[ty] Fix assignment in decorated method causing Unknown fallback#22778
Merged
[ty] Fix assignment in decorated method causing Unknown fallback#22778
Conversation
Typing conformance resultsNo changes detected ✅ |
|
carljm
requested review from
AlexWaygood,
dcreager and
sharkdp
as code owners
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
possibly-missing-attribute |
80 | 94 | 42 |
unresolved-attribute |
94 | 0 | 0 |
invalid-argument-type |
18 | 2 | 11 |
no-matching-overload |
21 | 0 | 0 |
invalid-assignment |
6 | 1 | 5 |
invalid-parameter-default |
0 | 0 | 7 |
not-subscriptable |
7 | 0 | 0 |
invalid-return-type |
1 | 1 | 4 |
not-iterable |
3 | 0 | 0 |
unsupported-operator |
0 | 0 | 3 |
unused-ignore-comment |
0 | 2 | 0 |
parameter-already-assigned |
0 | 1 | 0 |
| Total | 230 | 101 | 72 |
Contributor
Author
|
The new diagnostics in the ecosystem report look like true positives. Home-assistant is relying on a pattern like this: class CoinbaseData:
def __init__(self, client, exchange_base):
self.accounts = None
@Throttle(MIN_TIME_BETWEEN_UPDATES) # <- Unknown decorator
def update(self):
self.accounts = get_accounts(self.client) # <- Assigns listThis code implicitly relies on the The reason why The new parso diagnostics have the same basic cause, and are also true positives. |
Contributor
Author
|
The sklearn diagnostics are similar to home-assistant and parso; they generally follow the pattern "we understand more types as something other than Unknown, so we get more diagnostics". The one wrinkle with sklearn is that some of the new types we understand also trigger #350. |
MichaReiser
approved these changes
Jan 21, 2026
Fix issue #2566 where assigning to an instance attribute in a decorated method (like @cache) incorrectly resulted in `Unknown | int` instead of just `int` when the attribute was declared in `__init__`. The bug was in the `is_valid_scope` closure in `implicit_attribute_inner`. When looking up class members for the descriptor protocol, it calls `implicit_attribute` with `MethodDecorator::ClassMethod`. The previous code relied on the final evaluated type of the method (after all decorators are applied) to determine if it's a classmethod. For methods wrapped by `@cache`, the final type becomes `_lru_cache_wrapper[...]` instead of `FunctionLiteral`, which caused incorrect behavior. The fix changes the approach to directly iterate through the decorators on the AST node and check if any of their types evaluate to `classmethod` or `staticmethod`. This is more reliable because it checks the original decorator types, not the final wrapped result. Also removes the assumption that unknown decorators might be classmethod/staticmethod aliases. This caused attributes defined in methods with unknown decorators to pollute class-level lookups and potentially override instance-level attributes.
carljm
force-pushed
the
cjm/attrs-in-decorated-methods
branch
from
d75004d to
5a32863
Compare
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.
Fixes astral-sh/ty#2566
Fix a bug where assigning to an instance attribute in a decorated method (like @cache) incorrectly resulted in
Unknown | intinstead of justintwhen the attribute was explicitly declared in__init__.The bug was in the
is_valid_scopeclosure inimplicit_attribute_inner. When looking up class members for the descriptor protocol, it callsimplicit_attributewithMethodDecorator::ClassMethod. The previous code relied on the final evaluated type of the method (after all decorators are applied) to determine if it's a classmethod. For methods wrapped by@cache, the final type becomes_lru_cache_wrapper[...]instead ofFunctionLiteral, which caused incorrect behavior.The fix changes the approach to directly iterate through the decorators on the AST node and check if any of their types evaluate to
classmethodorstaticmethod. This is more reliable because it checks the original decorator types, not the final wrapped result. It also results in fewer cycles, because attribute lookup no longer depends on a full attribute lookup of every method; instead it just depends on the types of the specific decorator nodes on each method.Also removes the assumption that unknown decorators might be classmethod/staticmethod aliases. This caused attributes defined in methods with unknown decorators to pollute class-level lookups and potentially override instance-level attributes.