Merge pull request #1528 from PyCQA/plugin_paths_plugin_options

combine local_plugin_paths and parse_plugin_options

docs/source/internal/plugin_handling.rst

@@ -36,8 +36,8 @@ reporting each check in the ``--version`` output, we only report
 API Documentation
 -----------------
 
+.. autofunction:: flake8.plugins.finder.parse_plugin_options
+
 .. autofunction:: flake8.plugins.finder.find_plugins
 
-.. autofunction:: flake8.plugins.finder.find_local_plugin_paths
-
 .. autofunction:: flake8.plugins.finder.load_plugins

src/flake8/main/application.py

@@ -117,10 +117,9 @@ class Application:
 
         Set :attr:`plugins` based on loaded plugins.
         """
+        opts = finder.parse_plugin_options(cfg, cfg_dir, enable_extensions)
         raw = finder.find_plugins(cfg)
-        local_plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir)
+        self.plugins = finder.load_plugins(raw, opts)
-        opts = finder.parse_plugin_options(cfg, enable_extensions)
-        self.plugins = finder.load_plugins(raw, local_plugin_paths, opts)
 
     def register_plugin_options(self) -> None:
         """Register options provided by plugins to our option manager."""

src/flake8/plugins/finder.py

@@ -11,6 +11,7 @@ from typing import Iterable
 from typing import List
 from typing import NamedTuple
 from typing import Optional
+from typing import Tuple
 
 from flake8 import utils
 from flake8._compat import importlib_metadata
@@ -169,23 +170,19 @@ def find_plugins(cfg: configparser.RawConfigParser) -> List[Plugin]:
     return ret
 
 
-def find_local_plugin_paths(
-    cfg: configparser.RawConfigParser,
-    cfg_dir: str,
-) -> List[str]:
-    """Discovers the list of ``flake8:local-plugins`` ``paths``."""
-    paths_s = cfg.get("flake8:local-plugins", "paths", fallback="").strip()
-    paths = utils.parse_comma_separated_list(paths_s)
-    return utils.normalize_paths(paths, cfg_dir)
-
-
 class PluginOptions(NamedTuple):
     """Options related to plugin loading."""
 
+    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,
@@ -208,10 +205,16 @@ def _parse_option(
 
 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),
         ),
@@ -259,9 +262,10 @@ def _load_plugin(plugin: Plugin) -> LoadedPlugin:
 
 
 def _import_plugins(
-    plugins: List[Plugin], paths: List[str]
+    plugins: List[Plugin],
+    opts: PluginOptions,
 ) -> List[LoadedPlugin]:
-    sys.path.extend(paths)
+    sys.path.extend(opts.local_plugin_paths)
     return [_load_plugin(p) for p in plugins]
 
 
@@ -305,7 +309,6 @@ def _classify_plugins(
 
 def load_plugins(
     plugins: List[Plugin],
-    paths: List[str],
     opts: PluginOptions,
 ) -> Plugins:
     """Load and classify all flake8 plugins.
@@ -314,4 +317,4 @@ def load_plugins(
     - next: converts the ``Plugin``s to ``LoadedPlugins``
     - finally: classifies plugins into their specific types
     """
-    return _classify_plugins(_import_plugins(plugins, paths), opts)
+    return _classify_plugins(_import_plugins(plugins, opts), opts)

tests/integration/test_checker.py

@@ -90,8 +90,8 @@ def mock_file_checker_with_plugin(plugin_target):
             ),
         ),
     ]
-    opts = finder.PluginOptions(frozenset())
+    opts = finder.PluginOptions.blank()
-    plugins = finder.load_plugins(to_load, [], opts)
+    plugins = finder.load_plugins(to_load, opts)
 
     # Prevent it from reading lines from stdin or somewhere else
     with mock.patch(

tests/integration/test_plugins.py

@@ -53,10 +53,9 @@ 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)
-    plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir)
+    loaded_plugins = finder.load_plugins(plugins, opts)
-    opts = finder.PluginOptions(frozenset())
-    loaded_plugins = finder.load_plugins(plugins, plugin_paths, opts)
 
     (custom_extension,) = (
         loaded
@@ -81,10 +80,9 @@ 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)
-    plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir)
+    loaded_plugins = finder.load_plugins(plugins, opts)
-    opts = finder.PluginOptions(frozenset())
-    loaded_plugins = finder.load_plugins(plugins, plugin_paths, opts)
 
     option_manager = OptionManager(
         version="123",

tests/unit/plugins/finder_test.py

@@ -376,27 +376,43 @@ def test_find_local_plugins(local_plugin_cfg):
     }
 
 
-def test_parse_plugin_options_not_specified():
+def test_parse_plugin_options_not_specified(tmp_path):
     cfg = configparser.RawConfigParser()
-    ret = finder.parse_plugin_options(cfg, None)
+    ret = finder.parse_plugin_options(cfg, str(tmp_path), None)
-    assert ret == finder.PluginOptions(frozenset())
+    assert ret == finder.PluginOptions((), frozenset())
 
 
-def test_parse_enabled_from_commandline():
+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, "D,E,F")
+    ret = finder.parse_plugin_options(cfg, str(tmp_path), "D,E,F")
-    assert ret == finder.PluginOptions(frozenset(("D", "E", "F")))
+    assert ret == finder.PluginOptions((), frozenset(("D", "E", "F")))
 
 
 @pytest.mark.parametrize("opt", ("enable_extensions", "enable-extensions"))
-def test_parse_enabled_from_config(opt):
+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, None)
+    ret = finder.parse_plugin_options(cfg, str(tmp_path), None)
-    assert ret == finder.PluginOptions(frozenset(("A", "B", "C")))
+    assert ret == finder.PluginOptions((), 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)
+    assert opts.local_plugin_paths == ()
+
+
+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)
+
+    expected = (str(tmp_path.joinpath("a")), str(tmp_path.joinpath("b")))
+    assert opts.local_plugin_paths == expected
 
 
 def test_find_plugins(
@@ -489,20 +505,6 @@ def test_find_plugins(
     ]
 
 
-def test_find_local_plugin_paths_missing(tmp_path):
-    cfg = configparser.RawConfigParser()
-    assert finder.find_local_plugin_paths(cfg, str(tmp_path)) == []
-
-
-def test_find_local_plugin_paths(tmp_path):
-    cfg = configparser.RawConfigParser()
-    cfg.add_section("flake8:local-plugins")
-    cfg.set("flake8:local-plugins", "paths", "./a, ./b")
-    ret = finder.find_local_plugin_paths(cfg, str(tmp_path))
-
-    assert ret == [str(tmp_path.joinpath("a")), str(tmp_path.joinpath("b"))]
-
-
 def test_parameters_for_class_plugin():
     """Verify that we can retrieve the parameters for a class plugin."""
 
@@ -576,7 +578,11 @@ def reset_sys():
 def test_import_plugins_extends_sys_path():
     plugin = _plugin(ep=_ep(value="aplugin:ExtensionTestPlugin2"))
 
-    ret = finder._import_plugins([plugin], ["tests/integration/subdir"])
+    opts = finder.PluginOptions(
+        local_plugin_paths=("tests/integration/subdir",),
+        enable_extensions=frozenset(),
+    )
+    ret = finder._import_plugins([plugin], opts)
 
     import aplugin
 
@@ -604,7 +610,7 @@ def test_classify_plugins():
             logical_line_plugin,
             physical_line_plugin,
         ],
-        finder.PluginOptions(frozenset()),
+        finder.PluginOptions.blank(),
     )
 
     assert classified == finder.Plugins(
@@ -623,9 +629,15 @@ def test_classify_plugins_enable_a_disabled_plugin():
     plugin = _plugin(ep=_ep(name="ABC"))
     loaded = _loaded(plugin=plugin, parameters={"tree": True}, obj=obj)
 
-    normal_opts = finder.PluginOptions(frozenset())
+    normal_opts = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(),
+    )
     classified_normal = finder._classify_plugins([loaded], normal_opts)
-    enabled_opts = finder.PluginOptions(frozenset(("ABC",)))
+    enabled_opts = finder.PluginOptions(
+        local_plugin_paths=(),
+        enable_extensions=frozenset(("ABC",)),
+    )
     classified_enabled = finder._classify_plugins([loaded], enabled_opts)
 
     assert classified_normal == finder.Plugins(
@@ -644,8 +656,11 @@ def test_classify_plugins_enable_a_disabled_plugin():
 def test_load_plugins():
     plugin = _plugin(ep=_ep(value="aplugin:ExtensionTestPlugin2"))
 
-    opts = finder.PluginOptions(frozenset())
+    opts = finder.PluginOptions(
-    ret = finder.load_plugins([plugin], ["tests/integration/subdir"], opts)
+        local_plugin_paths=("tests/integration/subdir",),
+        enable_extensions=frozenset(),
+    )
+    ret = finder.load_plugins([plugin], opts)
 
     import aplugin