Repository: PyCQA/flake8

Issue:
Add a "require plugin" option
In GitLab by @mschwager on Jan 7, 2019, 07:34

Hi all,

I'd like to request an additional option to `flake8` for requiring certain plugin(s) are installed and selected during a run. For example,

```
$ flake8 --require=XYZ --select=XYZ /path/to/lint
```

I could also envision the new option implies a `--select`. Naming like `--require-plugins` also makes sense to me.

This would be very helpful for reminding developers to have a set list of `flake8` plugins installed on their machine. For example, a common use case may be that a repository uses a `Makefile` for various common operations including linting:

```
lint:
    flake8 --select=XYZ /path/to/lint

.PHONY: lint
```

Both the CI/CD system and developers use the `Makefile` to run these common operations, i.e. `make lint`. However, if the CI/CD system has the required plugins, but the developer's machine does not, then the local dev machine will silently ignore the selected rules:

```
$ flake8 --select=DOESNTEXIST /path/to/lint
$ echo $?
0
```

I'm envisioning a flag that would loudly complain if a required plugin is not installed:

```
lint:
    flake8 --require=ABC,XYZ /path/to/lint

.PHONY: lint
```

Keying off the plugin's prefix, similar to `--select` and `--ignore`, seems to be the most consistent solution in my opinion.

Another option could be for `flake8` to complain loudly if a plugin in a `--select` flag doesn't exist. However, a new flag makes more sense to me for ensuring backwards compatibility and because this seems like a net-new feature.

Thoughts?

Discussion:

Patch:
diff --git a/docs/source/user/options.rst b/docs/source/user/options.rst
index 8be702c..8bf7e33 100644
--- a/docs/source/user/options.rst
+++ b/docs/source/user/options.rst
@@ -78,6 +78,8 @@ Index of Options
 
 - :option:`flake8 --statistics`
 
+- :option:`flake8 --require-plugins`
+
 - :option:`flake8 --enable-extensions`
 
 - :option:`flake8 --exit-zero`
@@ -772,6 +774,32 @@ Options and their Descriptions
         statistics = True
 
 
+.. option:: --require-plugins=<names>
+
+    :ref:`Go back to index <top>`
+
+    Require specific plugins to be installed before running.
+
+    This option takes a list of distribution names (usually the name you would
+    use when running ``pip install``).
+
+    Command-line example:
+
+    .. prompt:: bash
+
+        flake8 --require-plugins=flake8-2020,flake8-typing-extensions dir/
+
+    This **can** be specified in config files.
+
+    Example config file usage:
+
+    .. code-block:: ini
+
+        require-plugins =
+            flake8-2020
+            flake8-typing-extensions
+
+
 .. option:: --enable-extensions=<errors>
 
     :ref:`Go back to index <top>`
@@ -779,8 +807,8 @@ Options and their Descriptions
     Enable off-by-default extensions.
 
     Plugins to |Flake8| have the option of registering themselves as
-    off-by-default. These plugins effectively add themselves to the
-    default ignore list.
+    off-by-default. These plugins will not be loaded unless enabled by this
+    option.
 
     Command-line example:
 
diff --git a/src/flake8/api/legacy.py b/src/flake8/api/legacy.py
index 089543d..8fb7ab6 100644
--- a/src/flake8/api/legacy.py
+++ b/src/flake8/api/legacy.py
@@ -212,7 +212,12 @@ def get_style_guide(**kwargs: Any) -> StyleGuide:
         isolated=prelim_opts.isolated,
     )
 
-    application.find_plugins(cfg, cfg_dir, prelim_opts.enable_extensions)
+    application.find_plugins(
+        cfg,
+        cfg_dir,
+        enable_extensions=prelim_opts.enable_extensions,
+        require_plugins=prelim_opts.require_plugins,
+    )
     application.register_plugin_options()
     application.parse_configuration_and_cli(cfg, cfg_dir, remaining_args)
     # We basically want application.initialize to be called but with these
diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py
index 7552528..df32f92 100644
--- a/src/flake8/main/application.py
+++ b/src/flake8/main/application.py
@@ -111,14 +111,21 @@ class Application:
         self,
         cfg: configparser.RawConfigParser,
         cfg_dir: str,
+        *,
         enable_extensions: Optional[str],
+        require_plugins: Optional[str],
     ) -> None:
         """Find and load the plugins for this application.
 
         Set :attr:`plugins` based on loaded plugins.
         """
-        opts = finder.parse_plugin_options(cfg, cfg_dir, enable_extensions)
-        raw = finder.find_plugins(cfg)
+        opts = finder.parse_plugin_options(
+            cfg,
+            cfg_dir,
+            enable_extensions=enable_extensions,
+            require_plugins=require_plugins,
+        )
+        raw = finder.find_plugins(cfg, opts)
         self.plugins = finder.load_plugins(raw, opts)
 
     def register_plugin_options(self) -> None:
@@ -295,7 +302,12 @@ class Application:
             isolated=prelim_opts.isolated,
         )
 
-        self.find_plugins(cfg, cfg_dir, prelim_opts.enable_extensions)
+        self.find_plugins(
+            cfg,
+            cfg_dir,
+            enable_extensions=prelim_opts.enable_extensions,
+            require_plugins=prelim_opts.require_plugins,
+        )
         self.register_plugin_options()
         self.parse_configuration_and_cli(cfg, cfg_dir, remaining_args)
         self.make_formatter()
diff --git a/src/flake8/main/options.py b/src/flake8/main/options.py
index 201483b..cb4cd73 100644
--- a/src/flake8/main/options.py
+++ b/src/flake8/main/options.py
@@ -68,6 +68,11 @@ def stage1_arg_parser() -> argparse.ArgumentParser:
         "by default",
     )
 
+    parser.add_argument(
+        "--require-plugins",
+        help="Require specific plugins to be installed before running",
+    )
+
     return parser
 
 
diff --git a/src/flake8/plugins/finder.py b/src/flake8/plugins/finder.py
index e40b176..43b5417 100644
--- a/src/flake8/plugins/finder.py
+++ b/src/flake8/plugins/finder.py
@@ -15,6 +15,7 @@ from typing import Tuple
 
 from flake8 import utils
 from flake8._compat import importlib_metadata
+from flake8.exceptions import ExecutionError
 from flake8.exceptions import FailedToLoadPlugin
 
 LOG = logging.getLogger(__name__)
@@ -88,6 +89,65 @@ class Plugins(NamedTuple):
         )
 
 
+class PluginOptions(NamedTuple):
+    """Options related to plugin loading."""
+
+    local_plugin_paths: Tuple[str, ...]
+    enable_extensions: FrozenSet[str]
+    require_plugins: FrozenSet[str]
+
+    @classmethod
+    def blank(cls) -> "PluginOptions":
+        """Make a blank PluginOptions, mostly used for tests."""
+        return cls(
+            local_plugin_paths=(),
+            enable_extensions=frozenset(),
+            require_plugins=frozenset(),
+        )
+
+
+def _parse_option(
+    cfg: configparser.RawConfigParser,
+    cfg_opt_name: str,
+    opt: Optional[str],
+) -> List[str]:
+    # specified on commandline: use that
+    if opt is not None:
+        return utils.parse_comma_separated_list(opt)
+    else:
+        # ideally this would reuse our config parsing framework but we need to
+        # parse this from preliminary options before plugins are enabled
+        for opt_name in (cfg_opt_name, cfg_opt_name.replace("_", "-")):
+            val = cfg.get("flake8", opt_name, fallback=None)
+            if val is not None:
+                return utils.parse_comma_separated_list(val)
+        else:
+            return []
+
+
+def parse_plugin_options(
+    cfg: configparser.RawConfigParser,
+    cfg_dir: str,
+    *,
+    enable_extensions: Optional[str],
+    require_plugins: Optional[str],
+) -> PluginOptions:
+    """Parse plugin loading related options."""
+    paths_s = cfg.get("flake8:local-plugins", "paths", fallback="").strip()
+    paths = utils.parse_comma_separated_list(paths_s)
+    paths = utils.normalize_paths(paths, cfg_dir)
+
+    return PluginOptions(
+        local_plugin_paths=tuple(paths),
+        enable_extensions=frozenset(
+            _parse_option(cfg, "enable_extensions", enable_extensions),
+        ),
+        require_plugins=frozenset(
+            _parse_option(cfg, "require_plugins", require_plugins),
+        ),
+    )
+
+
 def _flake8_plugins(
     eps: Iterable[importlib_metadata.EntryPoint],
     name: str,
@@ -160,65 +220,38 @@ def _find_local_plugins(
             yield Plugin("local", "local", ep)
 
 
-def find_plugins(cfg: configparser.RawConfigParser) -> List[Plugin]:
+def _check_required_plugins(
+    plugins: List[Plugin],
+    expected: FrozenSet[str],
+) -> None:
+    plugin_names = {
+        utils.normalize_pypi_name(plugin.package) for plugin in plugins
+    }
+    expected_names = {utils.normalize_pypi_name(name) for name in expected}
+    missing_plugins = expected_names - plugin_names
+
+    if missing_plugins:
+        raise ExecutionError(
+            f"required plugins were not installed!\n"
+            f"- installed: {', '.join(sorted(plugin_names))}\n"
+            f"- expected: {', '.join(sorted(expected_names))}\n"
+            f"- missing: {', '.join(sorted(missing_plugins))}"
+        )
+
+
+def find_plugins(
+    cfg: configparser.RawConfigParser,
+    opts: PluginOptions,
+) -> List[Plugin]:
     """Discovers all plugins (but does not load them)."""
     ret = [*_find_importlib_plugins(), *_find_local_plugins(cfg)]
 
     # for determinism, sort the list
     ret.sort()
 
-    return ret
-
-
-class PluginOptions(NamedTuple):
-    """Options related to plugin loading."""
+    _check_required_plugins(ret, opts.require_plugins)
 
-    local_plugin_paths: Tuple[str, ...]
-    enable_extensions: FrozenSet[str]
-    # TODO: more options here!
-    # require_plugins: Tuple[str, ...]
-
-    @classmethod
-    def blank(cls) -> "PluginOptions":
-        """Make a blank PluginOptions, mostly used for tests."""
-        return cls(local_plugin_paths=(), enable_extensions=frozenset())
-
-
-def _parse_option(
-    cfg: configparser.RawConfigParser,
-    cfg_opt_name: str,
-    opt: Optional[str],
-) -> List[str]:
-    # specified on commandline: use that
-    if opt is not None:
-        return utils.parse_comma_separated_list(opt)
-    else:
-        # ideally this would reuse our config parsing framework but we need to
-        # parse this from preliminary options before plugins are enabled
-        for opt_name in (cfg_opt_name, cfg_opt_name.replace("_", "-")):
-            val = cfg.get("flake8", opt_name, fallback=None)
-            if val is not None:
-                return utils.parse_comma_separated_list(val)
-        else:
-            return []
-
-
-def parse_plugin_options(
-    cfg: configparser.RawConfigParser,
-    cfg_dir: str,
-    enable_extensions: Optional[str],
-) -> PluginOptions:
-    """Parse plugin loading related options."""
-    paths_s = cfg.get("flake8:local-plugins", "paths", fallback="").strip()
-    paths = utils.parse_comma_separated_list(paths_s)
-    paths = utils.normalize_paths(paths, cfg_dir)
-
-    return PluginOptions(
-        local_plugin_paths=tuple(paths),
-        enable_extensions=frozenset(
-            _parse_option(cfg, "enable_extensions", enable_extensions),
-        ),
-    )
+    return ret
 
 
 def _parameters_for(func: Any) -> Dict[str, bool]:
diff --git a/src/flake8/utils.py b/src/flake8/utils.py
index 71f9dd0..cc47ffc 100644
--- a/src/flake8/utils.py
+++ b/src/flake8/utils.py
@@ -25,6 +25,7 @@ from flake8 import exceptions
 DIFF_HUNK_REGEXP = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@.*$")
 COMMA_SEPARATED_LIST_RE = re.compile(r"[,\s]")
 LOCAL_PLUGIN_LIST_RE = re.compile(r"[,\t\n\r\f\v]")
+NORMALIZE_PACKAGE_NAME_RE = re.compile(r"[-_.]+")
 
 
 def parse_comma_separated_list(
@@ -343,3 +344,8 @@ def get_python_version() -> str:
         platform.python_version(),
         platform.system(),
     )
+
+
+def normalize_pypi_name(s: str) -> str:
+    """Normalize a distribution name according to PEP 503."""
+    return NORMALIZE_PACKAGE_NAME_RE.sub("-", s).lower()


Test patch:
diff --git a/tests/integration/test_plugins.py b/tests/integration/test_plugins.py
index 0950b6c..c3be049 100644
--- a/tests/integration/test_plugins.py
+++ b/tests/integration/test_plugins.py
@@ -53,8 +53,13 @@ report =
 def test_enable_local_plugin_from_config(local_config):
     """App can load a local plugin from config file."""
     cfg, cfg_dir = config.load_config(local_config, [], isolated=False)
-    opts = finder.parse_plugin_options(cfg, cfg_dir, None)
-    plugins = finder.find_plugins(cfg)
+    opts = finder.parse_plugin_options(
+        cfg,
+        cfg_dir,
+        enable_extensions=None,
+        require_plugins=None,
+    )
+    plugins = finder.find_plugins(cfg, opts)
     loaded_plugins = finder.load_plugins(plugins, opts)
 
     (custom_extension,) = (
@@ -80,8 +85,13 @@ def test_local_plugin_can_add_option(local_config):
         config=stage1_args.config, extra=[], isolated=False
     )
 
-    opts = finder.parse_plugin_options(cfg, cfg_dir, None)
-    plugins = finder.find_plugins(cfg)
+    opts = finder.parse_plugin_options(
+        cfg,
+        cfg_dir,
+        enable_extensions=None,
+        require_plugins=None,
+    )
+    plugins = finder.find_plugins(cfg, opts)
     loaded_plugins = finder.load_plugins(plugins, opts)
 
     option_manager = OptionManager(
diff --git a/tests/unit/plugins/finder_test.py b/tests/unit/plugins/finder_test.py
index d87a77c..15f15a2 100644
--- a/tests/unit/plugins/finder_test.py
+++ b/tests/unit/plugins/finder_test.py
@@ -5,6 +5,7 @@ from unittest import mock
 import pytest
 
 from flake8._compat import importlib_metadata
+from flake8.exceptions import ExecutionError
 from flake8.exceptions import FailedToLoadPlugin
 from flake8.plugins import finder
 from flake8.plugins.pyflakes import FlakesChecker
@@ -378,16 +379,31 @@ def test_find_local_plugins(local_plugin_cfg):
 
 def test_parse_plugin_options_not_specified(tmp_path):
     cfg = configparser.RawConfigParser()
-    ret = finder.parse_plugin_options(cfg, str(tmp_path), None)
-    assert ret == finder.PluginOptions((), frozenset())
+    opts = finder.parse_plugin_options(
+        cfg,
+        str(tmp_path),
+        enable_extensions=None,
+        require_plugins=None,
+    )
+    expected = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(),
+        require_plugins=frozenset(),
+    )
+    assert opts == expected
 
 
 def test_parse_enabled_from_commandline(tmp_path):
     cfg = configparser.RawConfigParser()
     cfg.add_section("flake8")
     cfg.set("flake8", "enable_extensions", "A,B,C")
-    ret = finder.parse_plugin_options(cfg, str(tmp_path), "D,E,F")
-    assert ret == finder.PluginOptions((), frozenset(("D", "E", "F")))
+    opts = finder.parse_plugin_options(
+        cfg,
+        str(tmp_path),
+        enable_extensions="D,E,F",
+        require_plugins=None,
+    )
+    assert opts.enable_extensions == frozenset(("D", "E", "F"))
 
 
 @pytest.mark.parametrize("opt", ("enable_extensions", "enable-extensions"))
@@ -395,13 +411,23 @@ def test_parse_enabled_from_config(opt, tmp_path):
     cfg = configparser.RawConfigParser()
     cfg.add_section("flake8")
     cfg.set("flake8", opt, "A,B,C")
-    ret = finder.parse_plugin_options(cfg, str(tmp_path), None)
-    assert ret == finder.PluginOptions((), frozenset(("A", "B", "C")))
+    opts = finder.parse_plugin_options(
+        cfg,
+        str(tmp_path),
+        enable_extensions=None,
+        require_plugins=None,
+    )
+    assert opts.enable_extensions == frozenset(("A", "B", "C"))
 
 
 def test_parse_plugin_options_local_plugin_paths_missing(tmp_path):
     cfg = configparser.RawConfigParser()
-    opts = finder.parse_plugin_options(cfg, str(tmp_path), None)
+    opts = finder.parse_plugin_options(
+        cfg,
+        str(tmp_path),
+        enable_extensions=None,
+        require_plugins=None,
+    )
     assert opts.local_plugin_paths == ()
 
 
@@ -409,7 +435,12 @@ def test_parse_plugin_options_local_plugin_paths(tmp_path):
     cfg = configparser.RawConfigParser()
     cfg.add_section("flake8:local-plugins")
     cfg.set("flake8:local-plugins", "paths", "./a, ./b")
-    opts = finder.parse_plugin_options(cfg, str(tmp_path), None)
+    opts = finder.parse_plugin_options(
+        cfg,
+        str(tmp_path),
+        enable_extensions=None,
+        require_plugins=None,
+    )
 
     expected = (str(tmp_path.joinpath("a")), str(tmp_path.joinpath("b")))
     assert opts.local_plugin_paths == expected
@@ -422,12 +453,13 @@ def test_find_plugins(
     mock_distribution,
     local_plugin_cfg,
 ):
+    opts = finder.PluginOptions.blank()
     with mock.patch.object(
         importlib_metadata,
         "distributions",
         return_value=[flake8_dist, flake8_foo_dist],
     ):
-        ret = finder.find_plugins(local_plugin_cfg)
+        ret = finder.find_plugins(local_plugin_cfg, opts)
 
     assert ret == [
         finder.Plugin(
@@ -505,6 +537,80 @@ def test_find_plugins(
     ]
 
 
+def test_find_plugins_plugin_is_present(flake8_foo_dist):
+    cfg = configparser.RawConfigParser()
+    options_flake8_foo_required = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(),
+        require_plugins=frozenset(("flake8-foo",)),
+    )
+    options_not_required = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(),
+        require_plugins=frozenset(),
+    )
+
+    with mock.patch.object(
+        importlib_metadata,
+        "distributions",
+        return_value=[flake8_foo_dist],
+    ):
+        # neither of these raise, `flake8-foo` is satisfied
+        finder.find_plugins(cfg, options_flake8_foo_required)
+        finder.find_plugins(cfg, options_not_required)
+
+
+def test_find_plugins_plugin_is_missing(flake8_dist, flake8_foo_dist):
+    cfg = configparser.RawConfigParser()
+    options_flake8_foo_required = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(),
+        require_plugins=frozenset(("flake8-foo",)),
+    )
+    options_not_required = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(),
+        require_plugins=frozenset(),
+    )
+
+    with mock.patch.object(
+        importlib_metadata,
+        "distributions",
+        return_value=[flake8_dist],
+    ):
+        # this is ok, no special requirements
+        finder.find_plugins(cfg, options_not_required)
+
+        # but we get a nice error for missing plugins here!
+        with pytest.raises(ExecutionError) as excinfo:
+            finder.find_plugins(cfg, options_flake8_foo_required)
+
+        (msg,) = excinfo.value.args
+        assert msg == (
+            "required plugins were not installed!\n"
+            "- installed: flake8, pycodestyle, pyflakes\n"
+            "- expected: flake8-foo\n"
+            "- missing: flake8-foo"
+        )
+
+
+def test_find_plugins_name_normalization(flake8_foo_dist):
+    cfg = configparser.RawConfigParser()
+    opts = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(),
+        # this name will be normalized before checking
+        require_plugins=frozenset(("Flake8_Foo",)),
+    )
+
+    with mock.patch.object(
+        importlib_metadata,
+        "distributions",
+        return_value=[flake8_foo_dist],
+    ):
+        finder.find_plugins(cfg, opts)
+
+
 def test_parameters_for_class_plugin():
     """Verify that we can retrieve the parameters for a class plugin."""
 
@@ -581,6 +687,7 @@ def test_import_plugins_extends_sys_path():
     opts = finder.PluginOptions(
         local_plugin_paths=("tests/integration/subdir",),
         enable_extensions=frozenset(),
+        require_plugins=frozenset(),
     )
     ret = finder._import_plugins([plugin], opts)
 
@@ -632,11 +739,13 @@ def test_classify_plugins_enable_a_disabled_plugin():
     normal_opts = finder.PluginOptions(
         local_plugin_paths=(),
         enable_extensions=frozenset(),
+        require_plugins=frozenset(),
     )
     classified_normal = finder._classify_plugins([loaded], normal_opts)
     enabled_opts = finder.PluginOptions(
         local_plugin_paths=(),
         enable_extensions=frozenset(("ABC",)),
+        require_plugins=frozenset(),
     )
     classified_enabled = finder._classify_plugins([loaded], enabled_opts)
 
@@ -659,6 +768,7 @@ def test_load_plugins():
     opts = finder.PluginOptions(
         local_plugin_paths=("tests/integration/subdir",),
         enable_extensions=frozenset(),
+        require_plugins=frozenset(),
     )
     ret = finder.load_plugins([plugin], opts)
 
diff --git a/tests/unit/test_legacy_api.py b/tests/unit/test_legacy_api.py
index 4b44fb8..169969b 100644
--- a/tests/unit/test_legacy_api.py
+++ b/tests/unit/test_legacy_api.py
@@ -20,6 +20,7 @@ def test_get_style_guide():
         output_file=None,
         verbose=0,
         enable_extensions=None,
+        require_plugins=None,
     )
     mockedapp = mock.Mock()
     mockedapp.parse_preliminary_options.return_value = (prelim_opts, [])
@@ -35,7 +36,12 @@ def test_get_style_guide():
 
     application.assert_called_once_with()
     mockedapp.parse_preliminary_options.assert_called_once_with([])
-    mockedapp.find_plugins.assert_called_once_with(cfg, cfg_dir, None)
+    mockedapp.find_plugins.assert_called_once_with(
+        cfg,
+        cfg_dir,
+        enable_extensions=None,
+        require_plugins=None,
+    )
     mockedapp.register_plugin_options.assert_called_once_with()
     mockedapp.parse_configuration_and_cli.assert_called_once_with(
         cfg, cfg_dir, []
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 5aadf2f..fefe662 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -231,3 +231,19 @@ def test_stdin_unknown_coding_token():
     stdin = io.TextIOWrapper(io.BytesIO(b"# coding: unknown\n"), "UTF-8")
     with mock.patch.object(sys, "stdin", stdin):
         assert utils.stdin_get_value.__wrapped__() == "# coding: unknown\n"
+
+
+@pytest.mark.parametrize(
+    ("s", "expected"),
+    (
+        ("", ""),
+        ("my-plugin", "my-plugin"),
+        ("MyPlugin", "myplugin"),
+        ("my_plugin", "my-plugin"),
+        ("my.plugin", "my-plugin"),
+        ("my--plugin", "my-plugin"),
+        ("my__plugin", "my-plugin"),
+    ),
+)
+def test_normalize_pypi_name(s, expected):
+    assert utils.normalize_pypi_name(s) == expected