[ty] Enforce `typing.Final` by sharkdp · Pull Request #19178 · astral-sh/ruff

sharkdp · 2025-07-07T14:03:19Z

Summary

Emit a diagnostic when a Final-qualified symbol is modified. This first iteration only works for name targets. Tests with TODO comments were added for attribute assignments as well.

related ticket: astral-sh/ty#158

Ecosystem impact

Correctly identified modification of a Final symbol (behind a # type: ignore):

- warning[unused-ignore-comment] sphinx/__init__.py:44:56: Unused blanket `type: ignore` directive

And the same here:

- warning[unused-ignore-comment] src/trio/_core/_run.py:128:45: Unused blanket `type: ignore` directive

Test Plan

New Markdown tests

github-actions · 2025-07-07T14:10:30Z

`mypy_primer` results

Changes were detected when running on open source projects

trio (https://github.com/python-trio/trio)
- warning[unused-ignore-comment] src/trio/_core/_run.py:128:45: Unused blanket `type: ignore` directive
- Found 799 diagnostics
+ Found 798 diagnostics

sphinx (https://github.com/sphinx-doc/sphinx)
- warning[unused-ignore-comment] sphinx/__init__.py:44:56: Unused blanket `type: ignore` directive
- Found 606 diagnostics
+ Found 605 diagnostics

Memory usage changes were detected when running on open source projects

flake8 (https://github.com/pycqa/flake8)
-     memo fields = ~63MB
+     memo fields = ~66MB

trio (https://github.com/python-trio/trio)
-     memo fields = ~152MB
+     memo fields = ~159MB

sphinx (https://github.com/sphinx-doc/sphinx)
- TOTAL MEMORY USAGE: ~316MB
+ TOTAL MEMORY USAGE: ~332MB

prefect (https://github.com/PrefectHQ/prefect)
-     memo fields = ~541MB
+     memo fields = ~568MB

AlexWaygood

🚀

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

AlexWaygood · 2025-07-08T13:07:04Z

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

+from mod import FINAL_A, FINAL_B, FINAL_C, FINAL_D, FINAL_E, FINAL_F
+
+FINAL_A = 2  # error: [invalid-assignment] "Assignment to `Final` symbol is not allowed"
+FINAL_B = 2  # error: [invalid-assignment] "Assignment to `Final` symbol is not allowed"
+FINAL_C = 2  # error: [invalid-assignment] "Assignment to `Final` symbol is not allowed"
+FINAL_D = 2  # error: [invalid-assignment] "Assignment to `Final` symbol is not allowed"
+FINAL_E = 2  # error: [invalid-assignment] "Assignment to `Final` symbol is not allowed"
+FINAL_F = 2  # error: [invalid-assignment] "Assignment to `Final` symbol is not allowed"


relevant discussion on this: https://discuss.python.org/t/imported-final-variable/82429

I sort-of disagree with the resolution there, but don't have the energy to relitigate the question now 😆 and it comes up very rarely. It's good for us to follow the behaviour the spec mandates, anyway (no change requested!).

AlexWaygood · 2025-07-08T13:13:54Z

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

+Objects qualified with `Final` *can be modified*. `Final` represents a constant reference to an
+object, but that object itself may still be mutable:


Yes -- pyre (and possibly pyrefly? Can't remember the exact status) has a feature called ImmutableRef, which they presented on at the typing summit this year. It's similar to Final but it also prevents any mutations to the symbol. It's a far more invasive and more complicated feature to implement; I'm not sure if it's ever going to be proposed for standardisation in the typing system.

AlexWaygood · 2025-07-08T13:19:27Z

crates/ty_python_semantic/src/types/infer.rs

+        let (declared_ty, is_modifiable) = place_from_declarations(self.db(), declarations)
            .and_then(|place_and_quals| {
                Ok(
                    if matches!(place_and_quals.place, Place::Type(_, Boundness::Bound)) {


doesn't need to be done in this PR: should we add a Place::is_definitely_bound() method? It feels like we have a lot of these if matches!() conditions around the codebase

will look into it

I looked into it, but it seems like such a function would only have this callsite. In most other cases, we also match on the type at the same time.

Oh, fair enough!

AlexWaygood · 2025-07-08T13:20:57Z

crates/ty_python_semantic/src/types/infer.rs

+                if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, node) {
+                    builder.into_diagnostic("Assignment to `Final` symbol is not allowed");
+                }


Could we add a subdiagnostic pointing to the previous definition where it was declared as Final?

Yes, thanks for the suggestion! It's hard to do this for imported symbols (unless @Gankra adds a vector of Definitions to Place 😄), but I added a subdiagnostic for same-module Finals:

AlexWaygood · 2025-07-08T14:16:38Z

...ources/mdtest/snapshots/final.md_-_`typing.Final`_-_Full_diagnostics_(174fdd8134fb325b).snap

+3 | MY_CONSTANT: Final[int] = 1
+  | ----------- Original definition
+4 |
+5 | # more code
+6 |
+7 | MY_CONSTANT = 2  # error: [invalid-assignment]
+  | ^^^^^^^^^^^ Reassignment of `Final` symbol


even better might be

Suggested change

3 | MY_CONSTANT: Final[int] = 1

| ----------- Original definition

4 |

5 | # more code

6 |

7 | MY_CONSTANT = 2 # error: [invalid-assignment]

| ^^^^^^^^^^^ Reassignment of `Final` symbol

3 | MY_CONSTANT: Final[int] = 1

| ---------- Symbol declared as `Final` here

4 |

5 | # more code

6 |

7 | MY_CONSTANT = 2 # error: [invalid-assignment]

| ^^^^^^^^^^^^^^^ Symbol later reassigned here

but probably not worth spending time on that now if that's hard to achieve!

I tried to highlight the full range for the assignment, but for (annotated) assignments, full_range apparently only includes the left hand side target. I'll try to change that in a follow-up, since it might affect other diagnostics.

Highlighting the annotation for the original definition would also be nice, but is a bit harder because we would need to explicitly match on different definition kinds.

@AlexWaygood

## Summary Implement [this suggestion](#19178 (comment)) by @AlexWaygood. ![image](https://github.com/user-attachments/assets/f183d691-ef6e-43a2-b005-3a32205bc408)

oconnor663 · 2025-07-09T18:30:11Z

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

+    X: Final[int] = 1
+    def inner():
+        nonlocal X
+        # TODO: this should be an error


This TODO isn't going to last long: rebasing #19112 on top of this PR makes this line fail :)

I added that just for you 🎁

sharkdp added the ty Multi-file analysis & type inference label Jul 7, 2025

sharkdp force-pushed the david/enforce-final branch from 535561f to 07e539f Compare July 7, 2025 14:05

sharkdp added the ecosystem-analyzer label Jul 7, 2025

sharkdp force-pushed the david/enforce-final branch 2 times, most recently from 3c65586 to 7c267d0 Compare July 8, 2025 09:12

[ty] Enforce typing.Final

0ecfcc1

sharkdp force-pushed the david/enforce-final branch from 7c267d0 to 0ecfcc1 Compare July 8, 2025 09:32

sharkdp marked this pull request as ready for review July 8, 2025 11:12

sharkdp requested review from AlexWaygood, carljm and dcreager as code owners July 8, 2025 11:12

AlexWaygood approved these changes Jul 8, 2025

View reviewed changes

Add subdiagnostic pointing to previous definition

3f95824

AlexWaygood approved these changes Jul 8, 2025

View reviewed changes

sharkdp merged commit 149350b into main Jul 8, 2025
37 checks passed

sharkdp deleted the david/enforce-final branch July 8, 2025 14:26

sharkdp mentioned this pull request Jul 8, 2025

[ty] Improved diagnostic for reassignments of Final symbols #19214

Merged

oconnor663 reviewed Jul 9, 2025

View reviewed changes

		Objects qualified with `Final` can be modified. `Final` represents a constant reference to an
		object, but that object itself may still be mutable:

Comments

Conversation

sharkdp commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Ecosystem impact

Test Plan

Uh oh!

github-actions bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharkdp Jul 8, 2025 • 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 Jul 8, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sharkdp commented Jul 7, 2025 •

edited

Loading

github-actions bot commented Jul 7, 2025 •

edited

Loading

`mypy_primer` results

sharkdp Jul 8, 2025 •

edited

Loading

AlexWaygood Jul 8, 2025 •

edited

Loading