Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ruff] Omit diagnostic for shadowed private function parameters in used-dummy-variable (RUF052) #15376

Merged
merged 4 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF052.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
5 changes: 3 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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);
}
}
Expand Down
23 changes: 21 additions & 2 deletions crates/ruff_linter/src/rules/ruff/rules/used_dummy_variable.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
|
Expand All @@ -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
Loading