From f9364dda690f5df12c29682eab1eefdba8937941 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Fri, 6 Dec 2024 13:46:50 +0000 Subject: [PATCH] [`ruff`] Dont emit RUF052 on function parameters --- .../resources/test/fixtures/ruff/RUF052.py | 2 +- .../rules/ruff/rules/used_dummy_variable.rs | 26 ++++++-- ..._rules__ruff__tests__RUF052_RUF052.py.snap | 24 +------- ...var_regexp_preset__RUF052_RUF052.py_1.snap | 24 +------- ...var_regexp_preset__RUF052_RUF052.py_2.snap | 59 ------------------- 5 files changed, 23 insertions(+), 112 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py index 4f37b1d7fac4e..dd90a2bfe82d5 100644 --- 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 index 3be6928f8af49..4147badc30b9f 100644 --- 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 . + // + // Moreover, autofixing the diagnostic for function parameters is much more troublesome than + // autofixing the diagnostic for assignments. See: + // - + // - + 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/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF052_RUF052.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF052_RUF052.py.snap index f81ceaccac880..eef08f0aeb33b 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF052_RUF052.py.snap +++ b/crates/ruff_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/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_1.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_1.snap index f81ceaccac880..eef08f0aeb33b 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_1.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/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/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_2.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_2.snap index b519bae0f8170..dd918e2ea1e72 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__custom_dummy_var_regexp_preset__RUF052_RUF052.py_2.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/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():