diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py index b99ce111081650..4f37b1d7fac4ec 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py @@ -14,7 +14,7 @@ _valid_var_3 = _valid_var_1 + _valid_var_2 def _valid_fun(): - pass + pass _valid_fun() @@ -35,7 +35,7 @@ def fun(): def fun(): global _x _x = "reassigned global" - return _x + return _x class _ValidClass: pass @@ -56,7 +56,7 @@ def _valid_method(self): def method(arg): _valid_unused_var = arg - return + return def fun(x): _ = 1 @@ -104,3 +104,28 @@ def fun(): x = "local" _x = "shadows local" # [RUF052] return _x + + +GLOBAL_1 = "global 1" +GLOBAL_1_ = "global 1 with trailing underscore" + +def unfixables(): + _GLOBAL_1 = "foo" + # unfixable because the rename would shadow a global variable + print(_GLOBAL_1) # [RUF052] + + local = "local" + local_ = "local2" + + # unfixable because the rename would shadow a local variable + _local = "local3" # [RUF052] + print(_local) + + def nested(): + _GLOBAL_1 = "foo" + # unfixable because the rename would shadow a global variable + print(_GLOBAL_1) # [RUF052] + + # unfixable because the rename would shadow a variable from the outer function + _local = "local4" + print(_local) 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 d40716dbd36c88..3be6928f8af494 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 @@ -10,16 +10,19 @@ use ruff_text_size::Ranged; use crate::{checkers::ast::Checker, renamer::Renamer}; /// ## What it does -/// Checks for accesses of local dummy variables, excluding `_` and dunder variables. +/// Checks for "dummy variables" (variables that are named as if to indicate they are unused) +/// that are in fact used. /// /// By default, "dummy variables" are any variables with names that start with leading -/// underscores. However, this is customisable using the `dummy-variable-rgx` setting). +/// underscores. However, this is customisable using the [`lint.dummy-variable-rgx`] setting). +/// +/// Dunder variables are ignored by this rule, as are variables named `_`. /// /// ## Why is this bad? /// Marking a variable with a leading underscore conveys that it is intentionally unused within the function or method. /// When these variables are later referenced in the code, it causes confusion and potential misunderstandings about -/// the code's intention. A variable marked as "unused" being subsequently used suggests oversight or unintentional use. -/// This detracts from the clarity and maintainability of the codebase. +/// the code's intention. If a variable marked as "unused" is subsequently used, it suggests that either the variable +/// could be given a clearer name, or that the code is accidentally making use of the wrong variable. /// /// Sometimes leading underscores are used to avoid variables shadowing other variables, Python builtins, or Python /// keywords. However, [PEP 8] recommends to use trailing underscores for this rather than leading underscores. @@ -39,12 +42,14 @@ use crate::{checkers::ast::Checker, renamer::Renamer}; /// ``` /// /// ## Fix availability -/// An fix is only available for variables that start with leading underscores. +/// The rule's fix is only available for variables that start with leading underscores. +/// It will also only be available if the "obvious" new name for the variable +/// would not shadow any other known variables already accessible from the scope +/// in which the variable is defined. /// /// ## Options /// - [`lint.dummy-variable-rgx`] /// -/// /// [PEP 8]: https://peps.python.org/pep-0008/ #[derive(ViolationMetadata)] pub(crate) struct UsedDummyVariable { @@ -61,21 +66,18 @@ impl Violation for UsedDummyVariable { } fn fix_title(&self) -> Option { - if let Some(shadowed_kind) = self.shadowed_kind { - return Some(match shadowed_kind { - ShadowedKind::BuiltIn => { - "Prefer using trailing underscores to avoid shadowing a built-in".to_string() - } - ShadowedKind::Keyword => { - "Prefer using trailing underscores to avoid shadowing a keyword".to_string() - } - ShadowedKind::Some => { - "Prefer using trailing underscores to avoid shadowing a variable".to_string() - } - ShadowedKind::None => "Remove leading underscores".to_string(), - }); - } - None + self.shadowed_kind.map(|kind| match kind { + ShadowedKind::BuiltIn => { + "Prefer using trailing underscores to avoid shadowing a built-in".to_string() + } + ShadowedKind::Keyword => { + "Prefer using trailing underscores to avoid shadowing a keyword".to_string() + } + ShadowedKind::Some => { + "Prefer using trailing underscores to avoid shadowing a variable".to_string() + } + ShadowedKind::None => "Remove leading underscores".to_string(), + }) } } 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 ad8e5355311b19..f81ceaccac8805 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 @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs -snapshot_kind: text --- RUF052.py:77:9: RUF052 [*] Local dummy variable `_var` is accessed | @@ -130,3 +129,44 @@ RUF052.py:105:5: RUF052 [*] Local dummy variable `_x` is accessed 106 |- return _x 105 |+ x_ = "shadows local" # [RUF052] 106 |+ return x_ +107 107 | +108 108 | +109 109 | GLOBAL_1 = "global 1" + +RUF052.py:113:5: RUF052 Local dummy variable `_GLOBAL_1` is accessed + | +112 | def unfixables(): +113 | _GLOBAL_1 = "foo" + | ^^^^^^^^^ RUF052 +114 | # unfixable because the rename would shadow a global variable +115 | print(_GLOBAL_1) # [RUF052] + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:121:5: RUF052 Local dummy variable `_local` is accessed + | +120 | # unfixable because the rename would shadow a local variable +121 | _local = "local3" # [RUF052] + | ^^^^^^ RUF052 +122 | print(_local) + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:125:9: RUF052 Local dummy variable `_GLOBAL_1` is accessed + | +124 | def nested(): +125 | _GLOBAL_1 = "foo" + | ^^^^^^^^^ RUF052 +126 | # unfixable because the rename would shadow a global variable +127 | print(_GLOBAL_1) # [RUF052] + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:130:9: RUF052 Local dummy variable `_local` is accessed + | +129 | # unfixable because the rename would shadow a variable from the outer function +130 | _local = "local4" + | ^^^^^^ RUF052 +131 | print(_local) + | + = help: Prefer using trailing underscores to avoid shadowing a variable 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 ad8e5355311b19..f81ceaccac8805 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 @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs -snapshot_kind: text --- RUF052.py:77:9: RUF052 [*] Local dummy variable `_var` is accessed | @@ -130,3 +129,44 @@ RUF052.py:105:5: RUF052 [*] Local dummy variable `_x` is accessed 106 |- return _x 105 |+ x_ = "shadows local" # [RUF052] 106 |+ return x_ +107 107 | +108 108 | +109 109 | GLOBAL_1 = "global 1" + +RUF052.py:113:5: RUF052 Local dummy variable `_GLOBAL_1` is accessed + | +112 | def unfixables(): +113 | _GLOBAL_1 = "foo" + | ^^^^^^^^^ RUF052 +114 | # unfixable because the rename would shadow a global variable +115 | print(_GLOBAL_1) # [RUF052] + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:121:5: RUF052 Local dummy variable `_local` is accessed + | +120 | # unfixable because the rename would shadow a local variable +121 | _local = "local3" # [RUF052] + | ^^^^^^ RUF052 +122 | print(_local) + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:125:9: RUF052 Local dummy variable `_GLOBAL_1` is accessed + | +124 | def nested(): +125 | _GLOBAL_1 = "foo" + | ^^^^^^^^^ RUF052 +126 | # unfixable because the rename would shadow a global variable +127 | print(_GLOBAL_1) # [RUF052] + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:130:9: RUF052 Local dummy variable `_local` is accessed + | +129 | # unfixable because the rename would shadow a variable from the outer function +130 | _local = "local4" + | ^^^^^^ RUF052 +131 | print(_local) + | + = help: Prefer using trailing underscores to avoid shadowing a variable 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 ea20bafed686ee..b519bae0f81705 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,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs -snapshot_kind: text --- RUF052.py:21:9: RUF052 Local dummy variable `arg` is accessed | @@ -38,12 +37,12 @@ RUF052.py:57:16: RUF052 Local dummy variable `arg` is accessed 57 | def method(arg): | ^^^ RUF052 58 | _valid_unused_var = arg -59 | return +59 | return | RUF052.py:61:9: RUF052 Local dummy variable `x` is accessed | -59 | return +59 | return 60 | 61 | def fun(x): | ^ RUF052 @@ -119,3 +118,41 @@ RUF052.py:105:5: RUF052 Local dummy variable `_x` is accessed 106 | return _x | = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:113:5: RUF052 Local dummy variable `_GLOBAL_1` is accessed + | +112 | def unfixables(): +113 | _GLOBAL_1 = "foo" + | ^^^^^^^^^ RUF052 +114 | # unfixable because the rename would shadow a global variable +115 | print(_GLOBAL_1) # [RUF052] + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:121:5: RUF052 Local dummy variable `_local` is accessed + | +120 | # unfixable because the rename would shadow a local variable +121 | _local = "local3" # [RUF052] + | ^^^^^^ RUF052 +122 | print(_local) + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:125:9: RUF052 Local dummy variable `_GLOBAL_1` is accessed + | +124 | def nested(): +125 | _GLOBAL_1 = "foo" + | ^^^^^^^^^ RUF052 +126 | # unfixable because the rename would shadow a global variable +127 | print(_GLOBAL_1) # [RUF052] + | + = help: Prefer using trailing underscores to avoid shadowing a variable + +RUF052.py:130:9: RUF052 Local dummy variable `_local` is accessed + | +129 | # unfixable because the rename would shadow a variable from the outer function +130 | _local = "local4" + | ^^^^^^ RUF052 +131 | print(_local) + | + = help: Prefer using trailing underscores to avoid shadowing a variable