Respect parent-scoping rules for NamedExpr assignments (#4145)

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)

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)

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 {

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__"

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`
+
 

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`
+
 

crates/ruff_python_semantic/src/binding.rs

@@ -253,6 +253,7 @@ pub enum BindingKind<'a> {
     Annotation,
     Argument,
     Assignment,
+    NamedExprAssignment,
     Binding,
     LoopVar,
     Global,