diff --git a/resources/test/fixtures/pyflakes/F841.py b/resources/test/fixtures/pyflakes/F841.py index cef1a76a58..1dfd3a1654 100644 --- a/resources/test/fixtures/pyflakes/F841.py +++ b/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") diff --git a/src/ast/operations.rs b/src/ast/operations.rs index c69df87323..6d26596d25 100644 --- a/src/ast/operations.rs +++ b/src/ast/operations.rs @@ -94,23 +94,99 @@ pub fn in_nested_block<'a>(parents: &mut impl Iterator) -> bool }) } -/// Check if a node represents an unpacking assignment. -pub fn is_unpacking_assignment(stmt: &Stmt) -> bool { - let StmtKind::Assign { targets, value, .. } = &stmt.node else { - return false; - }; - if !targets.iter().any(|child| { - matches!( - child.node, - ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. } - ) - }) { - return false; +/// Returns `true` if `parent` contains `child`. +fn contains(parent: &Expr, child: &Expr) -> bool { + match &parent.node { + ExprKind::BoolOp { values, .. } => values.iter().any(|parent| contains(parent, child)), + ExprKind::NamedExpr { target, value } => contains(target, child) || contains(value, child), + ExprKind::BinOp { left, right, .. } => contains(left, child) || contains(right, child), + ExprKind::UnaryOp { operand, .. } => contains(operand, child), + ExprKind::Lambda { body, .. } => contains(body, child), + ExprKind::IfExp { test, body, orelse } => { + contains(test, child) || contains(body, child) || contains(orelse, child) + } + 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 = Located; diff --git a/src/check_ast.rs b/src/check_ast.rs index 4dfd5286a8..fb66ec4f92 100644 --- a/src/check_ast.rs +++ b/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 { diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F841_F841.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F841_F841.py.snap index a425c5b896..415df71798 100644 --- a/src/pyflakes/snapshots/ruff__pyflakes__tests__F841_F841.py.snap +++ b/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: ~ diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__f841_dummy_variable_rgx.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__f841_dummy_variable_rgx.snap index c4db961d75..ea98273d71 100644 --- a/src/pyflakes/snapshots/ruff__pyflakes__tests__f841_dummy_variable_rgx.snap +++ b/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: ~