[ty] Diagnostic when combining Final and ClassVar#23365
Conversation
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 83.88% to 83.93%. The percentage of expected errors that received a diagnostic increased from 74.91% to 75.18%. Summary
True positives addedDetails
|
Memory usage reportMemory usage unchanged ✅ |
|
7ac276c to
5a0a91a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
invalid-return-type |
1 | 0 | 0 |
type-assertion-failure |
1 | 0 | 0 |
| Total | 42 | 0 | 0 |
5a0a91a to
deaa69d
Compare
deaa69d to
41287ac
Compare
| pub(crate) static REDUNDANT_FINAL_CLASSVAR = { | ||
| summary: "detects redundant combinations of `ClassVar` and `Final`", | ||
| status: LintStatus::stable("0.0.18"), | ||
| default_level: Level::Warn, |
There was a problem hiding this comment.
I decided to create a new rule for this, since it's more of a lint. However, the conformance tests currently require that we emit an error, so it's useful to implement this in ty. I chose a "warning" severity though, and remapped that to "error" for running the conformance tests.
Final and ClassVar
crates/ty_python_semantic/resources/mdtest/type_qualifiers/classvar.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/src/types/infer/builder/annotation_expression.rs
Outdated
Show resolved
Hide resolved
| let inside_non_dataclass_class = | ||
| nearest_enclosing_class(self.db(), self.index, self.scope()) | ||
| .is_none_or(|class| !class.is_dataclass_like(self.db())); |
There was a problem hiding this comment.
I guess for something like this, the "nearest enclosing class" would be Some():
from dataclasses import dataclass
@dataclass
class F:
def method(self):
X: Final[ClassVar[int]] = 42but I guess that would be a separate error anyway, since ClassVar is only valid in the class body, so a false negative here probably doesn't matter too much.
Hmm, unrelatedly, should we make this a lazy closure? It's unnecessary work to compute this variable if classvar_and_final, already computed above, was false
There was a problem hiding this comment.
Inlined the nearest_enclosing_class test to make sure that we short-circuit. Now we only execute the check if we see ClassVar + Final
crates/ty_python_semantic/src/types/infer/builder/annotation_expression.rs
Outdated
Show resolved
Hide resolved
## Summary This PR implements the following paragraph in the typing spec: > Type checkers should infer a final attribute that is initialized in a class body as being a class variable, except in the case of [Dataclasses](https://typing.python.org/en/latest/spec/dataclasses.html), where `x: Final[int] = 3` creates a dataclass field and instance-level final attribute `x` with default value `3`; `x: ClassVar[Final[int]] = 3` is necessary to create a final class variable with value `3`. In non-dataclasses, combining `ClassVar` and `Final` is redundant, and type checkers may choose to warn or error on the redundancy. > > https://typing.python.org/en/latest/spec/qualifiers.html#semantics-and-examples ## Test Plan New Markdown tests
Summary
This PR implements the following paragraph in the typing spec:
Test Plan
New Markdown tests