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

Better fill invalid generic instances #15656

Closed
wants to merge 2 commits into from

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Jul 13, 2023

Fill invalid generic instances with existing args if possible instead of adding Any for TypeVar. This matches the pyright behavior. There is already an error for the invalid arg count. By reusing the existing args, mypy can at least provide some type checking. That's especially useful if a TypeVar is removed from a class and now there is one to many.

from typing import Generic, TypeVar
T = TypeVar("T")
class A(Generic[T]): ...

a: A[int, bool]
reveal_type(a)
"A" expects 1 type argument, but 2 given  [type-arg]

-Revealed type is "test.A[Any]"
+Revealed type is "test.A[builtins.int]"

@github-actions
Copy link
Contributor

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

steam.py (https://github.com/Gobot1234/steam.py)
+ steam/abc.py:217: error: Need type annotation for "comments"  [var-annotated]

@cdce8p
Copy link
Collaborator Author

cdce8p commented Jul 13, 2023

steam.py (https://github.com/Gobot1234/steam.py)
+ steam/abc.py:217: error: Need type annotation for "comments"  [var-annotated]

The primer issue is expected. Gobot1234/steam.py already uses TypeVar defaults, this is one of these cases.
The error is emitted because we now use the first generic arg while still defaulting to Any for the second which has a default value.

@A5rocks
Copy link
Contributor

A5rocks commented Jul 18, 2023

Unsolicited opinion: I don't really like this. If someone removes anything but the last typevar, then this might cause more errors (because the typevars are now mis-ordered, meaning that specific actions get the wrong types meaning more errors for valid code, where the only error would be the typevars).

On the other hand, although typevars aren't always removed from the end (at least I see no reason why they would be), typevars are normally added to the end IMO. Maybe that specifically could be special cased for better errors?

@cdce8p
Copy link
Collaborator Author

cdce8p commented Jan 24, 2024

There hasn't been much interest in this change. Going to close the PR.
I rewrote parts of the TypeVar fill logic for PEP 696 in #16812 while following the current design (replacing everything with Any if too many args are passed). It should be fairly easy to implement this change later on if we decide to do so.

@cdce8p cdce8p closed this Jan 24, 2024
@cdce8p cdce8p deleted the fill-invalid-generics branch January 24, 2024 12:47
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.

2 participants