[ty] Improve assignability/subtyping between two protocol types #20368
Conversation
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
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-10-08 18:35:15.522415266 +0000
+++ new-output.txt 2025-10-08 18:35:18.851436775 +0000
@@ -104,6 +104,7 @@
callables_annotation.py:57:18: error[invalid-type-form] List literals are not allowed in this context in a type expression: Did you mean `list[int]`?
callables_annotation.py:58:5: error[invalid-type-form] Special form `typing.Callable` expected exactly two arguments (parameter types and return type)
callables_annotation.py:58:14: error[invalid-type-form] The first argument to `Callable` must be either a list of types, ParamSpec, Concatenate, or `...`
+callables_annotation.py:157:5: error[invalid-assignment] Object of type `Proto7` is not assignable to `Proto6`
callables_kwargs.py:24:5: error[type-assertion-failure] Argument does not have asserted type `int`
callables_kwargs.py:32:9: error[type-assertion-failure] Argument does not have asserted type `str`
callables_kwargs.py:35:5: error[type-assertion-failure] Argument does not have asserted type `str`
@@ -130,6 +131,35 @@
callables_protocol.py:311:1: error[invalid-assignment] Object of type `def cb14_no_default(*, path: str) -> str` is not assignable to `Proto14_Default`
callables_subtyping.py:26:5: error[invalid-assignment] Object of type `(int, /) -> int` is not assignable to `(int | float, /) -> int | float`
callables_subtyping.py:29:5: error[invalid-assignment] Object of type `(int | float, /) -> int | float` is not assignable to `(int, /) -> int`
+callables_subtyping.py:51:5: error[invalid-assignment] Object of type `PosOnly2` is not assignable to `Standard2`
+callables_subtyping.py:52:5: error[invalid-assignment] Object of type `KwOnly2` is not assignable to `Standard2`
+callables_subtyping.py:55:5: error[invalid-assignment] Object of type `KwOnly2` is not assignable to `PosOnly2`
+callables_subtyping.py:58:5: error[invalid-assignment] Object of type `PosOnly2` is not assignable to `KwOnly2`
+callables_subtyping.py:82:5: error[invalid-assignment] Object of type `NoArgs3` is not assignable to `IntArgs3`
+callables_subtyping.py:85:5: error[invalid-assignment] Object of type `NoArgs3` is not assignable to `FloatArgs3`
+callables_subtyping.py:86:5: error[invalid-assignment] Object of type `IntArgs3` is not assignable to `FloatArgs3`
+callables_subtyping.py:116:5: error[invalid-assignment] Object of type `IntArgs4` is not assignable to `PosOnly4`
+callables_subtyping.py:119:5: error[invalid-assignment] Object of type `StrArgs4` is not assignable to `IntStrArgs4`
+callables_subtyping.py:120:5: error[invalid-assignment] Object of type `IntArgs4` is not assignable to `IntStrArgs4`
+callables_subtyping.py:122:5: error[invalid-assignment] Object of type `IntArgs4` is not assignable to `StrArgs4`
+callables_subtyping.py:124:5: error[invalid-assignment] Object of type `StrArgs4` is not assignable to `IntArgs4`
+callables_subtyping.py:126:5: error[invalid-assignment] Object of type `StrArgs4` is not assignable to `Standard4`
+callables_subtyping.py:151:5: error[invalid-assignment] Object of type `NoKwargs5` is not assignable to `IntKwargs5`
+callables_subtyping.py:154:5: error[invalid-assignment] Object of type `NoKwargs5` is not assignable to `FloatKwargs5`
+callables_subtyping.py:155:5: error[invalid-assignment] Object of type `IntKwargs5` is not assignable to `FloatKwargs5`
+callables_subtyping.py:187:5: error[invalid-assignment] Object of type `IntKwargs6` is not assignable to `KwOnly6`
+callables_subtyping.py:190:5: error[invalid-assignment] Object of type `StrKwargs6` is not assignable to `IntStrKwargs6`
+callables_subtyping.py:191:5: error[invalid-assignment] Object of type `IntKwargs6` is not assignable to `IntStrKwargs6`
+callables_subtyping.py:193:5: error[invalid-assignment] Object of type `IntKwargs6` is not assignable to `StrKwargs6`
+callables_subtyping.py:195:5: error[invalid-assignment] Object of type `StrKwargs6` is not assignable to `IntKwargs6`
+callables_subtyping.py:196:5: error[invalid-assignment] Object of type `IntStrKwargs6` is not assignable to `Standard6`
+callables_subtyping.py:197:5: error[invalid-assignment] Object of type `StrKwargs6` is not assignable to `Standard6`
+callables_subtyping.py:236:5: error[invalid-assignment] Object of type `NoDefaultArg8` is not assignable to `DefaultArg8`
+callables_subtyping.py:237:5: error[invalid-assignment] Object of type `NoX8` is not assignable to `DefaultArg8`
+callables_subtyping.py:240:5: error[invalid-assignment] Object of type `NoX8` is not assignable to `NoDefaultArg8`
+callables_subtyping.py:243:5: error[invalid-assignment] Object of type `NoDefaultArg8` is not assignable to `NoX8`
+callables_subtyping.py:273:5: error[invalid-assignment] Object of type `Overloaded9` is not assignable to `FloatArg9`
+callables_subtyping.py:297:5: error[invalid-assignment] Object of type `StrArg10` is not assignable to `Overloaded10`
classes_classvar.py:38:11: error[invalid-type-form] Type qualifier `typing.ClassVar` expected exactly 1 argument, got 2
classes_classvar.py:39:14: error[invalid-type-form] Int literals are not allowed in this context in a type expression
classes_classvar.py:40:14: error[unresolved-reference] Name `var` used when not defined
@@ -669,6 +699,7 @@
narrowing_typeguard.py:89:9: error[type-assertion-failure] Argument does not have asserted type `B`
narrowing_typeguard.py:93:9: error[type-assertion-failure] Argument does not have asserted type `B`
narrowing_typeis.py:21:9: error[type-assertion-failure] Argument does not have asserted type `tuple[str, ...]`
+narrowing_typeis.py:35:9: error[invalid-assignment] Object of type `object` is not assignable to `int`
narrowing_typeis.py:38:9: error[type-assertion-failure] Argument does not have asserted type `int`
narrowing_typeis.py:72:9: error[type-assertion-failure] Argument does not have asserted type `int`
narrowing_typeis.py:76:9: error[type-assertion-failure] Argument does not have asserted type `int`
@@ -713,6 +744,7 @@
overloads_evaluation.py:291:33: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `T@example6`
protocols_class_objects.py:58:1: error[invalid-assignment] Object of type `<class 'ConcreteA'>` is not assignable to `ProtoA1`
protocols_class_objects.py:59:1: error[invalid-assignment] Object of type `<class 'ConcreteA'>` is not assignable to `ProtoA2`
+protocols_definition.py:30:11: error[invalid-argument-type] Argument to function `close_all` is incorrect: Expected `Iterable[SupportsClose]`, found `list[Unknown | int]`
protocols_definition.py:114:1: error[invalid-assignment] Object of type `Concrete2_Bad1` is not assignable to `Template2`
protocols_definition.py:115:1: error[invalid-assignment] Object of type `Concrete2_Bad2` is not assignable to `Template2`
protocols_definition.py:116:1: error[invalid-assignment] Object of type `Concrete2_Bad3` is not assignable to `Template2`
@@ -726,6 +758,10 @@
protocols_definition.py:288:1: error[invalid-assignment] Object of type `Concrete5_Bad4` is not assignable to `Template5`
protocols_definition.py:289:1: error[invalid-assignment] Object of type `Concrete5_Bad5` is not assignable to `Template5`
protocols_generic.py:40:1: error[invalid-assignment] Object of type `Concrete1` is not assignable to `Proto1[int, str]`
+protocols_generic.py:56:5: error[invalid-assignment] Object of type `Box[int | float]` is not assignable to `Box[int]`
+protocols_generic.py:66:5: error[invalid-assignment] Object of type `Sender[int]` is not assignable to `Sender[int | float]`
+protocols_generic.py:74:5: error[invalid-assignment] Object of type `AttrProto[int]` is not assignable to `AttrProto[int | float]`
+protocols_generic.py:75:5: error[invalid-assignment] Object of type `AttrProto[int | float]` is not assignable to `AttrProto[int]`
protocols_merging.py:52:1: error[invalid-assignment] Object of type `SCConcrete2` is not assignable to `SizedAndClosable1`
protocols_merging.py:53:1: error[invalid-assignment] Object of type `SCConcrete2` is not assignable to `SizedAndClosable2`
protocols_merging.py:54:1: error[invalid-assignment] Object of type `SCConcrete2` is not assignable to `SizedAndClosable3`
@@ -742,6 +778,10 @@
protocols_subtyping.py:16:6: error[call-non-callable] Cannot instantiate class `Proto1`: This call will raise `TypeError` at runtime
protocols_subtyping.py:38:5: error[invalid-assignment] Object of type `Proto2` is not assignable to `Concrete2`
protocols_subtyping.py:55:5: error[invalid-assignment] Object of type `Proto2` is not assignable to `Proto3`
+protocols_subtyping.py:79:5: error[invalid-assignment] Object of type `Proto5[int]` is not assignable to `Proto4[int, int | float]`
+protocols_subtyping.py:80:5: error[invalid-assignment] Object of type `Proto4[int, int]` is not assignable to `Proto5[int | float]`
+protocols_subtyping.py:102:5: error[invalid-assignment] Object of type `Proto6[int | float, int | float]` is not assignable to `Proto7[int, int | float]`
+protocols_subtyping.py:103:5: error[invalid-assignment] Object of type `Proto6[int | float, int | float]` is not assignable to `Proto7[int | float, object]`
protocols_variance.py:85:62: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `R@__call__`
qualifiers_annotated.py:36:42: error[invalid-syntax] named expression cannot be used within a type annotation
qualifiers_annotated.py:40:28: error[invalid-syntax] await expression cannot be used within a type annotation
@@ -858,5 +898,5 @@
typeddicts_usage.py:28:17: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
typeddicts_usage.py:28:18: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "title"
typeddicts_usage.py:40:24: error[invalid-type-form] The special form `typing.TypedDict` is not allowed in type expressions. Did you mean to use a concrete TypedDict or `collections.abc.Mapping[str, object]` instead?
-Found 859 diagnostics
+Found 899 diagnostics
WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details. |
According to the conformance test, we're not meant to emit an error on this one (here). All other new diagnostics on the conformance suite look correct. Edit: this is a pre-existing bug in |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
141 | 27 | 60 |
unused-ignore-comment |
10 | 51 | 0 |
invalid-return-type |
4 | 44 | 8 |
possibly-missing-attribute |
36 | 2 | 3 |
invalid-assignment |
19 | 10 | 8 |
no-matching-overload |
35 | 1 | 0 |
unresolved-attribute |
11 | 15 | 0 |
unsupported-operator |
6 | 7 | 0 |
redundant-cast |
0 | 10 | 0 |
type-assertion-failure |
5 | 3 | 0 |
index-out-of-bounds |
3 | 3 | 0 |
not-iterable |
5 | 1 | 0 |
call-non-callable |
3 | 1 | 0 |
possibly-missing-implicit-call |
3 | 0 | 0 |
| Total | 281 | 175 | 79 |
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
4a4c181 to
21e7391
Compare
|
CodSpeed Performance ReportMerging #20368 will degrade performances by 4.64%Comparing Summary
Benchmarks breakdown
|
This comment was marked as off-topic.
This comment was marked as off-topic.
This was referenced Sep 16, 2025
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
1e427f1 to
bc107c6
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
2 times, most recently
from
60ae600 to
de26419
Compare
AlexWaygood
force-pushed
the
alex/iter-union-tests
branch
from
f7652c4 to
801e6d4
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
de26419 to
3e7b3e0
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
3e7b3e0 to
ced0e48
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
ced0e48 to
46dda94
Compare
This was referenced Sep 26, 2025
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
46dda94 to
0c78745
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
0c78745 to
9c1e0ea
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
2 times, most recently
from
5ccc8b2 to
98a1eb6
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
98a1eb6 to
ad00f21
Compare
Ecosystem analysisI was expecting this PR to reduce diagnostics across the ecosystem overall, but it seems like on net it leads to more diagnostics, which is unfortunate. I have spent a while now trying to reduce the number of underlying issues here, and the diagnostic diff is much better than it was when I first opened the PR (we've gone from +297,-128 a few weeks ago, to +280,-157 now). PRs such as #20602, #20629, #20603, #20591 and github.com//pull/20593 have all helped reduce the impact of this PR by fixing underlying issues. It's possible I could carry on with this a little bit, but I think the remaining diagnostics are mostly acceptable and/or hard to fix. Many new diagnostics on this PR are due to narrowing behaviour that is technically correct, but which I worry may appear quite pedantic to users. For example, an adapted repro from from typing import Iterable
import socket
def f(x: Iterable[socket.socket] | socket.socket):
if isinstance(x, Iterable):
reveal_type(x) # revealed: Iterable[socket] | (socket & Iterable[object])
for s in x:
reveal_type(s) # revealed: object
needs_a_socket(s) # error: "Expected `socket`, found `object`"
else:
needs_a_socket(x)
def needs_a_socket(s: socket.socket): ...This is, of course, strictly correct: typeshed (correctly) does not mark There are lots of diagnostics like this going away, due to astral-sh/ty#733 being fixed by this PR: And there's also lots of churning regarding diagnostics for Not an exhaustive analysis, but here's a spot check of some of the other diagnostics in the ecosystem that stood out to me while I skimmed through: bokehOne of the bokeh hits is: Debug-printing the types gives: Here's the diff between the two types: https://www.diffchecker.com/ZxRy9rMI/. It appears that one of the I suppose that this is only an issue because the diagnostic is being emitted while checking This is quite strange but seems unrelated to this PR, and it was a pre-existing problem. I've opened astral-sh/ty#1306. homeassistantThere's lots of new diagnostics like this: This relates to astral-sh/ty#500 (so I suppose it will be solved by astral-sh/ty#623). A repro of this false positive that occurs on from typing import Any, Callable
class Foo[T = dict[str, Any]]:
def __init__(self, data: Callable[..., T]): ...
def returns_bool() -> bool:
return True
# error: [invalid-argument-type] "Argument to bound method `__init__` is incorrect: Expected `(...) -> dict[str, Any]`, found `def returns_bool() -> bool`"
# revealed: `Foo[dict[str, Any]]`
reveal_type(Foo(returns_bool))ibisThese are because on from typing import Iterable
def f[T](x: T | Iterable[T]) -> T:
raise NotImplementedError
reveal_type(f(["foo"])) # revealed: Unknownwhereas with this PR, we infer this: from typing import Iterable
def f[T](x: T | Iterable[T]) -> T:
raise NotImplementedError
reveal_type(f(["foo"])) # revealed: list[Unknown | str]when what is desired is of course: from typing import Iterable
def f[T](x: T | Iterable[T]) -> T:
raise NotImplementedError
reveal_type(f(["foo"])) # revealed: `str`So this should also hopefully be solved by astral-sh/ty#623. isortHere's a minimal repro of what's going on here: from typing import Iterable, Self
class chain[T]:
def __new__(cls, *iterables: Iterable[T]) -> Self:
raise NotImplementedError
def yield_ints() -> Iterable[int]:
yield 42
# revealed: chain[None]
reveal_type(chain((None,), (None,)))
# revealed: chain[Unknown]
reveal_type(chain(yield_ints(), yield_ints()))
# revealed: chain[Unknown]
# error: [invalid-argument-type] "Argument to function `__new__` is incorrect: Expected `Iterable[None]`, found `Iterable[int]`"
reveal_type(chain(yield_ints(), (None,)))On mesonThese are all astral-sh/ty#1248: it's an unannotated heterogeneous dictionary. |
This was referenced Oct 4, 2025
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
ad00f21 to
b2de1fc
Compare
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
b2de1fc to
b6f5dc4
Compare
AlexWaygood
added a commit
that referenced
this pull request
Oct 8, 2025
… same fully qualified names (#20756) ## Summary Even disambiguating classes using their fully qualified names is not enough for some diagnostics. We've seen real-world examples in the ecosystem (and #20368 introduces some more!) where two types can be different, but can still have the same fully qualified name. In these cases, our disambiguation machinery needs to print the file path and line number of the class in order to disambiguate classes with similar names in our diagnostics. Helps with astral-sh/ty#1306 ## Test Plan Mdtests
AlexWaygood
force-pushed
the
alex/subtyping-between-protocols
branch
from
b6f5dc4 to
407b850
Compare
carljm
approved these changes
Oct 8, 2025
Comment on lines
+549
to
+550
| relation_visitor: &HasRelationToVisitor<'db>, | ||
| disjointness_visitor: &IsDisjointVisitor<'db>, |
There was a problem hiding this comment.
Disjointness is also a relation. If we added TypeRelation::Disjoint and used has_relation_to_impl for implementing is_disjoint_from, we could reduce this back to a single HasRelationToVisitor and eliminate IsDisjointVisitor entirely.
In an ideal world I might say we should prefer to do that refactor as a prior PR, and then make this a much smaller PR on top of it. But I won't argue if you prefer to land this PR as-is and consider that as a possible follow-up. Maybe it's not even worth it; there's just a big increase in LoC in this PR from all the has_relation_to_impl calls that now have to explode onto seven lines.
There was a problem hiding this comment.
Disjointness is also a relation.
That's true, but in terms of the implementation it's very different from the others (at least currently). I think what you'd be looking at would be greatly expanding the size of the Type::has_relation_to_impl function, which is already very complicated, hard to refactor, and hard to understand.
there's just a big increase in LoC in this PR from all the
has_relation_to_implcalls that now have to explode onto seven lines.
That's true, but I feel less bad about it given what Doug has in store for us in #20677 ;)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Summary
We currently have reasonably good assignability and subtyping checks between nominal types and protocol types if the protocol has a method member or an attribute member. For example, these assertions all pass:
However, we do not currently have a good implementation of subtyping and assignability between protocol types: we currently think that
Iterator[str]is a subtype ofIterator[int], and vice versa, for example, because both protocols have__iter__and__next__method members, and we do not currently compare the types of these members when doing subtyping/assignability checks between protocols (we only check for the presence of those members).This PR fixes that.
Fixes astral-sh/ty#306
Fixes astral-sh/ty#733
Fixes astral-sh/ty#1089
Implementation details
The change that I wanted to make in this PR is the change to
protocol_class.rs. Unfortunately, I quickly ran into stack overflows after making that change, however, due to large cycles that were spread acrossis_subtype_ofandis_disjoint_from. Looking at backtraces, the root of the problem was clear:has_relation_tofor intersections callsis_disjoint_from:ruff/crates/ty_python_semantic/src/types.rs
Lines 1653 to 1665 in 3771f15
...and
is_disjoint_fromfor intersections callshas_relation_to...ruff/crates/ty_python_semantic/src/types.rs
Lines 2196 to 2209 in 3771f15
...meaning that existing mdtests ended up running into an infinite loop when checking whether intersections of protocols were subtypes of other types. This infinite loop could not be caught by our existing cycle-detector infrastructure, because a fresh cycle detector would be created in the call to
has_relation_tocall fromis_disjoint_from_impl, and this cycle detector would not be passed into theis_disjoint_fromcall insidehas_relation_to.I therefore had to make a much more invasive change here than I wanted to, in order to fix these stack overflows. Rather than accepting only one cycle detector as an input parameter,
has_relation_to_implmust now accept two cycle detectors: oneHasRelationToVisitorand oneIsDisjointFromVisitor. This allowshas_relation_toto callis_disjoint_from_implrather thanis_disjoint_from, meaning that the same cycle detector can be passed betweenhas_relation_tocalls andis_disjoint_fromcalls. The cycle can therefore be detected and stopped in its tracks before a stack overflow occurs.Similar changes also needed to be made to
is_disjoint_from_impl: it now accepts aHasRelationToVisitoras well as anIsDisjointFromVisitor.Performance
There are some regressions of up to 5% or so on some benchmarks. I think this is just because we're doing more work now, but I'm happy to hear suggestions if anybody has any ideas for how to optimise this.
Conformances suite impact
See #20368 (comment)
Ecosystem impact
See #20368 (comment)
Test Plan
Mdtests updated