From 1a099886abfc535e1e08e73674e84038b52a82ef Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 8 Jul 2025 20:29:07 +0200 Subject: [PATCH] [ty] Improved diagnostic for reassignments of `Final` symbols (#19214) ## Summary Implement [this suggestion](https://github.com/astral-sh/ruff/pull/19178#discussion_r2192658146) by @AlexWaygood. ![image](https://github.com/user-attachments/assets/f183d691-ef6e-43a2-b005-3a32205bc408) --- ...-_Full_diagnostics_(174fdd8134fb325b).snap | 37 ++++++++++++++----- .../resources/mdtest/type_qualifiers/final.md | 10 +++++ crates/ty_python_semantic/src/types/infer.rs | 28 +++++++++++--- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_`typing.Final`_-_Full_diagnostics_(174fdd8134fb325b).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_`typing.Final`_-_Full_diagnostics_(174fdd8134fb325b).snap index 8d0adf1810..77b57442e8 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_`typing.Final`_-_Full_diagnostics_(174fdd8134fb325b).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_`typing.Final`_-_Full_diagnostics_(174fdd8134fb325b).snap @@ -12,31 +12,48 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md ## mdtest_snippet.py ``` -1 | from typing import Final -2 | -3 | MY_CONSTANT: Final[int] = 1 -4 | -5 | # more code -6 | -7 | MY_CONSTANT = 2 # error: [invalid-assignment] + 1 | from typing import Final + 2 | + 3 | MY_CONSTANT: Final[int] = 1 + 4 | + 5 | # more code + 6 | + 7 | MY_CONSTANT = 2 # error: [invalid-assignment] + 8 | from _stat import ST_INO + 9 | +10 | ST_INO = 1 # error: [invalid-assignment] ``` # Diagnostics ``` error[invalid-assignment]: Reassignment of `Final` symbol `MY_CONSTANT` is not allowed - --> src/mdtest_snippet.py:3:1 + --> src/mdtest_snippet.py:3:14 | 1 | from typing import Final 2 | 3 | MY_CONSTANT: Final[int] = 1 - | --------------------------- Original definition + | ---------- Symbol declared as `Final` here 4 | 5 | # more code 6 | 7 | MY_CONSTANT = 2 # error: [invalid-assignment] - | ^^^^^^^^^^^ Reassignment of `Final` symbol + | ^^^^^^^^^^^^^^^ Symbol later reassigned here +8 | from _stat import ST_INO | info: rule `invalid-assignment` is enabled by default ``` + +``` +error[invalid-assignment]: Reassignment of `Final` symbol `ST_INO` is not allowed + --> src/mdtest_snippet.py:10:1 + | + 8 | from _stat import ST_INO + 9 | +10 | ST_INO = 1 # error: [invalid-assignment] + | ^^^^^^^^^^ Reassignment of `Final` symbol + | +info: rule `invalid-assignment` is enabled by default + +``` diff --git a/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md b/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md index 0b75bb85e6..57f64cd875 100644 --- a/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md +++ b/crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md @@ -260,6 +260,8 @@ class C: +Annotated assignment: + ```py from typing import Final @@ -270,4 +272,12 @@ MY_CONSTANT: Final[int] = 1 MY_CONSTANT = 2 # error: [invalid-assignment] ``` +Imported `Final` symbol: + +```py +from _stat import ST_INO + +ST_INO = 1 # error: [invalid-assignment] +``` + [`typing.final`]: https://docs.python.org/3/library/typing.html#typing.Final diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 9575ea738c..cad60f6c00 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -1663,7 +1663,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { if !is_local || previous_definition.is_some() { let place = place_table.place_expr(binding.place(db)); - if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, node) { + if let Some(builder) = self.context.report_lint( + &INVALID_ASSIGNMENT, + binding.full_range(self.db(), self.module()), + ) { let mut diagnostic = builder.into_diagnostic(format_args!( "Reassignment of `Final` symbol `{place}` is not allowed" )); @@ -1676,10 +1679,25 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // module, but that information is currently not threaded through attribute // lookup. if !previous_definition.kind(db).is_import() { - let range = previous_definition.full_range(self.db(), self.module()); - diagnostic.annotate( - self.context.secondary(range).message("Original definition"), - ); + if let DefinitionKind::AnnotatedAssignment(assignment) = + previous_definition.kind(db) + { + let range = assignment.annotation(self.module()).range(); + diagnostic.annotate( + self.context + .secondary(range) + .message("Symbol declared as `Final` here"), + ); + } else { + let range = + previous_definition.full_range(self.db(), self.module()); + diagnostic.annotate( + self.context + .secondary(range) + .message("Symbol declared as `Final` here"), + ); + } + diagnostic.set_primary_message("Symbol later reassigned here"); } } }