[ty] Correctly instantiate generic class that inherits __init__ from generic base class#19693
Merged
[ty] Correctly instantiate generic class that inherits __init__ from generic base class#19693
__init__ from generic base class#19693Conversation
Contributor
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
Contributor
|
AlexWaygood
approved these changes
Aug 1, 2025
Member
There was a problem hiding this comment.
Nice! I think you also need to pass the generic context into the __new__ method we synthesize for tuple types here:
ruff/crates/ty_python_semantic/src/types/class.rs
Lines 783 to 827 in 06cd249
Example failing test case on your branch:
class Foo[T, U](tuple[T, U]): ... # false positive error: Argument is incorrect: Expected `tuple[T@Foo, U@Foo]`, found `tuple[Literal[1], Literal[2]]` (invalid-argument-type)
reveal_type(Foo((1, 2))) # revealed: Foo[Unknown, Unknown]
Member
|
Love that primer report!! |
Member
Author
Thank you! Done |
AlexWaygood
approved these changes
Aug 1, 2025
|
|
||
| class C(tuple[T, U]): ... | ||
|
|
||
| reveal_type(C((1, 2))) # revealed: C[int, int] |
Member
There was a problem hiding this comment.
I sort of wish that this was C[Literal[1], Literal[2]] -- I don't think there's a strong reason to upcast the Literal types to int types here, since tuple is covariant. We'd obviously infer tuple[Literal[1], Literal[2]] rather than tuple[int, int] for (1, 2), for similar reasons: the covariance of tuple means that tuple[Literal[1], Literal[2]] is assignable to tuple[int, int], so there's no way that inferring the more precise type can produce false positives.
But that's not for this PR! It's obviously a pre-existing issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is subtle, and the root cause became more apparent with #19604, since we now have many more cases of superclasses and subclasses using different typevars. The issue is easiest to see in the following:
When instantiating a generic class, the
__init__method inherits the generic context of that class. This lets our call binding machinery infer a specialization for that context.Prior to this PR, the instantiation of
Cworked just fine. Its__init__method would inherit the[T]generic context, and we would infer{T = int}as the specialization based on the argument parameters.It didn't work for
D. The issue is that the__init__method was inheriting the generic context of the class where__init__was defined (here,Cand[T]). At the call site, we would then infer{T = int}as the specialization — but that wouldn't help us specializeD[U], sinceDdoes not haveTin its generic context!Instead, the
__init__method should inherit the generic context of the class that we are performing the lookup on (here,Dand[U]). That lets us correctly infer{U = int}as the specialization, which we can successfully apply toD[U].(Note that
__init__refers toC's typevars in its signature, but that's okay; our member lookup logic already applies theT = Uspecialization when returning a member ofCwhile performing a lookup onD, transforming its signature from(Self, T) -> Noneto(Self, U) -> None.)Closes astral-sh/ty#588