Merge branch 'separate-prelim-options' into 'master'

Separate pre-configuration CLI parsing

See merge request pycqa/flake8!364

src/flake8/api/legacy.py

@@ -28,14 +28,14 @@ def get_style_guide(**kwargs):
         :class:`StyleGuide`
     """
     application = app.Application()
-    prelim_opts, prelim_args = application.parse_preliminary_options_and_args(
+    prelim_opts, remaining_args = application.parse_preliminary_options_and_args(  # noqa: E501
         []
     )
     flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file)
     application.make_config_finder(prelim_opts.append_config)
     application.find_plugins(prelim_opts.config, prelim_opts.isolated)
     application.register_plugin_options()
-    application.parse_configuration_and_cli([])
+    application.parse_configuration_and_cli(remaining_args)
     # We basically want application.initialize to be called but with these
     # options set instead before we make our formatter, notifier, internal
     # style guide and file checker manager.

src/flake8/main/application.py

@@ -45,10 +45,16 @@ class Application(object):
         self.program = program
         #: The version of the program being run
         self.version = version
+        #: The prelimary argument parser for handling options required for
+        #: obtaining and parsing the configuration file.
+        self.prelim_arg_parser = argparse.ArgumentParser(add_help=False)
+        options.register_preliminary_options(self.prelim_arg_parser)
         #: The instance of :class:`flake8.options.manager.OptionManager` used
         #: to parse and handle the options and arguments passed by the user
         self.option_manager = manager.OptionManager(
-            prog="flake8", version=flake8.__version__
+            prog="flake8",
+            version=flake8.__version__,
+            parents=[self.prelim_arg_parser],
         )
         options.register_default_options(self.option_manager)
         #: The instance of :class:`flake8.options.config.ConfigFileFinder`
@@ -110,32 +116,7 @@ class Application(object):
         :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
-        # and the command-line. First, let's clone our arguments on the CLI,
-        # then we'll attempt to remove ``--version`` so that we can avoid
-        # triggering the "version" action in argparse. If it's not there, we
-        # do not need to worry and we can continue. If it is, we successfully
-        # defer printing the version until just a little bit later.
-        # Similarly we have to defer printing the help text until later.
-        args = argv[:]
-        try:
-            args.remove("--version")
-        except ValueError:
-            pass
-        try:
-            args.remove("--help")
-        except ValueError:
-            pass
-        try:
-            args.remove("-h")
-        except ValueError:
-            pass
-
-        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("-")]
-        return opts, args
+        return self.prelim_arg_parser.parse_known_args(argv)
 
     def exit(self):
         # type: () -> None
@@ -357,14 +338,14 @@ class Application(object):
         """
         # NOTE(sigmavirus24): When updating this, make sure you also update
         # our legacy API calls to these same methods.
-        prelim_opts, prelim_args = self.parse_preliminary_options_and_args(
+        prelim_opts, remaining_args = self.parse_preliminary_options_and_args(
             argv
         )
         flake8.configure_logging(prelim_opts.verbose, prelim_opts.output_file)
         self.make_config_finder(prelim_opts.append_config)
         self.find_plugins(prelim_opts.config, prelim_opts.isolated)
         self.register_plugin_options()
-        self.parse_configuration_and_cli(argv)
+        self.parse_configuration_and_cli(remaining_args)
         self.make_formatter()
         self.make_guide()
         self.make_file_checker_manager()

src/flake8/main/options.py

@@ -1,4 +1,5 @@
 """Contains the logic for all of the default options for Flake8."""
+import argparse
 import functools
 
 from flake8 import defaults
@@ -6,12 +7,66 @@ from flake8.main import debug
 from flake8.main import vcs
 
 
+def register_preliminary_options(parser):
+    # type: (argparse.ArgumentParser) -> None
+    """Register the preliminary options on our OptionManager.
+
+    The preliminary options include:
+
+    - ``-v``/``--verbose``
+    - ``--output-file``
+    - ``--append-config``
+    - ``--config``
+    - ``--isolated``
+    """
+    add_argument = parser.add_argument
+
+    add_argument(
+        "-v",
+        "--verbose",
+        default=0,
+        action="count",
+        help="Print more information about what is happening in flake8."
+        " This option is repeatable and will increase verbosity each "
+        "time it is repeated.",
+    )
+
+    add_argument(
+        "--output-file", default=None, help="Redirect report to a file."
+    )
+
+    # Config file options
+
+    add_argument(
+        "--append-config",
+        action="append",
+        help="Provide extra config files to parse in addition to the files "
+        "found by Flake8 by default. These files are the last ones read "
+        "and so they take the highest precedence when multiple files "
+        "provide the same option.",
+    )
+
+    add_argument(
+        "--config",
+        default=None,
+        help="Path to the config file that will be the authoritative config "
+        "source. This will cause Flake8 to ignore all other "
+        "configuration files.",
+    )
+
+    add_argument(
+        "--isolated",
+        default=False,
+        action="store_true",
+        help="Ignore all configuration files.",
+    )
+
+
 def register_default_options(option_manager):
     """Register the default options on our OptionManager.
 
     The default options include:
 
-    - ``-v``/``--verbose``
     - ``-q``/``--quiet``
     - ``--count``
     - ``--diff``
@@ -32,26 +87,13 @@ def register_default_options(option_manager):
     - ``--enable-extensions``
     - ``--exit-zero``
     - ``-j``/``--jobs``
-    - ``--output-file``
     - ``--tee``
-    - ``--append-config``
-    - ``--config``
-    - ``--isolated``
     - ``--benchmark``
     - ``--bug-report``
     """
     add_option = option_manager.add_option
 
     # pep8 options
-    add_option(
-        "-v",
-        "--verbose",
-        default=0,
-        action="count",
-        help="Print more information about what is happening in flake8."
-        " This option is repeatable and will increase verbosity each "
-        "time it is repeated.",
-    )
     add_option(
         "-q",
         "--quiet",
@@ -257,10 +299,6 @@ def register_default_options(option_manager):
         " (Default: %(default)s)",
     )
 
-    add_option(
-        "--output-file", default=None, help="Redirect report to a file."
-    )
-
     add_option(
         "--tee",
         default=False,
@@ -269,32 +307,6 @@ def register_default_options(option_manager):
         help="Write to stdout and output-file.",
     )
 
-    # Config file options
-
-    add_option(
-        "--append-config",
-        action="append",
-        help="Provide extra config files to parse in addition to the files "
-        "found by Flake8 by default. These files are the last ones read "
-        "and so they take the highest precedence when multiple files "
-        "provide the same option.",
-    )
-
-    add_option(
-        "--config",
-        default=None,
-        help="Path to the config file that will be the authoritative config "
-        "source. This will cause Flake8 to ignore all other "
-        "configuration files.",
-    )
-
-    add_option(
-        "--isolated",
-        default=False,
-        action="store_true",
-        help="Ignore all configuration files.",
-    )
-
     # Benchmarking
 
     add_option(

src/flake8/options/manager.py

@@ -338,8 +338,12 @@ class OptionManager(object):
     """Manage Options and OptionParser while adding post-processing."""
 
     def __init__(
-        self, prog, version, usage="%(prog)s [options] file file ..."
-    ):  # type: (str, str, str) -> None
+        self,
+        prog,
+        version,
+        usage="%(prog)s [options] file file ...",
+        parents=None,
+    ):  # type: (str, str, str, Optional[List[argparse.ArgumentParser]]) -> None  # noqa: E501
         """Initialize an instance of an OptionManager.
 
         :param str prog:
@@ -348,9 +352,15 @@ class OptionManager(object):
             Version string for the program.
         :param str usage:
             Basic usage string used by the OptionParser.
+        :param argparse.ArgumentParser parents:
+            A list of ArgumentParser objects whose arguments should also be
+            included.
         """
+        if parents is None:
+            parents = []
+
         self.parser = argparse.ArgumentParser(
-            prog=prog, usage=usage
+            prog=prog, usage=usage, parents=parents
         )  # type: argparse.ArgumentParser
         self._current_group = None  # type: Optional[argparse._ArgumentGroup]
         self.version_action = cast(

tests/integration/test_aggregator.py

@@ -1,4 +1,5 @@
 """Test aggregation of config files and command-line options."""
+import argparse
 import os
 
 import pytest
@@ -14,9 +15,12 @@ CLI_SPECIFIED_CONFIG = 'tests/fixtures/config_files/cli-specified.ini'
 @pytest.fixture
 def optmanager():
     """Create a new OptionManager."""
+    prelim_parser = argparse.ArgumentParser(add_help=False)
+    options.register_preliminary_options(prelim_parser)
     option_manager = manager.OptionManager(
         prog='flake8',
         version='3.0.0',
+        parents=[prelim_parser],
     )
     options.register_default_options(option_manager)
     return option_manager

tests/unit/test_application.py

@@ -93,11 +93,24 @@ def test_returns_specified_plugin(application):
 def test_prelim_opts_args(application):
     """Verify we get sensible prelim opts and args."""
     opts, args = application.parse_preliminary_options_and_args(
-        ['flake8', '--foo', '--verbose', 'src', 'setup.py', '--statistics'])
+        ['--foo', '--verbose', 'src', 'setup.py', '--statistics', '--version'])
 
-    assert opts.statistics
     assert opts.verbose
-    assert args == ['src', 'setup.py']
+    assert args == ['--foo', 'src', 'setup.py', '--statistics', '--version']
+
+
+def test_prelim_opts_ignore_help(application):
+    """Verify -h/--help is not handled."""
+    # GIVEN
+
+    # WHEN
+    _, args = application.parse_preliminary_options_and_args([
+        '--help',
+        '-h',
+    ])
+
+    # THEN
+    assert args == ['--help', '-h']
 
 
 def test_prelim_opts_handles_empty(application):

tests/unit/test_option_manager.py

@@ -22,6 +22,23 @@ def test_option_manager_creates_option_parser(optmanager):
     assert isinstance(optmanager.parser, argparse.ArgumentParser)
 
 
+def test_option_manager_including_parent_options():
+    """Verify parent options are included in the parsed options."""
+    # GIVEN
+    parent_parser = argparse.ArgumentParser(add_help=False)
+    parent_parser.add_argument('--parent')
+
+    # WHEN
+    optmanager = manager.OptionManager(
+        prog='flake8',
+        version=TEST_VERSION,
+        parents=[parent_parser])
+    option, _ = optmanager.parse_args(['--parent', 'foo'])
+
+    # THEN
+    assert option.parent == 'foo'
+
+
 def test_parse_args_forwarding_default_values(optmanager):
     """Verify default provided values are present in the final result."""
     namespace = argparse.Namespace(foo='bar')