[ruff] Don't emit used-dummy-variable on function parameters (`RU…

…F052`) (#14818)
astral-sh · Dec 6, 2024 · 4fdd4dd · 4fdd4dd
1 parent 6b9f3d7
commit 4fdd4dd
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 112 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py
@@ -77,7 +77,7 @@ def fun(self):
         _var = "method variable" # [RUF052]
         return _var
 
-def fun(_var): # [RUF052]
+def fun(_var): # parameters are ignored
     return _var
 
 def fun():

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, BindingKind, ScopeId};
+use ruff_python_semantic::{Binding, ScopeId};
 use ruff_python_stdlib::{
     builtins::is_python_builtin, identifiers::is_identifier, keyword::is_keyword,
 };
@@ -93,13 +93,27 @@ pub(crate) fn used_dummy_variable(checker: &Checker, binding: &Binding) -> Optio
     if binding.is_unused() {
         return None;
     }
-    // Only variables defined via function arguments or assignments.
-    if !matches!(
-        binding.kind,
-        BindingKind::Argument | BindingKind::Assignment
-    ) {
+
+    // We only emit the lint on variables defined via assignments.
+    //
+    // ## Why not also emit the lint on function parameters?
+    //
+    // There isn't universal agreement that leading underscores indicate "unused" parameters
+    // in Python (many people use them for "private" parameters), so this would be a lot more
+    // controversial than emitting the lint on assignments. Even if it's decided that it's
+    // desirable to emit a lint on function parameters with "dummy variable" names, it would
+    // possibly have to be a separate rule or we'd have to put it behind a configuration flag,
+    // as there's much less community consensus about the issue.
+    // See <https://github.com/astral-sh/ruff/issues/14796>.
+    //
+    // Moreover, autofixing the diagnostic for function parameters is much more troublesome than
+    // autofixing the diagnostic for assignments. See:
+    // - <https://github.com/astral-sh/ruff/issues/14790>
+    // - <https://github.com/astral-sh/ruff/issues/14799>
+    if !binding.kind.is_assignment() {
         return None;
     }
+
     // This excludes `global` and `nonlocal` variables.
     if binding.is_global() || binding.is_nonlocal() {
         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
@@ -20,31 +20,9 @@ RUF052.py:77:9: RUF052 [*] Local dummy variable `_var` is accessed
    77 |+        var = "method variable" # [RUF052]
    78 |+        return var
 79 79 | 
-80 80 | def fun(_var): # [RUF052]
+80 80 | def fun(_var): # parameters are ignored
 81 81 |     return _var
 
-RUF052.py:80:9: RUF052 [*] Local dummy variable `_var` is accessed
-   |
-78 |         return _var
-79 | 
-80 | def fun(_var): # [RUF052]
-   |         ^^^^ RUF052
-81 |     return _var
-   |
-   = help: Remove leading underscores
-
-ℹ Safe fix
-77 77 |         _var = "method variable" # [RUF052]
-78 78 |         return _var
-79 79 | 
-80    |-def fun(_var): # [RUF052]
-81    |-    return _var
-   80 |+def fun(var): # [RUF052]
-   81 |+    return var
-82 82 | 
-83 83 | def fun():
-84 84 |     _list = "built-in" # [RUF052]
-
 RUF052.py:84:5: RUF052 [*] Local dummy variable `_list` is accessed
    |
 83 | def fun():

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
@@ -20,31 +20,9 @@ RUF052.py:77:9: RUF052 [*] Local dummy variable `_var` is accessed
    77 |+        var = "method variable" # [RUF052]
    78 |+        return var
 79 79 | 
-80 80 | def fun(_var): # [RUF052]
+80 80 | def fun(_var): # parameters are ignored
 81 81 |     return _var
 
-RUF052.py:80:9: RUF052 [*] Local dummy variable `_var` is accessed
-   |
-78 |         return _var
-79 | 
-80 | def fun(_var): # [RUF052]
-   |         ^^^^ RUF052
-81 |     return _var
-   |
-   = help: Remove leading underscores
-
-ℹ Safe fix
-77 77 |         _var = "method variable" # [RUF052]
-78 78 |         return _var
-79 79 | 
-80    |-def fun(_var): # [RUF052]
-81    |-    return _var
-   80 |+def fun(var): # [RUF052]
-   81 |+    return var
-82 82 | 
-83 83 | def fun():
-84 84 |     _list = "built-in" # [RUF052]
-
 RUF052.py:84:5: RUF052 [*] Local dummy variable `_list` is accessed
    |
 83 | def fun():

diff --git a/.../ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_2.snap b/.../ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_2.snap
@@ -1,55 +1,6 @@
 ---
 source: crates/ruff_linter/src/rules/ruff/mod.rs
 ---
-RUF052.py:21:9: RUF052 Local dummy variable `arg` is accessed
-   |
-19 | _valid_fun()
-20 | 
-21 | def fun(arg):
-   |         ^^^ RUF052
-22 |     _valid_unused_var = arg
-23 |     pass
-   |
-
-RUF052.py:50:18: RUF052 Local dummy variable `self` is accessed
-   |
-48 |     print(_valid_private_cls_attr)
-49 | 
-50 |     def __init__(self):
-   |                  ^^^^ RUF052
-51 |         self._valid_private_ins_attr = 2
-52 |         print(self._valid_private_ins_attr)
-   |
-
-RUF052.py:54:23: RUF052 Local dummy variable `self` is accessed
-   |
-52 |         print(self._valid_private_ins_attr)
-53 | 
-54 |     def _valid_method(self):
-   |                       ^^^^ RUF052
-55 |         return self._valid_private_ins_attr
-   |
-
-RUF052.py:57:16: RUF052 Local dummy variable `arg` is accessed
-   |
-55 |         return self._valid_private_ins_attr
-56 | 
-57 |     def method(arg):
-   |                ^^^ RUF052
-58 |         _valid_unused_var = arg
-59 |         return
-   |
-
-RUF052.py:61:9: RUF052 Local dummy variable `x` is accessed
-   |
-59 |         return
-60 | 
-61 | def fun(x):
-   |         ^ RUF052
-62 |     _ = 1
-63 |     __ = 2
-   |
-
 RUF052.py:77:9: RUF052 Local dummy variable `_var` is accessed
    |
 75 | class Class_:
@@ -60,16 +11,6 @@ RUF052.py:77:9: RUF052 Local dummy variable `_var` is accessed
    |
    = help: Remove leading underscores
 
-RUF052.py:80:9: RUF052 Local dummy variable `_var` is accessed
-   |
-78 |         return _var
-79 | 
-80 | def fun(_var): # [RUF052]
-   |         ^^^^ RUF052
-81 |     return _var
-   |
-   = help: Remove leading underscores
-
 RUF052.py:84:5: RUF052 Local dummy variable `_list` is accessed
    |
 83 | def fun():