Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-commit mypy run fails to catch and report type errors #9502

Closed
obi1kenobi opened this issue Jan 24, 2021 · 2 comments · Fixed by #9700
Closed

Pre-commit mypy run fails to catch and report type errors #9502

obi1kenobi opened this issue Jan 24, 2021 · 2 comments · Fixed by #9700

Comments

@obi1kenobi
Copy link

Currently, pip uses mypy via a pre-commit hook and with some custom configuration specified in the setup.cfg file.

However, due to a surprising difference in behavior in mypy between specifying individual files vs specifying directories to be checked, the way that pre-commit calls mypy ends up unexpectedly hiding 51 type errors that are present in the files being checked and that are prohibited by the mypy configuration specified in setup.cfg.

Environment

  • pip version: Issue found on commit 8d042c448718355447f133c440f7a931a5ae57c0, which is the tip of the master branch as of the time of this post.
  • Python version: Python 3.8.5
  • OS: Ubuntu 20.04.1 LTS (focal) inside WSL2
  • mypy: version 0.790, same as specified in the pre-commit hook config

Expected behavior
The following two commands should produce equivalent results i.e. neither of them finds any type errors on the current master branch (exact commit hash specified above):

  • pre-commit run mypy --all-files
  • mypy src/

Actual behavior & How to Reproduce
Running pre-commit run mypy --all-files finds no type errors, as expected — this is enforced by CI.

However, running mypy src/ finds 51 type errors. Adding the --verbose flag to the invocation confirms that mypy indeed found and applied the setup.cfg config, see this excerpt from the (extremely verbose) mypy log:

LOG:  Mypy Version:           0.790
LOG:  Config File:            setup.cfg

Cause
Some digging into pre-commit shows that it uses a slightly different but nevertheless reasonable mypy invocation: it explicitly lists all the files to be checked rather than specifying an entire directory. This is probably in order to be able to apply the exclude rules specified in the pre-commit config. The abbreviated form of the mypy invocation it uses is the following (I include the full and rather long invocation at the bottom of the issue):

mypy --pretty < ... long list of space-separated .py files in the src/ directory ... >

Unlike the mypy src/ invocation, this mypy invocation surprisingly finds no issues!

I have verified the following things:

  • The --pretty flag has no effect on the output of the mypy invocation used by pre-commit.
  • The files where mypy finds errors in the mypy src/ invocation exist in the list of files in the invocation used by pre-commit.
  • Both invocations successfully locate and use the mypy configuration specified in setup.cfg (confirmed by using the --verbose flag and checking that mypy reports setup.cfg as the source of the applied configuration).

Additionally, the type errors reported by mypy src/ appear to be real issues. Just a couple of examples:

src/pip/_internal/resolution/resolvelib/factory.py:195: error: Item "None" of "Optional[Requirement]" has no attribute "name"
src/pip/_internal/resolution/resolvelib/factory.py:199: error: Item "None" of "Optional[Requirement]" has no attribute "specifier"

Here is the corresponding code: https://github.com/pypa/pip/blob/master/src/pip/_internal/resolution/resolvelib/factory.py#L179-L199

On line 195, template has type InstallRequirement, so template.req has type Optional[Requirement] per: https://github.com/pypa/pip/blob/master/src/pip/_internal/req/req_install.py#L95-L120

At no point is template.req checked for None before line 195 accesses template.req.name, so this is a real issue. Similarly, on line 199, ireq.req is also of type Optional[Requirement] but is never checked for None before ireq.req.specifier is accessed, so this is similarly a real issue.

One could argue that this surprising behavior is a bug in mypy and/or pre-commit and therefore should not be opened in the pip project. I will be opening a corresponding GitHub issue on mypy about this surprising behavior. However, I wanted to directly notify the pip maintainers of this problem as well, because currently the fact that mypy is running via pre-commit is in a way self-deceptive as the specified mypy configuration does actually produce real errors when run outside of pre-commit.

If the maintainers would like me to, I'd be happy to help contribute to resolving this issue.

Output

Below is the full output of mypy src/. As mentioned above, I have verified that this output takes into account the mypy settings configured in setup.cfg.

$ mypy src/
src/pip/_internal/utils/misc.py:526: error: "None" has no attribute "require"src/pip/_internal/utils/logging.py:27: error: Incompatible types in assignment (expression has type "None", variable has type Module)src/pip/_internal/req/req_set.py:36: error: Argument 1 to "canonicalize_name" has incompatible type "Optional[str]"; expected "str"src/pip/_internal/req/req_set.py:44: error: Argument 1 to "canonicalize_name" has incompatible type "Optional[str]"; expected "str"src/pip/_internal/req/req_set.py:130: error: Item "None" of "Optional[Requirement]" has no attribute "specifier"src/pip/_internal/req/req_install.py:367: error: Incompatible types in assignment (expression has type "str", variable has type "NormalizedName")src/pip/_internal/network/auth.py:283: error: "Response" has no attribute "connection"src/pip/_internal/cli/progress_bars.py:21: error: Incompatible types in assignment (expression has type "None", variable has type Module)src/pip/_internal/cli/progress_bars.py:52: error: Argument 1 to "_select_progress_class" has incompatible type "Type[IncrementalBar]"; expected "Bar"src/pip/_internal/cli/progress_bars.py:52: error: Argument 2 to "_select_progress_class" has incompatible type "Type[Bar]"; expected "Bar"
src/pip/_internal/cli/progress_bars.py:125: error: Incompatible types in assignment (expression has type "Tuple[str, str, str]", base class "IncrementalBar" defined the type as "Tuple[str, str, str, str, str, str, str, str, str]")
src/pip/_internal/cli/progress_bars.py:191: error: "AnsiToWin32" has no attribute "isatty"
src/pip/_internal/cli/progress_bars.py:195: error: "AnsiToWin32" has no attribute "flush"
src/pip/_internal/cli/progress_bars.py:211: error: Cannot determine type of 'hide_cursor' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:211: error: Cannot determine type of 'file' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:215: error: Cannot determine type of 'hide_cursor' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:215: error: Cannot determine type of 'file' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:220: error: Cannot determine type of 'hide_cursor' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:220: error: Cannot determine type of 'file' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:225: error: Cannot determine type of 'hide_cursor' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:225: error: Cannot determine type of 'file' in base class 'WindowsMixin'
src/pip/_internal/cli/progress_bars.py:230: error: Cannot determine type of 'hide_cursor' in base class 'WindowsMixin'
src/pip/_internal/resolution/resolvelib/base.py:66: error: Argument 1 to "contains" of "SpecifierSet" has incompatible type "_BaseVersion"; expected "Union[Union[Version, LegacyVersion], str]"
src/pip/_internal/req/constructors.py:191: error: Incompatible types in assignment (expression has type "None", variable has type "Requirement")
src/pip/_internal/req/constructors.py:369: error: Incompatible types in assignment (expression has type "None", variable has type "Requirement")
src/pip/_internal/commands/show.py:65: error: "None" has no attribute "__iter__" (not iterable)
src/pip/_internal/commands/show.py:79: error: "None" has no attribute "__iter__" (not iterable)
src/pip/_internal/commands/debug.py:212: error: Argument 2 to "show_value" has incompatible type "bool"; expected "Optional[str]"
src/pip/_internal/wheel_builder.py:189: error: Argument 1 to "canonicalize_name" has incompatible type "Optional[str]"; expected "str"
src/pip/_internal/wheel_builder.py:205: error: Unsupported operand types for <= ("Version" and "None")
src/pip/_internal/wheel_builder.py:205: note: Left operand is of type "Optional[Version]"
src/pip/_internal/wheel_builder.py:261: error: Argument "backend" to "build_wheel_pep517" has incompatible type "Optional[Pep517HookCaller]"; expected "Pep517HookCaller"
src/pip/_internal/resolution/resolvelib/requirements.py:77: error: Item "None" of "Optional[Requirement]" has no attribute "name"
src/pip/_internal/resolution/resolvelib/requirements.py:111: error: Item "None" of "Optional[Requirement]" has no attribute "specifier"
src/pip/_internal/resolution/resolvelib/requirements.py:112: error: Argument 1 to "contains" of "SpecifierSet" has incompatible type "_BaseVersion"; expected "Union[Union[Version, LegacyVersion], str]"
src/pip/_internal/resolution/resolvelib/requirements.py:150: error: Argument 1 to "contains" of "SpecifierSet" has incompatible type "_BaseVersion"; expected "Union[Union[Version, LegacyVersion], str]"
src/pip/_internal/resolution/resolvelib/requirements.py:160: error: Argument 1 to "contains" of "SpecifierSet" has incompatible type "_BaseVersion"; expected "Union[Union[Version, LegacyVersion], str]"
src/pip/_internal/operations/check.py:52: error: Incompatible return value type (got "Tuple[Dict[NormalizedName, PackageDetails], bool]", expected "Tuple[Dict[str, PackageDetails], bool]")
src/pip/_internal/operations/check.py:139: error: Incompatible return value type (got "Set[NormalizedName]", expected "Set[str]")
src/pip/_internal/operations/check.py:139: note: Perhaps you need a type annotation for "installed"? Suggestion: "Set[str]"
src/pip/_internal/resolution/resolvelib/factory.py:195: error: Item "None" of "Optional[Requirement]" has no attribute "name"
src/pip/_internal/resolution/resolvelib/factory.py:199: error: Item "None" of "Optional[Requirement]" has no attribute "specifier"
src/pip/_internal/resolution/resolvelib/factory.py:364: error: No overload variant of "get" of "Mapping" matches argument type "str"
src/pip/_internal/resolution/resolvelib/factory.py:364: note: Possible overload variant:
src/pip/_internal/resolution/resolvelib/factory.py:364: note:     def get(self, key: NormalizedName) -> Optional[Distribution]
src/pip/_internal/resolution/resolvelib/factory.py:364: note:     <1 more non-matching overload not shown>
src/pip/_internal/resolution/resolvelib/resolver.py:31: error: Module 'pip._vendor.resolvelib.structs' has no attribute 'Graph'
src/pip/_internal/resolution/resolvelib/resolver.py:92: error: Argument 1 to "canonicalize_name" has incompatible type "Optional[str]"; expected "str"
src/pip/_internal/resolution/resolvelib/resolver.py:116: error: Incompatible types in assignment (expression has type "PipReporter", variable has type "PipDebuggingReporter")
src/pip/_internal/resolution/resolvelib/resolver.py:130: error: Item "None" of "Optional[Result]" has no attribute "mapping"
src/pip/_internal/commands/search.py:130: error: "None" has no attribute "__iter__" (not iterable)
src/pip/_internal/commands/list.py:212: error: "_BaseVersion" has no attribute "is_prerelease"
src/pip/_internal/commands/list.py:219: error: Incompatible return value type (got "None", expected "Distribution")
src/pip/_internal/commands/list.py:227: error: "Distribution" has no attribute "latest_version"; maybe "has_version" or "_get_version"?
src/pip/_internal/commands/list.py:228: error: "Distribution" has no attribute "latest_filetype"
src/pip/_internal/commands/install.py:53: error: Argument 1 to "canonicalize_name" has incompatible type "Optional[str]"; expected "str"
Found 51 errors in 18 files (checked 374 source files)

The full mypy invocation used by pre-commit and its output are the following:

$ mypy --pretty src/pip/_internal/commands/install.py src/pip/_internal/utils/glibc.py src/pip/_internal/models/candidate.py src/pip/_internal/commands/debug.py src/pip/_internal/req/req_tracker.py src/pip/_internal/cli/cmdoptions.py src/pip/_internal/commands/freeze.py src/pip/_internal/resolution/base.py src/pip/_internal/resolution/legacy/resolver.py src/pip/_internal/operations/install/legacy.py src/pip/_internal/commands/__init__.py src/pip/_internal/resolution/resolvelib/resolver.py src/pip/_internal/cli/status_codes.py src/pip/_internal/cli/main.py src/pip/_internal/operations/__init__.py src/pip/_internal/network/auth.py src/pip/_internal/req/req_file.py src/pip/_internal/models/search_scope.py src/pip/_internal/operations/prepare.py src/pip/_internal/network/download.py src/pip/_internal/vcs/git.py src/pip/_internal/utils/subprocess.py src/pip/_internal/req/req_install.py src/pip/_internal/utils/datetime.py src/pip/_internal/resolution/resolvelib/found_candidates.py src/pip/_internal/distributions/installed.py src/pip/_internal/utils/wheel.py src/pip/_internal/models/selection_prefs.py src/pip/_internal/vcs/subversion.py src/pip/_internal/utils/models.py src/pip/_internal/cli/main_parser.py src/pip/_internal/operations/install/wheel.py src/pip/_internal/utils/encoding.py src/pip/_internal/utils/inject_securetransport.py src/pip/_internal/utils/packaging.py src/pip/_internal/models/wheel.py src/pip/_internal/commands/list.py src/pip/_internal/cli/req_command.py src/pip/_internal/utils/compat.py src/pip/_internal/cli/spinners.py src/pip/_internal/req/req_uninstall.py setup.py src/pip/_internal/resolution/resolvelib/base.py src/pip/_internal/utils/logging.py src/pip/_internal/distributions/__init__.py src/pip/_internal/vcs/mercurial.py src/pip/_internal/pyproject.py src/pip/_internal/commands/show.py src/pip/_internal/utils/unpacking.py src/pip/_internal/utils/parallel.py src/pip/_internal/self_outdated_check.py src/pip/_internal/utils/hashes.py src/pip/_internal/utils/direct_url_helpers.py src/pip/_internal/operations/build/wheel.py src/pip/_internal/index/package_finder.py src/pip/_internal/distributions/wheel.py src/pip/_internal/network/utils.py src/pip/_internal/resolution/resolvelib/candidates.py src/pip/_internal/models/scheme.py src/pip/_internal/utils/__init__.py src/pip/_internal/operations/install/editable_legacy.py src/pip/_internal/network/__init__.py src/pip/_internal/operations/build/__init__.py src/pip/_internal/cli/parser.py src/pip/_internal/resolution/__init__.py src/pip/_internal/utils/entrypoints.py src/pip/_internal/distributions/sdist.py src/pip/_internal/models/target_python.py src/pip/_internal/cli/progress_bars.py tools/automation/release/__init__.py src/pip/_internal/vcs/__init__.py src/pip/_internal/operations/check.py src/pip/_internal/resolution/resolvelib/provider.py src/pip/_internal/commands/configuration.py src/pip/_internal/resolution/resolvelib/factory.py src/pip/_internal/__init__.py src/pip/_internal/distributions/base.py src/pip/_internal/vcs/versioncontrol.py src/pip/_internal/operations/build/metadata.py src/pip/_internal/utils/virtualenv.py src/pip/_internal/utils/pkg_resources.py src/pip/_internal/vcs/bazaar.py src/pip/__main__.py src/pip/_internal/resolution/resolvelib/reporter.py src/pip/_internal/operations/install/__init__.py src/pip/_internal/operations/freeze.py src/pip/_internal/index/__init__.py src/pip/_internal/operations/build/metadata_legacy.py src/pip/_internal/req/constructors.py src/pip/_internal/utils/typing.py src/pip/_internal/resolution/resolvelib/requirements.py src/pip/_internal/utils/deprecation.py tools/tox_pip.py src/pip/_internal/main.py src/pip/_internal/models/direct_url.py src/pip/_internal/cli/base_command.py src/pip/_internal/commands/help.py src/pip/_internal/resolution/resolvelib/__init__.py noxfile.py src/pip/_internal/utils/urls.py src/pip/_internal/network/session.py src/pip/_internal/commands/download.py src/pip/_internal/network/xmlrpc.py src/pip/_internal/commands/hash.py src/pip/_internal/cli/__init__.py src/pip/__init__.py src/pip/_internal/operations/build/wheel_legacy.py src/pip/_internal/resolution/legacy/__init__.py src/pip/_internal/configuration.py src/pip/_internal/cache.py src/pip/_internal/utils/misc.py src/pip/_internal/build_env.py src/pip/_internal/locations.py src/pip/_internal/commands/search.py src/pip/_internal/wheel_builder.py src/pip/_internal/utils/appdirs.py src/pip/_internal/network/lazy_wheel.py src/pip/_internal/utils/filetypes.py src/pip/_internal/models/format_control.py src/pip/_internal/exceptions.py src/pip/_internal/cli/command_context.py src/pip/_internal/models/__init__.py src/pip/_internal/cli/autocompletion.py src/pip/_internal/index/collector.py src/pip/_internal/models/index.py src/pip/_internal/commands/uninstall.py src/pip/_internal/utils/distutils_args.py src/pip/_internal/commands/check.py tools/automation/release/check_version.py src/pip/_internal/utils/filesystem.py src/pip/_internal/commands/cache.py src/pip/_internal/commands/completion.py src/pip/_internal/utils/compatibility_tags.py src/pip/_internal/commands/wheel.py src/pip/_internal/network/cache.py src/pip/_internal/utils/temp_dir.py src/pip/_internal/req/req_set.py src/pip/_internal/req/__init__.py src/pip/_internal/models/link.py src/pip/_internal/utils/setuptools_build.py
Success: no issues found in 140 source files
@obi1kenobi
Copy link
Author

Per python/mypy#9954 the root cause is the follow_imports = skip setting which seems to be inappropriately set here. Changing that setting to silent (exact config below) results in mypy correctly finding the 51 type errors:

[mypy]
follow_imports = silent
ignore_missing_imports = True
disallow_untyped_defs = True
disallow_any_generics = True
warn_unused_ignores = True

[mypy-pip/_vendor/*]
follow_imports = silent
ignore_errors = True

I'd be happy to open a PR with this config change, but the mypy CI pass will (correctly!) fail due to the errors. What would be the best way forward here?

@sbidoul
Copy link
Member

sbidoul commented Jan 24, 2021

@obi1kenobi Interesting find. I guess the only way forward is to file several reasonably small PRs to fix all errors until we can flip the switch in CI.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants