Skip to content

Better fill invalid generic instances - #15656

Closed
cdce8p wants to merge 2 commits into
python:masterfrom
cdce8p:fill-invalid-generics
Closed

Better fill invalid generic instances#15656
cdce8p wants to merge 2 commits into
python:masterfrom
cdce8p:fill-invalid-generics

Conversation

@cdce8p

@cdce8p cdce8p commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator

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

github-actions Bot commented Jul 13, 2023

Copy link
Copy Markdown
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

cdce8p commented Jul 13, 2023

Copy link
Copy Markdown
Collaborator Author 
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.

@cdce8p cdce8p mentioned this pull request Jul 13, 2023
Closed
@A5rocks

A5rocks commented Jul 18, 2023

Copy link
Copy Markdown
Collaborator

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

cdce8p commented Jan 24, 2024

Copy link
Copy Markdown
Collaborator Author

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

2 participants

@cdce8p @A5rocks