Merge branch 'error-when-duplicate-entry-points' into 'master'

Emit an error when entry points are duplicated

Closes #634

See merge request pycqa/flake8!425

src/flake8/exceptions.py

@@ -1,6 +1,10 @@
 """Exception classes for all of Flake8."""
 from typing import Dict
 
+if False:  # `typing.TYPE_CHECKING` was introduced in 3.5.2
+    from flake8.plugins.manager import Plugin
+    from flake8._compat import importlib_metadata
+
 
 class Flake8Exception(Exception):
     """Plain Flake8 exception."""
@@ -14,6 +18,33 @@ class ExecutionError(Flake8Exception):
     """Exception raised during execution of Flake8."""
 
 
+class DuplicatePluginEntryPoint(ExecutionError):
+    """Exception raised when a plugin entry point is already taken."""
+
+    FORMAT = (
+        'Plugin entry point "%(entry_point)s" for "%(new)s" already taken by '
+        '"%(existing)s"'
+    )
+
+    def __init__(self, entry_point, existing_plugin):
+        # type: (importlib_metadata.EntryPoint, Plugin) -> None
+        """Initialize our DuplicatePluginEntryPoint exception."""
+        self.entry_point = entry_point
+        self.existing_plugin = existing_plugin
+        super(DuplicatePluginEntryPoint, self).__init__(
+            entry_point,
+            existing_plugin,
+        )
+
+    def __str__(self):  # type: () -> str
+        """Format our exception message."""
+        return self.FORMAT % {
+            "entry_point": self.entry_point.name,
+            "new": self.entry_point.value,
+            "existing": self.existing_plugin.entry_point.value,
+        }
+
+
 class FailedToLoadPlugin(Flake8Exception):
     """Exception raised when a plugin fails to load."""
 

src/flake8/main/application.py

@@ -126,15 +126,18 @@ class Application(object):
         This should be the last thing called on the application instance. It
         will check certain options and exit appropriately.
         """
+        if self.catastrophic_failure:
+            # Don't rely on any attributes being set if things failued
+            # catastrophically
+            raise SystemExit(True)
+
         if self.options.count:
             print(self.result_count)
 
         if self.options.exit_zero:
-            raise SystemExit(self.catastrophic_failure)
+            raise SystemExit(False)
         else:
-            raise SystemExit(
+            raise SystemExit(self.result_count > 0)
-                (self.result_count > 0) or self.catastrophic_failure
-            )
 
     def find_plugins(self, config_finder):
         # type: (config.ConfigFileFinder) -> None

src/flake8/plugins/manager.py

@@ -273,6 +273,12 @@ class PluginManager(object):  # pylint: disable=too-few-public-methods
             Is this a repo-local plugin?
         """
         name = entry_point.name
+
+        if name in self.plugins:
+            raise exceptions.DuplicatePluginEntryPoint(
+                entry_point, self.plugins[name],
+            )
+
         self.plugins[name] = Plugin(name, entry_point, local=local)
         self.names.append(name)
         LOG.debug('Loaded %r for plugin "%s".', self.plugins[name], name)

tests/fixtures/config_files/local-plugin-duplicate-entry-point.ini

@@ -0,0 +1,4 @@
+[flake8:local-plugins]
+extension =
+  XE = aplugin:ExtensionTestPluginA
+  XE = aplugin:ExtensionTestPluginB

tests/integration/test_plugins.py

@@ -1,9 +1,13 @@
 """Integration tests for plugin loading."""
+import pytest
+
 from flake8.main import application
+from flake8 import exceptions
 
 
 LOCAL_PLUGIN_CONFIG = 'tests/fixtures/config_files/local-plugin.ini'
 LOCAL_PLUGIN_PATH_CONFIG = 'tests/fixtures/config_files/local-plugin-path.ini'
+LOCAL_PLUGIN_DUPLICATE_CONFIG = 'tests/fixtures/config_files/local-plugin-duplicate-entry-point.ini'
 
 
 class ExtensionTestPlugin(object):
@@ -61,3 +65,10 @@ def test_enable_local_plugin_at_non_installed_path():
     app.initialize(['flake8', '--config', LOCAL_PLUGIN_PATH_CONFIG])
 
     assert app.check_plugins['XE'].plugin.name == 'ExtensionTestPlugin2'
+
+
+def test_reject_local_plugins_with_duplicate_entry_point():
+    """Reject duplicate entry points in local-plugins config section."""
+    with pytest.raises(exceptions.DuplicatePluginEntryPoint):
+        app = application.Application()
+        app.initialize(['flake8', '--config', LOCAL_PLUGIN_DUPLICATE_CONFIG])

tests/unit/test_application.py

@@ -48,6 +48,18 @@ def test_exit_does_raise(result_count, catastrophic, exit_zero, value,
     assert excinfo.value.args[0] is value
 
 
+def test_exit_raises(application):
+    """Verify Application.exit raises SystemExit under configuration failure."""
+    application.catastrophic_failure = True
+    # Note: no application.options set -- configuration issues can lead to
+    # errors before it's assigned.
+
+    with pytest.raises(SystemExit) as excinfo:
+        application.exit()
+
+    assert excinfo.value.args[0] is True
+
+
 def test_warns_on_unknown_formatter_plugin_name(application):
     """Verify we log a warning with an unfound plugin."""
     default = mock.Mock()