Track star imports on Scope directly (#3822)

crates/ruff/src/checkers/ast/mod.rs

@@ -1194,22 +1194,10 @@ where
                             ));
                         }
                     } else if alias.node.name == "*" {
-                        self.add_binding(
+                        self.ctx.scope_mut().add_star_import(StarImportation {
-                            "*",
+                            module: module.as_ref().map(String::as_str),
-                            Binding {
+                            level: *level,
-                                kind: BindingKind::StarImportation(StarImportation {
+                        });
-                                    level: *level,
-                                    module: module.clone(),
-                                }),
-                                runtime_usage: None,
-                                synthetic_usage: None,
-                                typing_usage: None,
-                                range: Range::from(stmt),
-                                source: Some(*self.ctx.current_stmt()),
-                                context: self.ctx.execution_context(),
-                                exceptions: self.ctx.exceptions(),
-                            },
-                        );
 
                         if self
                             .settings
@@ -1242,9 +1230,6 @@ where
                                 Range::from(stmt),
                             ));
                         }
-
-                        let scope = self.ctx.scope_mut();
-                        scope.import_starred = true;
                     } else {
                         if let Some(asname) = &alias.node.asname {
                             self.check_builtin_shadowing(asname, stmt, false);
@@ -4045,7 +4030,6 @@ impl<'a> Checker<'a> {
                     BindingKind::Importation(..)
                         | BindingKind::FromImportation(..)
                         | BindingKind::SubmoduleImportation(..)
-                        | BindingKind::StarImportation(..)
                         | BindingKind::FutureImportation
                 );
                 if binding.kind.is_loop_var() && existing_is_import {
@@ -4252,32 +4236,31 @@ impl<'a> Checker<'a> {
 
             first_iter = false;
             in_generator = matches!(scope.kind, ScopeKind::Generator);
-            import_starred = import_starred || scope.import_starred;
+            import_starred = import_starred || scope.uses_star_imports();
         }
 
         if import_starred {
+            // F405
             if self
                 .settings
                 .rules
                 .enabled(Rule::UndefinedLocalWithImportStarUsage)
             {
-                let mut from_list = vec![];
+                let sources: Vec<String> = self
-                for scope_index in self.ctx.scope_stack.iter() {
+                    .ctx
-                    let scope = &self.ctx.scopes[*scope_index];
+                    .scopes
-                    for binding in scope.binding_ids().map(|index| &self.ctx.bindings[*index]) {
+                    .iter()
-                        if let BindingKind::StarImportation(StarImportation { level, module }) =
+                    .flat_map(Scope::star_imports)
-                            &binding.kind
+                    .map(|StarImportation { level, module }| {
-                        {
+                        helpers::format_import_from(*level, *module)
-                            from_list.push(helpers::format_import_from(*level, module.as_deref()));
+                    })
-                        }
+                    .sorted()
-                    }
+                    .dedup()
-                }
+                    .collect();
-                from_list.sort();
-
                 self.diagnostics.push(Diagnostic::new(
                     pyflakes::rules::UndefinedLocalWithImportStarUsage {
                         name: id.to_string(),
-                        sources: from_list,
+                        sources,
                     },
                     Range::from(expr),
                 ));
@@ -4755,7 +4738,6 @@ 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
@@ -4829,13 +4811,45 @@ impl<'a> Checker<'a> {
         for (index, stack) in self.ctx.dead_scopes.iter().rev() {
             let scope = &self.ctx.scopes[*index];
 
-            // F822
             if index.is_global() {
+                // F822
                 if self.settings.rules.enabled(Rule::UndefinedExport) {
+                    if !self.path.ends_with("__init__.py") {
+                        if let Some((names, range)) = &all_names {
+                            diagnostics
+                                .extend(pyflakes::rules::undefined_export(names, range, scope));
+                        }
+                    }
+                }
+
+                // F405
+                if self
+                    .settings
+                    .rules
+                    .enabled(Rule::UndefinedLocalWithImportStarUsage)
+                {
                     if let Some((names, range)) = &all_names {
-                        diagnostics.extend(pyflakes::rules::undefined_export(
+                        let sources: Vec<String> = scope
-                            names, range, self.path, scope,
+                            .star_imports()
-                        ));
+                            .map(|StarImportation { level, module }| {
+                                helpers::format_import_from(*level, *module)
+                            })
+                            .sorted()
+                            .dedup()
+                            .collect();
+                        if !sources.is_empty() {
+                            for &name in names {
+                                if !scope.defines(name) {
+                                    diagnostics.push(Diagnostic::new(
+                                        pyflakes::rules::UndefinedLocalWithImportStarUsage {
+                                            name: name.to_string(),
+                                            sources: sources.clone(),
+                                        },
+                                        *range,
+                                    ));
+                                }
+                            }
+                        }
                     }
                 }
             }
@@ -4876,7 +4890,6 @@ impl<'a> Checker<'a> {
                         BindingKind::Importation(..)
                             | BindingKind::FromImportation(..)
                             | BindingKind::SubmoduleImportation(..)
-                            | BindingKind::StarImportation(..)
                             | BindingKind::FutureImportation
                     ) {
                         if binding.used() {
@@ -4907,39 +4920,6 @@ impl<'a> Checker<'a> {
                 }
             }
 
-            if self
-                .settings
-                .rules
-                .enabled(Rule::UndefinedLocalWithImportStarUsage)
-            {
-                if scope.import_starred {
-                    if let Some((names, range)) = &all_names {
-                        let mut from_list = vec![];
-                        for binding in scope.binding_ids().map(|index| &self.ctx.bindings[*index]) {
-                            if let BindingKind::StarImportation(StarImportation { level, module }) =
-                                &binding.kind
-                            {
-                                from_list
-                                    .push(helpers::format_import_from(*level, module.as_deref()));
-                            }
-                        }
-                        from_list.sort();
-
-                        for &name in names {
-                            if !scope.defines(name) {
-                                diagnostics.push(Diagnostic::new(
-                                    pyflakes::rules::UndefinedLocalWithImportStarUsage {
-                                        name: name.to_string(),
-                                        sources: from_list.clone(),
-                                    },
-                                    *range,
-                                ));
-                            }
-                        }
-                    }
-                }
-            }
-
             if enforce_typing_imports {
                 let runtime_imports: Vec<&Binding> = if self.settings.flake8_type_checking.strict {
                     vec![]

crates/ruff/src/rules/pandas_vet/rules/check_attr.rs

@@ -82,7 +82,6 @@ pub fn check_attr(checker: &mut Checker, attr: &str, value: &Expr, attr_expr: &E
                     | BindingKind::FunctionDefinition
                     | BindingKind::Export(..)
                     | BindingKind::FutureImportation
-                    | BindingKind::StarImportation(..)
                     | BindingKind::Importation(..)
                     | BindingKind::FromImportation(..)
                     | BindingKind::SubmoduleImportation(..)

crates/ruff/src/rules/pandas_vet/rules/check_call.rs

@@ -99,7 +99,6 @@ pub fn check_call(checker: &mut Checker, func: &Expr) {
                         | BindingKind::FunctionDefinition
                         | BindingKind::Export(..)
                         | BindingKind::FutureImportation
-                        | BindingKind::StarImportation(..)
                         | BindingKind::Importation(..)
                         | BindingKind::FromImportation(..)
                         | BindingKind::SubmoduleImportation(..)

crates/ruff/src/rules/pyflakes/rules/undefined_export.rs

@@ -1,5 +1,3 @@
-use std::path::Path;
-
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::scope::Scope;
@@ -19,14 +17,9 @@ impl Violation for UndefinedExport {
 }
 
 /// F822
-pub fn undefined_export(
+pub fn undefined_export(names: &[&str], range: &Range, scope: &Scope) -> Vec<Diagnostic> {
-    names: &[&str],
-    range: &Range,
-    path: &Path,
-    scope: &Scope,
-) -> Vec<Diagnostic> {
     let mut diagnostics = Vec::new();
-    if !scope.import_starred && !path.ends_with("__init__.py") {
+    if !scope.uses_star_imports() {
         for name in names {
             if !scope.defines(name) {
                 diagnostics.push(Diagnostic::new(

crates/ruff_python_ast/src/scope.rs

@@ -9,8 +9,10 @@ use std::ops::{Deref, Index, IndexMut};
 pub struct Scope<'a> {
     pub id: ScopeId,
     pub kind: ScopeKind<'a>,
-    pub import_starred: bool,
     pub uses_locals: bool,
+    /// A list of star imports in this scope. These represent _module_ imports (e.g., `sys` in
+    /// `from sys import *`), rather than individual bindings (e.g., individual members in `sys`).
+    star_imports: Vec<StarImportation<'a>>,
     /// A map from bound name to binding index, for live bindings.
     bindings: FxHashMap<&'a str, BindingId>,
     /// A map from bound name to binding index, for bindings that were created
@@ -28,8 +30,8 @@ impl<'a> Scope<'a> {
         Scope {
             id,
             kind,
-            import_starred: false,
             uses_locals: false,
+            star_imports: Vec::default(),
             bindings: FxHashMap::default(),
             rebounds: FxHashMap::default(),
         }
@@ -60,9 +62,25 @@ impl<'a> Scope<'a> {
         self.bindings.values()
     }
 
+    /// Returns a tuple of the name and id of all bindings defined in this scope.
     pub fn bindings(&self) -> std::collections::hash_map::Iter<&'a str, BindingId> {
         self.bindings.iter()
     }
+
+    /// Adds a reference to a star import (e.g., `from sys import *`) to this scope.
+    pub fn add_star_import(&mut self, import: StarImportation<'a>) {
+        self.star_imports.push(import);
+    }
+
+    /// Returns `true` if this scope contains a star import (e.g., `from sys import *`).
+    pub fn uses_star_imports(&self) -> bool {
+        !self.star_imports.is_empty()
+    }
+
+    /// Returns an iterator over all star imports (e.g., `from sys import *`) in this scope.
+    pub fn star_imports(&self) -> impl Iterator<Item = &StarImportation<'a>> {
+        self.star_imports.iter()
+    }
 }
 
 /// Id uniquely identifying a scope in a program.
@@ -286,7 +304,6 @@ impl<'a> Binding<'a> {
                 | BindingKind::FunctionDefinition
                 | BindingKind::Builtin
                 | BindingKind::FutureImportation
-                | BindingKind::StarImportation(..)
                 | BindingKind::Importation(..)
                 | BindingKind::FromImportation(..)
                 | BindingKind::SubmoduleImportation(..)
@@ -340,9 +357,6 @@ impl<'a> Binding<'a> {
             BindingKind::FutureImportation => {
                 return false;
             }
-            BindingKind::StarImportation(..) => {
-                return false;
-            }
             _ => {}
         }
         existing.is_definition()
@@ -379,6 +393,14 @@ impl TryFrom<usize> for BindingId {
 
 impl nohash_hasher::IsEnabled for BindingId {}
 
+#[derive(Debug, Clone)]
+pub struct StarImportation<'a> {
+    /// The level of the import. `None` or `Some(0)` indicate an absolute import.
+    pub level: Option<usize>,
+    /// The module being imported. `None` indicates a wildcard import.
+    pub module: Option<&'a str>,
+}
+
 // Pyflakes defines the following binding hierarchy (via inheritance):
 //   Binding
 //    ExportBinding
@@ -393,7 +415,6 @@ impl nohash_hasher::IsEnabled for BindingId {}
 //      Importation
 //        SubmoduleImportation
 //        ImportationFrom
-//        StarImportation
 //        FutureImportation
 
 #[derive(Clone, Debug)]
@@ -402,14 +423,6 @@ pub struct Export {
     pub names: Vec<String>,
 }
 
-#[derive(Clone, Debug)]
-pub struct StarImportation {
-    /// The level of the import. `None` or `Some(0)` indicate an absolute import.
-    pub level: Option<usize>,
-    /// The module being imported. `None` indicates a wildcard import.
-    pub module: Option<String>,
-}
-
 #[derive(Clone, Debug)]
 pub struct Importation<'a> {
     /// The name to which the import is bound.
@@ -458,7 +471,6 @@ pub enum BindingKind<'a> {
     FunctionDefinition,
     Export(Export),
     FutureImportation,
-    StarImportation(StarImportation),
     Importation(Importation<'a>),
     FromImportation(FromImportation<'a>),
     SubmoduleImportation(SubmoduleImportation<'a>),