From 6043e908552cd38a75098c99a739497744170e81 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 1 Oct 2019 08:48:18 +0200 Subject: [PATCH 1/5] application: Return namespace and args from preliminary arg parsing This is the initial step towards removing state from the `Application` object during argument parsing and handling. The goal is to remove `Application.prelim_opts` and `Application.prelim_args`. --- src/flake8/main/application.py | 9 +++++++-- tests/unit/test_application.py | 8 ++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 6393dad..65a17d5 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -5,7 +5,7 @@ import argparse import logging import sys import time -from typing import Dict, List, Optional, Set +from typing import Dict, List, Optional, Set, Tuple import flake8 from flake8 import checker @@ -98,7 +98,7 @@ class Application(object): self.parsed_diff = {} # type: Dict[str, Set[int]] def parse_preliminary_options_and_args(self, argv): - # type: (List[str]) -> None + # type: (List[str]) -> Tuple[argparse.Namespace, List[str]] """Get preliminary options and args from CLI, pre-plugin-loading. We need to know the values of a few standard options and args now, so @@ -112,6 +112,10 @@ class Application(object): :param list argv: Command-line arguments passed in directly. + :returns: + Populated namespace and list of remaining argument strings. + :rtype: + (argparse.Namespace, list) """ # We haven't found or registered our plugins yet, so let's defer # printing the version until we aggregate options from config files @@ -139,6 +143,7 @@ class Application(object): # parse_known_args includes unknown options as args args = [a for a in args if not a.startswith("-")] self.prelim_opts, self.prelim_args = opts, args + return opts, args def exit(self): # type: () -> None diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 8e5c9d5..ea4f090 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -92,18 +92,22 @@ def test_returns_specified_plugin(application): def test_prelim_opts_args(application): """Verify we get sensible prelim opts and args.""" - application.parse_preliminary_options_and_args( + opts, args = application.parse_preliminary_options_and_args( ['flake8', '--foo', '--verbose', 'src', 'setup.py', '--statistics']) assert application.prelim_opts.statistics assert application.prelim_opts.verbose assert application.prelim_args == ['src', 'setup.py'] + assert opts.statistics is application.prelim_opts.statistics + assert opts.verbose is application.prelim_opts.verbose + assert args is application.prelim_args def test_prelim_opts_handles_empty(application): """Verify empty argv lists are handled correctly.""" irrelevant_args = ['myexe', '/path/to/foo'] with mock.patch.object(sys, 'argv', irrelevant_args): - application.parse_preliminary_options_and_args([]) + opts, args = application.parse_preliminary_options_and_args([]) assert application.prelim_args == [] + assert args is application.prelim_args From 55ef2c6f5eae1617dcbbb51636e9280aa8870e02 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 1 Oct 2019 08:48:18 +0200 Subject: [PATCH 2/5] application: Pass returned prelim options to `.configure_logging()` The verbosity and output file options can be obtained from options returned by `.parse_preliminary_options_and_args()`, instead of state from the `Application` object. --- src/flake8/api/legacy.py | 6 +++--- src/flake8/main/application.py | 6 +++--- tests/unit/test_legacy_api.py | 13 +++++++++++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/flake8/api/legacy.py b/src/flake8/api/legacy.py index 16a33f4..6cff7ad 100644 --- a/src/flake8/api/legacy.py +++ b/src/flake8/api/legacy.py @@ -28,10 +28,10 @@ def get_style_guide(**kwargs): :class:`StyleGuide` """ application = app.Application() - application.parse_preliminary_options_and_args([]) - flake8.configure_logging( - application.prelim_opts.verbose, application.prelim_opts.output_file + prelim_opts, prelim_args = application.parse_preliminary_options_and_args( + [] ) + flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) application.make_config_finder() application.find_plugins() application.register_plugin_options() diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 65a17d5..dc0f237 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -357,10 +357,10 @@ class Application(object): """ # NOTE(sigmavirus24): When updating this, make sure you also update # our legacy API calls to these same methods. - self.parse_preliminary_options_and_args(argv) - flake8.configure_logging( - self.prelim_opts.verbose, self.prelim_opts.output_file + prelim_opts, prelim_args = self.parse_preliminary_options_and_args( + argv ) + flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) self.make_config_finder() self.find_plugins() self.register_plugin_options() diff --git a/tests/unit/test_legacy_api.py b/tests/unit/test_legacy_api.py index 1d6a3e2..e71d89a 100644 --- a/tests/unit/test_legacy_api.py +++ b/tests/unit/test_legacy_api.py @@ -1,4 +1,6 @@ """Tests for Flake8's legacy API.""" +import argparse + import mock import pytest @@ -8,9 +10,16 @@ from flake8.formatting import base as formatter def test_get_style_guide(): """Verify the methods called on our internal Application.""" + prelim_opts = argparse.Namespace( + output_file=None, + verbose=0, + ) mockedapp = mock.Mock() - mockedapp.prelim_opts.verbose = 0 - mockedapp.prelim_opts.output_file = None + mockedapp.prelim_opts = prelim_opts + mockedapp.parse_preliminary_options_and_args.return_value = ( + prelim_opts, + [], + ) with mock.patch('flake8.main.application.Application') as application: application.return_value = mockedapp style_guide = api.get_style_guide() From 32ebb4fa55ae49f28ab7aa175b6519db21d42dbb Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 1 Oct 2019 08:48:18 +0200 Subject: [PATCH 3/5] application: Pass prelim opts and args to `.make_config_finder()` Now that `.parse_preliminary_options_and_args()` returns options and arguments, the boolean for appending configuration and the arguments can be threaded through to the creation of the `ConfigFileFinder`. --- src/flake8/api/legacy.py | 2 +- src/flake8/main/application.py | 19 ++++++++++++------- tests/unit/test_legacy_api.py | 3 ++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/flake8/api/legacy.py b/src/flake8/api/legacy.py index 6cff7ad..b680346 100644 --- a/src/flake8/api/legacy.py +++ b/src/flake8/api/legacy.py @@ -32,7 +32,7 @@ def get_style_guide(**kwargs): [] ) flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) - application.make_config_finder() + application.make_config_finder(prelim_opts.append_config, prelim_args) application.find_plugins() application.register_plugin_options() application.parse_configuration_and_cli([]) diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index dc0f237..0eb1abd 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -57,7 +57,7 @@ class Application(object): #: The preliminary arguments parsed from CLI before plugins are loaded self.prelim_args = None # type: List[str] #: The instance of :class:`flake8.options.config.ConfigFileFinder` - self.config_finder = None + self.config_finder = None # type: config.ConfigFileFinder #: The :class:`flake8.options.config.LocalPlugins` found in config self.local_plugins = None # type: config.LocalPlugins @@ -160,13 +160,18 @@ class Application(object): (self.result_count > 0) or self.catastrophic_failure ) - def make_config_finder(self): - """Make our ConfigFileFinder based on preliminary opts and args.""" + def make_config_finder(self, append_config, args): + # type: (List[str], List[str]) -> None + """Make our ConfigFileFinder based on preliminary opts and args. + + :param list append_config: + List of configuration files to be parsed for configuration. + :param list args: + The list of file arguments passed from the CLI. + """ if self.config_finder is None: self.config_finder = config.ConfigFileFinder( - self.option_manager.program_name, - self.prelim_args, - self.prelim_opts.append_config, + self.option_manager.program_name, args, append_config ) def find_plugins(self): @@ -361,7 +366,7 @@ class Application(object): argv ) flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) - self.make_config_finder() + self.make_config_finder(prelim_opts.append_config, prelim_args) self.find_plugins() self.register_plugin_options() self.parse_configuration_and_cli(argv) diff --git a/tests/unit/test_legacy_api.py b/tests/unit/test_legacy_api.py index e71d89a..23c975e 100644 --- a/tests/unit/test_legacy_api.py +++ b/tests/unit/test_legacy_api.py @@ -11,6 +11,7 @@ from flake8.formatting import base as formatter def test_get_style_guide(): """Verify the methods called on our internal Application.""" prelim_opts = argparse.Namespace( + append_config=[], output_file=None, verbose=0, ) @@ -26,7 +27,7 @@ def test_get_style_guide(): application.assert_called_once_with() mockedapp.parse_preliminary_options_and_args.assert_called_once_with([]) - mockedapp.make_config_finder.assert_called_once_with() + mockedapp.make_config_finder.assert_called_once_with([], []) mockedapp.find_plugins.assert_called_once_with() mockedapp.register_plugin_options.assert_called_once_with() mockedapp.parse_configuration_and_cli.assert_called_once_with([]) From 95a88e3fcd84f45eb6c0869cb095ea9089dfed44 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 1 Oct 2019 08:48:18 +0200 Subject: [PATCH 4/5] application: Pass prelim opts to `.find_plugins()` The configuration file and boolean to ignore configuration files can be threaded through now that `.parse_preliminary_options_and_args()` returns options and arguments. --- src/flake8/api/legacy.py | 2 +- src/flake8/main/application.py | 17 +++++++++++------ tests/unit/test_legacy_api.py | 4 +++- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/flake8/api/legacy.py b/src/flake8/api/legacy.py index b680346..a620930 100644 --- a/src/flake8/api/legacy.py +++ b/src/flake8/api/legacy.py @@ -33,7 +33,7 @@ def get_style_guide(**kwargs): ) flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) application.make_config_finder(prelim_opts.append_config, prelim_args) - application.find_plugins() + application.find_plugins(prelim_opts.config, prelim_opts.isolated) application.register_plugin_options() application.parse_configuration_and_cli([]) # We basically want application.initialize to be called but with these diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 0eb1abd..0580766 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -174,8 +174,8 @@ class Application(object): self.option_manager.program_name, args, append_config ) - def find_plugins(self): - # type: () -> None + def find_plugins(self, config_file, ignore_config_files): + # type: (Optional[str], bool) -> None """Find and load the plugins for this application. If :attr:`check_plugins`, or :attr:`formatting_plugins` are ``None`` @@ -183,12 +183,17 @@ class Application(object): instance. Given the expense of finding plugins (via :mod:`entrypoints`) we want this to be idempotent and so only update those attributes if they are ``None``. + + :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). """ if self.local_plugins is None: self.local_plugins = config.get_local_plugins( - self.config_finder, - self.prelim_opts.config, - self.prelim_opts.isolated, + self.config_finder, config_file, ignore_config_files ) sys.path.extend(self.local_plugins.paths) @@ -367,7 +372,7 @@ class Application(object): ) flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file) self.make_config_finder(prelim_opts.append_config, prelim_args) - self.find_plugins() + self.find_plugins(prelim_opts.config, prelim_opts.isolated) self.register_plugin_options() self.parse_configuration_and_cli(argv) self.make_formatter() diff --git a/tests/unit/test_legacy_api.py b/tests/unit/test_legacy_api.py index 23c975e..f36246c 100644 --- a/tests/unit/test_legacy_api.py +++ b/tests/unit/test_legacy_api.py @@ -12,6 +12,8 @@ def test_get_style_guide(): """Verify the methods called on our internal Application.""" prelim_opts = argparse.Namespace( append_config=[], + config=None, + isolated=False, output_file=None, verbose=0, ) @@ -28,7 +30,7 @@ def test_get_style_guide(): application.assert_called_once_with() mockedapp.parse_preliminary_options_and_args.assert_called_once_with([]) mockedapp.make_config_finder.assert_called_once_with([], []) - mockedapp.find_plugins.assert_called_once_with() + mockedapp.find_plugins.assert_called_once_with(None, False) mockedapp.register_plugin_options.assert_called_once_with() mockedapp.parse_configuration_and_cli.assert_called_once_with([]) mockedapp.make_formatter.assert_called_once_with() From b54164f916922725c17e6d0df75998ada6b27eef Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Tue, 1 Oct 2019 08:48:18 +0200 Subject: [PATCH 5/5] application: Remove preliminary argument parsing state The preliminary options and arguments returned from `.parse_preliminary_options_and_args()` are now all threaded through to the appropriate methods during initialization. --- src/flake8/main/application.py | 8 -------- tests/unit/test_application.py | 12 ++++-------- tests/unit/test_legacy_api.py | 1 - 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py index 0580766..9b31b56 100644 --- a/src/flake8/main/application.py +++ b/src/flake8/main/application.py @@ -51,11 +51,6 @@ class Application(object): prog="flake8", version=flake8.__version__ ) options.register_default_options(self.option_manager) - #: The preliminary options parsed from CLI before plugins are loaded, - #: into a :class:`argparse.Namespace` instance - self.prelim_opts = None # type: argparse.Namespace - #: The preliminary arguments parsed from CLI before plugins are loaded - self.prelim_args = None # type: List[str] #: The instance of :class:`flake8.options.config.ConfigFileFinder` self.config_finder = None # type: config.ConfigFileFinder @@ -108,8 +103,6 @@ class Application(object): options; we ignore those for now, they'll be parsed later when we do real option parsing. - Sets self.prelim_opts and self.prelim_args. - :param list argv: Command-line arguments passed in directly. :returns: @@ -142,7 +135,6 @@ class Application(object): opts, args = self.option_manager.parse_known_args(args) # parse_known_args includes unknown options as args args = [a for a in args if not a.startswith("-")] - self.prelim_opts, self.prelim_args = opts, args return opts, args def exit(self): diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index ea4f090..09edfe4 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -95,12 +95,9 @@ def test_prelim_opts_args(application): opts, args = application.parse_preliminary_options_and_args( ['flake8', '--foo', '--verbose', 'src', 'setup.py', '--statistics']) - assert application.prelim_opts.statistics - assert application.prelim_opts.verbose - assert application.prelim_args == ['src', 'setup.py'] - assert opts.statistics is application.prelim_opts.statistics - assert opts.verbose is application.prelim_opts.verbose - assert args is application.prelim_args + assert opts.statistics + assert opts.verbose + assert args == ['src', 'setup.py'] def test_prelim_opts_handles_empty(application): @@ -109,5 +106,4 @@ def test_prelim_opts_handles_empty(application): with mock.patch.object(sys, 'argv', irrelevant_args): opts, args = application.parse_preliminary_options_and_args([]) - assert application.prelim_args == [] - assert args is application.prelim_args + assert args == [] diff --git a/tests/unit/test_legacy_api.py b/tests/unit/test_legacy_api.py index f36246c..369fb2f 100644 --- a/tests/unit/test_legacy_api.py +++ b/tests/unit/test_legacy_api.py @@ -18,7 +18,6 @@ def test_get_style_guide(): verbose=0, ) mockedapp = mock.Mock() - mockedapp.prelim_opts = prelim_opts mockedapp.parse_preliminary_options_and_args.return_value = ( prelim_opts, [],