[pyupgrade] [ruff] Don't apply renamings if the new name is shadowed in a scope of one of the references to the binding (UP049, RUF052) (#16032)

Co-authored-by: Alex Waygood <alex.waygood@gmail.com>

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP049_1.py

@@ -54,3 +54,52 @@ def f[_](x: _) -> _: ...
 def g[__](x: __) -> __: ...
 def h[_T_](x: _T_) -> _T_: ...
 def i[__T__](x: __T__) -> __T__: ...
+
+
+# https://github.com/astral-sh/ruff/issues/16024
+
+from typing import cast, Literal
+
+
+class C[_0]: ...
+
+
+class C[T, _T]: ...
+class C[_T, T]: ...
+
+
+class C[_T]:
+    v1 = cast(_T, ...)
+    v2 = cast('_T', ...)
+    v3 = cast("\u005fT", ...)
+
+    def _(self):
+        v1 = cast(_T, ...)
+        v2 = cast('_T', ...)
+        v3 = cast("\u005fT", ...)
+
+
+class C[_T]:
+    v = cast('Literal[\'foo\'] | _T', ...)
+
+
+## Name collision
+class C[T]:
+    def f[_T](self):  # No fix, collides with `T` from outer scope
+        v1 = cast(_T, ...)
+        v2 = cast('_T', ...)
+
+
+# Unfixable as the new name collides with a variable visible from one of the inner scopes
+class C[_T]:
+    T = 42
+
+    v1 = cast(_T, ...)
+    v2 = cast('_T', ...)
+
+
+# Unfixable as the new name collides with a variable visible from one of the inner scopes
+class C[_T]:
+    def f[T](self):
+        v1 = cast(_T, ...)
+        v2 = cast('_T', ...)

crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py

@@ -176,3 +176,22 @@ class Node:
         _seen.add(self)
         for other in self.connected:
             other.recurse(_seen=_seen)
+
+
+def foo():
+    _dummy_var = 42
+
+    def bar():
+        dummy_var = 43
+        print(_dummy_var)
+
+
+def foo():
+    # Unfixable because both possible candidates for the new name are shadowed
+    # in the scope of one of the references to the variable
+    _dummy_var = 42
+
+    def bar():
+        dummy_var = 43
+        dummy_var_ = 44
+        print(_dummy_var)

crates/ruff_linter/src/renamer.rs

@@ -387,25 +387,36 @@ pub(crate) enum ShadowedKind {
 }
 
 impl ShadowedKind {
-    /// Determines the kind of shadowing or conflict for a given variable name.
+    /// Determines the kind of shadowing or conflict for the proposed new name of a given [`Binding`].
     ///
-    /// This function is useful for checking whether or not the `target` of a [`Rename::rename`]
+    /// This function is useful for checking whether or not the `target` of a [`Renamer::rename`]
     /// will shadow another binding.
-    pub(crate) fn new(name: &str, checker: &Checker, scope_id: ScopeId) -> ShadowedKind {
+    pub(crate) fn new(binding: &Binding, new_name: &str, checker: &Checker) -> ShadowedKind {
         // Check the kind in order of precedence
-        if is_keyword(name) {
+        if is_keyword(new_name) {
             return ShadowedKind::Keyword;
         }
 
         if is_python_builtin(
-            name,
+            new_name,
             checker.settings.target_version.minor(),
             checker.source_type.is_ipynb(),
         ) {
             return ShadowedKind::BuiltIn;
         }
 
-        if !checker.semantic().is_available_in_scope(name, scope_id) {
+        let semantic = checker.semantic();
+
+        if !semantic.is_available_in_scope(new_name, binding.scope) {
+            return ShadowedKind::Some;
+        }
+
+        if binding
+            .references()
+            .map(|reference_id| semantic.reference(reference_id).scope_id())
+            .dedup()
+            .any(|scope| !semantic.is_available_in_scope(new_name, scope))
+        {
             return ShadowedKind::Some;
         }
 

crates/ruff_linter/src/rules/flake8_pyi/rules/custom_type_var_for_self.rs

@@ -570,7 +570,7 @@ fn replace_typevar_usages_with_self<'a>(
         let reference_range = reference.range();
         if &source[reference_range] != tvar_name {
             bail!(
-                "Cannot autofix: feference in the source code (`{}`) is not equal to the typevar name (`{}`)",
+                "Cannot autofix: reference in the source code (`{}`) is not equal to the typevar name (`{}`)",
                 &source[reference_range],
                 tvar_name
             );

crates/ruff_linter/src/rules/pyupgrade/rules/pep695/private_type_parameter.rs

@@ -1,7 +1,9 @@
-use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
+use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_ast::Stmt;
 use ruff_python_semantic::Binding;
+use ruff_python_stdlib::identifiers::is_identifier;
+use ruff_text_size::Ranged;
 
 use crate::{
     checkers::ast::Checker,
@@ -93,7 +95,7 @@ impl Violation for PrivateTypeParameter {
     }
 
     fn fix_title(&self) -> Option<String> {
-        Some("Remove the leading underscores".to_string())
+        Some("Rename type parameter to remove leading underscores".to_string())
     }
 }
 
@@ -129,20 +131,35 @@ pub(crate) fn private_type_parameter(checker: &Checker, binding: &Binding) -> Op
 
     // if the new name would shadow another variable, keyword, or builtin, emit a diagnostic without
     // a suggested fix
-    if ShadowedKind::new(new_name, checker, binding.scope).shadows_any() {
+    if ShadowedKind::new(binding, new_name, checker).shadows_any() {
         return Some(diagnostic);
     }
 
+    if !is_identifier(new_name) {
+        return Some(diagnostic);
+    }
+
+    let source = checker.source();
+
     diagnostic.try_set_fix(|| {
         let (first, rest) = Renamer::rename(
             old_name,
             new_name,
             &semantic.scopes[binding.scope],
-            checker.semantic(),
+            semantic,
             checker.stylist(),
         )?;
 
-        Ok(Fix::safe_edits(first, rest))
+        let applicability = if binding
+            .references()
+            .any(|id| &source[semantic.reference(id).range()] != old_name)
+        {
+            Applicability::DisplayOnly
+        } else {
+            Applicability::Safe
+        };
+
+        Ok(Fix::applicable_edits(first, rest, applicability))
     });
 
     Some(diagnostic)

crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_0.py.snap

@@ -8,7 +8,7 @@ UP049_0.py:2:15: UP049 [*] Generic class uses private type parameters
   |               ^^ UP049
 3 |     buf: list[_T]
   |
-  = help: Remove the leading underscores
+  = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 1 1 | # simple case, replace _T in signature and body
@@ -31,7 +31,7 @@ UP049_0.py:10:12: UP049 [*] Generic function uses private type parameters
 11 |     y: _T = var[1]
 12 |     return y
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 7  7  | 
@@ -54,7 +54,7 @@ UP049_0.py:17:5: UP049 [*] Generic function uses private type parameters
 18 |     _U,  # second generic
 19 | ](args):
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 14 14 | 
@@ -75,7 +75,7 @@ UP049_0.py:18:5: UP049 [*] Generic function uses private type parameters
 19 | ](args):
 20 |     return args
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 15 15 | # one diagnostic for each variable, comments are preserved
@@ -94,7 +94,7 @@ UP049_0.py:27:7: UP049 [*] Generic function uses private type parameters
 28 |     cast("_T", v)
 29 |     cast("Literal['_T']")
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 24 24 | from typing import Literal, cast

crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP049_1.py.snap

@@ -8,7 +8,7 @@ UP049_1.py:2:11: UP049 [*] Generic class uses private type parameters
   |           ^^ UP049
 3 |     var: _T
   |
-  = help: Remove the leading underscores
+  = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 1 1 | # bound
@@ -27,7 +27,7 @@ UP049_1.py:7:11: UP049 [*] Generic class uses private type parameters
   |           ^^ UP049
 8 |     var: _T
   |
-  = help: Remove the leading underscores
+  = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 4 4 | 
@@ -48,7 +48,7 @@ UP049_1.py:12:11: UP049 [*] Generic class uses private type parameters
    |           ^^ UP049
 13 |     var: _T
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 9  9  | 
@@ -69,7 +69,7 @@ UP049_1.py:17:12: UP049 [*] Generic class uses private type parameters
    |            ^^^ UP049
 18 |     var: tuple[*_Ts]
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 14 14 | 
@@ -90,7 +90,7 @@ UP049_1.py:22:11: UP049 [*] Generic class uses private type parameters
    |           ^^ UP049
 23 |     var: _P
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 19 19 | 
@@ -113,7 +113,7 @@ UP049_1.py:31:18: UP049 [*] Generic class uses private type parameters
 32 |     @staticmethod
 33 |     def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None:
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 28 28 | 
@@ -137,7 +137,7 @@ UP049_1.py:31:22: UP049 [*] Generic class uses private type parameters
 32 |     @staticmethod
 33 |     def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None:
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 28 28 | 
@@ -161,7 +161,7 @@ UP049_1.py:31:31: UP049 [*] Generic class uses private type parameters
 32 |     @staticmethod
 33 |     def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None:
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 28 28 | 
@@ -185,7 +185,7 @@ UP049_1.py:31:50: UP049 [*] Generic class uses private type parameters
 32 |     @staticmethod
 33 |     def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None:
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 28 28 | 
@@ -209,7 +209,7 @@ UP049_1.py:31:56: UP049 [*] Generic class uses private type parameters
 32 |     @staticmethod
 33 |     def transform(t: _T, u: _U, v: _V) -> tuple[*_W] | Callable[_X, _T] | None:
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 ℹ Safe fix
 28 28 | 
@@ -231,7 +231,7 @@ UP049_1.py:39:9: UP049 Generic class uses private type parameters
 39 | class F[_async]: ...
    |         ^^^^^^ UP049
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
 
 UP049_1.py:47:25: UP049 Generic class uses private type parameters
    |
@@ -242,4 +242,110 @@ UP049_1.py:47:25: UP049 Generic class uses private type parameters
 48 |         var: _X
 49 |         x: X
    |
-   = help: Remove the leading underscores
+   = help: Rename type parameter to remove leading underscores
+
+UP049_1.py:64:9: UP049 Generic class uses private type parameters
+   |
+64 | class C[_0]: ...
+   |         ^^ UP049
+   |
+   = help: Rename type parameter to remove leading underscores
+
+UP049_1.py:67:12: UP049 Generic class uses private type parameters
+   |
+67 | class C[T, _T]: ...
+   |            ^^ UP049
+68 | class C[_T, T]: ...
+   |
+   = help: Rename type parameter to remove leading underscores
+
+UP049_1.py:68:9: UP049 Generic class uses private type parameters
+   |
+67 | class C[T, _T]: ...
+68 | class C[_T, T]: ...
+   |         ^^ UP049
+   |
+   = help: Rename type parameter to remove leading underscores
+
+UP049_1.py:71:9: UP049 Generic class uses private type parameters
+   |
+71 | class C[_T]:
+   |         ^^ UP049
+72 |     v1 = cast(_T, ...)
+73 |     v2 = cast('_T', ...)
+   |
+   = help: Rename type parameter to remove leading underscores
+
+ℹ Display-only fix
+68 68 | class C[_T, T]: ...
+69 69 | 
+70 70 | 
+71    |-class C[_T]:
+72    |-    v1 = cast(_T, ...)
+73    |-    v2 = cast('_T', ...)
+74    |-    v3 = cast("\u005fT", ...)
+   71 |+class C[T]:
+   72 |+    v1 = cast(T, ...)
+   73 |+    v2 = cast('T', ...)
+   74 |+    v3 = cast(T, ...)
+75 75 | 
+76 76 |     def _(self):
+77    |-        v1 = cast(_T, ...)
+78    |-        v2 = cast('_T', ...)
+79    |-        v3 = cast("\u005fT", ...)
+   77 |+        v1 = cast(T, ...)
+   78 |+        v2 = cast('T', ...)
+   79 |+        v3 = cast(T, ...)
+80 80 | 
+81 81 | 
+82 82 | class C[_T]:
+
+UP049_1.py:82:9: UP049 Generic class uses private type parameters
+   |
+82 | class C[_T]:
+   |         ^^ UP049
+83 |     v = cast('Literal[\'foo\'] | _T', ...)
+   |
+   = help: Rename type parameter to remove leading underscores
+
+ℹ Display-only fix
+79 79 |         v3 = cast("\u005fT", ...)
+80 80 | 
+81 81 | 
+82    |-class C[_T]:
+83    |-    v = cast('Literal[\'foo\'] | _T', ...)
+   82 |+class C[T]:
+   83 |+    v = cast(T, ...)
+84 84 | 
+85 85 | 
+86 86 | ## Name collision
+
+UP049_1.py:88:11: UP049 Generic function uses private type parameters
+   |
+86 | ## Name collision
+87 | class C[T]:
+88 |     def f[_T](self):  # No fix, collides with `T` from outer scope
+   |           ^^ UP049
+89 |         v1 = cast(_T, ...)
+90 |         v2 = cast('_T', ...)
+   |
+   = help: Rename type parameter to remove leading underscores
+
+UP049_1.py:94:9: UP049 Generic class uses private type parameters
+   |
+93 | # Unfixable as the new name collides with a variable visible from one of the inner scopes
+94 | class C[_T]:
+   |         ^^ UP049
+95 |     T = 42
+   |
+   = help: Rename type parameter to remove leading underscores
+
+UP049_1.py:102:9: UP049 Generic class uses private type parameters
+    |
+101 | # Unfixable as the new name collides with a variable visible from one of the inner scopes
+102 | class C[_T]:
+    |         ^^ UP049
+103 |     def f[T](self):
+104 |         v1 = cast(_T, ...)
+    |
+    = help: Rename type parameter to remove leading underscores

crates/ruff_linter/src/rules/ruff/rules/used_dummy_variable.rs

@@ -1,7 +1,7 @@
 use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_ast::helpers::is_dunder;
-use ruff_python_semantic::{Binding, BindingId, ScopeId};
+use ruff_python_semantic::{Binding, BindingId};
 use ruff_python_stdlib::identifiers::is_identifier;
 use ruff_text_size::Ranged;
 
@@ -179,7 +179,7 @@ pub(crate) fn used_dummy_variable(
     // Trim the leading underscores for further checks
     let trimmed_name = name.trim_start_matches('_');
 
-    let shadowed_kind = ShadowedKind::new(trimmed_name, checker, binding.scope);
+    let shadowed_kind = ShadowedKind::new(binding, trimmed_name, checker);
 
     let mut diagnostic = Diagnostic::new(
         UsedDummyVariable {
@@ -190,9 +190,7 @@ pub(crate) fn used_dummy_variable(
     );
 
     // Get the possible fix based on the scope
-    if let Some(new_name) =
+    if let Some(new_name) = get_possible_new_name(binding, trimmed_name, shadowed_kind, checker) {
-        get_possible_new_name(trimmed_name, shadowed_kind, binding.scope, checker)
-    {
         diagnostic.try_set_fix(|| {
             Renamer::rename(name, &new_name, scope, semantic, checker.stylist())
                 .map(|(edit, rest)| Fix::unsafe_edits(edit, rest))
@@ -204,9 +202,9 @@ pub(crate) fn used_dummy_variable(
 
 /// Suggests a potential alternative name to resolve a shadowing conflict.
 fn get_possible_new_name(
+    binding: &Binding,
     trimmed_name: &str,
     kind: ShadowedKind,
-    scope_id: ScopeId,
     checker: &Checker,
 ) -> Option<String> {
     // Construct the potential fix name based on ShadowedKind
@@ -223,10 +221,7 @@ fn get_possible_new_name(
     }
 
     // Ensure the fix name is not already taken in the scope or enclosing scopes
-    if !checker
+    if ShadowedKind::new(binding, &fix_name, checker).shadows_any() {
-        .semantic()
-        .is_available_in_scope(&fix_name, scope_id)
-    {
         return None;
     }
 

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF052_RUF052.py.snap

@@ -365,3 +365,39 @@ RUF052.py:160:5: RUF052 [*] Local dummy variable `_NotADynamicClass` is accessed
 163 163 | 
 164 164 | # Do not emit diagnostic if parameter is private
 165 165 | # even if it is later shadowed in the body of the function
+
+RUF052.py:182:5: RUF052 [*] Local dummy variable `_dummy_var` is accessed
+    |
+181 | def foo():
+182 |     _dummy_var = 42
+    |     ^^^^^^^^^^ RUF052
+183 |
+184 |     def bar():
+    |
+    = help: Prefer using trailing underscores to avoid shadowing a variable
+
+ℹ Unsafe fix
+179 179 | 
+180 180 | 
+181 181 | def foo():
+182     |-    _dummy_var = 42
+    182 |+    dummy_var_ = 42
+183 183 | 
+184 184 |     def bar():
+185 185 |         dummy_var = 43
+186     |-        print(_dummy_var)
+    186 |+        print(dummy_var_)
+187 187 | 
+188 188 | 
+189 189 | def foo():
+
+RUF052.py:192:5: RUF052 Local dummy variable `_dummy_var` is accessed
+    |
+190 |     # Unfixable because both possible candidates for the new name are shadowed
+191 |     # in the scope of one of the references to the variable
+192 |     _dummy_var = 42
+    |     ^^^^^^^^^^ RUF052
+193 |
+194 |     def bar():
+    |
+    = help: Prefer using trailing underscores to avoid shadowing a variable

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_1.snap

@@ -365,3 +365,39 @@ RUF052.py:160:5: RUF052 [*] Local dummy variable `_NotADynamicClass` is accessed
 163 163 | 
 164 164 | # Do not emit diagnostic if parameter is private
 165 165 | # even if it is later shadowed in the body of the function
+
+RUF052.py:182:5: RUF052 [*] Local dummy variable `_dummy_var` is accessed
+    |
+181 | def foo():
+182 |     _dummy_var = 42
+    |     ^^^^^^^^^^ RUF052
+183 |
+184 |     def bar():
+    |
+    = help: Prefer using trailing underscores to avoid shadowing a variable
+
+ℹ Unsafe fix
+179 179 | 
+180 180 | 
+181 181 | def foo():
+182     |-    _dummy_var = 42
+    182 |+    dummy_var_ = 42
+183 183 | 
+184 184 |     def bar():
+185 185 |         dummy_var = 43
+186     |-        print(_dummy_var)
+    186 |+        print(dummy_var_)
+187 187 | 
+188 188 | 
+189 189 | def foo():
+
+RUF052.py:192:5: RUF052 Local dummy variable `_dummy_var` is accessed
+    |
+190 |     # Unfixable because both possible candidates for the new name are shadowed
+191 |     # in the scope of one of the references to the variable
+192 |     _dummy_var = 42
+    |     ^^^^^^^^^^ RUF052
+193 |
+194 |     def bar():
+    |
+    = help: Prefer using trailing underscores to avoid shadowing a variable

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_2.snap

@@ -1,6 +1,5 @@
 ---
 source: crates/ruff_linter/src/rules/ruff/mod.rs
-snapshot_kind: text
 ---
 RUF052.py:92:9: RUF052 Local dummy variable `_var` is accessed
    |
@@ -184,3 +183,24 @@ RUF052.py:160:5: RUF052 Local dummy variable `_NotADynamicClass` is accessed
 162 |     print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     |
     = help: Remove leading underscores
+
+RUF052.py:182:5: RUF052 Local dummy variable `_dummy_var` is accessed
+    |
+181 | def foo():
+182 |     _dummy_var = 42
+    |     ^^^^^^^^^^ RUF052
+183 |
+184 |     def bar():
+    |
+    = help: Prefer using trailing underscores to avoid shadowing a variable
+
+RUF052.py:192:5: RUF052 Local dummy variable `_dummy_var` is accessed
+    |
+190 |     # Unfixable because both possible candidates for the new name are shadowed
+191 |     # in the scope of one of the references to the variable
+192 |     _dummy_var = 42
+    |     ^^^^^^^^^^ RUF052
+193 |
+194 |     def bar():
+    |
+    = help: Prefer using trailing underscores to avoid shadowing a variable