From a5c17c1a19bd0478a8e7543d13cce8fdd30488f2 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 7 Jan 2020 12:39:21 -0500 Subject: [PATCH 1/3] config: Add 'ignore_config_files' parameter to ConfigFileFinder The `--isolated` flag is passed into `MergedConfigParser.parse()` and the module-level function `config.get_local_plugins()`. Since both of these places utilize the `ConfigFileFinder` object and isolation pertains to how the `ConfigFileFinder` should behave with respect to isolation, this incremental change more directly associates the `ConfigFileFinder` and configuration file isolate. --- src/flake8/main/application.py | 4 +++- src/flake8/options/config.py | 11 +++++++++-- tests/unit/test_config_file_finder.py | 20 ++++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 791d5af..7ea3fbb 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -335,7 +335,9 @@ class Application(object): prelim_opts, remaining_args = self.parse_preliminary_options(argv) flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) config_finder = config.ConfigFileFinder( - self.program, prelim_opts.append_config + self.program, + prelim_opts.append_config, + ignore_config_files=prelim_opts.isolated, ) self.find_plugins( config_finder, prelim_opts.config, prelim_opts.isolated diff --git a/src/flake8/options/config.py b/src/flake8/options/config.py index e1952be..e921f6e 100644 --- a/src/flake8/options/config.py +++ b/src/flake8/options/config.py @@ -16,19 +16,26 @@ __all__ = ("ConfigFileFinder", "MergedConfigParser") class ConfigFileFinder(object): """Encapsulate the logic for finding and reading config files.""" - def __init__(self, program_name, extra_config_files): - # type: (str, List[str]) -> None + def __init__( + self, program_name, extra_config_files, ignore_config_files=False + ): + # type: (str, List[str], bool) -> None """Initialize object to find config files. :param str program_name: Name of the current program (e.g., flake8). :param list extra_config_files: Extra configuration files specified by the user to read. + :param bool ignore_config_files: + Determine whether to ignore configuration files or not. """ # The values of --append-config from the CLI extra_config_files = extra_config_files or [] self.extra_config_files = utils.normalize_paths(extra_config_files) + # The value of --isolated from the CLI. + self.ignore_config_files = ignore_config_files + # Platform specific settings self.is_windows = sys.platform == "win32" self.xdg_home = os.environ.get( diff --git a/tests/unit/test_config_file_finder.py b/tests/unit/test_config_file_finder.py index b562c4a..44da8d4 100644 --- a/tests/unit/test_config_file_finder.py +++ b/tests/unit/test_config_file_finder.py @@ -124,3 +124,23 @@ def test_read_config_catches_decoding_errors(tmpdir): setup_cfg.write_binary(b'[x]\ny = \x81\x8d\x90\x9d') _, parsed = config.ConfigFileFinder._read_config(setup_cfg.strpath) assert parsed == [] + + +def test_ignore_config_files_default_value(): + """Verify the default 'ignore_config_files' attribute value.""" + finder = config.ConfigFileFinder('flake8', []) + assert finder.ignore_config_files is False + + +@pytest.mark.parametrize('ignore_config_files_arg', [ + False, + True, +]) +def test_setting_ignore_config_files_value(ignore_config_files_arg): + """Verify the 'ignore_config_files' attribute matches constructed value.""" + finder = config.ConfigFileFinder( + 'flake8', + [], + ignore_config_files=ignore_config_files_arg + ) + assert finder.ignore_config_files is ignore_config_files_arg From 3d546d448a22b131f42444657f0985b37b252097 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 7 Jan 2020 13:03:34 -0500 Subject: [PATCH 2/3] config: Switch code paths to use 'ConfigFileFinder.ignore_config_files' Now that the `ConfigFileFinder` has the `.ignore_config_files` attribute, switch the relevant code paths to utilize this public attribute. Tests have been updated to either construct `ConfigFileFinder` or mock the object appropriately. --- src/flake8/options/config.py | 4 ++-- tests/integration/test_aggregator.py | 3 ++- tests/unit/test_get_local_plugins.py | 1 + tests/unit/test_merged_config_parser.py | 1 + 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/flake8/options/config.py b/src/flake8/options/config.py index e921f6e..ead12dc 100644 --- a/src/flake8/options/config.py +++ b/src/flake8/options/config.py @@ -307,7 +307,7 @@ class MergedConfigParser(object): :rtype: dict """ - if isolated: + if self.config_finder.ignore_config_files: LOG.debug( "Refusing to parse configuration files due to user-" "requested isolation" @@ -344,7 +344,7 @@ def get_local_plugins(config_finder, cli_config=None, isolated=False): flake8.options.config.LocalPlugins """ local_plugins = LocalPlugins(extension=[], report=[], paths=[]) - if isolated: + if config_finder.ignore_config_files: LOG.debug( "Refusing to look for local plugins in configuration" "files due to user-requested isolation" diff --git a/tests/integration/test_aggregator.py b/tests/integration/test_aggregator.py index 18ce5e8..636b3d5 100644 --- a/tests/integration/test_aggregator.py +++ b/tests/integration/test_aggregator.py @@ -43,7 +43,8 @@ def test_aggregate_options_when_isolated(optmanager): """Verify we aggregate options and config values appropriately.""" arguments = ['flake8', '--select', 'E11,E34,E402,W,F', '--exclude', 'tests/*'] - config_finder = config.ConfigFileFinder('flake8', []) + config_finder = config.ConfigFileFinder( + 'flake8', [], ignore_config_files=True) optmanager.extend_default_ignore(['E8']) options, args = aggregator.aggregate_options( optmanager, config_finder, None, True, arguments) diff --git a/tests/unit/test_get_local_plugins.py b/tests/unit/test_get_local_plugins.py index fa9c1a4..db62e33 100644 --- a/tests/unit/test_get_local_plugins.py +++ b/tests/unit/test_get_local_plugins.py @@ -21,6 +21,7 @@ def test_get_local_plugins_uses_cli_config(): config_obj = mock.Mock() config_finder = mock.MagicMock() config_finder.cli_config.return_value = config_obj + config_finder.ignore_config_files = False config_obj.get.return_value = '' config.get_local_plugins(config_finder, cli_config='foo.ini') diff --git a/tests/unit/test_merged_config_parser.py b/tests/unit/test_merged_config_parser.py index a28a4c3..186d853 100644 --- a/tests/unit/test_merged_config_parser.py +++ b/tests/unit/test_merged_config_parser.py @@ -155,6 +155,7 @@ def test_parse_isolates_config(optmanager): def test_parse_uses_cli_config(optmanager): """Verify behaviour of the parse method with a specified config.""" config_finder = mock.MagicMock() + config_finder.ignore_config_files = False parser = config.MergedConfigParser(optmanager, config_finder) parser.parse(cli_config='foo.ini') From c918e7249699ab91ccbd6d69a898d19aa009c971 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 7 Jan 2020 13:14:34 -0500 Subject: [PATCH 3/3] Remove unused 'isolated' parameter Now that `ConfigFileFinder.ignore_config_files` attribute is used everywhere and is constructed from the `--isolated` CLI option, the now unused `isolated` parameters can be safely removed. --- src/flake8/api/legacy.py | 13 +++++------- src/flake8/main/application.py | 27 ++++++------------------- src/flake8/options/aggregator.py | 6 +----- src/flake8/options/config.py | 10 ++------- tests/integration/test_aggregator.py | 4 ++-- tests/unit/test_get_local_plugins.py | 3 ++- tests/unit/test_legacy_api.py | 4 ++-- tests/unit/test_merged_config_parser.py | 3 ++- 8 files changed, 22 insertions(+), 48 deletions(-) diff --git a/src/flake8/api/legacy.py b/src/flake8/api/legacy.py index fe3b405..40da7db 100644 --- a/src/flake8/api/legacy.py +++ b/src/flake8/api/legacy.py @@ -32,18 +32,15 @@ def get_style_guide(**kwargs): prelim_opts, remaining_args = application.parse_preliminary_options([]) flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) config_finder = config.ConfigFileFinder( - application.program, prelim_opts.append_config + application.program, + prelim_opts.append_config, + ignore_config_files=prelim_opts.isolated, ) - application.find_plugins( - config_finder, prelim_opts.config, prelim_opts.isolated - ) + application.find_plugins(config_finder, prelim_opts.config) application.register_plugin_options() application.parse_configuration_and_cli( - config_finder, - prelim_opts.config, - prelim_opts.isolated, - remaining_args, + config_finder, prelim_opts.config, remaining_args, ) # We basically want application.initialize to be called but with these # options set instead before we make our formatter, notifier, internal diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 7ea3fbb..a30f195 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -131,8 +131,8 @@ class Application(object): (self.result_count > 0) or self.catastrophic_failure ) - def find_plugins(self, config_finder, config_file, ignore_config_files): - # type: (config.ConfigFileFinder, Optional[str], bool) -> None + def find_plugins(self, config_finder, config_file): + # type: (config.ConfigFileFinder, Optional[str]) -> None """Find and load the plugins for this application. Set the :attr:`check_plugins` and :attr:`formatting_plugins` attributes @@ -147,9 +147,7 @@ class Application(object): Determine whether to parse configuration files or not. (i.e., the --isolated option). """ - local_plugins = config.get_local_plugins( - config_finder, config_file, ignore_config_files - ) + local_plugins = config.get_local_plugins(config_finder, config_file) sys.path.extend(local_plugins.paths) @@ -173,7 +171,6 @@ class Application(object): self, config_finder, # type: config.ConfigFileFinder config_file, # type: Optional[str] - ignore_config_files, # type: bool argv, # type: List[str] ): # type: (...) -> None @@ -184,18 +181,11 @@ class Application(object): :param str config_file: The optional configuraiton file to override all other configuration files (i.e., the --config option). - :param bool ignore_config_files: - Determine whether to parse configuration files or not. (i.e., the - --isolated option). :param list argv: Command-line arguments passed in directly. """ self.options, self.args = aggregator.aggregate_options( - self.option_manager, - config_finder, - config_file, - ignore_config_files, - argv, + self.option_manager, config_finder, config_file, argv, ) self.running_against_diff = self.options.diff @@ -339,15 +329,10 @@ class Application(object): prelim_opts.append_config, ignore_config_files=prelim_opts.isolated, ) - self.find_plugins( - config_finder, prelim_opts.config, prelim_opts.isolated - ) + self.find_plugins(config_finder, prelim_opts.config) self.register_plugin_options() self.parse_configuration_and_cli( - config_finder, - prelim_opts.config, - prelim_opts.isolated, - remaining_args, + config_finder, prelim_opts.config, remaining_args, ) self.make_formatter() self.make_guide() diff --git a/src/flake8/options/aggregator.py b/src/flake8/options/aggregator.py index 58d1022..939dfb3 100644 --- a/src/flake8/options/aggregator.py +++ b/src/flake8/options/aggregator.py @@ -17,7 +17,6 @@ def aggregate_options( manager, # type: OptionManager config_finder, # type: config.ConfigFileFinder cli_config, # type: Optional[str] - isolated, # type: bool argv, # type: List[str] ): # type: (...) -> Tuple[argparse.Namespace, List[str]] """Aggregate and merge CLI and config file options. @@ -29,9 +28,6 @@ def aggregate_options( :param str cli_config: Value of --config when specified at the command-line. Overrides all other config files. - :param bool isolated: - Determines if we should parse configuration files at all or not. - If running in isolated mode, we ignore all configuration files :param list argv: The list of remaining command-line argumentsthat were unknown during preliminary option parsing to pass to ``manager.parse_args``. @@ -50,7 +46,7 @@ def aggregate_options( ) # Get the parsed config - parsed_config = config_parser.parse(cli_config, isolated) + parsed_config = config_parser.parse(cli_config) # Extend the default ignore value with the extended default ignore list, # registered by plugins. diff --git a/src/flake8/options/config.py b/src/flake8/options/config.py index ead12dc..7261bcd 100644 --- a/src/flake8/options/config.py +++ b/src/flake8/options/config.py @@ -289,7 +289,7 @@ class MergedConfigParser(object): return config - def parse(self, cli_config=None, isolated=False): + def parse(self, cli_config=None): """Parse and return the local and user config files. First this copies over the parsed local configuration and then @@ -299,9 +299,6 @@ class MergedConfigParser(object): :param str cli_config: Value of --config when specified at the command-line. Overrides all other config files. - :param bool isolated: - Determines if we should parse configuration files at all or not. - If running in isolated mode, we ignore all configuration files :returns: Dictionary of parsed configuration options :rtype: @@ -326,7 +323,7 @@ class MergedConfigParser(object): return self.merge_user_and_local_config() -def get_local_plugins(config_finder, cli_config=None, isolated=False): +def get_local_plugins(config_finder, cli_config=None): """Get local plugins lists from config files. :param flake8.options.config.ConfigFileFinder config_finder: @@ -334,9 +331,6 @@ def get_local_plugins(config_finder, cli_config=None, isolated=False): :param str cli_config: Value of --config when specified at the command-line. Overrides all other config files. - :param bool isolated: - Determines if we should parse configuration files at all or not. - If running in isolated mode, we ignore all configuration files :returns: LocalPlugins namedtuple containing two lists of plugin strings, one for extension (checker) plugins and one for report plugins. diff --git a/tests/integration/test_aggregator.py b/tests/integration/test_aggregator.py index 636b3d5..672c865 100644 --- a/tests/integration/test_aggregator.py +++ b/tests/integration/test_aggregator.py @@ -32,7 +32,7 @@ def test_aggregate_options_with_config(optmanager): 'E11,E34,E402,W,F', '--exclude', 'tests/*'] config_finder = config.ConfigFileFinder('flake8', []) options, args = aggregator.aggregate_options( - optmanager, config_finder, CLI_SPECIFIED_CONFIG, False, arguments) + optmanager, config_finder, CLI_SPECIFIED_CONFIG, arguments) assert options.select == ['E11', 'E34', 'E402', 'W', 'F'] assert options.ignore == ['E123', 'W234', 'E111'] @@ -47,7 +47,7 @@ def test_aggregate_options_when_isolated(optmanager): 'flake8', [], ignore_config_files=True) optmanager.extend_default_ignore(['E8']) options, args = aggregator.aggregate_options( - optmanager, config_finder, None, True, arguments) + optmanager, config_finder, None, arguments) assert options.select == ['E11', 'E34', 'E402', 'W', 'F'] assert sorted(options.ignore) == [ diff --git a/tests/unit/test_get_local_plugins.py b/tests/unit/test_get_local_plugins.py index db62e33..6311677 100644 --- a/tests/unit/test_get_local_plugins.py +++ b/tests/unit/test_get_local_plugins.py @@ -7,8 +7,9 @@ from flake8.options import config def test_get_local_plugins_respects_isolated(): """Verify behaviour of get_local_plugins with isolated=True.""" config_finder = mock.MagicMock() + config_finder.ignore_config_files = True - local_plugins = config.get_local_plugins(config_finder, isolated=True) + local_plugins = config.get_local_plugins(config_finder) assert local_plugins.extension == [] assert local_plugins.report == [] diff --git a/tests/unit/test_legacy_api.py b/tests/unit/test_legacy_api.py index d3b9eb5..2ec836c 100644 --- a/tests/unit/test_legacy_api.py +++ b/tests/unit/test_legacy_api.py @@ -31,10 +31,10 @@ 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(config_finder, None, False) + mockedapp.find_plugins.assert_called_once_with(config_finder, None) mockedapp.register_plugin_options.assert_called_once_with() mockedapp.parse_configuration_and_cli.assert_called_once_with( - config_finder, None, False, []) + config_finder, None, []) mockedapp.make_formatter.assert_called_once_with() mockedapp.make_guide.assert_called_once_with() mockedapp.make_file_checker_manager.assert_called_once_with() diff --git a/tests/unit/test_merged_config_parser.py b/tests/unit/test_merged_config_parser.py index 186d853..f2d70d9 100644 --- a/tests/unit/test_merged_config_parser.py +++ b/tests/unit/test_merged_config_parser.py @@ -145,9 +145,10 @@ def test_merge_user_and_local_config(optmanager, config_finder): def test_parse_isolates_config(optmanager): """Verify behaviour of the parse method with isolated=True.""" config_finder = mock.MagicMock() + config_finder.ignore_config_files = True parser = config.MergedConfigParser(optmanager, config_finder) - assert parser.parse(isolated=True) == {} + assert parser.parse() == {} assert config_finder.local_configs.called is False assert config_finder.user_config.called is False