From ec8fcfc8f8fb3a4dd69e3679c5076baf01481e06 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Fri, 30 Aug 2019 15:17:16 -0400 Subject: [PATCH 1/3] Add typing to `OptionManager.parse_args()` Note that the `assert` is necessary to "cast" `self.parser` since it is specified as a `Union`. --- src/flake8/options/manager.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/flake8/options/manager.py b/src/flake8/options/manager.py index a5e5906..3aea4ad 100644 --- a/src/flake8/options/manager.py +++ b/src/flake8/options/manager.py @@ -425,13 +425,21 @@ class OptionManager(object): plugin_version_format ) - def parse_args(self, args=None, values=None): + def parse_args( + self, + args=None, # type: Optional[List[str]] + values=None, # type: Optional[argparse.Namespace] + ): + # type: (...) -> Tuple[argparse.Namespace, List[str]] """Proxy to calling the OptionParser's parse_args method.""" self.generate_epilog() self.update_version_string() - args = self.parser.parse_args(args, values) + assert isinstance( # nosec (for bandit) + self.parser, argparse.ArgumentParser + ), self.parser + parsed_args = self.parser.parse_args(args, values) # TODO: refactor callers to not need this - return args, args.filenames + return parsed_args, parsed_args.filenames def parse_known_args(self, args=None): # type: (Optional[List[str]]) -> Tuple[argparse.Namespace, List[str]] From b231c10016fa50faf7ea87b2e0530655c2184bf4 Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Fri, 30 Aug 2019 15:17:16 -0400 Subject: [PATCH 2/3] Test default provided options are forwarded Ensure options provided external to the command-line (i.e., configuration files) are present in the final result of options. --- tests/unit/test_option_manager.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit/test_option_manager.py b/tests/unit/test_option_manager.py index d3cc728..b6ea55c 100644 --- a/tests/unit/test_option_manager.py +++ b/tests/unit/test_option_manager.py @@ -22,6 +22,13 @@ def test_option_manager_creates_option_parser(optmanager): assert isinstance(optmanager.parser, argparse.ArgumentParser) +def test_parse_args_forwarding_default_values(optmanager): + """Verify default provided values are present in the final result.""" + namespace = argparse.Namespace(foo='bar') + options, args = optmanager.parse_args([], namespace) + assert options.foo == 'bar' + + def test_add_option_short_option_only(optmanager): """Verify the behaviour of adding a short-option only.""" assert optmanager.options == [] From aadd09dd8bd91092b22b24b860d4ff2476313b1c Mon Sep 17 00:00:00 2001 From: "Eric N. Vander Weele" Date: Fri, 30 Aug 2019 15:17:16 -0400 Subject: [PATCH 3/3] Set configuration file-provided values via ArgumentParser.set_defaults() When calling `ArgumentParser.parse_args()` with the `namespace` argument, command-line options are just added to the namespace without going through any of the argument parsing and type conversion logic (e.g., the `type` keyword argument of `ArgumentParser.add_argument()`). In other words, it is assumed that a namespace is well-formed from a previous invocation of `ArgumentParser.parse_args()`. The `values` parameter is intended to be values already-provided from configuration files. To take advantage of the logic defined by `ArgumentParser.add_argument()`, utilize `ArgumentParser.set_defaults()` instead. --- src/flake8/options/manager.py | 4 +++- tests/unit/test_option_manager.py | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/flake8/options/manager.py b/src/flake8/options/manager.py index 3aea4ad..f745388 100644 --- a/src/flake8/options/manager.py +++ b/src/flake8/options/manager.py @@ -437,7 +437,9 @@ class OptionManager(object): assert isinstance( # nosec (for bandit) self.parser, argparse.ArgumentParser ), self.parser - parsed_args = self.parser.parse_args(args, values) + if values: + self.parser.set_defaults(**vars(values)) + parsed_args = self.parser.parse_args(args) # TODO: refactor callers to not need this return parsed_args, parsed_args.filenames diff --git a/tests/unit/test_option_manager.py b/tests/unit/test_option_manager.py index b6ea55c..859dca1 100644 --- a/tests/unit/test_option_manager.py +++ b/tests/unit/test_option_manager.py @@ -29,6 +29,14 @@ def test_parse_args_forwarding_default_values(optmanager): assert options.foo == 'bar' +def test_parse_args_forwarding_type_coercion(optmanager): + """Verify default provided values are type converted from add_option.""" + optmanager.add_option('--foo', type=int) + namespace = argparse.Namespace(foo='5') + options, args = optmanager.parse_args([], namespace) + assert options.foo == 5 + + def test_add_option_short_option_only(optmanager): """Verify the behaviour of adding a short-option only.""" assert optmanager.options == []