Re-support F841 detection for single context managers (#1099)

resources/test/fixtures/pyflakes/F841.py

@@ -65,3 +65,8 @@ def f7():
 
     with connect() as (connection, cursor):
         cursor.execute("SELECT * FROM users")
+
+
+def f8():
+    with open("file") as f, open("") as ((a, b)):
+        print("hello")

src/ast/operations.rs

@@ -94,23 +94,99 @@ pub fn in_nested_block<'a>(parents: &mut impl Iterator<Item = &'a Stmt>) -> bool
     })
 }
 
-/// Check if a node represents an unpacking assignment.
+/// Returns `true` if `parent` contains `child`.
-pub fn is_unpacking_assignment(stmt: &Stmt) -> bool {
+fn contains(parent: &Expr, child: &Expr) -> bool {
-    let StmtKind::Assign { targets, value, .. } = &stmt.node else {
+    match &parent.node {
-        return false;
+        ExprKind::BoolOp { values, .. } => values.iter().any(|parent| contains(parent, child)),
-    };
+        ExprKind::NamedExpr { target, value } => contains(target, child) || contains(value, child),
-    if !targets.iter().any(|child| {
+        ExprKind::BinOp { left, right, .. } => contains(left, child) || contains(right, child),
-        matches!(
+        ExprKind::UnaryOp { operand, .. } => contains(operand, child),
-            child.node,
+        ExprKind::Lambda { body, .. } => contains(body, child),
-            ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. }
+        ExprKind::IfExp { test, body, orelse } => {
-        )
+            contains(test, child) || contains(body, child) || contains(orelse, child)
-    }) {
+        }
-        return false;
+        ExprKind::Dict { keys, values } => keys
+            .iter()
+            .chain(values.iter())
+            .any(|parent| contains(parent, child)),
+        ExprKind::Set { elts } => elts.iter().any(|parent| contains(parent, child)),
+        ExprKind::ListComp { elt, .. } => contains(elt, child),
+        ExprKind::SetComp { elt, .. } => contains(elt, child),
+        ExprKind::DictComp { key, value, .. } => contains(key, child) || contains(value, child),
+        ExprKind::GeneratorExp { elt, .. } => contains(elt, child),
+        ExprKind::Await { value } => contains(value, child),
+        ExprKind::Yield { value } => value.as_ref().map_or(false, |value| contains(value, child)),
+        ExprKind::YieldFrom { value } => contains(value, child),
+        ExprKind::Compare {
+            left, comparators, ..
+        } => contains(left, child) || comparators.iter().any(|parent| contains(parent, child)),
+        ExprKind::Call {
+            func,
+            args,
+            keywords,
+        } => {
+            contains(func, child)
+                || args.iter().any(|parent| contains(parent, child))
+                || keywords
+                    .iter()
+                    .any(|keyword| contains(&keyword.node.value, child))
+        }
+        ExprKind::FormattedValue {
+            value, format_spec, ..
+        } => {
+            contains(value, child)
+                || format_spec
+                    .as_ref()
+                    .map_or(false, |value| contains(value, child))
+        }
+        ExprKind::JoinedStr { values } => values.iter().any(|parent| contains(parent, child)),
+        ExprKind::Constant { .. } => false,
+        ExprKind::Attribute { value, .. } => contains(value, child),
+        ExprKind::Subscript { value, slice, .. } => {
+            contains(value, child) || contains(slice, child)
+        }
+        ExprKind::Starred { value, .. } => contains(value, child),
+        ExprKind::Name { .. } => parent == child,
+        ExprKind::List { elts, .. } => elts.iter().any(|parent| contains(parent, child)),
+        ExprKind::Tuple { elts, .. } => elts.iter().any(|parent| contains(parent, child)),
+        ExprKind::Slice { lower, upper, step } => {
+            lower.as_ref().map_or(false, |value| contains(value, child))
+                || upper.as_ref().map_or(false, |value| contains(value, child))
+                || step.as_ref().map_or(false, |value| contains(value, child))
+        }
+    }
+}
+
+/// Check if a node represents an unpacking assignment.
+pub fn is_unpacking_assignment(parent: &Stmt, child: &Expr) -> bool {
+    match &parent.node {
+        StmtKind::With { items, .. } => items.iter().any(|item| {
+            if let Some(optional_vars) = &item.optional_vars {
+                if matches!(optional_vars.node, ExprKind::Tuple { .. }) {
+                    if contains(optional_vars, child) {
+                        return true;
+                    }
+                }
+            }
+            false
+        }),
+        StmtKind::Assign { targets, value, .. } => {
+            // TODO(charlie): Match based on `child`.
+            if !targets.iter().any(|child| {
+                matches!(
+                    child.node,
+                    ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. }
+                )
+            }) {
+                return false;
+            }
+            !matches!(
+                &value.node,
+                ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. }
+            )
+        }
+        _ => false,
     }
-    !matches!(
-        &value.node,
-        ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. }
-    )
 }
 
 pub type LocatedCmpop<U = ()> = Located<Cmpop, U>;

src/check_ast.rs

@@ -5,7 +5,6 @@ use std::path::Path;
 
 use log::error;
 use rustc_hash::{FxHashMap, FxHashSet};
-use rustpython_ast::Withitem;
 use rustpython_parser::ast::{
     Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind,
     KeywordData, Operator, Stmt, StmtKind, Suite,
@@ -20,7 +19,7 @@ use crate::ast::relocate::relocate_expr;
 use crate::ast::types::{
     Binding, BindingContext, BindingKind, ClassScope, FunctionScope, Node, Range, Scope, ScopeKind,
 };
-use crate::ast::visitor::{walk_excepthandler, walk_withitem, Visitor};
+use crate::ast::visitor::{walk_excepthandler, Visitor};
 use crate::ast::{helpers, operations, visitor};
 use crate::checks::{Check, CheckCode, CheckKind, DeferralKeyword};
 use crate::docstrings::definition::{Definition, DefinitionKind, Documentable};
@@ -82,7 +81,6 @@ pub struct Checker<'a> {
     in_deferred_type_definition: bool,
     in_literal: bool,
     in_subscript: bool,
-    in_withitem: bool,
     seen_import_boundary: bool,
     futures_allowed: bool,
     annotations_future_enabled: bool,
@@ -130,7 +128,6 @@ impl<'a> Checker<'a> {
             in_deferred_type_definition: false,
             in_literal: false,
             in_subscript: false,
-            in_withitem: false,
             seen_import_boundary: false,
             futures_allowed: true,
             annotations_future_enabled: false,
@@ -2485,13 +2482,6 @@ where
 
         self.check_builtin_arg_shadowing(&arg.node.arg, Range::from_located(arg));
     }
-
-    fn visit_withitem(&mut self, withitem: &'b Withitem) {
-        let prev_in_withitem = self.in_withitem;
-        self.in_withitem = true;
-        walk_withitem(self, withitem);
-        self.in_withitem = prev_in_withitem;
-    }
 }
 
 fn try_mark_used(scope: &mut Scope, scope_id: usize, id: &str, expr: &Expr) -> bool {
@@ -2798,7 +2788,7 @@ impl<'a> Checker<'a> {
             return;
         }
 
-        if self.in_withitem || operations::is_unpacking_assignment(parent) {
+        if operations::is_unpacking_assignment(parent, expr) {
             self.add_binding(
                 id,
                 Binding {

src/pyflakes/snapshots/ruff__pyflakes__tests__F841_F841.py.snap

@@ -56,4 +56,13 @@ expression: checks
     row: 51
     column: 9
   fix: ~
+- kind:
+    UnusedVariable: f
+  location:
+    row: 71
+    column: 25
+  end_location:
+    row: 71
+    column: 26
+  fix: ~
 

src/pyflakes/snapshots/ruff__pyflakes__tests__f841_dummy_variable_rgx.snap

@@ -74,4 +74,13 @@ expression: checks
     row: 51
     column: 9
   fix: ~
+- kind:
+    UnusedVariable: f
+  location:
+    row: 71
+    column: 25
+  end_location:
+    row: 71
+    column: 26
+  fix: ~