[ty] Enforce Final attribute assignment rules for annotated and augmented writes by charliermarsh · Pull Request #23880 · astral-sh/ruff

charliermarsh · 2026-03-10T19:08:10Z

Summary

Reject Final attribute writes that previously slipped through the annotated-assignment and augmented-assignment code paths, e.g.:

self.x: Final[int] = 1

Closes astral-sh/ty#1888.

astral-sh-bot · 2026-03-10T19:10:18Z

Typing conformance results improved 🎉

The percentage of diagnostics emitted that were expected errors increased from 85.05% to 85.19%. The percentage of expected errors that received a diagnostic increased from 78.05% to 78.33%. The number of fully passing files improved from 63/132 to 64/132.

Summary

How are test cases classified?

Each test case represents one expected error annotation or a group of annotations sharing a tag. Counts are per test case, not per diagnostic — multiple diagnostics on the same line count as one. Required annotations (E) are true positives when ty flags the expected location and false negatives when it does not. Optional annotations (E?) are true positives when flagged but true negatives (not false negatives) when not. Tagged annotations (E[tag]) require ty to flag exactly one of the tagged lines; tagged multi-annotations (E[tag+]) allow any number up to the tag count. Flagging unexpected locations counts as a false positive.

Metric	Old	New	Diff	Outcome
True Positives	825	828	+3	⏫ (✅)
False Positives	145	144	-1	⏬ (✅)
False Negatives	232	229	-3	⏬ (✅)
Total Diagnostics	1047	1049	+2	⏫
Precision	85.05%	85.19%	+0.13%	⏫ (✅)
Recall	78.05%	78.33%	+0.28%	⏫ (✅)
Passing Files	63/132	64/132	+1	⏫ (✅)

Test file breakdown

2 files altered

File	True Positives	False Positives	False Negatives	Status
qualifiers_final_annotation.py	26 (+3) ✅	0	0 (-3) ✅	✅ Newly Passing 🎉
generics_syntax_infer_variance.py	16	6 (-1) ✅	3	📈 Improving
Total (all files)	828 (+3) ✅	144 (-1) ✅	229 (-3) ✅	64/132

True positives added (3)

3 diagnostics

Test case

Diff

qualifiers_final_annotation.py:62

+error[invalid-assignment] Cannot assign to final attribute `id3` on type `Self@method1`

qualifiers_final_annotation.py:63

+error[invalid-assignment] Cannot assign to final attribute `id4` on type `Self@method1`

qualifiers_final_annotation.py:67

+error[invalid-assignment] Cannot assign to final attribute `ID7` on type `Self@method1`

False positives removed (1)

1 diagnostic

Test case	Diff
generics_syntax_infer_variance.py:92	-error[invalid-assignment] Cannot assign to final attribute `x` on type `Self@__init__`

astral-sh-bot · 2026-03-10T19:12:12Z

`mypy_primer` results

Changes were detected when running on open source projects

meson (https://github.com/mesonbuild/meson)
- mesonbuild/options.py:330:47: warning[unused-type-ignore-comment] Unused blanket `type: ignore` directive
- Found 2356 diagnostics
+ Found 2355 diagnostics

hydpy (https://github.com/hydpy-dev/hydpy)
- hydpy/core/modeltools.py:288:9: error[invalid-assignment] Cannot assign to final attribute `optional` on type `Self@__init__`
- hydpy/core/modeltools.py:289:9: error[invalid-assignment] Cannot assign to final attribute `sidemodel` on type `Self@__init__`
- Found 1062 diagnostics
+ Found 1060 diagnostics

astral-sh-bot · 2026-03-10T19:12:23Z

Memory usage report

Memory usage unchanged ✅

charliermarsh · 2026-03-10T19:23:17Z

Tons of false positives, working on it...

charliermarsh · 2026-03-10T21:03:27Z

I believe the hydpy removals were false positives.

astral-sh-bot · 2026-03-10T21:10:43Z

`ecosystem-analyzer` results

Lint rule	Added	Removed	Changed
`invalid-assignment`	0	2	0
`unused-type-ignore-comment`	0	1	0
Total	0	3	0

Full report with detailed diff (timing results)

AlexWaygood

(Not a full review, it's late here 😄)

crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md

crates/ty_python_semantic/resources/mdtest/attributes.md

AlexWaygood · 2026-03-10T22:08:29Z

crates/ty_python_semantic/resources/mdtest/attributes.md

 reveal_type(C().w)  # revealed: Unknown | Weird
 ```

+#### Ecosystem regression: pip SSL context options


Can you add some prose to say what exactly this is testing, and how it differs from the ecosystem regression test below it? It feels a bit unclear right now how this interacts with Final variables.

crates/ty_python_semantic/src/types/infer/builder/final.rs

crates/ty_python_semantic/src/types/infer/builder.rs

…nted writes

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

charliermarsh · 2026-03-11T15:35:34Z

Looks like this also closes astral-sh/ty#1888.

AlexWaygood

A few more issues I spotted. It looks like we also have false negatives on protocol classes, possibly due to some special-casing of nominal-instance types somewhere? On this branch, we're still the only type checker not to report any diagnostics on this snippet:

from typing import Final, Protocol

class Foo(Protocol):
    value: Final[int] = 42

    def foo(self, value: int):
        self.value = value

AlexWaygood · 2026-03-11T14:41:38Z

crates/ty_python_semantic/resources/mdtest/attributes.md

 reveal_type(C().w)  # revealed: Unknown | Weird
 ```

+#### Nested augmented assignments after narrowing


I still think it would be great to add a line or two of prose here to describe what it is that this is testing. What was the ecosystem regression here that led to you adding this test?

AlexWaygood · 2026-03-11T14:43:20Z

crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs

+                builder.into_diagnostic(format_args!(
+                    "Cannot assign to final attribute `{attribute}` on type `{}`",
+                    object_ty.display(db)
+                ));


Ideally here I feel like we'd add a subdiagnostic pointing to the location where the attribute was declared as Final. But if you want to leave that for now, it's fine to just add a TODO comment for that.

AlexWaygood · 2026-03-11T14:52:50Z

crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs

+        object_ty: Type<'db>,
+        attribute: &str,
+        qualifiers: TypeQualifiers,
+        emit_diagnostics: bool,


I'm a bit confused by the handling of this flag. It looks like we could short-circuit much earlier in some cases and save ourselves a bunch of work? All tests pass with this patch applied to your branch:

diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 85be7c6336..f4e13f8d8a 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -2245,13 +2245,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // Resolve `Self` type variables to the concrete instance type. let meta_attr_ty = meta_attr_ty.bind_self_typevars(db, object_ty); - if self.invalid_assignment_to_final_attribute( - target.range(), - object_ty, - attribute, - qualifiers, - emit_diagnostics, - ) { + if emit_diagnostics + && self.invalid_assignment_to_final_attribute( + target.range(), + object_ty, + attribute, + qualifiers, + ) + { return false; } @@ -2306,13 +2307,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { instance_attr_ty.bind_self_typevars(db, object_ty); let value_ty = infer_value_ty .infer_silent(self, TypeContext::new(Some(instance_attr_ty))); - if self.invalid_assignment_to_final_attribute( - target.range(), - object_ty, - attribute, - qualifiers, - emit_diagnostics, - ) { + if emit_diagnostics + && self.invalid_assignment_to_final_attribute( + target.range(), + object_ty, + attribute, + qualifiers, + ) + { return false; } @@ -2360,13 +2362,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { instance_attr_ty.bind_self_typevars(db, object_ty); let value_ty = infer_value_ty .infer_silent(self, TypeContext::new(Some(instance_attr_ty))); - if self.invalid_assignment_to_final_attribute( - target.range(), - object_ty, - attribute, - qualifiers, - emit_diagnostics, - ) { + if emit_diagnostics + && self.invalid_assignment_to_final_attribute( + target.range(), + object_ty, + attribute, + qualifiers, + ) + { return false; } @@ -2436,13 +2439,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { }), qualifiers, } => { - if self.invalid_assignment_to_final_attribute( - target.range(), - object_ty, - attribute, - qualifiers, - emit_diagnostics, - ) { + if emit_diagnostics + && self.invalid_assignment_to_final_attribute( + target.range(), + object_ty, + attribute, + qualifiers, + ) + { infer_value_ty(self, TypeContext::default()); return false; } @@ -2540,13 +2544,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { { let value_ty = infer_value_ty(self, TypeContext::new(Some(class_attr_ty))); - if self.invalid_assignment_to_final_attribute( - target.range(), - object_ty, - attribute, - qualifiers, - emit_diagnostics, - ) { + if emit_diagnostics + && self.invalid_assignment_to_final_attribute( + target.range(), + object_ty, + attribute, + qualifiers, + ) + { return false; } @@ -3881,7 +3886,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { object_ty, attr_expr.attr.id(), annotated.qualifiers, - true, ); } diff --git a/crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs b/crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs index 619ebc7b69..6390e4f39a 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs @@ -13,7 +13,6 @@ impl<'db> TypeInferenceBuilder<'db, '_> { object_ty: Type<'db>, attribute: &str, qualifiers: TypeQualifiers, - emit_diagnostics: bool, ) -> bool { if !qualifiers.contains(TypeQualifiers::FINAL) { return false; @@ -22,10 +21,9 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let db = self.db(); let emit_invalid_final = || { - if emit_diagnostics - && let Some(builder) = self - .context - .report_lint(&INVALID_ASSIGNMENT, diagnostic_range) + if let Some(builder) = self + .context + .report_lint(&INVALID_ASSIGNMENT, diagnostic_range) { builder.into_diagnostic(format_args!( "Cannot assign to final attribute `{attribute}` on type `{}`", @@ -81,10 +79,9 @@ impl<'db> TypeInferenceBuilder<'db, '_> { if let Some(symbol) = place_table.symbol_by_name(attribute) && symbol.is_bound() { - if emit_diagnostics - && let Some(diag_builder) = self - .context - .report_lint(&INVALID_ASSIGNMENT, diagnostic_range) + if let Some(diag_builder) = self + .context + .report_lint(&INVALID_ASSIGNMENT, diagnostic_range) { diag_builder.into_diagnostic(format_args!( "Cannot assign to final attribute `{attribute}` in `__init__` \ @@ -108,6 +105,10 @@ impl<'db> TypeInferenceBuilder<'db, '_> { ) { let db = self.db(); + if !emit_diagnostics { + return; + } + match object_ty { Type::Union(union) => { for elem in union.elements(db) { @@ -169,7 +170,6 @@ impl<'db> TypeInferenceBuilder<'db, '_> { object_ty, attribute, meta_attr.qualifiers, - emit_diagnostics, ) && let Some(fallback_attr) = fallback_attr { self.invalid_assignment_to_final_attribute( @@ -177,7 +177,6 @@ impl<'db> TypeInferenceBuilder<'db, '_> { object_ty, attribute, fallback_attr.qualifiers, - emit_diagnostics, ); } }

AlexWaygood · 2026-03-11T15:03:42Z

crates/ty_python_semantic/src/types/infer/builder.rs


-                        if invalid_assignment_to_final(self, qualifiers) {
+                        if self.invalid_assignment_to_final_attribute(
+                            target.range(),


it looks like it predates your PR, but the diagnostic range for these errors looks incorrect to me. I think the underline should cover the whole assignment here, not just the target on the left-hand side. The current range to me implies that it's an error about invalid attribute access or something:

AlexWaygood · 2026-03-11T15:18:40Z

crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs

+        // When `__init__` has a `self: Self` annotation, `object_ty` may be a `Self` typevar
+        // (a class type) rather than an instance type. Bind the typevar, then try `to_instance`
+        // to convert e.g. `type[C]` -> `C`; fall back to the bound type if it's already an
+        // instance.
+        let object_instance_ty = object_ty
+            .bind_self_typevars(db, class_instance_ty)
+            .to_instance(db)
+            .unwrap_or_else(|| object_ty.bind_self_typevars(db, class_instance_ty));


I don't understand this comment at all (the upper bound of Self in the context of self: Self is a nominal instance type referring to the current class, so calling .to_instance() on it should always return None). No tests fail if I remove this .to_instance(db).unwrap_or_else() business on your branch:

Suggested change

// When `__init__` has a `self: Self` annotation, `object_ty` may be a `Self` typevar

// (a class type) rather than an instance type. Bind the typevar, then try `to_instance`

// to convert e.g. `type[C]` -> `C`; fall back to the bound type if it's already an

// instance.

let object_instance_ty = object_ty

.bind_self_typevars(db, class_instance_ty)

.to_instance(db)

.unwrap_or_else(|| object_ty.bind_self_typevars(db, class_instance_ty));

let object_instance_ty = object_ty.bind_self_typevars(db, class_instance_ty);

AlexWaygood · 2026-03-11T15:32:57Z

crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs

+        if !is_in_init {
+            emit_invalid_final();
+            return true;
+        }


again, it doesn't need to block merging the PR, but it would be great to have some more information in the diagnostic about why this is invalid. This diagnostic:

error[invalid-assignment]: Cannot assign to final attribute `x` on type `Foo` --> foo.py:5:9 | 3 | class Foo: 4 | def foo(self: Foo): 5 | self.x: Final[int] = 42 | ^^^^^^ | info: rule `invalid-assignment` is enabled by default

could be much more helpful: it could say that the specific rule the user is violating is that you're only allowed to assign to Final attributes in a class's __init__ method

Also, it might not necessarily be this point where the problem is, but something somewhere is a bit too naive. Even on this branch, we're the only type checker not to issue any complaints about this code:

from typing import Final class Foo: def __init__(sssself, self: Foo): self.x: Final[int] = 42

(notice that self there is not the first parameter to __init__...)

AlexWaygood · 2026-03-11T15:41:45Z

crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs

+        let is_current_class_instance =
+            object_instance_ty
+                .as_nominal_instance()
+                .is_some_and(|instance| {
+                    instance
+                        .class(db)
+                        .iter_mro(db)
+                        .filter_map(crate::types::ClassBase::into_class)
+                        .any(|mro_class| mro_class.class_literal(db) == class_literal)
+                })
+                || object_instance_ty.is_subtype_of(db, class_instance_ty);


You can get rid of the elaborate special-casing of nominal-instance types by instead doing this:

--- a/crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/final_attribute.rs @@ -48,8 +48,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { return true; }; - let class_instance_ty = Type::instance(db, class_ty); - let class_literal = class_ty.class_literal(db); + let class_instance_ty = Type::instance(db, class_ty).top_materialization(db); // When `__init__` has a `self: Self` annotation, `object_ty` may be a `Self` typevar // (a class type) rather than an instance type. Bind the typevar, then try `to_instance` // to convert e.g. `type[C]` -> `C`; fall back to the bound type if it's already an @@ -58,17 +57,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { .bind_self_typevars(db, class_instance_ty) .to_instance(db) .unwrap_or_else(|| object_ty.bind_self_typevars(db, class_instance_ty)); - let is_current_class_instance = - object_instance_ty - .as_nominal_instance() - .is_some_and(|instance| { - instance - .class(db) - .iter_mro(db) - .filter_map(crate::types::ClassBase::into_class) - .any(|mro_class| mro_class.class_literal(db) == class_literal) - }) - || object_instance_ty.is_subtype_of(db, class_instance_ty); + let is_current_class_instance = object_instance_ty.is_subtype_of(db, class_instance_ty); if !is_current_class_instance { emit_invalid_final(); return true;

(all your tests pass with that change applied)

astral-sh-bot bot added the ty Multi-file analysis & type inference label Mar 10, 2026

charliermarsh force-pushed the charlie/final-attr branch 6 times, most recently from e8f72bf to 59c2102 Compare March 10, 2026 20:36

charliermarsh marked this pull request as ready for review March 10, 2026 21:03

charliermarsh requested review from AlexWaygood, carljm, dcreager, ibraheemdev and sharkdp as code owners March 10, 2026 21:03

astral-sh-bot bot assigned ibraheemdev Mar 10, 2026

charliermarsh added the ecosystem-analyzer label Mar 10, 2026

AlexWaygood reviewed Mar 10, 2026

View reviewed changes

charliermarsh and others added 3 commits March 10, 2026 19:57

[ty] Enforce Final attribute assignment rules for annotated and augme…

e537867

…nted writes

Review feedback

8e8e34c

Apply suggestion from @AlexWaygood

5b2b1d1

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

charliermarsh force-pushed the charlie/final-attr branch from 8c08657 to 5b2b1d1 Compare March 10, 2026 23:57

carljm removed their request for review March 11, 2026 02:12

AlexWaygood assigned AlexWaygood and unassigned ibraheemdev Mar 11, 2026

AlexWaygood reviewed Mar 11, 2026

View reviewed changes

Conversation

charliermarsh commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

astral-sh-bot bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Typing conformance results improved 🎉

Summary

Test file breakdown

True positives added (3)

False positives removed (1)

Uh oh!

astral-sh-bot bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

astral-sh-bot bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Memory usage report

Uh oh!

charliermarsh commented Mar 10, 2026

Uh oh!

charliermarsh commented Mar 10, 2026

Uh oh!

astral-sh-bot bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ecosystem-analyzer results

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexWaygood Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

charliermarsh commented Mar 11, 2026

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

charliermarsh commented Mar 10, 2026 •

edited

Loading

astral-sh-bot bot commented Mar 10, 2026 •

edited

Loading

astral-sh-bot bot commented Mar 10, 2026 •

edited

Loading

`mypy_primer` results

astral-sh-bot bot commented Mar 10, 2026 •

edited

Loading

astral-sh-bot bot commented Mar 10, 2026 •

edited

Loading

`ecosystem-analyzer` results

AlexWaygood Mar 11, 2026 •

edited

Loading

AlexWaygood Mar 11, 2026 •

edited

Loading