[ty] Ensure annotation/type expressions in stub files are always deferred #21401
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
AlexWaygood
changed the title
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-11-13 13:18:20.973834959 +0000
+++ new-output.txt 2025-11-13 13:18:24.521875587 +0000
@@ -39,7 +39,7 @@
aliases_newtype.py:11:8: error[invalid-argument-type] Argument is incorrect: Expected `int`, found `Literal["user"]`
aliases_newtype.py:12:1: error[invalid-assignment] Object of type `Literal[42]` is not assignable to `UserId`
aliases_newtype.py:18:1: error[invalid-assignment] Object of type `<NewType pseudo-class 'UserId'>` is not assignable to `type`
-aliases_newtype.py:23:16: error[invalid-argument-type] Argument to function `isinstance` is incorrect: Expected `type | UnionType | tuple[Unknown, ...]`, found `<NewType pseudo-class 'UserId'>`
+aliases_newtype.py:23:16: error[invalid-argument-type] Argument to function `isinstance` is incorrect: Expected `type | UnionType | tuple[Divergent, ...]`, found `<NewType pseudo-class 'UserId'>`
aliases_newtype.py:26:21: error[invalid-base] Cannot subclass an instance of NewType
aliases_newtype.py:35:1: error[invalid-newtype] The name of a `NewType` (`BadName`) must match the name of the variable it is assigned to (`GoodName`)
aliases_newtype.py:41:6: error[invalid-type-form] `GoodNewType1` is a `NewType` and cannot be specialized
@@ -54,7 +54,7 @@
aliases_type_statement.py:19:1: error[call-non-callable] Object of type `TypeAliasType` is not callable
aliases_type_statement.py:23:7: error[unresolved-attribute] Object of type `typing.TypeAliasType` has no attribute `other_attrib`
aliases_type_statement.py:26:18: error[invalid-base] Invalid class base with type `typing.TypeAliasType`
-aliases_type_statement.py:31:22: error[invalid-argument-type] Argument to function `isinstance` is incorrect: Expected `type | UnionType | tuple[Unknown, ...]`, found `typing.TypeAliasType`
+aliases_type_statement.py:31:22: error[invalid-argument-type] Argument to function `isinstance` is incorrect: Expected `type | UnionType | tuple[Divergent, ...]`, found `typing.TypeAliasType`
aliases_type_statement.py:37:22: error[invalid-type-form] Function calls are not allowed in type expressions
aliases_type_statement.py:38:22: error[invalid-type-form] List literals are not allowed in this context in a type expression: Did you mean `tuple[int, str]`?
aliases_type_statement.py:39:22: error[invalid-type-form] Tuple literals are not allowed in this context in a type expression
|
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
no-matching-overload |
229 | 0 | 0 |
invalid-argument-type |
25 | 33 | 103 |
unresolved-attribute |
0 | 9 | 59 |
unresolved-reference |
0 | 62 | 0 |
possibly-missing-attribute |
0 | 3 | 40 |
unsupported-operator |
20 | 12 | 11 |
invalid-return-type |
4 | 3 | 20 |
non-subscriptable |
0 | 0 | 8 |
unused-ignore-comment |
0 | 8 | 0 |
invalid-assignment |
0 | 0 | 6 |
not-iterable |
0 | 5 | 0 |
index-out-of-bounds |
0 | 0 | 3 |
call-non-callable |
0 | 0 | 1 |
| Total | 278 | 135 | 251 |
CodSpeed Performance ReportMerging #21401 will degrade performances by 4.55%Comparing Summary
Benchmarks breakdown
Footnotes
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
AlexWaygood
force-pushed
the
alex/tvar-bound-stubs2
branch
from
d331ee3 to
3ab14c2
Compare
AlexWaygood
force-pushed
the
alex/tvar-bound-stubs2
branch
2 times, most recently
from
27b39f6 to
3ab14c2
Compare
AlexWaygood
force-pushed
the
alex/bad-dunder-gets
branch
from
a02d9f6 to
cdf1f5e
Compare
AlexWaygood
force-pushed
the
alex/tvar-bound-stubs2
branch
from
3ab14c2 to
929a6b1
Compare
AlexWaygood
force-pushed
the
alex/tvar-bound-stubs2
branch
from
929a6b1 to
be2be48
Compare
Member
Author
Ecosystem analysisOverall this gets rid of a lot of There are lots of new + scipy/interpolate/tests/test_bary_rational.py:173:42: error[no-matching-overload] No overload of function `assert_allclose` matches argumentsThese appear due to a bug in numpy's stubs. That But none of numpy's overloads for |
Member
Author
|
I believe the performance regression is mostly due to the fact that we now accurately understand a lot more typeshed types that use forward references. Note that the regression is most significant on microbenchmarks and tomllib, i.e., the smaller codebases in which inferring typeshed types will take up a greater proportion of the overall type-checking time. |
carljm
approved these changes
Nov 13, 2025
Comment on lines
+23
to
+33
| // `DeferredExpressionState::InStringAnnotation` takes precedence over other states. | ||
| // However, if it's not a stringified annotation, we must still ensure that annotation expressions | ||
| // are always deferred in stub files. | ||
| match self.deferred_state { | ||
| DeferredExpressionState::None => { | ||
| if self.in_stub() { | ||
| self.deferred_state = DeferredExpressionState::Deferred; | ||
| } | ||
| } | ||
| DeferredExpressionState::InStringAnnotation(_) | DeferredExpressionState::Deferred => {} | ||
| } |
Contributor
There was a problem hiding this comment.
In general, setting the deferred state without a paired revert back to the previous deferred state is not right; it will cause the new deferred state to leak back up the stack, outside of this type expression inference.
I feel like there might be some refactor called for here between this method and infer_type_expression_with_state, but I haven't quite sorted yet what it should be...
Member
Author
There was a problem hiding this comment.
In general, setting the deferred state without a paired revert back to the previous deferred state is not right; it will cause the new deferred state to leak back up the stack, outside of this type expression inference.
Yes, but it can never leak so far as leak into a different file, correct? And when you're inferring types for one scope in a stub file, you know that all other scopes in that stub file will also be in a stub file. So I think this is fine for this specific scenario.
My first draft of this PR just switched a bunch of callsites from self.infer_type_expression(expr) to self.infer_type_expression_with_state(expr, state) where state was DeferredExpressionState::Deferred if we were in a stub file. But that doesn't feel like a robust way of doing it at all, because you have to remember to do that every time, and there were loads of callsites where we were forgetting to do that.
Member
Author
There was a problem hiding this comment.
I feel like there might be some refactor called for here between this method and
infer_type_expression_with_state, but I haven't quite sorted yet what it should be...
Yeah, I also looked briefly at whether a refactor would be possible, but it seemed nontrivial...
Contributor
There was a problem hiding this comment.
Yes, but it can never leak so far as leak into a different file, correct? And when you're inferring types for one scope in a stub file, you know that all other scopes in that stub file will also be in a stub file. So I think this is fine for this specific scenario.
I don't think it's fine, because the updated DeferredExpressionState::Deferred that is set here can leak into non-type-expressions in that stub file. And (at least in this PR) it is not the intent that all name references in stub files are deferred, only those in type expressions.
Member
Author
There was a problem hiding this comment.
Oh, good point. Argh, that seems serious enough that I maybe wouldn't have approved if I'd been reviewing 😆 Will fix in a followup!
AlexWaygood
added a commit
that referenced
this pull request
Nov 14, 2025
…red in stub files (#21456) ## Summary - Always restore the previous `deferred_state` after parsing a type expression: we don't want that state leaking out into other contexts where we shouldn't be deferring expression inference - Always defer the right-hand-side of a PEP-613 type alias in a stub file, allowing for forward references on the right-hand side of `T: TypeAlias = X | Y` in a stub file Addresses @carljm's review in #21401 (comment) ## Test Plan I added a regression test for a regression that the first version of this PR introduced (we need to make sure the r.h.s. of a PEP-613 `TypeAlias`es is always deferred in a stub file)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Inference of type expressions should always be deferred when analysing a stub file: stubs are allowed to (and often do) do things like this:
bound=Foohere would cause aNameErrorif you tried to run this snippet with an interpreter at runtime, sinceFoois not yet defined at the point when theTypeVaris assigned. But it's nonetheless permitted in a stub file, because stub files are never intended to be executed at runtime. They're just "data files" for the type checker.This pattern is used in quite a few typeshed stub files. Among other things, the fact that we don't yet support it means that we aren't able to correctly infer the type of the
default=argument for thisTypeVar(inthas not yet been defined at that point inbuiltins.pyi):ruff/crates/ty_vendored/vendor/typeshed/stdlib/builtins.pyi
Line 93 in 9dd666d
And therefore we think that a bare
memoryviewannotation should signifymemoryview[Unknown], whereas in fact it should signifymemoryview[int].Test Plan