Use references for Export binding type (#3853)

This commit is contained in:
Charlie Marsh 2023-04-03 11:26:42 -04:00 committed by GitHub
parent 924bebbb4a
commit 25771cd4b9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 65 additions and 62 deletions

View file

@ -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<BindingId>, Range)> = {
let global_scope = self.ctx.global_scope();
let all_names: Option<(&Vec<String>, 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 {

View file

@ -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<String>, AllNamesFlags) {
fn add_to_names(names: &mut Vec<String>, 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<F>(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<Expr>>, AllNamesFlags) {
fn extract_elts<F>(expr: &Expr, is_builtin: F) -> (Option<&Vec<Expr>>, 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<String> = 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);
}
}
}

View file

@ -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<String>,
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>),