Skip to content

fix: warn unused ignores when not all specified error codes are used #11178

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

Merged

Conversation

hi-ogawa
Copy link
Contributor

Description

Fixes #11157

For --warn-unused-ignored, currently Errors.used_ignored_lines keeps track of only the line numbers without error codes when errors appeares on the line with # type: ignore or # type: ignore[...].

This PR changes used_ignored_lines so that it also keeps track of error codes and that information used in generate_unused_ignore_errors to show an error message like unused "type: ignore[attr-defined]" comment.

I'm very new to mypy code base and one thing I'm not so confident about my change is that I might be using MISC error codes in an unintended way.
Please let me know if this approach is correct and I appreciate any feedback.
Thank for the review!

Test Plan

There are some tests using --warn-unused-ignores, but this PR didn't change the result of those tests. So, I added a few test named testErrorCodeWarnUnusedIgnores in test-data/unit/check-errorcodes.test which have different behavior between this PR and current master.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

core (https://github.com/home-assistant/core.git)
- homeassistant/components/http/__init__.py:299: error: unused "type: ignore" comment
+ homeassistant/components/http/__init__.py:299: error: unused "type: ignore[arg-type, misc]" comment

ignite (https://github.com/pytorch/ignite)
- ignite/engine/deterministic.py:255: error: unused "type: ignore" comment
+ ignite/engine/deterministic.py:255: error: unused "type: ignore[arg-type, attr-defined]" comment
- ignite/engine/deterministic.py:260: error: unused "type: ignore" comment
+ ignite/engine/deterministic.py:260: error: unused "type: ignore[attr-defined, union-attr]" comment

pandas (https://github.com/pandas-dev/pandas.git)
- pandas/core/arrays/_mixins.py:238: error: unused "type: ignore" comment
+ pandas/core/arrays/_mixins.py:238: error: unused "type: ignore[assignment, type-var]" comment
- pandas/core/arrays/datetimelike.py:497: error: unused "type: ignore" comment
+ pandas/core/arrays/datetimelike.py:497: error: unused "type: ignore[arg-type, return-value]" comment
- pandas/core/arrays/string_arrow.py:739: error: unused "type: ignore" comment
+ pandas/core/arrays/string_arrow.py:739: error: unused "type: ignore[arg-type, type-var]" comment
- pandas/core/arrays/string_.py:551: error: unused "type: ignore" comment
+ pandas/core/arrays/string_.py:551: error: unused "type: ignore[arg-type, type-var]" comment
- pandas/io/parsers/base_parser.py:988: error: unused "type: ignore" comment
+ pandas/io/parsers/base_parser.py:988: error: unused "type: ignore[arg-type, return-value]" comment
- pandas/core/reshape/merge.py:2153: error: unused "type: ignore" comment
+ pandas/core/reshape/merge.py:2153: error: unused "type: ignore[assignment, union-attr]" comment

aiohttp (https://github.com/aio-libs/aiohttp.git)
+ aiohttp/client.py:1063: error: unused "type: ignore[arg-type]" comment

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good.

@JukkaL JukkaL merged commit 8b47a03 into python:master Sep 24, 2021
@hi-ogawa hi-ogawa deleted the 11157-warn-unused-ignores-for-multiple-codes branch September 24, 2021 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

warn-unused-ignores doesn't fire when ignoring multiple codes
2 participants