Move shadow tracking into Scope directly (#3854)

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

@@ -4081,7 +4081,7 @@ impl<'a> Checker<'a> {
                     }
                 } else if existing_is_import && binding.redefines(existing) {
                     self.ctx
-                        .redefinitions
+                        .shadowed_bindings
                         .entry(existing_binding_index)
                         .or_insert_with(Vec::new)
                         .push(binding_id);
@@ -4123,13 +4123,7 @@ impl<'a> Checker<'a> {
         // in scope.
         let scope = self.ctx.scope_mut();
         if !(binding.kind.is_annotation() && scope.defines(name)) {
-            if let Some(rebound_index) = scope.add(name, binding_id) {
+            scope.add(name, binding_id);
-                scope
-                    .rebounds
-                    .entry(name)
-                    .or_insert_with(Vec::new)
-                    .push(rebound_index);
-            }
         }
 
         self.ctx.bindings.push(binding);
@@ -4919,7 +4913,7 @@ impl<'a> Checker<'a> {
                             continue;
                         }
 
-                        if let Some(indices) = self.ctx.redefinitions.get(index) {
+                        if let Some(indices) = self.ctx.shadowed_bindings.get(index) {
                             for index in indices {
                                 let rebound = &self.ctx.bindings[*index];
                                 let mut diagnostic = Diagnostic::new(

crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs

@@ -166,17 +166,13 @@ pub fn unused_loop_control_variable(
             if certainty.into() && checker.patch(diagnostic.kind.rule()) {
                 // Find the `BindingKind::LoopVar` corresponding to the name.
                 let scope = checker.ctx.scope();
-                let binding = scope
+                let binding = scope.bindings_for_name(name).find_map(|index| {
-                    .get(name)
+                    let binding = &checker.ctx.bindings[*index];
-                    .into_iter()
+                    binding
-                    .chain(scope.rebounds.get(name).into_iter().flatten())
+                        .source
-                    .find_map(|index| {
+                        .as_ref()
-                        let binding = &checker.ctx.bindings[*index];
+                        .and_then(|source| (source == &RefEquality(stmt)).then_some(binding))
-                        binding
+                });
-                            .source
-                            .as_ref()
-                            .and_then(|source| (source == &RefEquality(stmt)).then_some(binding))
-                    });
                 if let Some(binding) = binding {
                     if binding.kind.is_loop_var() {
                         if !binding.used() {

crates/ruff_python_ast/src/context.rs

@@ -30,8 +30,8 @@ pub struct Context<'a> {
     pub child_to_parent: FxHashMap<RefEquality<'a, Stmt>, RefEquality<'a, Stmt>>,
     // A stack of all bindings created in any scope, at any point in execution.
     pub bindings: Bindings<'a>,
-    // Map from binding index to indices of bindings that redefine it in other scopes.
+    // Map from binding index to indexes of bindings that shadow it in other scopes.
-    pub redefinitions:
+    pub shadowed_bindings:
         std::collections::HashMap<BindingId, Vec<BindingId>, BuildNoHashHasher<BindingId>>,
     pub exprs: Vec<RefEquality<'a, Expr>>,
     pub scopes: Scopes<'a>,
@@ -71,7 +71,7 @@ impl<'a> Context<'a> {
             depths: FxHashMap::default(),
             child_to_parent: FxHashMap::default(),
             bindings: Bindings::default(),
-            redefinitions: IntMap::default(),
+            shadowed_bindings: IntMap::default(),
             exprs: Vec::default(),
             scopes: Scopes::default(),
             scope_stack: ScopeStack::default(),

crates/ruff_python_ast/src/scope.rs

@@ -14,12 +14,10 @@ pub struct Scope<'a> {
     /// 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.
+    /// A map from bound name to binding index, for current bindings.
     bindings: FxHashMap<&'a str, BindingId>,
-    /// A map from bound name to binding index, for bindings that were created
+    /// A map from bound name to binding index, for bindings that were shadowed later in the scope.
-    /// in the scope but rebound (and thus overridden) later on in the same
+    shadowed_bindings: FxHashMap<&'a str, Vec<BindingId>>,
-    /// scope.
-    pub rebounds: FxHashMap<&'a str, Vec<BindingId>>,
 }
 
 impl<'a> Scope<'a> {
@@ -34,18 +32,23 @@ impl<'a> Scope<'a> {
             uses_locals: false,
             star_imports: Vec::default(),
             bindings: FxHashMap::default(),
-            rebounds: FxHashMap::default(),
+            shadowed_bindings: FxHashMap::default(),
         }
     }
 
-    /// Returns the [id](BindingId) of the binding with the given name.
+    /// Returns the [id](BindingId) of the binding bound to the given name.
     pub fn get(&self, name: &str) -> Option<&BindingId> {
         self.bindings.get(name)
     }
 
     /// Adds a new binding with the given name to this scope.
     pub fn add(&mut self, name: &'a str, id: BindingId) -> Option<BindingId> {
-        self.bindings.insert(name, id)
+        if let Some(id) = self.bindings.insert(name, id) {
+            self.shadowed_bindings.entry(name).or_default().push(id);
+            Some(id)
+        } else {
+            None
+        }
     }
 
     /// Returns `true` if this scope defines a binding with the given name.
@@ -68,6 +71,15 @@ impl<'a> Scope<'a> {
         self.bindings.iter()
     }
 
+    /// Returns an iterator over all [bindings](BindingId) bound to the given name, including
+    /// those that were shadowed by later bindings.
+    pub fn bindings_for_name(&self, name: &str) -> impl Iterator<Item = &BindingId> {
+        self.bindings
+            .get(name)
+            .into_iter()
+            .chain(self.shadowed_bindings.get(name).into_iter().flatten().rev())
+    }
+
     /// 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);