From 4f3087b369afaad7563dbb555bb3603b285827a9 Mon Sep 17 00:00:00 2001 From: Germain Chazot Date: Tue, 24 Jul 2018 23:11:24 +0100 Subject: [PATCH] Add minimal code to handle --prepend-config Add code to handle config files specified with the --prepend-config argument. Options specified in prepend files get a higher precedence than flake8 defaults but a lower one than user configs. This allows to override the defaults without affecting the behaviour of user and local config files. This implements a solution for issue #349 --- src/flake8/main/application.py | 3 + src/flake8/options/config.py | 44 ++++++++++- tests/integration/test_aggregator.py | 4 +- tests/unit/test_config_file_finder.py | 101 +++++++++++++++--------- tests/unit/test_get_local_plugins.py | 2 +- tests/unit/test_merged_config_parser.py | 2 +- 6 files changed, 111 insertions(+), 45 deletions(-) diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 9c15629..3cdabac 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -152,11 +152,14 @@ class Application(object): def make_config_finder(self): """Make our ConfigFileFinder based on preliminary opts and args.""" if self.config_finder is None: + prepend_config_files = utils.normalize_paths( + self.prelim_opts.prepend_config) extra_config_files = utils.normalize_paths( self.prelim_opts.append_config) self.config_finder = config.ConfigFileFinder( self.option_manager.program_name, self.prelim_args, + prepend_config_files, extra_config_files, ) diff --git a/src/flake8/options/config.py b/src/flake8/options/config.py index ba9442a..dfd7192 100644 --- a/src/flake8/options/config.py +++ b/src/flake8/options/config.py @@ -17,16 +17,30 @@ class ConfigFileFinder(object): PROJECT_FILENAMES = ('setup.cfg', 'tox.ini') - def __init__(self, program_name, args, extra_config_files): + def __init__( + self, program_name, args, + prepend_config_files, extra_config_files + ): """Initialize object to find config files. :param str program_name: Name of the current program (e.g., flake8). :param list args: The extra arguments passed on the command-line. + :param list prepend_config_files: + Extra configuration files specified by the user to read before user + and project configs. :param list extra_config_files: - Extra configuration files specified by the user to read. + Extra configuration files specified by the user to read after user + and project configs. """ + # The values of --prepend-config from the CLI + prepend_config_files = prepend_config_files or [] + self.prepend_config_files = [ + # Ensure the paths are absolute paths for local_config_files + os.path.abspath(f) for f in prepend_config_files + ] + # The values of --append-config from the CLI extra_config_files = extra_config_files or [] self.extra_config_files = [ @@ -57,6 +71,7 @@ class ConfigFileFinder(object): self._local_found_files = [] self._user_config = None self._cli_configs = {} + self._prepend_config = None @staticmethod def _read_config(files): @@ -153,6 +168,16 @@ class ConfigFileFinder(object): self._user_config = config return self._user_config + def prepend_configs(self): + """Return configuration as per prepend files found.""" + if self._prepend_config is None: + prepend_files = filter(os.path.exists, self.prepend_config_files) + config, found_files = self._read_config(prepend_files) + if found_files: + LOG.debug('Found prepend configuration files: %s', found_files) + self._prepend_config = config + return self._prepend_config + class MergedConfigParser(object): """Encapsulate merging different types of configuration files. @@ -258,6 +283,17 @@ class MergedConfigParser(object): LOG.debug('Parsing CLI configuration files.') return self._parse_config(config) + def parse_prepend_config(self): + """Parse and return the prepend configuration files.""" + config = self.config_finder.prepend_configs() + if not self.is_configured_by(config): + LOG.debug('Prepend configuration files have no %s section', + self.program_name) + return {} + + LOG.debug('Parsing prepend configuration files.') + return self._parse_config(config) + def merge_user_and_local_config(self): """Merge the parsed user and local configuration files. @@ -266,12 +302,16 @@ class MergedConfigParser(object): :rtype: dict """ + prepend_config = self.parse_prepend_config() user_config = self.parse_user_config() config = self.parse_local_config() for option, value in user_config.items(): config.setdefault(option, value) + for option, value in prepend_config.items(): + config.setdefault(option, value) + return config def parse(self, cli_config=None, isolated=False): diff --git a/tests/integration/test_aggregator.py b/tests/integration/test_aggregator.py index c789624..7939864 100644 --- a/tests/integration/test_aggregator.py +++ b/tests/integration/test_aggregator.py @@ -26,7 +26,7 @@ def test_aggregate_options_with_config(optmanager): """Verify we aggregate options and config values appropriately.""" arguments = ['flake8', '--config', CLI_SPECIFIED_CONFIG, '--select', 'E11,E34,E402,W,F', '--exclude', 'tests/*'] - config_finder = config.ConfigFileFinder('flake8', arguments, []) + config_finder = config.ConfigFileFinder('flake8', arguments, [], []) options, args = aggregator.aggregate_options( optmanager, config_finder, arguments) @@ -40,7 +40,7 @@ def test_aggregate_options_when_isolated(optmanager): """Verify we aggregate options and config values appropriately.""" arguments = ['flake8', '--isolated', '--select', 'E11,E34,E402,W,F', '--exclude', 'tests/*'] - config_finder = config.ConfigFileFinder('flake8', arguments, []) + config_finder = config.ConfigFileFinder('flake8', arguments, [], []) optmanager.extend_default_ignore(['E8']) options, args = aggregator.aggregate_options( optmanager, config_finder, arguments) diff --git a/tests/unit/test_config_file_finder.py b/tests/unit/test_config_file_finder.py index 933feb8..61f6009 100644 --- a/tests/unit/test_config_file_finder.py +++ b/tests/unit/test_config_file_finder.py @@ -15,7 +15,7 @@ BROKEN_CONFIG_PATH = 'tests/fixtures/config_files/broken.ini' def test_uses_default_args(): """Show that we default the args value.""" - finder = config.ConfigFileFinder('flake8', None, []) + finder = config.ConfigFileFinder('flake8', None, [], []) assert finder.parent == os.path.abspath('.') @@ -27,14 +27,14 @@ def test_uses_default_args(): def test_windows_detection(platform, is_windows): """Verify we detect Windows to the best of our knowledge.""" with mock.patch.object(sys, 'platform', platform): - finder = config.ConfigFileFinder('flake8', None, []) + finder = config.ConfigFileFinder('flake8', None, [], []) assert finder.is_windows is is_windows def test_cli_config(): """Verify opening and reading the file specified via the cli.""" cli_filepath = CLI_SPECIFIED_FILEPATH - finder = config.ConfigFileFinder('flake8', None, []) + finder = config.ConfigFileFinder('flake8', None, [], []) parsed_config = finder.cli_config(cli_filepath) assert parsed_config.has_section('flake8') @@ -42,7 +42,7 @@ def test_cli_config(): def test_cli_config_double_read(): """Second request for CLI config is cached.""" - finder = config.ConfigFileFinder('flake8', None, []) + finder = config.ConfigFileFinder('flake8', None, [], []) parsed_config = finder.cli_config(CLI_SPECIFIED_FILEPATH) boom = Exception("second request for CLI config not cached") @@ -68,59 +68,82 @@ def test_cli_config_double_read(): ]) def test_generate_possible_local_files(args, expected): """Verify generation of all possible config paths.""" - finder = config.ConfigFileFinder('flake8', args, []) + finder = config.ConfigFileFinder('flake8', args, [], []) assert (list(finder.generate_possible_local_files()) == expected) -@pytest.mark.parametrize('args,extra_config_files,expected', [ - # No arguments, common prefix of abspath('.') - ([], - [], - [os.path.abspath('setup.cfg'), - os.path.abspath('tox.ini')]), - # Common prefix of "flake8/" - (['flake8/options', 'flake8/'], - [], - [os.path.abspath('setup.cfg'), - os.path.abspath('tox.ini')]), - # Common prefix of "flake8/options" - (['flake8/options', 'flake8/options/sub'], - [], - [os.path.abspath('setup.cfg'), - os.path.abspath('tox.ini')]), - # Common prefix of "flake8/" with extra config files specified - (['flake8/'], - [CLI_SPECIFIED_FILEPATH], - [os.path.abspath('setup.cfg'), - os.path.abspath('tox.ini'), - os.path.abspath(CLI_SPECIFIED_FILEPATH)]), - # Common prefix of "flake8/" with missing extra config files specified - (['flake8/'], - [CLI_SPECIFIED_FILEPATH, - 'tests/fixtures/config_files/missing.ini'], - [os.path.abspath('setup.cfg'), - os.path.abspath('tox.ini'), - os.path.abspath(CLI_SPECIFIED_FILEPATH)]), -]) -def test_local_config_files(args, extra_config_files, expected): +@pytest.mark.parametrize( + 'args,prepend_config_files,extra_config_files,expected', [ + # No arguments, common prefix of abspath('.') + ([], + [], + [], + [os.path.abspath('setup.cfg'), + os.path.abspath('tox.ini')]), + # Common prefix of "flake8/" + (['flake8/options', 'flake8/'], + [], + [], + [os.path.abspath('setup.cfg'), + os.path.abspath('tox.ini')]), + # Common prefix of "flake8/options" + (['flake8/options', 'flake8/options/sub'], + [], + [], + [os.path.abspath('setup.cfg'), + os.path.abspath('tox.ini')]), + # Common prefix of "flake8/" with extra config files specified + (['flake8/'], + [], + [CLI_SPECIFIED_FILEPATH], + [os.path.abspath('setup.cfg'), + os.path.abspath('tox.ini'), + os.path.abspath(CLI_SPECIFIED_FILEPATH)]), + # Common prefix of "flake8/" with missing extra config files specified + (['flake8/'], + [], + [CLI_SPECIFIED_FILEPATH, + 'tests/fixtures/config_files/missing.ini'], + [os.path.abspath('setup.cfg'), + os.path.abspath('tox.ini'), + os.path.abspath(CLI_SPECIFIED_FILEPATH)]), + # Common prefix of "flake8/" with prepend config files specified + (['flake8/'], + [CLI_SPECIFIED_FILEPATH], + [], + [os.path.abspath('setup.cfg'), + os.path.abspath('tox.ini')]), + # Common prefix of "flake8/" with prepend and extra config files + (['flake8/'], + [CLI_SPECIFIED_FILEPATH], + [CLI_SPECIFIED_FILEPATH], + [os.path.abspath('setup.cfg'), + os.path.abspath('tox.ini'), + os.path.abspath(CLI_SPECIFIED_FILEPATH)]), + ]) +def test_local_config_files( + args, prepend_config_files, extra_config_files, expected +): """Verify discovery of local config files.""" - finder = config.ConfigFileFinder('flake8', args, extra_config_files) + finder = config.ConfigFileFinder( + 'flake8', args, prepend_config_files, extra_config_files + ) assert list(finder.local_config_files()) == expected def test_local_configs(): """Verify we return a ConfigParser.""" - finder = config.ConfigFileFinder('flake8', None, []) + finder = config.ConfigFileFinder('flake8', None, [], []) assert isinstance(finder.local_configs(), configparser.RawConfigParser) def test_local_configs_double_read(): """Second request for local configs is cached.""" - finder = config.ConfigFileFinder('flake8', None, []) + finder = config.ConfigFileFinder('flake8', None, [], []) first_read = finder.local_configs() boom = Exception("second request for local configs not cached") diff --git a/tests/unit/test_get_local_plugins.py b/tests/unit/test_get_local_plugins.py index b2f985e..fa18ac2 100644 --- a/tests/unit/test_get_local_plugins.py +++ b/tests/unit/test_get_local_plugins.py @@ -31,7 +31,7 @@ def test_get_local_plugins_uses_cli_config(): def test_get_local_plugins(): """Verify get_local_plugins returns expected plugins.""" config_fixture_path = 'tests/fixtures/config_files/local-plugin.ini' - config_finder = config.ConfigFileFinder('flake8', [], []) + config_finder = config.ConfigFileFinder('flake8', [], [], []) with mock.patch.object(config_finder, 'local_config_files') as localcfs: localcfs.return_value = [config_fixture_path] diff --git a/tests/unit/test_merged_config_parser.py b/tests/unit/test_merged_config_parser.py index 37d5e6f..bdf2f41 100644 --- a/tests/unit/test_merged_config_parser.py +++ b/tests/unit/test_merged_config_parser.py @@ -17,7 +17,7 @@ def optmanager(): @pytest.fixture def config_finder(): """Generate a simple ConfigFileFinder.""" - return config.ConfigFileFinder('flake8', [], []) + return config.ConfigFileFinder('flake8', [], [], []) def test_parse_cli_config(optmanager, config_finder):