Skip to content

Commit 33b942c

Browse files
authored
[ty] Handle annotated self parameter in constructor of non-invariant generic classes (#21325)
This manifested as an error when inferring the type of a PEP-695 generic class via its constructor parameters: ```py class D[T, U]: @overload def __init__(self: "D[str, U]", u: U) -> None: ... @overload def __init__(self, t: T, u: U) -> None: ... def __init__(self, *args) -> None: ... # revealed: D[Unknown, str] # SHOULD BE: D[str, str] reveal_type(D("string")) ``` This manifested because `D` is inferred to be bivariant in both `T` and `U`. We weren't seeing this in the equivalent example for legacy typevars, since those default to invariant. (This issue also showed up for _covariant_ typevars, so this issue was not limited to bivariance.) The underlying cause was because of a heuristic that we have in our current constraint solver, which attempts to handle situations like this: ```py def f[T](t: T | None): ... f(None) ``` Here, the `None` argument matches the non-typevar union element, so this argument should not add any constraints on what `T` can specialize to. Our previous heuristic would check for this by seeing if the argument type is a subtype of the parameter annotation as a whole — even if it isn't a union! That would cause us to erroneously ignore the `self` parameter in our constructor call, since bivariant classes are equivalent to each other, regardless of their specializations. The quick fix is to move this heuristic "down a level", so that we only apply it when the parameter annotation is a union. This heuristic should go away completely 🤞 with the new constraint solver.
1 parent 9ce3230 commit 33b942c

File tree

4 files changed

+50
-37
lines changed

4 files changed

+50
-37
lines changed

crates/ty_python_semantic/resources/mdtest/assignment/annotations.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -674,8 +674,7 @@ x6: Covariant[Any] = covariant(1)
674674
x7: Contravariant[Any] = contravariant(1)
675675
x8: Invariant[Any] = invariant(1)
676676

677-
# TODO: This could reveal `Bivariant[Any]`.
678-
reveal_type(x5) # revealed: Bivariant[Literal[1]]
677+
reveal_type(x5) # revealed: Bivariant[Any]
679678
reveal_type(x6) # revealed: Covariant[Any]
680679
reveal_type(x7) # revealed: Contravariant[Any]
681680
reveal_type(x8) # revealed: Invariant[Any]

crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,9 +436,7 @@ def test_seq(x: Sequence[T]) -> Sequence[T]:
436436
def func8(t1: tuple[complex, list[int]], t2: tuple[int, *tuple[str, ...]], t3: tuple[()]):
437437
reveal_type(test_seq(t1)) # revealed: Sequence[int | float | complex | list[int]]
438438
reveal_type(test_seq(t2)) # revealed: Sequence[int | str]
439-
440-
# TODO: this should be `Sequence[Never]`
441-
reveal_type(test_seq(t3)) # revealed: Sequence[Unknown]
439+
reveal_type(test_seq(t3)) # revealed: Sequence[Never]
442440
```
443441

444442
### `__init__` is itself generic
@@ -466,6 +464,7 @@ wrong_innards: C[int] = C("five", 1)
466464
from typing_extensions import overload, Generic, TypeVar
467465

468466
T = TypeVar("T")
467+
U = TypeVar("U")
469468

470469
class C(Generic[T]):
471470
@overload
@@ -497,6 +496,17 @@ C[int](12)
497496
C[None]("string") # error: [no-matching-overload]
498497
C[None](b"bytes") # error: [no-matching-overload]
499498
C[None](12)
499+
500+
class D(Generic[T, U]):
501+
@overload
502+
def __init__(self: "D[str, U]", u: U) -> None: ...
503+
@overload
504+
def __init__(self, t: T, u: U) -> None: ...
505+
def __init__(self, *args) -> None: ...
506+
507+
reveal_type(D("string")) # revealed: D[str, str]
508+
reveal_type(D(1)) # revealed: D[str, int]
509+
reveal_type(D(1, "string")) # revealed: D[int, str]
500510
```
501511

502512
### Synthesized methods with dataclasses

crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,9 +375,7 @@ def test_seq[T](x: Sequence[T]) -> Sequence[T]:
375375
def func8(t1: tuple[complex, list[int]], t2: tuple[int, *tuple[str, ...]], t3: tuple[()]):
376376
reveal_type(test_seq(t1)) # revealed: Sequence[int | float | complex | list[int]]
377377
reveal_type(test_seq(t2)) # revealed: Sequence[int | str]
378-
379-
# TODO: this should be `Sequence[Never]`
380-
reveal_type(test_seq(t3)) # revealed: Sequence[Unknown]
378+
reveal_type(test_seq(t3)) # revealed: Sequence[Never]
381379
```
382380

383381
### `__init__` is itself generic
@@ -436,6 +434,17 @@ C[int](12)
436434
C[None]("string") # error: [no-matching-overload]
437435
C[None](b"bytes") # error: [no-matching-overload]
438436
C[None](12)
437+
438+
class D[T, U]:
439+
@overload
440+
def __init__(self: "D[str, U]", u: U) -> None: ...
441+
@overload
442+
def __init__(self, t: T, u: U) -> None: ...
443+
def __init__(self, *args) -> None: ...
444+
445+
reveal_type(D("string")) # revealed: D[str, str]
446+
reveal_type(D(1)) # revealed: D[str, int]
447+
reveal_type(D(1, "string")) # revealed: D[int, str]
439448
```
440449

441450
### Synthesized methods with dataclasses

crates/ty_python_semantic/src/types/generics.rs

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,31 +1393,6 @@ impl<'db> SpecializationBuilder<'db> {
13931393
return Ok(());
13941394
}
13951395

1396-
// If the actual type is a subtype of the formal type, then return without adding any new
1397-
// type mappings. (Note that if the formal type contains any typevars, this check will
1398-
// fail, since no non-typevar types are assignable to a typevar. Also note that we are
1399-
// checking _subtyping_, not _assignability_, so that we do specialize typevars to dynamic
1400-
// argument types; and we have a special case for `Never`, which is a subtype of all types,
1401-
// but which we also do want as a specialization candidate.)
1402-
//
1403-
// In particular, this handles a case like
1404-
//
1405-
// ```py
1406-
// def f[T](t: T | None): ...
1407-
//
1408-
// f(None)
1409-
// ```
1410-
//
1411-
// without specializing `T` to `None`.
1412-
if !matches!(formal, Type::ProtocolInstance(_))
1413-
&& !actual.is_never()
1414-
&& actual
1415-
.when_subtype_of(self.db, formal, self.inferable)
1416-
.is_always_satisfied(self.db)
1417-
{
1418-
return Ok(());
1419-
}
1420-
14211396
// Remove the union elements from `actual` that are not related to `formal`, and vice
14221397
// versa.
14231398
//
@@ -1473,10 +1448,30 @@ impl<'db> SpecializationBuilder<'db> {
14731448
self.add_type_mapping(*formal_bound_typevar, remaining_actual, filter);
14741449
}
14751450
(Type::Union(formal), _) => {
1476-
// Second, if the formal is a union, and precisely one union element _is_ a typevar (not
1477-
// _contains_ a typevar), then we add a mapping between that typevar and the actual
1478-
// type. (Note that we've already handled above the case where the actual is
1479-
// assignable to any _non-typevar_ union element.)
1451+
// Second, if the formal is a union, and precisely one union element is assignable
1452+
// from the actual type, then we don't add any type mapping. This handles a case like
1453+
//
1454+
// ```py
1455+
// def f[T](t: T | None): ...
1456+
//
1457+
// f(None)
1458+
// ```
1459+
//
1460+
// without specializing `T` to `None`.
1461+
//
1462+
// Otherwise, if precisely one union element _is_ a typevar (not _contains_ a
1463+
// typevar), then we add a mapping between that typevar and the actual type.
1464+
if !actual.is_never() {
1465+
let assignable_elements = (formal.elements(self.db).iter()).filter(|ty| {
1466+
actual
1467+
.when_subtype_of(self.db, **ty, self.inferable)
1468+
.is_always_satisfied(self.db)
1469+
});
1470+
if assignable_elements.exactly_one().is_ok() {
1471+
return Ok(());
1472+
}
1473+
}
1474+
14801475
let bound_typevars =
14811476
(formal.elements(self.db).iter()).filter_map(|ty| ty.as_typevar());
14821477
if let Ok(bound_typevar) = bound_typevars.exactly_one() {

0 commit comments

Comments
 (0)