From 153032f778d609b206cbfdb56ddf27e6f46925c4 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Sun, 12 Jan 2020 14:41:20 -0800 Subject: [PATCH 1/3] config: Add 'config_file' parameter to ConfigFileFinder The `--config` 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 the configuration file override pertains to how configuration behaves, this incremental change directly associates the `ConfigFileFinder` and the configuration file override. --- src/flake8/main/application.py | 1 + src/flake8/options/config.py | 15 ++++++++++++--- tests/unit/test_config_file_finder.py | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 6a59e1a..b96a605 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -324,6 +324,7 @@ class Application(object): config_finder = config.ConfigFileFinder( self.program, prelim_opts.append_config, + config_file=prelim_opts.config, ignore_config_files=prelim_opts.isolated, ) self.find_plugins(config_finder, prelim_opts.config) diff --git a/src/flake8/options/config.py b/src/flake8/options/config.py index dab16c4..76a81cf 100644 --- a/src/flake8/options/config.py +++ b/src/flake8/options/config.py @@ -4,7 +4,7 @@ import configparser import logging import os.path import sys -from typing import Dict, List, Tuple +from typing import Dict, List, Optional, Tuple from flake8 import utils @@ -17,15 +17,21 @@ class ConfigFileFinder(object): """Encapsulate the logic for finding and reading config files.""" def __init__( - self, program_name, extra_config_files, ignore_config_files=False + self, + program_name, + extra_config_files, + config_file=None, + ignore_config_files=False, ): - # type: (str, List[str], bool) -> None + # type: (str, List[str], Optional[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 str config_file: + Configuration file override to only read configuraiton from. :param bool ignore_config_files: Determine whether to ignore configuration files or not. """ @@ -33,6 +39,9 @@ class ConfigFileFinder(object): extra_config_files = extra_config_files or [] self.extra_config_files = utils.normalize_paths(extra_config_files) + # The value of --config from the CLI. + self.config_file = config_file + # The value of --isolated from the CLI. self.ignore_config_files = ignore_config_files diff --git a/tests/unit/test_config_file_finder.py b/tests/unit/test_config_file_finder.py index d03118a..93d515a 100644 --- a/tests/unit/test_config_file_finder.py +++ b/tests/unit/test_config_file_finder.py @@ -127,6 +127,23 @@ def test_read_config_catches_decoding_errors(tmpdir): assert parsed == [] +def test_config_file_default_value(): + """Verify the default 'config_file' attribute value.""" + finder = config.ConfigFileFinder('flake8', []) + assert finder.config_file is None + + +def test_setting_config_file_value(): + """Verify the 'config_file' attribute matches constructed value.""" + config_file_value = 'flake8.ini' + finder = config.ConfigFileFinder( + 'flake8', + [], + config_file=config_file_value, + ) + assert finder.config_file == config_file_value + + def test_ignore_config_files_default_value(): """Verify the default 'ignore_config_files' attribute value.""" finder = config.ConfigFileFinder('flake8', []) From 77b2506071bec65d75e5259e00ceefb91ae50cf1 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Sun, 12 Jan 2020 15:13:41 -0800 Subject: [PATCH 2/3] config: Switch code paths to use 'ConfigFileFinder.config_file' Now that the `ConfigFileFinder` has the `.config_file` 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 | 14 +++++++------- tests/integration/test_aggregator.py | 5 ++++- tests/unit/test_get_local_plugins.py | 6 ++++-- tests/unit/test_merged_config_parser.py | 6 ++++-- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/flake8/options/config.py b/src/flake8/options/config.py index 76a81cf..a4a9783 100644 --- a/src/flake8/options/config.py +++ b/src/flake8/options/config.py @@ -317,14 +317,14 @@ class MergedConfigParser(object): ) return {} - if cli_config: + if self.config_finder.config_file: LOG.debug( "Ignoring user and locally found configuration files. " 'Reading only configuration from "%s" specified via ' "--config by the user", - cli_config, + self.config_finder.config_file, ) - return self.parse_cli_config(cli_config) + return self.parse_cli_config(self.config_finder.config_file) return self.merge_user_and_local_config() @@ -351,14 +351,14 @@ def get_local_plugins(config_finder, cli_config=None): ) return local_plugins - if cli_config: + if config_finder.config_file: LOG.debug( 'Reading local plugins only from "%s" specified via ' "--config by the user", - cli_config, + config_finder.config_file, ) - config = config_finder.cli_config(cli_config) - config_files = [cli_config] + config = config_finder.cli_config(config_finder.config_file) + config_files = [config_finder.config_file] else: config, config_files = config_finder.local_configs_with_files() diff --git a/tests/integration/test_aggregator.py b/tests/integration/test_aggregator.py index 672c865..e365dfc 100644 --- a/tests/integration/test_aggregator.py +++ b/tests/integration/test_aggregator.py @@ -30,7 +30,10 @@ def test_aggregate_options_with_config(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', + [], + config_file=CLI_SPECIFIED_CONFIG) options, args = aggregator.aggregate_options( optmanager, config_finder, CLI_SPECIFIED_CONFIG, arguments) diff --git a/tests/unit/test_get_local_plugins.py b/tests/unit/test_get_local_plugins.py index 6311677..2a23d7a 100644 --- a/tests/unit/test_get_local_plugins.py +++ b/tests/unit/test_get_local_plugins.py @@ -24,10 +24,12 @@ def test_get_local_plugins_uses_cli_config(): config_finder.cli_config.return_value = config_obj config_finder.ignore_config_files = False config_obj.get.return_value = '' + config_file_value = 'foo.ini' + config_finder.config_file = config_file_value - config.get_local_plugins(config_finder, cli_config='foo.ini') + config.get_local_plugins(config_finder, cli_config=config_file_value) - config_finder.cli_config.assert_called_once_with('foo.ini') + config_finder.cli_config.assert_called_once_with(config_file_value) def test_get_local_plugins(): diff --git a/tests/unit/test_merged_config_parser.py b/tests/unit/test_merged_config_parser.py index f2d70d9..3881699 100644 --- a/tests/unit/test_merged_config_parser.py +++ b/tests/unit/test_merged_config_parser.py @@ -155,12 +155,14 @@ def test_parse_isolates_config(optmanager): def test_parse_uses_cli_config(optmanager): """Verify behaviour of the parse method with a specified config.""" + config_file_value = 'foo.ini' config_finder = mock.MagicMock() + config_finder.config_file = config_file_value config_finder.ignore_config_files = False parser = config.MergedConfigParser(optmanager, config_finder) - parser.parse(cli_config='foo.ini') - config_finder.cli_config.assert_called_once_with('foo.ini') + parser.parse(cli_config=config_file_value) + config_finder.cli_config.assert_called_once_with(config_file_value) @pytest.mark.parametrize('config_fixture_path', [ From 1e3bad20dd17981b66121d959b28faf6614edd7e Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Sun, 12 Jan 2020 15:33:54 -0800 Subject: [PATCH 3/3] Remove unused 'cli_config' parameter Now that `ConfigFileFinder.config_file` attribute is used everywhere and is constructed from the `--config` CLI option, the now unused `cli_config` parameters can be safely removed. --- src/flake8/api/legacy.py | 5 +++-- src/flake8/main/application.py | 19 ++++++------------- src/flake8/options/aggregator.py | 8 ++------ src/flake8/options/config.py | 10 ++-------- tests/integration/test_aggregator.py | 4 ++-- tests/unit/test_get_local_plugins.py | 2 +- tests/unit/test_legacy_api.py | 4 ++-- tests/unit/test_merged_config_parser.py | 2 +- 8 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/flake8/api/legacy.py b/src/flake8/api/legacy.py index 40da7db..68df9a2 100644 --- a/src/flake8/api/legacy.py +++ b/src/flake8/api/legacy.py @@ -34,13 +34,14 @@ def get_style_guide(**kwargs): config_finder = config.ConfigFileFinder( application.program, prelim_opts.append_config, + config_file=prelim_opts.config, ignore_config_files=prelim_opts.isolated, ) - application.find_plugins(config_finder, prelim_opts.config) + application.find_plugins(config_finder) application.register_plugin_options() application.parse_configuration_and_cli( - config_finder, prelim_opts.config, remaining_args, + config_finder, 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 b96a605..001ad6c 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): - # type: (config.ConfigFileFinder, Optional[str]) -> None + def find_plugins(self, config_finder): + # type: (config.ConfigFileFinder) -> None """Find and load the plugins for this application. Set the :attr:`check_plugins` and :attr:`formatting_plugins` attributes @@ -140,11 +140,8 @@ class Application(object): :param config.ConfigFileFinder config_finder: The finder for finding and reading configuration files. - :param str config_file: - The optional configuraiton file to override all other configuration - files (i.e., the --config option). """ - local_plugins = config.get_local_plugins(config_finder, config_file) + local_plugins = config.get_local_plugins(config_finder) sys.path.extend(local_plugins.paths) @@ -167,7 +164,6 @@ class Application(object): def parse_configuration_and_cli( self, config_finder, # type: config.ConfigFileFinder - config_file, # type: Optional[str] argv, # type: List[str] ): # type: (...) -> None @@ -175,14 +171,11 @@ class Application(object): :param config.ConfigFileFinder config_finder: The finder for finding and reading configuration files. - :param str config_file: - The optional configuraiton file to override all other configuration - files (i.e., the --config option). :param list argv: Command-line arguments passed in directly. """ self.options, self.args = aggregator.aggregate_options( - self.option_manager, config_finder, config_file, argv, + self.option_manager, config_finder, argv, ) self.running_against_diff = self.options.diff @@ -327,10 +320,10 @@ class Application(object): config_file=prelim_opts.config, ignore_config_files=prelim_opts.isolated, ) - self.find_plugins(config_finder, prelim_opts.config) + self.find_plugins(config_finder) self.register_plugin_options() self.parse_configuration_and_cli( - config_finder, prelim_opts.config, remaining_args, + config_finder, remaining_args, ) self.make_formatter() self.make_guide() diff --git a/src/flake8/options/aggregator.py b/src/flake8/options/aggregator.py index 939dfb3..bdcd6a3 100644 --- a/src/flake8/options/aggregator.py +++ b/src/flake8/options/aggregator.py @@ -5,7 +5,7 @@ applies the user-specified command-line configuration on top of it. """ import argparse import logging -from typing import List, Optional, Tuple +from typing import List, Tuple from flake8.options import config from flake8.options.manager import OptionManager @@ -16,7 +16,6 @@ LOG = logging.getLogger(__name__) def aggregate_options( manager, # type: OptionManager config_finder, # type: config.ConfigFileFinder - cli_config, # type: Optional[str] argv, # type: List[str] ): # type: (...) -> Tuple[argparse.Namespace, List[str]] """Aggregate and merge CLI and config file options. @@ -25,9 +24,6 @@ def aggregate_options( The instance of the OptionManager that we're presently using. :param flake8.options.config.ConfigFileFinder config_finder: The config file finder to use. - :param str cli_config: - Value of --config when specified at the command-line. Overrides - all other config files. :param list argv: The list of remaining command-line argumentsthat were unknown during preliminary option parsing to pass to ``manager.parse_args``. @@ -46,7 +42,7 @@ def aggregate_options( ) # Get the parsed config - parsed_config = config_parser.parse(cli_config) + parsed_config = config_parser.parse() # 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 a4a9783..af163ca 100644 --- a/src/flake8/options/config.py +++ b/src/flake8/options/config.py @@ -295,16 +295,13 @@ class MergedConfigParser(object): return config - def parse(self, cli_config=None): + def parse(self): """Parse and return the local and user config files. First this copies over the parsed local configuration and then iterates over the options in the user configuration and sets them if they were not set by the local configuration file. - :param str cli_config: - Value of --config when specified at the command-line. Overrides - all other config files. :returns: Dictionary of parsed configuration options :rtype: @@ -329,14 +326,11 @@ class MergedConfigParser(object): return self.merge_user_and_local_config() -def get_local_plugins(config_finder, cli_config=None): +def get_local_plugins(config_finder): """Get local plugins lists from config files. :param flake8.options.config.ConfigFileFinder config_finder: The config file finder to use. - :param str cli_config: - Value of --config when specified at the command-line. Overrides - all other config 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 e365dfc..16ed59b 100644 --- a/tests/integration/test_aggregator.py +++ b/tests/integration/test_aggregator.py @@ -35,7 +35,7 @@ def test_aggregate_options_with_config(optmanager): [], config_file=CLI_SPECIFIED_CONFIG) options, args = aggregator.aggregate_options( - optmanager, config_finder, CLI_SPECIFIED_CONFIG, arguments) + optmanager, config_finder, arguments) assert options.select == ['E11', 'E34', 'E402', 'W', 'F'] assert options.ignore == ['E123', 'W234', 'E111'] @@ -50,7 +50,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, arguments) + optmanager, config_finder, 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 2a23d7a..351be20 100644 --- a/tests/unit/test_get_local_plugins.py +++ b/tests/unit/test_get_local_plugins.py @@ -27,7 +27,7 @@ def test_get_local_plugins_uses_cli_config(): config_file_value = 'foo.ini' config_finder.config_file = config_file_value - config.get_local_plugins(config_finder, cli_config=config_file_value) + config.get_local_plugins(config_finder) config_finder.cli_config.assert_called_once_with(config_file_value) diff --git a/tests/unit/test_legacy_api.py b/tests/unit/test_legacy_api.py index 2ec836c..7c7a47e 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) + mockedapp.find_plugins.assert_called_once_with(config_finder) mockedapp.register_plugin_options.assert_called_once_with() mockedapp.parse_configuration_and_cli.assert_called_once_with( - config_finder, None, []) + config_finder, []) 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 3881699..6689776 100644 --- a/tests/unit/test_merged_config_parser.py +++ b/tests/unit/test_merged_config_parser.py @@ -161,7 +161,7 @@ def test_parse_uses_cli_config(optmanager): config_finder.ignore_config_files = False parser = config.MergedConfigParser(optmanager, config_finder) - parser.parse(cli_config=config_file_value) + parser.parse() config_finder.cli_config.assert_called_once_with(config_file_value)