[ty] Defer all parameter and return type annotations#21906
Merged
Conversation
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-12-11 19:38:32.777561529 +0000
+++ new-output.txt 2025-12-11 19:38:36.517590503 +0000
@@ -150,7 +150,7 @@
callables_protocol.py:169:7: error[invalid-assignment] Object of type `def cb8_bad1(x: int) -> Any` is not assignable to `Proto8`
callables_protocol.py:186:5: error[invalid-assignment] Object of type `Literal["str"]` is not assignable to attribute `other_attribute` of type `int`
callables_protocol.py:187:5: error[unresolved-attribute] Unresolved attribute `xxx` on type `Proto9[P@decorator1, R@decorator1]`.
-callables_protocol.py:197:7: error[unresolved-attribute] Object of type `Proto9[(x: int), Unknown] | Proto9[Divergent, Unknown]` has no attribute `other_attribute2`
+callables_protocol.py:197:7: error[unresolved-attribute] Object of type `Proto9[(x: int), Unknown]` has no attribute `other_attribute2`
callables_protocol.py:238:8: error[invalid-assignment] Object of type `def cb11_bad1(x: int, y: str, /) -> Any` is not assignable to `Proto11`
callables_protocol.py:260:8: error[invalid-assignment] Object of type `def cb12_bad1(*args: Any, *, kwarg0: Any) -> None` is not assignable to `Proto12`
callables_protocol.py:284:27: error[invalid-assignment] Object of type `def cb13_no_default(path: str) -> str` is not assignable to `Proto13_Default`
|
|
carljm
reviewed
Dec 10, 2025
CodSpeed Performance ReportMerging #21906 will improve performances by 5.61%Comparing Summary
Benchmarks breakdown
Footnotes
|
dcreager
commented
Dec 10, 2025
| let mut is_staticmethod = is_implicit_classmethod(function_name); | ||
| let mut is_classmethod = is_implicit_staticmethod(function_name); | ||
|
|
||
| let inference = infer_definition_types(db, definition); |
Member
Author
There was a problem hiding this comment.
All of this stuff is moved over to function.rs to avoid this cyclic call to infer_definition_types
Comment on lines
513
to
518
| if self.is_staticmethod(db) || self.is_classmethod(db) { | ||
| return None; | ||
| } | ||
|
|
||
| let method_may_be_generic = generic_context | ||
| .is_some_and(|context| context.variables(db).any(|v| v.typevar(db).is_self(db))); |
Member
Author
There was a problem hiding this comment.
...and over here, where it's moved, we don't need to call infer_definition_types on the function definition, because we can grab its generic context and decorator list directly.
| let class_scope = index.scope(class_scope_id.file_scope_id(db)); | ||
| let class_node = class_scope.node().as_class()?; | ||
| let class_def = index.expect_single_definition(class_node); | ||
| let (class_literal, class_is_generic) = match infer_definition_types(db, class_def) |
Member
Author
There was a problem hiding this comment.
This infer_definition_types is okay to keep, since it's referring to the containing class definition, not to the function we're in the middle of inferring a signature for
…-cycle * origin/main: [ty] Support implicit type of `cls` in signatures (#21771) [ty] add `SyntheticTypedDictType` and implement `normalized` and `is_equivalent_to` (#21784) [ty] Fix disjointness checks with type-of `@final` classes (#21770) [ty] Fix negation upper bounds in constraint sets (#21897)
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
9 | 84 | 61 |
unresolved-attribute |
1 | 31 | 1 |
missing-argument |
13 | 0 | 0 |
unknown-argument |
5 | 0 | 0 |
unused-ignore-comment |
4 | 1 | 0 |
no-matching-overload |
0 | 3 | 0 |
type-assertion-failure |
0 | 2 | 1 |
invalid-assignment |
0 | 2 | 0 |
invalid-return-type |
0 | 1 | 1 |
| Total | 32 | 124 | 64 |
Contributor
|
This is looking pretty good! The macos test failure looks like something fluky and unrelated to this PR? The ecosystem suggests we are not binding |
dcreager
commented
Dec 11, 2025
Member
Author
dcreager
left a comment
dcreager
left a comment
There was a problem hiding this comment.
Latest patch should fix the type[Self] issue. Added some regression mdtests for it too
| reveal_type(GenericShape().foo()) # revealed: GenericShape[Unknown] | ||
| reveal_type(GenericShape.bar()) # revealed: GenericShape[Unknown] | ||
| reveal_type(GenericShape[int].bar()) # revealed: GenericShape[int] | ||
| reveal_type(GenericShape.baz(1)) # revealed: GenericShape[Literal[1]] |
Member
Author
There was a problem hiding this comment.
This call (and the analogous one below) mimic the type[Self] errors we were seeing in the ecosystem report.
Member
Author
|
Contributor
|
A lot of the new diagnostics in the ecosystem report look like cases of astral-sh/ty#1787 that are just being revealed now that we are better able to infer the signature type at all. |
Member
|
Merged in |
carljm
approved these changes
Dec 11, 2025
Comment on lines
518
to
519
| let method_may_be_generic = generic_context | ||
| .is_some_and(|context| context.variables(db).any(|v| v.typevar(db).is_self(db))); |
Contributor
There was a problem hiding this comment.
It's not clear to me why we are checking whether Self is in the type context in order to determine whether method_may_be_generic? Maybe this is just a problem with the variable name?
Member
Author
There was a problem hiding this comment.
tbh I hadn't looked at this logic, just moved it from one file to the other! 😄
I'll update the variable name and comment below.
There's one thing that I thought we would want to ensure, which I'm not no longer sure about — that we should always treat an implicit Self parameter annotation the same as we would an explicit Self. The logic here (the actual logic, not the variable name/comment) breaks that [playground]:
class Implicit:
def method(self): ...
class Explicit:
def method(self: Self): ...
reveal_type(generic_context(Implicit().method)) # None
reveal_type(generic_context(Explicit().method)) # [Self@method]The else branch engages for implicit Self, since the method is not generic / Self does not appear explicitly in the parameter list. So we give the self param an implicit annotation of Implicit, not Self@Implicit.
For the explicit case, in theory we could perform the same check: realize that there are no other references to Self, and treat that annotation as Implicit, without creating a typevar for it. But we don't do that right now: you typed Self, and so we turn that into a typevar.
Anyway, I'm not suggesting we should jump through hoops to make the same simplification in the explicit case. It would be only be an optimization anyway, since it shouldn't™ be a problem to have the Self typevar in either case.
Contributor
There was a problem hiding this comment.
Makes sense! And sorry, didn't realize when I was initially reviewing this code that it had just been moved unchanged.
| Type::ClassLiteral(class_literal) => { | ||
| (class_literal, class_literal.generic_context(db).is_some()) | ||
| } | ||
| Type::GenericAlias(alias) => (alias.origin(db), true), |
Contributor
There was a problem hiding this comment.
I guess this case makes sense in principle, but I'm curious how it could ever occur. How could a class def node evaluate to a GenericAlias type?
Member
Author
There was a problem hiding this comment.
Good catch. I'll remove it and see if anything barfs.
Comment on lines
561
to
564
| // For methods of non-generic classes that are not otherwise generic (e.g. return `Self` or | ||
| // have additional type parameters), the implicit `Self` type of the `self`, or the implicit | ||
| // `type[Self]` type of the `cls` parameter, would be the only type variable, so we can just | ||
| // use the class directly. |
Contributor
There was a problem hiding this comment.
It doesn't seem like this matches what we check above, which is actually just that an explicit Self is in the type parameters -- we don't seem to care whether there are other type parameters. (That logic seems fine to me, it just doesn't match the text here, or the method_may_be_generic variable name.)
dcreager
added a commit
that referenced
this pull request
Dec 11, 2025
* origin/main: (36 commits) [ty] Defer all parameter and return type annotations (#21906) [ty] Fix workspace symbols to return members too (#21926) Document range suppressions, reorganize suppression docs (#21884) Ignore ruff:isort like ruff:noqa in new suppressions (#21922) [ty] Handle `Definition`s in `SemanticModel::scope` (#21919) [ty] Attach salsa db when running ide tests for easier debugging (#21917) [ty] Don't show hover for expressions with no inferred type (#21924) [ty] avoid unions of generic aliases of the same class in fixpoint (#21909) [ty] Squash false positive logs for failing to find `builtins` as a real module [ty] Uniformly use "not supported" in diagnostics (#21916) [ty] Reduce size of ty-ide snapshots (#21915) [ty] Adjust scope completions to use all reachable symbols [ty] Rename `all_members_of_scope` to `all_end_of_scope_members` [ty] Remove `all_` prefix from some routines on UseDefMap Enable `--document-private-items` for `ruff_python_formatter` (#21903) Remove `BackwardsTokenizer` based `parenthesized_range` references in `ruff_linter` (#21836) [ty] Revert "Do not infer types for invalid binary expressions in annotations" (#21914) Skip over trivia tokens after re-lexing (#21895) [ty] Avoid inferring types for invalid binary expressions in string annotations (#21911) [ty] Improve overload call resolution tracing (#21913) ...
sharkdp
added a commit
that referenced
this pull request
Dec 13, 2025
## Summary This check is not necessary thanks to #21906.
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
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.
As described in astral-sh/ty#1729, we previously had a salsa cycle when inferring the signature of many function definitions.
The most obvious case happened when (a) the function was decorated, (b) it had no PEP-695 type params, and (c) annotations were not always deferred (e.g. in a stub file). We currently evaluate and apply function decorators eagerly, as part of
infer_function_definition. Applying a decorator requires knowing the signature of the function being decorated. There were two places where signature construction calledinfer_definition_typescyclically.The simpler case was that we were looking up the generic context and decorator list of the function to determine whether it has an implicit
selfparameter. Before, we usedinfer_definition_typesto determine that information. But since we're in the middle of signature construction for the function, we can just thread the information through directly.The harder case is that signature construction requires knowing the inferred parameter and return type annotations. When (b) and (c) hold, those type annotations are inferred in
infer_function_definition! (In theory, we've already finished that by the time we start applying decorators, but signature construction doesn't know that.)If annotations are deferred, the params/return annotations are inferred in
infer_deferred_types; if there are PEP-695 type params, they're inferred ininfer_function_type_params. Both of those are different salsa queries, and don't induce this cycle.So the quick fix here is to always defer inference of the function params/return, so that they are always inferred under a different salsa query.
A more principled fix would be to apply decorators lazily, just like we construct signatures lazily. But that is a more invasive fix.
Fixes astral-sh/ty#1729