From 4761b25712caac664d0f6e5b793dea343bc3e323 Mon Sep 17 00:00:00 2001 From: ukyen Date: Fri, 21 Jun 2024 10:18:18 +0100 Subject: [PATCH 1/2] fix(F811): assignment can also be shadowed binding --- .../resources/test/fixtures/pyflakes/F811_30.py | 13 +++++++++++++ .../src/checkers/ast/analyze/deferred_scopes.rs | 7 +++++++ crates/ruff_linter/src/rules/pyflakes/mod.rs | 1 + ...er__rules__pyflakes__tests__F811_F811_30.py.snap | 12 ++++++++++++ crates/ruff_python_semantic/src/binding.rs | 5 +++-- 5 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py new file mode 100644 index 0000000000000..8125d75ef19ca --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py @@ -0,0 +1,13 @@ +"""Regression test for: https://github.com/astral-sh/ruff/issues/11828""" + + +class A: + """A.""" + + def foo(self) -> None: + """Foo.""" + + bar = foo + + def bar(self) -> None: + """Bar.""" diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index c4d0ee7944b78..a66dac8a7f71b 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -227,6 +227,13 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { continue; }; + // Ignore non-import redefinitions + if matches!(shadowed.kind, BindingKind::Assignment) + && matches!(binding.kind, BindingKind::Assignment) + { + continue; + } + // If this is an overloaded function, abort. if shadowed.kind.is_function_definition() { if checker diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index a700b7ed6c498..f2b1e6d114e1b 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -125,6 +125,7 @@ mod tests { #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_27.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_28.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_29.pyi"))] + #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_30.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_0.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_1.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_2.py"))] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap new file mode 100644 index 0000000000000..0ecd338c14c92 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap @@ -0,0 +1,12 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F811_30.py:12:9: F811 Redefinition of unused `bar` from line 10 + | +10 | bar = foo +11 | +12 | def bar(self) -> None: + | ^^^ F811 +13 | """Bar.""" + | + = help: Remove definition: `bar` diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 22ee07490c826..3573f4a4ad140 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -179,14 +179,15 @@ impl<'a> Binding<'a> { } _ => {} } - // Otherwise, the shadowed binding must be a class definition, function definition, or - // import to be considered a redefinition. + // Otherwise, the shadowed binding must be a class definition, function definition, + // import, or assignment to be considered a redefinition. matches!( existing.kind, BindingKind::ClassDefinition(_) | BindingKind::FunctionDefinition(_) | BindingKind::Import(_) | BindingKind::FromImport(_) + | BindingKind::Assignment ) } From 21ff0c6ad561a4e226484cba3e83ef7c10669f3d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 23 Jun 2024 13:13:08 -0400 Subject: [PATCH 2/2] Add more tests; move into redefines --- .../test/fixtures/pyflakes/F811_30.py | 24 +++++++++++++++++++ .../checkers/ast/analyze/deferred_scopes.rs | 7 ------ ...les__pyflakes__tests__F811_F811_30.py.snap | 18 ++++++++++++++ crates/ruff_python_semantic/src/binding.rs | 14 +++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py index 8125d75ef19ca..8c2b8c0e23140 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_30.py @@ -11,3 +11,27 @@ def foo(self) -> None: def bar(self) -> None: """Bar.""" + + +class B: + """B.""" + def baz(self) -> None: + """Baz.""" + + baz = 1 + + +class C: + """C.""" + def foo(self) -> None: + """Foo.""" + + bar = (foo := 1) + + +class D: + """D.""" + foo = 1 + foo = 2 + bar = (foo := 3) + bar = (foo := 4) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index a66dac8a7f71b..c4d0ee7944b78 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -227,13 +227,6 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { continue; }; - // Ignore non-import redefinitions - if matches!(shadowed.kind, BindingKind::Assignment) - && matches!(binding.kind, BindingKind::Assignment) - { - continue; - } - // If this is an overloaded function, abort. if shadowed.kind.is_function_definition() { if checker diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap index 0ecd338c14c92..e785583128b93 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_30.py.snap @@ -10,3 +10,21 @@ F811_30.py:12:9: F811 Redefinition of unused `bar` from line 10 13 | """Bar.""" | = help: Remove definition: `bar` + +F811_30.py:21:5: F811 Redefinition of unused `baz` from line 18 + | +19 | """Baz.""" +20 | +21 | baz = 1 + | ^^^ F811 + | + = help: Remove definition: `baz` + +F811_30.py:29:12: F811 Redefinition of unused `foo` from line 26 + | +27 | """Foo.""" +28 | +29 | bar = (foo := 1) + | ^^^ F811 + | + = help: Remove definition: `foo` diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 3573f4a4ad140..a4eb2340a4b28 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -177,6 +177,19 @@ impl<'a> Binding<'a> { | BindingKind::Builtin => { return false; } + // Assignment-assignment bindings are not considered redefinitions, as in: + // ```python + // x = 1 + // x = 2 + // ``` + BindingKind::Assignment | BindingKind::NamedExprAssignment => { + if matches!( + existing.kind, + BindingKind::Assignment | BindingKind::NamedExprAssignment + ) { + return false; + } + } _ => {} } // Otherwise, the shadowed binding must be a class definition, function definition, @@ -188,6 +201,7 @@ impl<'a> Binding<'a> { | BindingKind::Import(_) | BindingKind::FromImport(_) | BindingKind::Assignment + | BindingKind::NamedExprAssignment ) }