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

Support mypy: ignore inline comments to suppress mypy errors #12358

Open
jab opened this issue Mar 15, 2022 · 16 comments · May be fixed by #17875
Open

Support mypy: ignore inline comments to suppress mypy errors #12358

jab opened this issue Mar 15, 2022 · 16 comments · May be fixed by #17875
Labels
feature topic-type-ignore # type: ignore comments

Comments

@jab
Copy link
Contributor

jab commented Mar 15, 2022

Feature

Mypy should allow using inline mypy: ignore comments, treating them exactly the same as the equivalent type: ignore comment.

Pitch

Currently, mypy supports type: ignore comments for suppressing errors only on a given line. These are an essential tool for users hitting a bug in mypy to work around the bug.

However, type: ignore comments affect other type checkers as well. When a bug in mypy is causing it to flag a line that other type checkers are not flagging, the user can't add a type: ignore to that line without causing their other type checkers to complain about an unused ignore.

If users could add a mypy: ignore comment instead, this would not affect other type checkers, and would make it self-evident that the line is only being flagged by mypy, and not other type checkers in use.

Furthermore, since other type checkers now support similar comments just for them (e.g. Pyright now supports pyright: ignore comments), if this feature were implemented for mypy, users using multiple type checkers would have a clearer way of indicating what's going on on a given line:

  1. type: ignore: All type checkers agree there's an error here
  2. mypy: ignore: Only mypy thinks this is an error (false positive in mypy or false negative in others)
  3. pyright: ignore: Only pyright thinks this is an error (false positive in pyright or false negative in others)
    etc.

In summary, this feature would make it easier for users to use mypy with other type checkers on the same codebase, to discover new type checker bugs when they disagree (I've already found and reported several this way myself), and could ultimately lead to type checkers becoming less buggy more quickly and agreeing with one another more often.

@hauntsaninja
Copy link
Collaborator

This feature isn't likely to be implemented, since we use Python's ast as our parser, including for type comments (using ast.parse's type_comments=True).

One slightly verbose way of achieving the same effect would be using the MYPY constant:

MYPY = False  # somewhere
if MYPY:
    dirty_hack = cast(Any, dirty_hack)  # or whatever

@jab
Copy link
Contributor Author

jab commented Mar 16, 2022

Sorry, I didn’t follow that. The goal here is to provide an alternate syntax for an ignore comment that targets mypy only, and not other type checkers.

So adding support for “mypy: ignore” as an alternate spelling of “type: ignore” is a lot harder than it seems?

If so, is there any other syntax you can propose that would be easier to implement and achieve the same effect?

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 16, 2022

The first part of type checking is parsing a string into a syntax tree. mypy uses the stdlib's ast module to do this. ast only has support for type: ignore and there isn't a way to extend the standard library to parse other comments. So supporting this would require using some other library to parse Python or some kind of hack to associate comments with ast nodes, and so is unlikely to happen.

A thing that works today with current versions of mypy is:

MYPY = False
... your program ...
if MYPY:
    do_thing_mypy_doesnt_understand()  # type: ignore
    # or possibly
    # cast(Any, do_thing_mypy_doesnt_understand)()
else:
    do_thing_mypy_doesnt_understand()

A little verbose, but works today and could maybe made more concise depending on context.

@jab
Copy link
Contributor Author

jab commented Mar 17, 2022

Thanks @hauntsaninja, now I see what you meant, and I appreciate the additional explanation and the proposed workaround. That said, I think an important goal for any workaround here is to not impose such a high maintenance burden. It's a smell when a lot of extra code has to be introduced and maintained in a codebase just to work around false positive bugs in a linter.

I wonder if the best way forward here is to open a BPO feature request for ast.parse() to accept a new flag, e.g. directive_comments=True, which would cause any line of code with a comment of the form # <namespace>: <directive> to be exposed in a new directive_comments attribute of the ast.Module object, much like parse()'s type_comments flag causes the type_ignores attribute to be populated. E.g.

>>> dc = ast.parse('foo() # mypy: ignore', directive_comments=True).directive_comments[0]
>>> dc.namespace
'mypy'
>>> dc.directive
'ignore'

This would be more general than the current type_comments flag, and would allow using the ast module to see the millions of such comments that are already in use today (e.g. pylint: disable, flake8: noqa, pragma: no cover, etc.). That doesn't seem like it should be too hard to add? And for the purposes of this issue, it would allow mypy to detect mypy: ignore comments without having to change to a different parser than ast.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 17, 2022

Yeah, if something like that was added to ast, it'd be easy to support mypy: ignore in mypy (at least when mypy is run using Python >=3.xy). I don't have a good read of how CPython would feel about such a feature.

I'm sure you've considered this and found it inadequate, but I will note that turning off settings equivalent to mypy's warn_unused_ignores then just using type: ignore everywhere — possibly with additional non-machine-checked comments like # type: ignore # mypy, pytype — is 50% of a solution and has the advantage of working today.

@JelleZijlstra JelleZijlstra added topic-configuration Configuration files and flags topic-type-ignore # type: ignore comments and removed topic-configuration Configuration files and flags labels Mar 19, 2022
@jhance
Copy link
Collaborator

jhance commented Mar 21, 2022

# type: ignore[mypy:misc, other_checker:foo] seems like a better syntax and actually compatible with the parser. In the case of mypy we would simply recognize mypy:x as the same as x and ignore anything that has : but not mypy: prefix. If the set of pruned codes is empty, then we treat it as not even being there. Using error codes would be forced with this syntax but that seems like a good thing anyway.

I don't think we should introduce complexity into the ast parser to deal with a new case when we can workaround easily (and in a much better way, imo)

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 21, 2022

That's a good suggestion, but we'd need to coordinate with other type checkers for OP's use case to be served (since e.g. currently Pyright treats # type: ignore[whatever] as # type: ignore).

@jab
Copy link
Contributor Author

jab commented Mar 21, 2022

I actually had the same idea a couple weeks ago. (Note this GitHub bug affects that comment link, in case you want to read it in the context of the full discussion, which is relevant to this issue. For now I'll just quote a small excerpt.)

just to throw out another syntax idea, rules could be prefixed: # type: ignore [mypy:<mypy_rule>,pyright:<pyright_rule>]

Even though I proposed that syntax as an alternative to pyright: ignore, Pyright chose to implement only the pyright: ignore syntax. So the ship has possibly sailed for type: ignore [mypy:foo].

FWIW, the mypy: ignore syntax would not only better match the pyright: ignore syntax that Pyright already uses, but also that of many other widely-used linters (pylint: disable, flake8: noqa, etc.).


As long as we're brainstorming other solutions here, it's worth at least mentioning this one:

A simple, heuristic-based (e.g. regex) search for any lines containing # mypy: ignore, that we could even gate behind some experimental --enable-mypy-ignore flag for now, would be Good Enough for every case I've ever needed this, and could probably be implemented in a handful of lines? No need to touch ast or switch to another parser entirely. If it proves useful enough over time to graduate from an experimental implementation, it could be replaced by a more robust implementation (maybe even based on future ast APIs or some other convenient thing that isn't currently available). If this doesn't prove useful enough over time, the simple-but-unrobust implementation could just be removed. And if use of the flag caused a message like "mypy: ignore is experimental and may be removed in a future version", expectations would be set appropriately.

@jhance
Copy link
Collaborator

jhance commented Mar 21, 2022

Given that there is not a standardized behavior for these in the PEP I don't think we should be restricted by what choices Pyright makes. It would be a simple PR on Pyright end to ignore # type: ignore[mypy:foo]. I also think the syntax is much better when it comes to having >2 type checkers and you need to ignore something in >1 of them but not all. # pyright: ignore # mypy: ignore is not pleasing.

@erictraut
Copy link

I would be opposed to checking for errors that are specific to other type checkers within pyright. PEP 484 is pretty clear — a # type: ignore is supposed to suppress type-related errors on that line. It doesn't say anything about text that comes after the # type: ignore. If mypy wants to parse additional text after # type: ignore, that's its prerogative, but it's non-standard behavior. Other type checkers like pyright should ignore any additional contents IMO and follow the PEP 484 standard.

I think it would be better for mypy to support a mypy-specific ignore comment like # mypy: ignore[x, y, z]. I understand the challenges given that mypy uses the Python interpreter for parsing files, but as @jab mentioned, mypy could work around this limitation by performing a separate text scan for such comments.

@jab
Copy link
Contributor Author

jab commented Mar 28, 2022

Thanks for your input, @erictraut, helpful to have it here.

Wanted to mention one more point here: The description of this issue only mentions cases where mypy disagrees with another type checker due to a bug in one of them. However, mypy may also disagree with another type checker due to a deliberately different design decision. (See microsoft/pyright#3260 vs. #12390 for a recent example.) Since bugs are fixed in new releases hopefully more often than design decisions are overturned, this may add to the need for a mypy-specific suppression syntax (that can still be targeted at a specific line where the disagreement occurs, rather than globally -- globally suppressing all errors in a particular category can cause too much collateral damage when the category is broad).

@gvanrossum
Copy link
Member

gvanrossum commented Jun 8, 2022

https://mypy.readthedocs.io/en/stable/common_issues.html#spurious-errors-and-locally-silencing-the-checker

You can use the form # type: ignore[<code>] to only ignore specific errors on the line. This way you are less likely to silence unexpected errors that are not safe to ignore, and this will also document what the purpose of the comment is. See Error codes for more information.

@NeilGirdhar
Copy link
Contributor

NeilGirdhar commented Jun 8, 2022

@gvanrossum I believe this issue is about silencing MyPy in particular. Unfortunately type: ignore[code] doesn't help because Pyright ignores all of the text after type: ignore and treats the directive as "ignore everything". Although I can't find the comment, Eric has explained that that's consistent with the PEP.

@NeilGirdhar
Copy link
Contributor

NeilGirdhar commented Feb 10, 2023

In case anyone's interested, I opened a Python-ideas thread and corresponding CPython issue for expanding ast.parse (as proposed by @jab and @hauntsaninja) to facilitate mypy: ignore directives.

@davidhalter
Copy link

@jab Did you know that # mypy: ignore-errors=True exists? It feels like that's exactly what you want.

@jab jab changed the title Support mypy: ignore comments to suppress mypy errors Support mypy: ignore inline comments to suppress mypy errors May 16, 2024
@jab
Copy link
Contributor Author

jab commented May 16, 2024

@davidhalter, this issue calls for a way to tell mypy (and not other type checkers) to ignore a specific line, i.e. via an inline comment. Currently # mypy: ignore-errors=True only works on an entire-file basis when you put it on its own line (e.g. at the top of the file). As an inline comment it has no effect: https://mypy-play.net/?mypy=latest&python=3.12&gist=c4ca6a29f52e65a04601b80c1c9ac8d7

(Just added "inline" to the title of the issue to make this clearer without reading the description.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-type-ignore # type: ignore comments
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants