diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 63f4bac476..3ccbeeb0ce 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -4452,7 +4452,23 @@ impl<'a> Checker<'a> { } _ => false, } { - let (all_names, all_names_flags) = extract_all_names(&self.ctx, parent, current); + let (all_names, all_names_flags) = { + let (mut names, flags) = + extract_all_names(parent, |name| self.ctx.is_builtin(name)); + + // Grab the existing bound __all__ values. + if let StmtKind::AugAssign { .. } = &parent.node { + if let Some(index) = current.get("__all__") { + if let BindingKind::Export(Export { names: existing }) = + &self.ctx.bindings[*index].kind + { + names.extend_from_slice(existing); + } + } + } + + (names, flags) + }; if self.settings.rules.enabled(Rule::InvalidAllFormat) { if matches!(all_names_flags, AllNamesFlags::INVALID_FORMAT) { @@ -4749,7 +4765,7 @@ impl<'a> Checker<'a> { // Mark anything referenced in `__all__` as used. let all_bindings: Option<(Vec, Range)> = { let global_scope = self.ctx.global_scope(); - let all_names: Option<(&Vec, Range)> = global_scope + let all_names: Option<(&Vec<&str>, Range)> = global_scope .get("__all__") .map(|index| &self.ctx.bindings[*index]) .and_then(|binding| match &binding.kind { @@ -4761,7 +4777,7 @@ impl<'a> Checker<'a> { ( names .iter() - .filter_map(|name| global_scope.get(name.as_str()).copied()) + .filter_map(|name| global_scope.get(name).copied()) .collect(), range, ) @@ -4779,15 +4795,13 @@ impl<'a> Checker<'a> { } // Extract `__all__` names from the global scope. - let all_names: Option<(Vec<&str>, Range)> = self + let all_names: Option<(&[&str], Range)> = self .ctx .global_scope() .get("__all__") .map(|index| &self.ctx.bindings[*index]) .and_then(|binding| match &binding.kind { - BindingKind::Export(Export { names }) => { - Some((names.iter().map(String::as_str).collect(), binding.range)) - } + BindingKind::Export(Export { names }) => Some((names.as_slice(), binding.range)), _ => None, }); @@ -4847,7 +4861,7 @@ impl<'a> Checker<'a> { .dedup() .collect(); if !sources.is_empty() { - for &name in names { + for &name in names.iter() { if !scope.defines(name) { diagnostics.push(Diagnostic::new( pyflakes::rules::UndefinedLocalWithImportStarUsage { diff --git a/crates/ruff_python_ast/src/all.rs b/crates/ruff_python_ast/src/all.rs index f61a4f3077..4855525f90 100644 --- a/crates/ruff_python_ast/src/all.rs +++ b/crates/ruff_python_ast/src/all.rs @@ -1,10 +1,6 @@ use bitflags::bitflags; use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind}; -use crate::binding::{BindingKind, Export}; -use crate::context::Context; -use crate::scope::Scope; - bitflags! { #[derive(Default)] pub struct AllNamesFlags: u32 { @@ -14,29 +10,30 @@ bitflags! { } /// Extract the names bound to a given __all__ assignment. -pub fn extract_all_names( - ctx: &Context, - stmt: &Stmt, - scope: &Scope, -) -> (Vec, AllNamesFlags) { - fn add_to_names(names: &mut Vec, elts: &[Expr], flags: &mut AllNamesFlags) { +/// +/// Accepts a closure that determines whether a given name (e.g., `"list"`) is a Python builtin. +pub fn extract_all_names(stmt: &Stmt, is_builtin: F) -> (Vec<&str>, AllNamesFlags) +where + F: Fn(&str) -> bool, +{ + fn add_to_names<'a>(elts: &'a [Expr], names: &mut Vec<&'a str>, flags: &mut AllNamesFlags) { for elt in elts { if let ExprKind::Constant { value: Constant::Str(value), .. } = &elt.node { - names.push(value.to_string()); + names.push(value); } else { *flags |= AllNamesFlags::INVALID_OBJECT; } } } - fn extract_elts<'a>( - ctx: &'a Context, - expr: &'a Expr, - ) -> (Option<&'a Vec>, AllNamesFlags) { + fn extract_elts(expr: &Expr, is_builtin: F) -> (Option<&Vec>, AllNamesFlags) + where + F: Fn(&str) -> bool, + { match &expr.node { ExprKind::List { elts, .. } => { return (Some(elts), AllNamesFlags::empty()); @@ -56,27 +53,28 @@ pub fn extract_all_names( } => { // Allow `tuple()` and `list()` calls. if keywords.is_empty() && args.len() <= 1 { - if ctx.resolve_call_path(func).map_or(false, |call_path| { - call_path.as_slice() == ["", "tuple"] - || call_path.as_slice() == ["", "list"] - }) { - if args.is_empty() { - return (None, AllNamesFlags::empty()); - } - match &args[0].node { - ExprKind::List { elts, .. } - | ExprKind::Set { elts, .. } - | ExprKind::Tuple { elts, .. } => { - return (Some(elts), AllNamesFlags::empty()); + if let ExprKind::Name { id, .. } = &func.node { + if id == "tuple" || id == "list" { + if is_builtin(id) { + if args.is_empty() { + return (None, AllNamesFlags::empty()); + } + match &args[0].node { + ExprKind::List { elts, .. } + | ExprKind::Set { elts, .. } + | ExprKind::Tuple { elts, .. } => { + return (Some(elts), AllNamesFlags::empty()); + } + ExprKind::ListComp { .. } + | ExprKind::SetComp { .. } + | ExprKind::GeneratorExp { .. } => { + // Allow comprehensions, even though we can't statically analyze + // them. + return (None, AllNamesFlags::empty()); + } + _ => {} + } } - ExprKind::ListComp { .. } - | ExprKind::SetComp { .. } - | ExprKind::GeneratorExp { .. } => { - // Allow comprehensions, even though we can't statically analyze - // them. - return (None, AllNamesFlags::empty()); - } - _ => {} } } } @@ -86,18 +84,9 @@ pub fn extract_all_names( (None, AllNamesFlags::INVALID_FORMAT) } - let mut names: Vec = vec![]; + let mut names: Vec<&str> = vec![]; let mut flags = AllNamesFlags::empty(); - // Grab the existing bound __all__ values. - if let StmtKind::AugAssign { .. } = &stmt.node { - if let Some(index) = scope.get("__all__") { - if let BindingKind::Export(Export { names: existing }) = &ctx.bindings[*index].kind { - names.extend_from_slice(existing); - } - } - } - if let Some(value) = match &stmt.node { StmtKind::Assign { value, .. } => Some(value), StmtKind::AnnAssign { value, .. } => value.as_ref(), @@ -109,10 +98,10 @@ pub fn extract_all_names( let mut current_right = right; loop { // Process the right side, which should be a "real" value. - let (elts, new_flags) = extract_elts(ctx, current_right); + let (elts, new_flags) = extract_elts(current_right, |expr| is_builtin(expr)); flags |= new_flags; if let Some(elts) = elts { - add_to_names(&mut names, elts, &mut flags); + add_to_names(elts, &mut names, &mut flags); } // Process the left side, which can be a "real" value or the "rest" of the @@ -121,19 +110,19 @@ pub fn extract_all_names( current_left = left; current_right = right; } else { - let (elts, new_flags) = extract_elts(ctx, current_left); + let (elts, new_flags) = extract_elts(current_left, |expr| is_builtin(expr)); flags |= new_flags; if let Some(elts) = elts { - add_to_names(&mut names, elts, &mut flags); + add_to_names(elts, &mut names, &mut flags); } break; } } } else { - let (elts, new_flags) = extract_elts(ctx, value); + let (elts, new_flags) = extract_elts(value, |expr| is_builtin(expr)); flags |= new_flags; if let Some(elts) = elts { - add_to_names(&mut names, elts, &mut flags); + add_to_names(elts, &mut names, &mut flags); } } } diff --git a/crates/ruff_python_ast/src/binding.rs b/crates/ruff_python_ast/src/binding.rs index 95b4c0c29e..c977c689cb 100644 --- a/crates/ruff_python_ast/src/binding.rs +++ b/crates/ruff_python_ast/src/binding.rs @@ -207,9 +207,9 @@ pub struct StarImportation<'a> { // FutureImportation #[derive(Clone, Debug)] -pub struct Export { +pub struct Export<'a> { /// The names of the bindings exported via `__all__`. - pub names: Vec, + pub names: Vec<&'a str>, } #[derive(Clone, Debug)] @@ -258,7 +258,7 @@ pub enum BindingKind<'a> { Builtin, ClassDefinition, FunctionDefinition, - Export(Export), + Export(Export<'a>), FutureImportation, Importation(Importation<'a>), FromImportation(FromImportation<'a>),