[ty] Add a diagnostic for Final without assignment#23001
Merged
charliermarsh merged 1 commit intomainfrom Feb 3, 2026
Merged
Conversation
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 82.32% to 82.37%. The percentage of expected errors that received a diagnostic increased from 72.34% to 72.62%. Summary
True positives addedDetails
|
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-assignment |
0 | 4 | 1 |
invalid-return-type |
0 | 1 | 3 |
invalid-argument-type |
0 | 2 | 1 |
unused-type-ignore-comment |
0 | 2 | 0 |
| Total | 0 | 9 | 5 |
Member
Author
|
Two false positives in the ecosystem report around this pattern: from typing import TYPE_CHECKING
...
del TYPE_CHECKING |
6e0b808 to
5a9d772
Compare
charliermarsh
commented
Jan 31, 2026
|
|
||
| // Check for bindings from `__init__` (self.<name> = ...). | ||
| let symbol = place_table.symbol(symbol_id); | ||
| let has_init_binding = attribute_assignments(db, body_scope, symbol.name().as_str()) |
Member
Author
There was a problem hiding this comment.
I think this is too permissive right now, actually... I think it's supposed to be limited to __init__ (but this is not limited to __init__, as implemented).
5a9d772 to
a0ae328
Compare
a0ae328 to
1c5f5be
Compare
sharkdp
approved these changes
Feb 3, 2026
Contributor
sharkdp
left a comment
There was a problem hiding this comment.
Thank you, this looks good!
Comment on lines
511
to
513
| ```py | ||
| from typing import TYPE_CHECKING | ||
| ``` |
Contributor
There was a problem hiding this comment.
TYPE_CHECKING is heavily special-cased. I think it would be better to add a dedicated module here that we can import from, e.g.
Suggested change
| ```py | |
| from typing import TYPE_CHECKING | |
| ``` | |
| `module.py`: | |
| ```py | |
| from typing import Final | |
| MODULE_FINAL: Final[int] = 1 | |
| ``` | |
| ```py | |
| from module import MODULE_FINAL | |
| ``` |
(and similar for the test below)
Comment on lines
+1623
to
+1630
| for (symbol_id, declarations) in use_def.all_end_of_scope_symbol_declarations() { | ||
| let result = place_from_declarations(db, declarations); | ||
| let first_declaration = result.first_declaration; | ||
| let (place_and_quals, _) = result.into_place_and_conflicting_declarations(); | ||
|
|
||
| if !place_and_quals.qualifiers.contains(TypeQualifiers::FINAL) { | ||
| continue; | ||
| } |
Contributor
There was a problem hiding this comment.
Adding this as a global check that we run on every scope seems very reasonable, and the whole logic here looks good. My main concern is that this is a really expensive check. We look up all declarations for every symbol in every scope. But I guess the benchmarks speak for themselves: I only see minor 1% regressions. Someone might argue that this is still too much for a diagnostic of seemingly low impact, but I think I'm in favor of accepting this small hit.
Comment on lines
1663
to
1666
| /// | ||
| /// This is called from [`Self::check_static_class_definitions`] where the class literal | ||
| /// is available, so we can handle special cases like dataclasses (where the synthesized | ||
| /// `__init__` provides the value). |
Contributor
There was a problem hiding this comment.
Maybe just remove this?
Suggested change
| /// | |
| /// This is called from [`Self::check_static_class_definitions`] where the class literal | |
| /// is available, so we can handle special cases like dataclasses (where the synthesized | |
| /// `__init__` provides the value). |
1c5f5be to
46366fa
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.
Summary
Closes astral-sh/ty#872.