Replace # type: ignores without error codes

[srittau]

pyright recently added # pyright: ignore[...]. We should replace all plain # type: ignores with either # type: ignore[...] or # pyright: ignore[...] if possible. There might be a few ignores we can't replace.

$ rgrep "# type: ignore" stubs/ stdlib/ | grep -v "# type: ignore\[" | grep -v @python2 | wc -l
101

[srittau]

Cf #7495

[AlexWaygood]

If we do this for all the type: ignores that are irrelevant to mypy, we can also consider trying out the new ignore-without-code mypy option, which will complain if we have a type: ignore comment without a mypy error code.

[JelleZijlstra]

Some ignores may also be necessary for pytype (and maybe pyre, which we don't run in CI).

[rchen152]

Pytype discards # type: ignores without doing anything with them, so removing them shouldn't affect it :)

[Akuli]

Let's say someone makes a new type checker that doesn't attempt to parse # pyright: ignore[...] comments, but understands the standard # type: ignore comments. Should it display an error if it notices that there's something wrong with the stubs? Or just silence all errors coming from stub files?

I generally don't like introducing type checker specific things into stubs. While # type: ignore[error-code] is mypy-specific, other type checkers tend to treat it the same as # type: ignore. But how is a type checker supposed to know what to do with # pyright: ignore[...]?

I find it unlikely that we need to say "ignore this error in pyright but not in any other type checker". An error like that would be basically a bug in pyright, and those get fixed very quickly.

[srittau]

The ignores serve our internal tests. Type checkers shouldn't warn about perceived problems in typeshed or external stubs. (I also think it's also problematic for a type checker to treat "malformed" ignores as a regular ignore.) Everything else would mean we'd potentially have to annotate every line in typeshed with a ignore, because who knows what a type checkers decides to warn about.

Using plain # type: ignore in stubs is dangerous as it hides potential problems. It's also a documentation issue as it's often not immediately clear why a certain line is ignored.

[srittau]

One further advantage of using error codes is that it gives us the chance to collect statistics about the types of # type: ignores we use. This could influence the options we use in tests and point to areas of improvement in type checkers.

[AlexWaygood]

Having filed #7504, I can report that the type: ignores needed by pyright but not mypy are these, these and these. There appear to be three kinds of error that (with our current CI) pyright picks up on where mypy doesn't:

1. Instance methods where the first parameter isn't named self/classmethods where the first parameter isn't named cls.
2. Functions that use a TypeVar only once, leaving the TypeVar unbound.
3. Properties where the setter is "incompatible" with the getter.

There appear to be only 12 type: ignores across the codebase that are needed by pyright but not by other type checkers, following the changes made to the pyright config settings in #7498.

[srittau]

Point 3 is a warning we should probably just disable in typeshed. Point 1 could be disabled if stubtest checks the name of self/cls against the implementation. Point 2 is very useful, though, and we should probably review those cases.

[AlexWaygood]

Here's the rundown of what stubtest checks for us:

• stubtest doesn't currently check the name of self/cls for dunders. (In fact, it doesn't check any parameter names for dunders. And there are many which are incorrect.)
• stubtest does check parameter names for "special" dunder methods such as __init__ and __new__, and it does check all parameter names for non-dunder methods.
• For positional-only parameters, however, it declines to error if the parameter names "approximately" match the implementation.

So, there are a few holes in stubtest's checks with regards to parameter names, and it's possible that pyright could catch a few typos.

It's also equally possible that this pyright check is making it more difficult to match the runtime implementation, though. Most dunders in the fractions stdlib module don't have self as their first argument name. If we wanted to match that in the typeshed stubs, we'd currently have to litter the module with type: ignores.

It also probably just doesn't really matter that much to match the parameter names in dunders, though. So I don't think it's a massive problem keeping the pyright config as it is for point 1.