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

Warn about unused type: ignore comments when error code is disabled #18849

Merged

Conversation

brianschubert
Copy link
Collaborator

Fixes #11059

This comment has been minimized.

@hauntsaninja
Copy link
Collaborator

The primer hits aren't obvious to me, but maybe I'm missing something?

[case testErrorCodeWarnUnusedIgnores7_WarnWhenErrorCodeDisabled]
# flags: --warn-unused-ignores --disable-error-code name-defined
x # type: ignore[name-defined] # E: Unused "type: ignore" comment [unused-ignore]
"x".foobar(y) # type: ignore[name-defined, attr-defined] # E: Unused "type: ignore[name-defined]" comment [unused-ignore]
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe also check interactions with # type: ignore[unused-ignore]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some tests! The behavior seems to be what we'd want - adding # type: ignore[unused-ignore] suppresses any unused-ignore warnings. There's no way to make mypy warn about an unused # type: ignore[unused-ignore], but I don't think that's a problem?

Copy link
Contributor

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

scipy-stubs (https://github.com/scipy/scipy-stubs)
+ scipy-stubs/sparse/_lil.pyi:149: error: Unused "type: ignore" comment  [unused-ignore]
+ scipy-stubs/sparse/_lil.pyi:167: error: Unused "type: ignore" comment  [unused-ignore]
+ scipy-stubs/sparse/_csr.pyi:28: error: Unused "type: ignore" comment  [unused-ignore]
+ scipy-stubs/sparse/_csr.pyi:41: error: Unused "type: ignore" comment  [unused-ignore]
+ scipy-stubs/sparse/_csc.pyi:30: error: Unused "type: ignore" comment  [unused-ignore]
+ scipy-stubs/sparse/_csc.pyi:39: error: Unused "type: ignore" comment  [unused-ignore]

prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/server/orchestration/core_policy.py:418: error: Unused "type: ignore" comment  [unused-ignore]
+ src/prefect/server/orchestration/core_policy.py:486: error: Unused "type: ignore" comment  [unused-ignore]

static-frame (https://github.com/static-frame/static-frame)
+ static_frame/core/index.py:1509: error: Unused "type: ignore" comment  [unused-ignore]
+ static_frame/core/index_datetime.py:152: error: Unused "type: ignore" comment  [unused-ignore]
+ static_frame/core/index_datetime.py:304: error: Unused "type: ignore" comment  [unused-ignore]
+ static_frame/core/index_hierarchy.py:1034: error: Unused "type: ignore" comment  [unused-ignore]
+ static_frame/core/frame.py:3976: error: Unused "type: ignore" comment  [unused-ignore]
+ static_frame/core/frame.py:5610: error: Unused "type: ignore" comment  [unused-ignore]
+ static_frame/core/frame.py:6199: error: Unused "type: ignore" comment  [unused-ignore]

mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/coretypes/multidict.py:197: error: Unused "type: ignore" comment  [unused-ignore]
+ mitmproxy/proxy/layers/quic/_raw_layers.py:46: error: Unused "type: ignore" comment  [unused-ignore]

kornia (https://github.com/kornia/kornia)
+ kornia/augmentation/auto/base.py:44: error: Unused "type: ignore" comment  [unused-ignore]

werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/exceptions.py:208: error: Unused "type: ignore" comment  [unused-ignore]

sympy (https://github.com/sympy/sympy)
+ sympy/solvers/ode/single.py:1745: error: Unused "type: ignore" comment  [unused-ignore]

dedupe (https://github.com/dedupeio/dedupe)
+ dedupe/predicates.py:120: error: Unused "type: ignore" comment  [unused-ignore]

materialize (https://github.com/MaterializeInc/materialize)
+ misc/python/materialize/mzexplore/common.py:93: error: Unused "type: ignore" comment  [unused-ignore]

pwndbg (https://github.com/pwndbg/pwndbg)
+ pwndbg/aglib/disasm/__init__.py:92: error: Unused "type: ignore" comment  [unused-ignore]
+ pwndbg/aglib/heap/ptmalloc.py:144: error: Unused "type: ignore" comment  [unused-ignore]

jax (https://github.com/google/jax)
+ jax/experimental/mosaic/gpu/inference_utils.py:35: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/experimental/mosaic/gpu/inference_utils.py:46: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/experimental/mosaic/gpu/inference_utils.py:57: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/experimental/mosaic/gpu/inference_utils.py:68: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/experimental/mosaic/gpu/inference_utils.py:75: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/experimental/mosaic/gpu/inference_utils.py:111: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/traceback_util.py:134: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/sharding_specs.py:147: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/pretty_printer.py:54: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/sharding.py:203: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/named_sharding.py:201: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/linear_util.py:470: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/linear_util.py:471: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/core.py:1673: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/sharding_impls.py:177: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/array.py:784: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/interpreters/pxla.py:1637: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/export/_export.py:689: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/_src/export/_export.py:774: error: Unused "type: ignore" comment  [unused-ignore]
+ jax/experimental/jax2tf/jax2tf.py:1424: error: Unused "type: ignore" comment  [unused-ignore]

asynq (https://github.com/quora/asynq)
+ asynq/tools.pyi:89: error: Unused "type: ignore" comment  [unused-ignore]
+ asynq/tools.pyi:91: error: Unused "type: ignore" comment  [unused-ignore]

@brianschubert
Copy link
Collaborator Author

I did some analysis on the primer hits:

  • scipy-stubs: all # type: ignore[explicit-override] comments. Their presence seems to come from them using basedmypy, which I think enables this rule by default? I don't see explicit-override enabled in their pyproject.toml. In any case, I cloned the repo, made a venv to match the mypy_primer.projects definition, ran mypy master (4b1a255) , and got no errors. If I remove the # type: ignore[explicit-override] comments, I still get no errors. So, this seems to be a true positive, the # type: ignore isn't necessary from mypy's perspective. If I add --enable-error-code explicit-override, I do get explicit-override errors on the affected lines.
  • prefect, static-frame, mitmproxy, kornia, werkzeug, dedupe, materialize, asynq: all bare # type: ignore comments. Cloned each repo, removed the # type: ignore comments, ran mypy master, and got no errors on the affected lines. These all seem to be true positives. I didn't check what disabled error codes might be at play / why these # type: ignore comments were added in the first place.
  • pwndbg: one # type: ignore[attr-defined], one # type: ignore[assignment]. These error codes are indeed disabled for their respective modules in pyproject.toml, so the # type: ignore comments are unnecessary. Removing them and running mypy master produced no errors on the affected lines. true positive.
  • jax: only looked at the errors in jax/experimental/mosaic/gpu/inference_utils.py, all were bare # type: ignores. Removing these and running mypy master produced no errors on the affected lines. true positive (plus some unchecked)
  • sympy: not sure about this one. When I run this changeset locally, I don't get the unused-ignore error. I wasn't able to figure out what's different about my environment

@sterliakov
Copy link
Collaborator

sterliakov commented Mar 31, 2025

Re sympy: that seems to depend on how you invoke mypy.

$ mypy sympy --warn-unused-ignores --cache-dir /dev/null | grep ode/single
sympy/solvers/ode/single.py:1745: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:2661: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:2662: error: Unused "type: ignore" comment  [unused-ignore]

$ mypy sympy/solvers/ode/single.py --warn-unused-ignores --cache-dir /dev/null | grep ode/single
sympy/solvers/ode/single.py:22: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:1679: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:2661: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:2662: error: Unused "type: ignore" comment  [unused-ignore]

If you did the latter, unused-ignore doesn't show up.

And with 1.15.0 (compiled):

$ mypy sympy --warn-unused-ignores --cache-dir /dev/null | grep ode/single
sympy/solvers/ode/single.py:2661: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:2662: error: Unused "type: ignore" comment  [unused-ignore]

$ mypy sympy/solvers/ode/single.py --warn-unused-ignores --cache-dir /dev/null | grep ode/single
sympy/solvers/ode/single.py:22: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:1679: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:2661: error: Unused "type: ignore" comment  [unused-ignore]
sympy/solvers/ode/single.py:2662: error: Unused "type: ignore" comment  [unused-ignore]

(and that's a true positive - what they wanted to ignore was [truthy-bool] which isn't even part of --strict AFAIC, they don't have it enabled explicitly anywhere)

@brianschubert
Copy link
Collaborator Author

That's interesting! Yup, that seems to be the case. I was using a variant of mypy sympy/solvers/ode/single.py. Using mypy sympy instead, the unused-ignore error shows up, and I get no error when the # type: ignore comment is removed.

@hauntsaninja hauntsaninja merged commit 6b68661 into python:master Apr 3, 2025
18 checks passed
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.

disable_error_code + unused type:ignore comment
3 participants