[ruff] Omit diagnostic for shadowed private function parameters in …

…`used-dummy-variable` (`RUF052`) (#15376)
astral-sh · Jan 10, 2025 · 443bf38 · 443bf38
1 parent 23ad319
commit 443bf38
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 4 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py
@@ -145,3 +145,19 @@ def special_calls():
     _NotADynamicClass = type("_NotADynamicClass")
 
     print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+
+# Do not emit diagnostic if parameter is private
+# even if it is later shadowed in the body of the function
+# see https://github.com/astral-sh/ruff/issues/14968
+class Node:
+
+    connected: list[Node]
+
+    def recurse(self, *, _seen: set[Node] | None = None):
+        if _seen is None:
+            _seen = set()
+        elif self in _seen:
+            return
+        _seen.add(self)
+        for other in self.connected:
+            other.recurse(_seen=_seen)
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs
@@ -24,7 +24,7 @@ pub(crate) fn bindings(checker: &mut Checker) {
         return;
     }
 
-    for binding in &*checker.semantic.bindings {
+    for (binding_id, binding) in checker.semantic.bindings.iter_enumerated() {
         if checker.enabled(Rule::UnusedVariable) {
             if binding.kind.is_bound_exception()
                 && binding.is_unused()
@@ -90,7 +90,8 @@ pub(crate) fn bindings(checker: &mut Checker) {
             }
         }
         if checker.enabled(Rule::UsedDummyVariable) {
-            if let Some(diagnostic) = ruff::rules::used_dummy_variable(checker, binding) {
+            if let Some(diagnostic) = ruff::rules::used_dummy_variable(checker, binding, binding_id)
+            {
                 checker.diagnostics.push(diagnostic);
             }
         }

diff --git a/crates/ruff_linter/src/rules/ruff/rules/used_dummy_variable.rs b/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, ScopeId};
+use ruff_python_semantic::{Binding, BindingId, ScopeId};
 use ruff_python_stdlib::{
     builtins::is_python_builtin, identifiers::is_identifier, keyword::is_keyword,
 };
@@ -97,7 +97,11 @@ impl Violation for UsedDummyVariable {
 }
 
 /// RUF052
-pub(crate) fn used_dummy_variable(checker: &Checker, binding: &Binding) -> Option<Diagnostic> {
+pub(crate) fn used_dummy_variable(
+    checker: &Checker,
+    binding: &Binding,
+    binding_id: BindingId,
+) -> Option<Diagnostic> {
     let name = binding.name(checker.source());
 
     // Ignore `_` and dunder variables
@@ -141,6 +145,21 @@ pub(crate) fn used_dummy_variable(checker: &Checker, binding: &Binding) -> Optio
     if !scope.kind.is_function() {
         return None;
     }
+
+    // Recall from above that we do not wish to flag "private"
+    // function parameters. The previous early exit ensured
+    // that the binding in hand was not a function parameter.
+    // We now check that, in the body of our function, we are
+    // not looking at a shadowing of a private parameter.
+    //
+    // (Technically this also covers the case in the previous early exit,
+    // but it is more expensive so we keep both.)
+    if scope
+        .shadowed_bindings(binding_id)
+        .any(|shadow_id| semantic.binding(shadow_id).kind.is_argument())
+    {
+        return None;
+    }
     if !checker.settings.dummy_variable_rgx.is_match(name) {
         return None;
     }

diff --git a/...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF052_RUF052.py.snap b/...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF052_RUF052.py.snap
@@ -175,6 +175,9 @@ RUF052.py:138:5: RUF052 [*] Local dummy variable `_P` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:139:5: RUF052 [*] Local dummy variable `_T` is accessed
     |
@@ -201,6 +204,9 @@ RUF052.py:139:5: RUF052 [*] Local dummy variable `_T` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:140:5: RUF052 [*] Local dummy variable `_NT` is accessed
     |
@@ -227,6 +233,9 @@ RUF052.py:140:5: RUF052 [*] Local dummy variable `_NT` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:141:5: RUF052 [*] Local dummy variable `_E` is accessed
     |
@@ -252,6 +261,9 @@ RUF052.py:141:5: RUF052 [*] Local dummy variable `_E` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:142:5: RUF052 [*] Local dummy variable `_NT2` is accessed
     |
@@ -276,6 +288,9 @@ RUF052.py:142:5: RUF052 [*] Local dummy variable `_NT2` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:143:5: RUF052 [*] Local dummy variable `_NT3` is accessed
     |
@@ -299,6 +314,9 @@ RUF052.py:143:5: RUF052 [*] Local dummy variable `_NT3` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, _NT2, NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:144:5: RUF052 [*] Local dummy variable `_DynamicClass` is accessed
     |
@@ -320,6 +338,9 @@ RUF052.py:144:5: RUF052 [*] Local dummy variable `_DynamicClass` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, _NT2, _NT3, DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:145:5: RUF052 [*] Local dummy variable `_NotADynamicClass` is accessed
     |
@@ -341,3 +362,6 @@ RUF052.py:145:5: RUF052 [*] Local dummy variable `_NotADynamicClass` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
diff --git a/.../ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_1.snap b/.../ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_1.snap
@@ -175,6 +175,9 @@ RUF052.py:138:5: RUF052 [*] Local dummy variable `_P` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:139:5: RUF052 [*] Local dummy variable `_T` is accessed
     |
@@ -201,6 +204,9 @@ RUF052.py:139:5: RUF052 [*] Local dummy variable `_T` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:140:5: RUF052 [*] Local dummy variable `_NT` is accessed
     |
@@ -227,6 +233,9 @@ RUF052.py:140:5: RUF052 [*] Local dummy variable `_NT` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:141:5: RUF052 [*] Local dummy variable `_E` is accessed
     |
@@ -252,6 +261,9 @@ RUF052.py:141:5: RUF052 [*] Local dummy variable `_E` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:142:5: RUF052 [*] Local dummy variable `_NT2` is accessed
     |
@@ -276,6 +288,9 @@ RUF052.py:142:5: RUF052 [*] Local dummy variable `_NT2` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, NT2, _NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:143:5: RUF052 [*] Local dummy variable `_NT3` is accessed
     |
@@ -299,6 +314,9 @@ RUF052.py:143:5: RUF052 [*] Local dummy variable `_NT3` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, _NT2, NT3, _DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:144:5: RUF052 [*] Local dummy variable `_DynamicClass` is accessed
     |
@@ -320,6 +338,9 @@ RUF052.py:144:5: RUF052 [*] Local dummy variable `_DynamicClass` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, _NT2, _NT3, DynamicClass, _NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function
 
 RUF052.py:145:5: RUF052 [*] Local dummy variable `_NotADynamicClass` is accessed
     |
@@ -341,3 +362,6 @@ RUF052.py:145:5: RUF052 [*] Local dummy variable `_NotADynamicClass` is accessed
 146 146 | 
 147     |-    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, _NotADynamicClass)
     147 |+    print(_T, _P, _NT, _E, _NT2, _NT3, _DynamicClass, NotADynamicClass)
+148 148 | 
+149 149 | # Do not emit diagnostic if parameter is private
+150 150 | # even if it is later shadowed in the body of the function