Merge branch 'type_annotate_option_manager' into 'master'

Type annotate flake8.options.manager

See merge request pycqa/flake8!352

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.701
+    rev: v0.720
     hooks:
     -   id: mypy
         exclude: ^(docs/|example-plugin/|tests/fixtures)

setup.cfg

@@ -118,6 +118,8 @@ disallow_untyped_defs = true
 disallow_untyped_defs = true
 [mypy-flake8.formatting.*]
 disallow_untyped_defs = true
+[mypy-flake8.options.manager]
+disallow_untyped_defs = true
 [mypy-flake8.main.cli]
 disallow_untyped_defs = true
 [mypy-flake8.statistics]

src/flake8/checker.py

@@ -388,7 +388,7 @@ class FileChecker(object):
             self.should_process = not self.processor.should_ignore_file()
             self.statistics["physical lines"] = len(self.processor.lines)
 
-    def __repr__(self):
+    def __repr__(self):  # type: () -> str
         """Provide helpful debugging representation."""
         return "FileChecker for {}".format(self.filename)
 

src/flake8/options/manager.py

@@ -2,27 +2,53 @@
 import argparse
 import collections
 import contextlib
+import enum
 import functools
 import logging
-from typing import Any, Dict, Generator, List, Optional, Set, Tuple, Union
+from typing import Any, Callable, cast, Dict, Generator, List, Mapping
+from typing import Optional, Sequence, Set, Tuple, Union
 
 from flake8 import utils
 
+if False:  # TYPE_CHECKING
+    from typing import NoReturn
+    from typing import Type
+
 LOG = logging.getLogger(__name__)
 
-_NOARG = object()
+# represent a singleton of "not passed arguments".
+# an enum is chosen to trick mypy
+_ARG = enum.Enum("_ARG", "NO")
+
+
+_optparse_callable_map = {
+    "int": int,
+    "long": int,
+    "string": str,
+    "float": float,
+    "complex": complex,
+    "choice": _ARG.NO,
+}  # type: Dict[str, Union[Type[Any], _ARG]]
 
 
 class _CallbackAction(argparse.Action):
     """Shim for optparse-style callback actions."""
 
     def __init__(self, *args, **kwargs):
+        # type: (*Any, **Any) -> None
         self._callback = kwargs.pop("callback")
         self._callback_args = kwargs.pop("callback_args", ())
         self._callback_kwargs = kwargs.pop("callback_kwargs", {})
         super(_CallbackAction, self).__init__(*args, **kwargs)
 
-    def __call__(self, parser, namespace, values, option_string=None):
+    def __call__(
+        self,
+        parser,  # type: argparse.ArgumentParser
+        namespace,  # type: argparse.Namespace
+        values,  # type: Optional[Union[Sequence[str], str]]
+        option_string=None,  # type: Optional[str]
+    ):
+        # type: (...) -> None
         if not values:
             values = None
         elif isinstance(values, list) and len(values) > 1:
@@ -38,21 +64,23 @@ class _CallbackAction(argparse.Action):
 
 
 def _flake8_normalize(value, *args, **kwargs):
+    # type: (str, *str, **bool) -> Union[str, List[str]]
     comma_separated_list = kwargs.pop("comma_separated_list", False)
     normalize_paths = kwargs.pop("normalize_paths", False)
     if kwargs:
         raise TypeError("Unexpected keyword args: {}".format(kwargs))
 
-    if comma_separated_list and isinstance(value, utils.string_types):
+    ret = value  # type: Union[str, List[str]]
-        value = utils.parse_comma_separated_list(value)
+    if comma_separated_list and isinstance(ret, utils.string_types):
+        ret = utils.parse_comma_separated_list(value)
 
     if normalize_paths:
-        if isinstance(value, list):
+        if isinstance(ret, utils.string_types):
-            value = utils.normalize_paths(value, *args)
+            ret = utils.normalize_path(ret, *args)
         else:
-            value = utils.normalize_path(value, *args)
+            ret = utils.normalize_paths(ret, *args)
 
-    return value
+    return ret
 
 
 class Option(object):
@@ -60,29 +88,29 @@ class Option(object):
 
     def __init__(
         self,
-        short_option_name=_NOARG,
+        short_option_name=_ARG.NO,  # type: Union[str, _ARG]
-        long_option_name=_NOARG,
+        long_option_name=_ARG.NO,  # type: Union[str, _ARG]
         # Options below here are taken from the optparse.Option class
-        action=_NOARG,
+        action=_ARG.NO,  # type: Union[str, Type[argparse.Action], _ARG]
-        default=_NOARG,
+        default=_ARG.NO,  # type: Union[Any, _ARG]
-        type=_NOARG,
+        type=_ARG.NO,  # type: Union[str, Callable[..., Any], _ARG]
-        dest=_NOARG,
+        dest=_ARG.NO,  # type: Union[str, _ARG]
-        nargs=_NOARG,
+        nargs=_ARG.NO,  # type: Union[int, str, _ARG]
-        const=_NOARG,
+        const=_ARG.NO,  # type: Union[Any, _ARG]
-        choices=_NOARG,
+        choices=_ARG.NO,  # type: Union[Sequence[Any], _ARG]
-        help=_NOARG,
+        help=_ARG.NO,  # type: Union[str, _ARG]
-        metavar=_NOARG,
+        metavar=_ARG.NO,  # type: Union[str, _ARG]
         # deprecated optparse-only options
-        callback=_NOARG,
+        callback=_ARG.NO,  # type: Union[Callable[..., Any], _ARG]
-        callback_args=_NOARG,
+        callback_args=_ARG.NO,  # type: Union[Sequence[Any], _ARG]
-        callback_kwargs=_NOARG,
+        callback_kwargs=_ARG.NO,  # type: Union[Mapping[str, Any], _ARG]
         # Options below are taken from argparse.ArgumentParser.add_argument
-        required=_NOARG,
+        required=_ARG.NO,  # type: Union[bool, _ARG]
         # Options below here are specific to Flake8
-        parse_from_config=False,
+        parse_from_config=False,  # type: bool
-        comma_separated_list=False,
+        comma_separated_list=False,  # type: bool
-        normalize_paths=False,
+        normalize_paths=False,  # type: bool
-    ):
+    ):  # type: (...) -> None
         """Initialize an Option instance.
 
         The following are all passed directly through to argparse.
@@ -144,11 +172,15 @@ class Option(object):
             Whether the option is expecting a path or list of paths and should
             attempt to normalize the paths to absolute paths.
         """
-        if long_option_name is _NOARG and short_option_name.startswith("--"):
+        if (
-            short_option_name, long_option_name = _NOARG, short_option_name
+            long_option_name is _ARG.NO
+            and short_option_name is not _ARG.NO
+            and short_option_name.startswith("--")
+        ):
+            short_option_name, long_option_name = _ARG.NO, short_option_name
 
         # optparse -> argparse `%default` => `%(default)s`
-        if help is not _NOARG and "%default" in help:
+        if help is not _ARG.NO and "%default" in help:
             LOG.warning(
                 "option %s: please update `help=` text to use %%(default)s "
                 "instead of %%default -- this will be an error in the future",
@@ -165,7 +197,7 @@ class Option(object):
                 long_option_name,
             )
             action = _CallbackAction
-            if type is _NOARG:
+            if type is _ARG.NO:
                 nargs = 0
 
         # optparse -> argparse for `type`
@@ -175,14 +207,7 @@ class Option(object):
                 "argparse callable `type=` -- this will be an error in the "
                 "future"
             )
-            type = {
+            type = _optparse_callable_map[type]
-                "int": int,
-                "long": int,
-                "string": str,
-                "float": float,
-                "complex": complex,
-                "choice": _NOARG,
-            }[type]
 
         # flake8 special type normalization
         if comma_separated_list or normalize_paths:
@@ -197,25 +222,36 @@ class Option(object):
         self.option_args = [
             x
             for x in (short_option_name, long_option_name)
-            if x is not _NOARG
+            if x is not _ARG.NO
         ]
+        self.action = action
+        self.default = default
+        self.type = type
+        self.dest = dest
+        self.nargs = nargs
+        self.const = const
+        self.choices = choices
+        self.callback = callback
+        self.callback_args = callback_args
+        self.callback_kwargs = callback_kwargs
+        self.help = help
+        self.metavar = metavar
+        self.required = required
         self.option_kwargs = {
-            "action": action,
+            "action": self.action,
-            "default": default,
+            "default": self.default,
-            "type": type,
+            "type": self.type,
-            "dest": dest,
+            "dest": self.dest,
-            "nargs": nargs,
+            "nargs": self.nargs,
-            "const": const,
+            "const": self.const,
-            "choices": choices,
+            "choices": self.choices,
-            "callback": callback,
+            "callback": self.callback,
-            "callback_args": callback_args,
+            "callback_args": self.callback_args,
-            "callback_kwargs": callback_kwargs,
+            "callback_kwargs": self.callback_kwargs,
-            "help": help,
+            "help": self.help,
-            "metavar": metavar,
+            "metavar": self.metavar,
-        }
+            "required": self.required,
-        # Set attributes for our option arguments
+        }  # type: Dict[str, Union[Any, _ARG]]
-        for key, value in self.option_kwargs.items():
-            setattr(self, key, value)
 
         # Set our custom attributes
         self.parse_from_config = parse_from_config
@@ -224,7 +260,7 @@ class Option(object):
 
         self.config_name = None  # type: Optional[str]
         if parse_from_config:
-            if long_option_name is _NOARG:
+            if long_option_name is _ARG.NO:
                 raise ValueError(
                     "When specifying parse_from_config=True, "
                     "a long_option_name must also be specified."
@@ -237,10 +273,10 @@ class Option(object):
     def filtered_option_kwargs(self):  # type: () -> Dict[str, Any]
         """Return any actually-specified arguments."""
         return {
-            k: v for k, v in self.option_kwargs.items() if v is not _NOARG
+            k: v for k, v in self.option_kwargs.items() if v is not _ARG.NO
         }
 
-    def __repr__(self):  # noqa: D105
+    def __repr__(self):  # type: () -> str  # noqa: D105
         parts = []
         for arg in self.option_args:
             parts.append(arg)
@@ -249,6 +285,7 @@ class Option(object):
         return "Option({})".format(", ".join(parts))
 
     def normalize(self, value, *normalize_args):
+        # type: (Any, *str) -> Any
         """Normalize the value based on the option configuration."""
         if self.comma_separated_list and isinstance(
             value, utils.string_types
@@ -264,6 +301,7 @@ class Option(object):
         return value
 
     def normalize_from_setuptools(self, value):
+        # type: (str) -> Union[int, float, complex, bool, str]
         """Normalize the value received from setuptools."""
         value = self.normalize(value)
         if self.type is int or self.action == "count":
@@ -281,11 +319,12 @@ class Option(object):
         return value
 
     def to_argparse(self):
+        # type: () -> Tuple[List[str], Dict[str, Any]]
         """Convert a Flake8 Option to argparse ``add_argument`` arguments."""
         return self.option_args, self.filtered_option_kwargs
 
     @property
-    def to_optparse(self):
+    def to_optparse(self):  # type: () -> NoReturn
         """No longer functional."""
         raise AttributeError("to_optparse: flake8 now uses argparse")
 
@@ -299,11 +338,8 @@ class OptionManager(object):
     """Manage Options and OptionParser while adding post-processing."""
 
     def __init__(
-        self,
+        self, prog, version, usage="%(prog)s [options] file file ..."
-        prog=None,
+    ):  # type: (str, str, str) -> None
-        version=None,
-        usage="%(prog)s [options] file file ...",
-    ):
         """Initialize an instance of an OptionManager.
 
         :param str prog:
@@ -315,9 +351,13 @@ class OptionManager(object):
         """
         self.parser = argparse.ArgumentParser(
             prog=prog, usage=usage
-        )  # type: Union[argparse.ArgumentParser, argparse._ArgumentGroup]
+        )  # type: argparse.ArgumentParser
-        self.version_action = self.parser.add_argument(
+        self._current_group = None  # type: Optional[argparse._ArgumentGroup]
+        self.version_action = cast(
+            "argparse._VersionAction",
+            self.parser.add_argument(
                 "--version", action="version", version=version
+            ),
         )
         self.parser.add_argument("filenames", nargs="*", metavar="filename")
         self.config_options_dict = {}  # type: Dict[str, Option]
@@ -328,22 +368,17 @@ class OptionManager(object):
         self.extended_default_ignore = set()  # type: Set[str]
         self.extended_default_select = set()  # type: Set[str]
 
-    @staticmethod
-    def format_plugin(plugin):
-        """Convert a PluginVersion into a dictionary mapping name to value."""
-        return {attr: getattr(plugin, attr) for attr in ["name", "version"]}
-
     @contextlib.contextmanager
     def group(self, name):  # type: (str) -> Generator[None, None, None]
         """Attach options to an argparse group during this context."""
         group = self.parser.add_argument_group(name)
-        self.parser, orig_parser = group, self.parser
+        self._current_group, orig_group = group, self._current_group
         try:
             yield
         finally:
-            self.parser = orig_parser
+            self._current_group = orig_group
 
-    def add_option(self, *args, **kwargs):
+    def add_option(self, *args, **kwargs):  # type: (*Any, **Any) -> None
         """Create and register a new option.
 
         See parameters for :class:`~flake8.options.manager.Option` for
@@ -356,6 +391,9 @@ class OptionManager(object):
         """
         option = Option(*args, **kwargs)
         option_args, option_kwargs = option.to_argparse()
+        if self._current_group is not None:
+            self._current_group.add_argument(*option_args, **option_kwargs)
+        else:
             self.parser.add_argument(*option_args, **option_kwargs)
         self.options.append(option)
         if option.parse_from_config:
@@ -366,6 +404,7 @@ class OptionManager(object):
         LOG.debug('Registered option "%s".', option)
 
     def remove_from_default_ignore(self, error_codes):
+        # type: (Sequence[str]) -> None
         """Remove specified error codes from the default ignore list.
 
         :param list error_codes:
@@ -384,6 +423,7 @@ class OptionManager(object):
                 )
 
     def extend_default_ignore(self, error_codes):
+        # type: (Sequence[str]) -> None
         """Extend the default ignore list with the error codes provided.
 
         :param list error_codes:
@@ -394,6 +434,7 @@ class OptionManager(object):
         self.extended_default_ignore.update(error_codes)
 
     def extend_default_select(self, error_codes):
+        # type: (Sequence[str]) -> None
         """Extend the default select list with the error codes provided.
 
         :param list error_codes:
@@ -406,19 +447,20 @@ class OptionManager(object):
     def generate_versions(
         self, format_str="%(name)s: %(version)s", join_on=", "
     ):
+        # type: (str, str) -> str
         """Generate a comma-separated list of versions of plugins."""
         return join_on.join(
-            format_str % self.format_plugin(plugin)
+            format_str % plugin._asdict()
             for plugin in sorted(self.registered_plugins)
         )
 
-    def update_version_string(self):
+    def update_version_string(self):  # type: () -> None
         """Update the flake8 version string."""
         self.version_action.version = "{} ({}) {}".format(
             self.version, self.generate_versions(), utils.get_python_version()
         )
 
-    def generate_epilog(self):
+    def generate_epilog(self):  # type: () -> None
         """Create an epilog with the version and name of each of plugin."""
         plugin_version_format = "%(name)s: %(version)s"
         self.parser.epilog = "Installed plugins: " + self.generate_versions(
@@ -434,9 +476,6 @@ class OptionManager(object):
         """Proxy to calling the OptionParser's parse_args method."""
         self.generate_epilog()
         self.update_version_string()
-        assert isinstance(  # nosec (for bandit)
-            self.parser, argparse.ArgumentParser
-        ), self.parser
         if values:
             self.parser.set_defaults(**vars(values))
         parsed_args = self.parser.parse_args(args)
@@ -452,14 +491,10 @@ class OptionManager(object):
         """
         self.generate_epilog()
         self.update_version_string()
-        # TODO: Re-evaluate `self.parser` swap happening in `group()` to
-        #       avoid needing to assert to satify static type checking.
-        assert isinstance(  # nosec (for bandit)
-            self.parser, argparse.ArgumentParser
-        ), self.parser
         return self.parser.parse_known_args(args)
 
     def register_plugin(self, name, version, local=False):
+        # type: (str, str, bool) -> None
         """Register a plugin relying on the OptionManager.
 
         :param str name:

src/flake8/plugins/manager.py

@@ -1,6 +1,6 @@
 """Plugin loading and management logic and classes."""
 import logging
-from typing import Any, Dict, List, Set
+from typing import Any, Dict, List, Optional, Set
 
 import entrypoints
 
@@ -34,12 +34,12 @@ class Plugin(object):
         self.local = local
         self._plugin = None  # type: Any
         self._parameters = None
-        self._parameter_names = None
+        self._parameter_names = None  # type: Optional[List[str]]
         self._group = None
         self._plugin_name = None
         self._version = None
 
-    def __repr__(self):
+    def __repr__(self):  # type: () -> str
         """Provide an easy to read description of the current plugin."""
         return 'Plugin(name="{0}", entry_point="{1}")'.format(
             self.name, self.entry_point
@@ -85,7 +85,7 @@ class Plugin(object):
         return self._parameters
 
     @property
-    def parameter_names(self):
+    def parameter_names(self):  # type: () -> List[str]
         """List of argument names that need to be passed to the plugin."""
         if self._parameter_names is None:
             self._parameter_names = list(self.parameters)
@@ -101,15 +101,15 @@ class Plugin(object):
         return self._plugin
 
     @property
-    def version(self):
+    def version(self):  # type: () -> str
         """Return the version of the plugin."""
-        if self._version is None:
+        version = self._version
+        if version is None:
             if self.is_in_a_group():
-                self._version = version_for(self)
+                version = self._version = version_for(self)
             else:
-                self._version = self.plugin.version
+                version = self._version = self.plugin.version
-
+        return version
-        return self._version
 
     @property
     def plugin_name(self):

src/flake8/style_guide.py

@@ -24,7 +24,6 @@ else:
     from functools import lru_cache
 
 
-# TODO(sigmavirus24): Determine if we need to use enum/enum34
 class Selected(enum.Enum):
     """Enum representing an explicitly or implicitly selected code."""
 
@@ -451,7 +450,7 @@ class StyleGuide(object):
             self.filename = utils.normalize_path(self.filename)
         self._parsed_diff = {}  # type: Dict[str, Set[int]]
 
-    def __repr__(self):
+    def __repr__(self):  # type: () -> str
         """Make it easier to debug which StyleGuide we're using."""
         return "<StyleGuide [{}]>".format(self.filename)
 

tests/unit/test_option.py

@@ -28,7 +28,7 @@ def test_to_argparse():
 def test_to_optparse():
     """Test that .to_optparse() produces a useful error message."""
     with pytest.raises(AttributeError) as excinfo:
-        manager.Option('--foo').to_optparse()
+        manager.Option('--foo').to_optparse
     msg, = excinfo.value.args
     assert msg == 'to_optparse: flake8 now uses argparse'
 
@@ -58,4 +58,4 @@ def test_config_name_needs_long_option_name():
 def test_dest_is_not_overridden():
     """Show that we do not override custom destinations."""
     opt = manager.Option('-s', '--short', dest='something_not_short')
-    assert opt.dest == 'something_not_short'  # type: ignore
+    assert opt.dest == 'something_not_short'

tests/unit/test_option_manager.py

@@ -52,7 +52,7 @@ def test_add_option_long_option_only(optmanager):
     assert optmanager.config_options_dict == {}
 
     optmanager.add_option('--long', help='Test long opt')
-    assert optmanager.options[0].short_option_name is manager._NOARG
+    assert optmanager.options[0].short_option_name is manager._ARG.NO
     assert optmanager.options[0].long_option_name == '--long'
 
 
@@ -133,15 +133,6 @@ def test_parse_args_normalize_paths(optmanager):
     ]
 
 
-def test_format_plugin():
-    """Verify that format_plugin turns a tuple into a dictionary."""
-    plugin = manager.OptionManager.format_plugin(
-        manager.PluginVersion('Testing', '0.0.0', False)
-    )
-    assert plugin['name'] == 'Testing'
-    assert plugin['version'] == '0.0.0'
-
-
 def test_generate_versions(optmanager):
     """Verify a comma-separated string is generated of registered plugins."""
     optmanager.registered_plugins = [