move managing of off_by_default / enable_extensions to plugin loading

This commit is contained in:
Anthony Sottile 2022-01-01 18:28:11 -05:00
parent ff13916d74
commit a8333e2bf2
12 changed files with 137 additions and 111 deletions

View file

@ -38,7 +38,7 @@ def get_style_guide(**kwargs):
isolated=prelim_opts.isolated, isolated=prelim_opts.isolated,
) )
application.find_plugins(cfg, cfg_dir) application.find_plugins(cfg, cfg_dir, prelim_opts.enable_extensions)
application.register_plugin_options() application.register_plugin_options()
application.parse_configuration_and_cli(cfg, cfg_dir, remaining_args) application.parse_configuration_and_cli(cfg, cfg_dir, remaining_args)
# We basically want application.initialize to be called but with these # We basically want application.initialize to be called but with these

View file

@ -119,14 +119,16 @@ class Application:
self, self,
cfg: configparser.RawConfigParser, cfg: configparser.RawConfigParser,
cfg_dir: str, cfg_dir: str,
enable_extensions: Optional[str],
) -> None: ) -> None:
"""Find and load the plugins for this application. """Find and load the plugins for this application.
Set :attr:`plugins` based on loaded plugins. Set :attr:`plugins` based on loaded plugins.
""" """
raw_plugins = finder.find_plugins(cfg) raw = finder.find_plugins(cfg)
local_plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir) local_plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir)
self.plugins = finder.load_plugins(raw_plugins, local_plugin_paths) enabled = finder.parse_enabled(cfg, enable_extensions)
self.plugins = finder.load_plugins(raw, local_plugin_paths, enabled)
def register_plugin_options(self) -> None: def register_plugin_options(self) -> None:
"""Register options provided by plugins to our option manager.""" """Register options provided by plugins to our option manager."""
@ -302,7 +304,7 @@ class Application:
isolated=prelim_opts.isolated, isolated=prelim_opts.isolated,
) )
self.find_plugins(cfg, cfg_dir) self.find_plugins(cfg, cfg_dir, prelim_opts.enable_extensions)
self.register_plugin_options() self.register_plugin_options()
self.parse_configuration_and_cli(cfg, cfg_dir, remaining_args) self.parse_configuration_and_cli(cfg, cfg_dir, remaining_args)
self.make_formatter() self.make_formatter()

View file

@ -15,6 +15,7 @@ def stage1_arg_parser() -> argparse.ArgumentParser:
- ``--append-config`` - ``--append-config``
- ``--config`` - ``--config``
- ``--isolated`` - ``--isolated``
- ``--enable-extensions``
""" """
parser = argparse.ArgumentParser(add_help=False) parser = argparse.ArgumentParser(add_help=False)
@ -59,6 +60,14 @@ def stage1_arg_parser() -> argparse.ArgumentParser:
help="Ignore all configuration files.", help="Ignore all configuration files.",
) )
# Plugin enablement options
parser.add_argument(
"--enable-extensions",
help="Enable plugins and extensions that are otherwise disabled "
"by default",
)
return parser return parser
@ -116,7 +125,6 @@ def register_default_options(option_manager: OptionManager) -> None:
- ``--disable-noqa`` - ``--disable-noqa``
- ``--show-source`` - ``--show-source``
- ``--statistics`` - ``--statistics``
- ``--enable-extensions``
- ``--exit-zero`` - ``--exit-zero``
- ``-j``/``--jobs`` - ``-j``/``--jobs``
- ``--tee`` - ``--tee``
@ -331,14 +339,6 @@ def register_default_options(option_manager: OptionManager) -> None:
) )
# Flake8 options # Flake8 options
add_option(
"--enable-extensions",
default="",
parse_from_config=True,
comma_separated_list=True,
help="Enable plugins and extensions that are otherwise disabled "
"by default",
)
add_option( add_option(
"--exit-zero", "--exit-zero",

View file

@ -376,10 +376,6 @@ class OptionManager:
_set_group(loaded.plugin.package) _set_group(loaded.plugin.package)
add_options(self) add_options(self)
# if the plugin is off by default, disable it!
if getattr(loaded.obj, "off_by_default", False):
self.extend_default_ignore(loaded.entry_name)
else:
self.extend_default_select(loaded.entry_name) self.extend_default_select(loaded.entry_name)
# isn't strictly necessary, but seems cleaner # isn't strictly necessary, but seems cleaner

View file

@ -9,6 +9,8 @@ from typing import Generator
from typing import Iterable from typing import Iterable
from typing import List from typing import List
from typing import NamedTuple from typing import NamedTuple
from typing import Optional
from typing import Set
from flake8 import utils from flake8 import utils
from flake8._compat import importlib_metadata from flake8._compat import importlib_metadata
@ -63,6 +65,7 @@ class Plugins(NamedTuple):
checkers: Checkers checkers: Checkers
reporters: Dict[str, LoadedPlugin] reporters: Dict[str, LoadedPlugin]
disabled: List[LoadedPlugin]
def all_plugins(self) -> Generator[LoadedPlugin, None, None]: def all_plugins(self) -> Generator[LoadedPlugin, None, None]:
"""Return an iterator over all :class:`LoadedPlugin`s.""" """Return an iterator over all :class:`LoadedPlugin`s."""
@ -171,6 +174,24 @@ def find_local_plugin_paths(
return utils.normalize_paths(paths, cfg_dir) return utils.normalize_paths(paths, cfg_dir)
def parse_enabled(
cfg: configparser.RawConfigParser,
enable_extensions: Optional[str],
) -> Set[str]:
"""Parse --enable-extensions."""
if enable_extensions is not None:
return set(utils.parse_comma_separated_list(enable_extensions))
else:
# ideally this would reuse our config parsing framework but we need to
# parse this from preliminary options before plugins are enabled
for opt in ("enable_extensions", "enable-extensions"):
val = cfg.get("flake8", opt, fallback=None)
if val is not None:
return set(utils.parse_comma_separated_list(val))
else:
return set()
def _parameters_for(func: Any) -> Dict[str, bool]: def _parameters_for(func: Any) -> Dict[str, bool]:
"""Return the parameters for the plugin. """Return the parameters for the plugin.
@ -218,14 +239,23 @@ def _import_plugins(
return [_load_plugin(p) for p in plugins] return [_load_plugin(p) for p in plugins]
def _classify_plugins(plugins: List[LoadedPlugin]) -> Plugins: def _classify_plugins(
plugins: List[LoadedPlugin],
enabled: Set[str],
) -> Plugins:
tree = [] tree = []
logical_line = [] logical_line = []
physical_line = [] physical_line = []
reporters = {} reporters = {}
disabled = []
for loaded in plugins: for loaded in plugins:
if loaded.plugin.entry_point.group == "flake8.report": if (
getattr(loaded.obj, "off_by_default", False)
and loaded.plugin.entry_point.name not in enabled
):
disabled.append(loaded)
elif loaded.plugin.entry_point.group == "flake8.report":
reporters[loaded.entry_name] = loaded reporters[loaded.entry_name] = loaded
elif "tree" in loaded.parameters: elif "tree" in loaded.parameters:
tree.append(loaded) tree.append(loaded)
@ -243,14 +273,19 @@ def _classify_plugins(plugins: List[LoadedPlugin]) -> Plugins:
physical_line=physical_line, physical_line=physical_line,
), ),
reporters=reporters, reporters=reporters,
disabled=disabled,
) )
def load_plugins(plugins: List[Plugin], paths: List[str]) -> Plugins: def load_plugins(
plugins: List[Plugin],
paths: List[str],
enabled: Set[str],
) -> Plugins:
"""Load and classify all flake8 plugins. """Load and classify all flake8 plugins.
- first: extends ``sys.path`` with ``paths`` (to import local plugins) - first: extends ``sys.path`` with ``paths`` (to import local plugins)
- next: converts the ``Plugin``s to ``LoadedPlugins`` - next: converts the ``Plugin``s to ``LoadedPlugins``
- finally: classifies plugins into their specific types - finally: classifies plugins into their specific types
""" """
return _classify_plugins(_import_plugins(plugins, paths)) return _classify_plugins(_import_plugins(plugins, paths), enabled)

View file

@ -160,14 +160,9 @@ class DecisionEngine:
self.extended_selected = tuple( self.extended_selected = tuple(
sorted(options.extended_default_select, reverse=True) sorted(options.extended_default_select, reverse=True)
) )
self.enabled_extensions = tuple(options.enable_extensions)
self.all_selected = tuple( self.all_selected = tuple(
sorted( sorted(
itertools.chain( itertools.chain(self.selected, options.extend_select),
self.selected,
options.extend_select,
self.enabled_extensions,
),
reverse=True, reverse=True,
) )
) )

View file

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

View file

@ -55,7 +55,7 @@ def test_enable_local_plugin_from_config(local_config):
cfg, cfg_dir = config.load_config(local_config, [], isolated=False) cfg, cfg_dir = config.load_config(local_config, [], isolated=False)
plugins = finder.find_plugins(cfg) plugins = finder.find_plugins(cfg)
plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir) plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir)
loaded_plugins = finder.load_plugins(plugins, plugin_paths) loaded_plugins = finder.load_plugins(plugins, plugin_paths, set())
(custom_extension,) = ( (custom_extension,) = (
loaded loaded
@ -82,7 +82,8 @@ def test_local_plugin_can_add_option(local_config):
plugins = finder.find_plugins(cfg) plugins = finder.find_plugins(cfg)
plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir) plugin_paths = finder.find_local_plugin_paths(cfg, cfg_dir)
loaded_plugins = finder.load_plugins(plugins, plugin_paths) enabled = finder.parse_enabled(cfg, stage1_args.enable_extensions)
loaded_plugins = finder.load_plugins(plugins, plugin_paths, enabled)
option_manager = OptionManager( option_manager = OptionManager(
version="123", version="123",

View file

@ -49,6 +49,7 @@ def test_plugins_all_plugins():
physical_line=[physical_line_plugin], physical_line=[physical_line_plugin],
), ),
reporters={"R": report_plugin}, reporters={"R": report_plugin},
disabled=[],
) )
assert tuple(plugins.all_plugins()) == ( assert tuple(plugins.all_plugins()) == (
@ -72,6 +73,7 @@ def test_plugins_versions_str():
# ignore local plugins # ignore local plugins
"custom": _loaded(_plugin(package="local")), "custom": _loaded(_plugin(package="local")),
}, },
disabled=[],
) )
assert plugins.versions_str() == "pkg1: 1, pkg2: 2" assert plugins.versions_str() == "pkg1: 1, pkg2: 2"
@ -374,6 +376,25 @@ def test_find_local_plugins(local_plugin_cfg):
} }
def test_parse_enabled_not_specified():
assert finder.parse_enabled(configparser.RawConfigParser(), None) == set()
def test_parse_enabled_from_commandline():
cfg = configparser.RawConfigParser()
cfg.add_section("flake8")
cfg.set("flake8", "enable_extensions", "A,B,C")
assert finder.parse_enabled(cfg, "D,E,F") == {"D", "E", "F"}
@pytest.mark.parametrize("opt", ("enable_extensions", "enable-extensions"))
def test_parse_enabled_from_config(opt):
cfg = configparser.RawConfigParser()
cfg.add_section("flake8")
cfg.set("flake8", opt, "A,B,C")
assert finder.parse_enabled(cfg, None) == {"A", "B", "C"}
def test_find_plugins( def test_find_plugins(
tmp_path, tmp_path,
flake8_dist, flake8_dist,
@ -573,7 +594,13 @@ def test_classify_plugins():
physical_line_plugin = _loaded(parameters={"physical_line": True}) physical_line_plugin = _loaded(parameters={"physical_line": True})
classified = finder._classify_plugins( classified = finder._classify_plugins(
[report_plugin, tree_plugin, logical_line_plugin, physical_line_plugin] [
report_plugin,
tree_plugin,
logical_line_plugin,
physical_line_plugin,
],
set(),
) )
assert classified == finder.Plugins( assert classified == finder.Plugins(
@ -583,6 +610,27 @@ def test_classify_plugins():
physical_line=[physical_line_plugin], physical_line=[physical_line_plugin],
), ),
reporters={"R": report_plugin}, reporters={"R": report_plugin},
disabled=[],
)
def test_classify_plugins_enable_a_disabled_plugin():
obj = mock.Mock(off_by_default=True)
plugin = _plugin(ep=_ep(name="ABC"))
loaded = _loaded(plugin=plugin, parameters={"tree": True}, obj=obj)
classified_normal = finder._classify_plugins([loaded], set())
classified_enabled = finder._classify_plugins([loaded], {"ABC"})
assert classified_normal == finder.Plugins(
checkers=finder.Checkers([], [], []),
reporters={},
disabled=[loaded],
)
assert classified_enabled == finder.Plugins(
checkers=finder.Checkers([loaded], [], []),
reporters={},
disabled=[],
) )
@ -590,7 +638,7 @@ def test_classify_plugins():
def test_load_plugins(): def test_load_plugins():
plugin = _plugin(ep=_ep(value="aplugin:ExtensionTestPlugin2")) plugin = _plugin(ep=_ep(value="aplugin:ExtensionTestPlugin2"))
ret = finder.load_plugins([plugin], ["tests/integration/subdir"]) ret = finder.load_plugins([plugin], ["tests/integration/subdir"], set())
import aplugin import aplugin
@ -607,4 +655,5 @@ def test_load_plugins():
physical_line=[], physical_line=[],
), ),
reporters={}, reporters={},
disabled=[],
) )

View file

@ -30,6 +30,7 @@ def test_debug_information():
physical_line=[], physical_line=[],
), ),
reporters={}, reporters={},
disabled=[],
) )
info = debug.information("9001", plugins) info = debug.information("9001", plugins)

View file

@ -16,7 +16,6 @@ def create_options(**kwargs):
kwargs.setdefault("ignore", []) kwargs.setdefault("ignore", [])
kwargs.setdefault("extend_ignore", []) kwargs.setdefault("extend_ignore", [])
kwargs.setdefault("disable_noqa", False) kwargs.setdefault("disable_noqa", False)
kwargs.setdefault("enable_extensions", [])
return argparse.Namespace(**kwargs) return argparse.Namespace(**kwargs)
@ -64,30 +63,25 @@ def test_was_ignored_implicitly_selects_errors(
@pytest.mark.parametrize( @pytest.mark.parametrize(
"select_list,extend_select,enable_extensions,error_code", ("select_list", "extend_select", "error_code"),
[ (
(["E111", "E121"], [], [], "E111"), (["E111", "E121"], [], "E111"),
(["E111", "E121"], [], [], "E121"), (["E111", "E121"], [], "E121"),
(["E11", "E12"], [], [], "E121"), (["E11", "E12"], [], "E121"),
(["E2", "E12"], [], [], "E121"), (["E2", "E12"], [], "E121"),
(["E2", "E12"], [], [], "E211"), (["E2", "E12"], [], "E211"),
(["E1"], ["E2"], [], "E211"), (["E1"], ["E2"], "E211"),
(["E1"], [], ["E2"], "E211"), ([], ["E2"], "E211"),
([], ["E2"], [], "E211"), (["E1"], ["E2"], "E211"),
([], [], ["E2"], "E211"), (["E111"], ["E121"], "E121"),
(["E1"], ["E2"], [], "E211"), ),
(["E111"], ["E121"], ["E2"], "E121"),
],
) )
def test_was_selected_selects_errors( def test_was_selected_selects_errors(select_list, extend_select, error_code):
select_list, extend_select, enable_extensions, error_code
):
"""Verify we detect users explicitly selecting an error.""" """Verify we detect users explicitly selecting an error."""
decider = style_guide.DecisionEngine( decider = style_guide.DecisionEngine(
options=create_options( options=create_options(
select=select_list, select=select_list,
extend_select=extend_select, extend_select=extend_select,
enable_extensions=enable_extensions,
), ),
) )
@ -199,15 +193,20 @@ def test_decision_for(
@pytest.mark.parametrize( @pytest.mark.parametrize(
"select,ignore,extended_default_ignore,extended_default_select," (
"enabled_extensions,error_code,expected", "select",
"ignore",
"extended_default_ignore",
"extended_default_select",
"error_code",
"expected",
),
[ [
( (
defaults.SELECT, defaults.SELECT,
[], [],
[], [],
["I1"], ["I1"],
[],
"I100", "I100",
style_guide.Decision.Selected, style_guide.Decision.Selected,
), ),
@ -216,7 +215,6 @@ def test_decision_for(
[], [],
[], [],
["I1"], ["I1"],
[],
"I201", "I201",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
@ -225,7 +223,6 @@ def test_decision_for(
["I2"], ["I2"],
[], [],
["I1"], ["I1"],
[],
"I101", "I101",
style_guide.Decision.Selected, style_guide.Decision.Selected,
), ),
@ -234,7 +231,6 @@ def test_decision_for(
["I2"], ["I2"],
[], [],
["I1"], ["I1"],
[],
"I201", "I201",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
@ -243,7 +239,6 @@ def test_decision_for(
["I1"], ["I1"],
[], [],
["I10"], ["I10"],
[],
"I101", "I101",
style_guide.Decision.Selected, style_guide.Decision.Selected,
), ),
@ -252,43 +247,22 @@ def test_decision_for(
["I10"], ["I10"],
[], [],
["I1"], ["I1"],
[],
"I101", "I101",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
(
defaults.SELECT,
[],
[],
[],
["U4"],
"U401",
style_guide.Decision.Selected,
),
( (
defaults.SELECT, defaults.SELECT,
["U401"], ["U401"],
[], [],
[], [],
["U4"],
"U401", "U401",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
(
defaults.SELECT,
["U401"],
[],
[],
["U4"],
"U402",
style_guide.Decision.Selected,
),
( (
["E", "W"], ["E", "W"],
["E13"], ["E13"],
[], [],
[], [],
[],
"E131", "E131",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
@ -297,18 +271,16 @@ def test_decision_for(
["E13"], ["E13"],
[], [],
[], [],
[],
"E126", "E126",
style_guide.Decision.Selected, style_guide.Decision.Selected,
), ),
(["E2"], ["E21"], [], [], [], "E221", style_guide.Decision.Selected), (["E2"], ["E21"], [], [], "E221", style_guide.Decision.Selected),
(["E2"], ["E21"], [], [], [], "E212", style_guide.Decision.Ignored), (["E2"], ["E21"], [], [], "E212", style_guide.Decision.Ignored),
( (
["F", "W"], ["F", "W"],
["C90"], ["C90"],
[], [],
["I1"], ["I1"],
[],
"C901", "C901",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
@ -317,25 +289,14 @@ def test_decision_for(
["C"], ["C"],
[], [],
[], [],
[],
"E131", "E131",
style_guide.Decision.Selected, style_guide.Decision.Selected,
), ),
(
defaults.SELECT,
defaults.IGNORE,
[],
[],
["I"],
"I101",
style_guide.Decision.Selected,
),
( (
defaults.SELECT, defaults.SELECT,
defaults.IGNORE, defaults.IGNORE,
[], [],
["G"], ["G"],
["I"],
"G101", "G101",
style_guide.Decision.Selected, style_guide.Decision.Selected,
), ),
@ -344,25 +305,14 @@ def test_decision_for(
["G1"], ["G1"],
[], [],
["G"], ["G"],
["I"],
"G101", "G101",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
(
defaults.SELECT,
["E126"],
[],
[],
["I"],
"I101",
style_guide.Decision.Selected,
),
( (
["E", "W"], ["E", "W"],
defaults.IGNORE, defaults.IGNORE,
[], [],
["I"], ["I"],
[],
"I101", "I101",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
@ -371,7 +321,6 @@ def test_decision_for(
defaults.IGNORE + ("I101",), defaults.IGNORE + ("I101",),
["I101"], ["I101"],
[], [],
[],
"I101", "I101",
style_guide.Decision.Selected, style_guide.Decision.Selected,
), ),
@ -380,7 +329,6 @@ def test_decision_for(
defaults.IGNORE + ("I101",), defaults.IGNORE + ("I101",),
["I101"], ["I101"],
[], [],
[],
"I101", "I101",
style_guide.Decision.Ignored, style_guide.Decision.Ignored,
), ),
@ -393,7 +341,6 @@ def test_more_specific_decision_for_logic(
ignore, ignore,
extended_default_ignore, extended_default_ignore,
extended_default_select, extended_default_select,
enabled_extensions,
error_code, error_code,
expected, expected,
): ):
@ -404,7 +351,6 @@ def test_more_specific_decision_for_logic(
ignore=ignore, ignore=ignore,
extended_default_select=extended_default_select, extended_default_select=extended_default_select,
extended_default_ignore=extended_default_ignore, extended_default_ignore=extended_default_ignore,
enable_extensions=enabled_extensions,
), ),
) )

View file

@ -19,6 +19,7 @@ def test_get_style_guide():
isolated=False, isolated=False,
output_file=None, output_file=None,
verbose=0, verbose=0,
enable_extensions=None,
) )
mockedapp = mock.Mock() mockedapp = mock.Mock()
mockedapp.parse_preliminary_options.return_value = (prelim_opts, []) mockedapp.parse_preliminary_options.return_value = (prelim_opts, [])
@ -34,7 +35,7 @@ def test_get_style_guide():
application.assert_called_once_with() application.assert_called_once_with()
mockedapp.parse_preliminary_options.assert_called_once_with([]) mockedapp.parse_preliminary_options.assert_called_once_with([])
mockedapp.find_plugins.assert_called_once_with(cfg, cfg_dir) mockedapp.find_plugins.assert_called_once_with(cfg, cfg_dir, None)
mockedapp.register_plugin_options.assert_called_once_with() mockedapp.register_plugin_options.assert_called_once_with()
mockedapp.parse_configuration_and_cli.assert_called_once_with( mockedapp.parse_configuration_and_cli.assert_called_once_with(
cfg, cfg_dir, [] cfg, cfg_dir, []