From 64b7280eb824d0e5f9da887e82dcda53838dd38d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 29 Apr 2023 18:45:30 -0400 Subject: [PATCH] Respect parent-scoping rules for `NamedExpr` assignments (#4145) --- .../test/fixtures/pyflakes/F821_0.py | 5 ++ .../test/fixtures/pyflakes/F841_0.py | 5 ++ crates/ruff/src/checkers/ast/mod.rs | 53 ++++++++++++++++--- .../rules/pyflakes/rules/unused_variable.rs | 2 +- ...ules__pyflakes__tests__F841_F841_0.py.snap | 9 ++++ ...lakes__tests__f841_dummy_variable_rgx.snap | 9 ++++ crates/ruff_python_semantic/src/binding.rs | 1 + 7 files changed, 76 insertions(+), 8 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F821_0.py b/crates/ruff/resources/test/fixtures/pyflakes/F821_0.py index 03b5560638..82c098db1c 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F821_0.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F821_0.py @@ -132,3 +132,8 @@ def in_ipython_notebook() -> bool: except NameError: return False # not in notebook return True + + +def named_expr(): + if any((key := (value := x)) for x in ["ok"]): + print(key) diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F841_0.py b/crates/ruff/resources/test/fixtures/pyflakes/F841_0.py index 90c44f761c..c0e33502d6 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F841_0.py +++ b/crates/ruff/resources/test/fixtures/pyflakes/F841_0.py @@ -121,3 +121,8 @@ def f(x: int): print("A") case y: pass + + +def f(): + if any((key := (value := x)) for x in ["ok"]): + print(key) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 8594d58dd8..7b348e1488 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -4266,7 +4266,20 @@ impl<'a> Checker<'a> { } } - let scope = self.ctx.scope(); + // Per [PEP 572](https://peps.python.org/pep-0572/#scope-of-the-target), named + // expressions in generators and comprehensions bind to the scope that contains the + // outermost comprehension. + let scope_id = if binding.kind.is_named_expr_assignment() { + self.ctx + .scopes + .ancestor_ids(self.ctx.scope_id) + .find_or_last(|scope_id| !self.ctx.scopes[*scope_id].kind.is_generator()) + .unwrap_or(self.ctx.scope_id) + } else { + self.ctx.scope_id + }; + let scope = &mut self.ctx.scopes[scope_id]; + let binding = if let Some(index) = scope.get(name) { let existing = &self.ctx.bindings[*index]; match &existing.kind { @@ -4298,11 +4311,15 @@ impl<'a> Checker<'a> { // Don't treat annotations as assignments if there is an existing value // in scope. - let scope = self.ctx.scope_mut(); - if !(binding.kind.is_annotation() && scope.defines(name)) { - scope.add(name, binding_id); + if binding.kind.is_annotation() && scope.defines(name) { + self.ctx.bindings.push(binding); + return; } + // Add the binding to the scope. + scope.add(name, binding_id); + + // Add the binding to the arena. self.ctx.bindings.push(binding); } @@ -4579,9 +4596,10 @@ impl<'a> Checker<'a> { return; } - let current = self.ctx.scope(); + let scope = self.ctx.scope(); + if id == "__all__" - && matches!(current.kind, ScopeKind::Module) + && scope.kind.is_module() && matches!( parent.node, StmtKind::Assign { .. } | StmtKind::AugAssign { .. } | StmtKind::AnnAssign { .. } @@ -4619,7 +4637,7 @@ impl<'a> Checker<'a> { // Grab the existing bound __all__ values. if let StmtKind::AugAssign { .. } = &parent.node { - if let Some(index) = current.get("__all__") { + if let Some(index) = scope.get("__all__") { if let BindingKind::Export(Export { names: existing }) = &self.ctx.bindings[*index].kind { @@ -4662,6 +4680,27 @@ impl<'a> Checker<'a> { } } + if self + .ctx + .expr_ancestors() + .any(|expr| matches!(expr.node, ExprKind::NamedExpr { .. })) + { + self.add_binding( + id, + Binding { + kind: BindingKind::NamedExprAssignment, + runtime_usage: None, + synthetic_usage: None, + typing_usage: None, + range: expr.range(), + source: Some(*self.ctx.current_stmt()), + context: self.ctx.execution_context(), + exceptions: self.ctx.exceptions(), + }, + ); + return; + } + self.add_binding( id, Binding { diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs index c4f0024f65..2868b5112e 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs +++ b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs @@ -322,7 +322,7 @@ pub fn unused_variable(checker: &mut Checker, scope: ScopeId) { .map(|(name, index)| (name, &checker.ctx.bindings[*index])) { if !binding.used() - && binding.kind.is_assignment() + && (binding.kind.is_assignment() || binding.kind.is_named_expr_assignment()) && !checker.settings.dummy_variable_rgx.is_match(name) && name != &"__tracebackhide__" && name != &"__traceback_info__" diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_0.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_0.py.snap index a3ed914c1e..95da0702b4 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_0.py.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_0.py.snap @@ -210,4 +210,13 @@ F841_0.py:122:14: F841 [*] Local variable `y` is assigned to but never used | = help: Remove assignment to unused variable `y` +F841_0.py:127:21: F841 [*] Local variable `value` is assigned to but never used + | +127 | def f(): +128 | if any((key := (value := x)) for x in ["ok"]): + | ^^^^^ F841 +129 | print(key) + | + = help: Remove assignment to unused variable `value` + diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__f841_dummy_variable_rgx.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__f841_dummy_variable_rgx.snap index cb7da80550..55ad23a760 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__f841_dummy_variable_rgx.snap +++ b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__f841_dummy_variable_rgx.snap @@ -248,4 +248,13 @@ F841_0.py:122:14: F841 [*] Local variable `y` is assigned to but never used | = help: Remove assignment to unused variable `y` +F841_0.py:127:21: F841 [*] Local variable `value` is assigned to but never used + | +127 | def f(): +128 | if any((key := (value := x)) for x in ["ok"]): + | ^^^^^ F841 +129 | print(key) + | + = help: Remove assignment to unused variable `value` + diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index f531ff4f79..9c50cfd132 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -253,6 +253,7 @@ pub enum BindingKind<'a> { Annotation, Argument, Assignment, + NamedExprAssignment, Binding, LoopVar, Global,