No clone

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

@@ -3852,8 +3852,8 @@ where
                             );
                         }
 
-                        // Grab the existing binding.
-                        let existing_id = self.semantic.scope().get(name);
+                        // Store the existing binding, if any.
+                        let existing_id = self.semantic.lookup(name);
 
                         // Add the bound exception name to the scope.
                         let binding_id = self.add_binding(
@@ -3885,22 +3885,12 @@ where
                             }
                         }
 
-                        if let Some(existing_id) = existing_id {
-                            // If the name was already bound, restore the existing binding. We can't
-                            // know whether this handler will trigger, so we proceed as if it
-                            // didn't. Treat it as an entirely new binding to avoid creating a cycle
-                            // in the shadowing graph.
-                            let binding_id = self.semantic.copy_binding(existing_id);
-                            self.semantic.scope_mut().add(name, binding_id);
-                        } else {
-                            // If the name wasn't already bound, mark it as unbound.
-                            self.add_binding(
-                                name,
-                                range,
-                                BindingKind::UnboundException,
-                                BindingFlags::empty(),
-                            );
-                        }
+                        self.add_binding(
+                            name,
+                            range,
+                            BindingKind::UnboundException(existing_id),
+                            BindingFlags::empty(),
+                        );
                     }
                     None => walk_except_handler(self, except_handler),
                 }
@@ -4248,7 +4238,7 @@ impl<'a> Checker<'a> {
             let shadowed = &self.semantic.bindings[shadowed_id];
             if !matches!(
                 shadowed.kind,
-                BindingKind::Builtin | BindingKind::Deletion | BindingKind::UnboundException,
+                BindingKind::Builtin | BindingKind::Deletion | BindingKind::UnboundException(_),
             ) {
                 let references = shadowed.references.clone();
                 let is_global = shadowed.is_global();

crates/ruff/src/renamer.rs

@@ -251,7 +251,7 @@ impl Renamer {
             | BindingKind::ClassDefinition
             | BindingKind::FunctionDefinition
             | BindingKind::Deletion
-            | BindingKind::UnboundException => {
+            | BindingKind::UnboundException(_) => {
                 Some(Edit::range_replacement(target.to_string(), binding.range))
             }
         }

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

@@ -360,6 +360,25 @@ mod tests {
             "#,
         "print_in_body_after_shadowing_except"
     )]
+    #[test_case(
+        r#"
+        def f():
+            x = 1
+
+            try:
+                1 / 0
+            except ValueError as x:
+                pass
+            except ImportError as x:
+                pass
+
+            # No error here, though it should arguably be an F821 error. `x` will
+            # be unbound after the `except` block (assuming an exception is raised
+            # and caught).
+            print(x)
+            "#,
+        "print_in_body_after_double_shadowing_except"
+    )]
     #[test_case(
         r#"
         def f():
@@ -397,6 +416,79 @@ mod tests {
             "#,
         "double_del"
     )]
+    #[test_case(
+        r#"
+        x = 1
+
+        def f():
+            try:
+                pass
+            except ValueError as x:
+                pass
+
+            # This should resolve to the `x` in `x = 1`.
+            print(x)
+            "#,
+        "load_after_unbind_from_module_scope"
+    )]
+    #[test_case(
+        r#"
+        x = 1
+
+        def f():
+            try:
+                pass
+            except ValueError as x:
+                pass
+
+            try:
+                pass
+            except ValueError as x:
+                pass
+
+            # This should resolve to the `x` in `x = 1`.
+            print(x)
+            "#,
+        "load_after_multiple_unbinds_from_module_scope"
+    )]
+    #[test_case(
+        r#"
+        x = 1
+
+        def f():
+            try:
+                pass
+            except ValueError as x:
+                pass
+
+            def g():
+                try:
+                    pass
+                except ValueError as x:
+                    pass
+
+                # This should resolve to the `x` in `x = 1`.
+                print(x)
+            "#,
+        "load_after_unbind_from_nested_module_scope"
+    )]
+    #[test_case(
+        r#"
+        class C:
+            x = 1
+
+            def f():
+                try:
+                    pass
+                except ValueError as x:
+                    pass
+
+                # This should raise an F821 error, rather than resolving to the
+                # `x` in `x = 1`.
+                print(x)
+            "#,
+        "load_after_unbind_from_class_scope"
+    )]
     fn contents(contents: &str, snapshot: &str) {
         let diagnostics = test_snippet(contents, &Settings::for_rules(&Linter::Pyflakes));
         assert_messages!(snapshot, diagnostics);
@@ -2022,11 +2114,7 @@ mod tests {
         try: pass
         except Exception as fu: pass
         "#,
-            &[
-                Rule::UnusedImport,
-                Rule::RedefinedWhileUnused,
-                Rule::UnusedVariable,
-            ],
+            &[Rule::RedefinedWhileUnused, Rule::UnusedVariable],
         );
     }
 

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

@@ -100,16 +100,9 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
     let mut unused: FxHashMap<(NodeId, Exceptions), Vec<Import>> = FxHashMap::default();
     let mut ignored: FxHashMap<(NodeId, Exceptions), Vec<Import>> = FxHashMap::default();
 
-    for (name, binding_id) in scope.bindings() {
+    for binding_id in scope.binding_ids() {
         let binding = checker.semantic().binding(binding_id);
 
-        if !binding.kind.is_builtin() {
-            println!("binding: {:?}", binding);
-            for b in scope.get_all(name) {
-                println!("  {:?}", b);
-            }
-        }
-
         if binding.is_used()
             || binding.is_explicit_export()
             || binding.is_nonlocal()

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__load_after_multiple_unbinds_from_module_scope.snap

@@ -0,0 +1,44 @@
+---
+source: crates/ruff/src/rules/pyflakes/mod.rs
+---
+<filename>:7:26: F841 [*] Local variable `x` is assigned to but never used
+  |
+5 |     try:
+6 |         pass
+7 |     except ValueError as x:
+  |                          ^ F841
+8 |         pass
+  |
+  = help: Remove assignment to unused variable `x`
+
+ℹ Fix
+4 4 | def f():
+5 5 |     try:
+6 6 |         pass
+7   |-    except ValueError as x:
+  7 |+    except ValueError:
+8 8 |         pass
+9 9 | 
+10 10 |     try:
+
+<filename>:12:26: F841 [*] Local variable `x` is assigned to but never used
+   |
+10 |     try:
+11 |         pass
+12 |     except ValueError as x:
+   |                          ^ F841
+13 |         pass
+   |
+   = help: Remove assignment to unused variable `x`
+
+ℹ Fix
+9  9  | 
+10 10 |     try:
+11 11 |         pass
+12    |-    except ValueError as x:
+   12 |+    except ValueError:
+13 13 |         pass
+14 14 | 
+15 15 |     # This should resolve to the `x` in `x = 1`.
+
+

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__load_after_unbind_from_class_scope.snap

@@ -0,0 +1,32 @@
+---
+source: crates/ruff/src/rules/pyflakes/mod.rs
+---
+<filename>:8:30: F841 [*] Local variable `x` is assigned to but never used
+  |
+6 |         try:
+7 |             pass
+8 |         except ValueError as x:
+  |                              ^ F841
+9 |             pass
+  |
+  = help: Remove assignment to unused variable `x`
+
+ℹ Fix
+5 5 |     def f():
+6 6 |         try:
+7 7 |             pass
+8   |-        except ValueError as x:
+  8 |+        except ValueError:
+9 9 |             pass
+10 10 | 
+11 11 |         # This should raise an F821 error, rather than resolving to the
+
+<filename>:13:15: F821 Undefined name `x`
+   |
+11 |         # This should raise an F821 error, rather than resolving to the
+12 |         # `x` in `x = 1`.
+13 |         print(x)
+   |               ^ F821
+   |
+
+

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__load_after_unbind_from_module_scope.snap

@@ -0,0 +1,24 @@
+---
+source: crates/ruff/src/rules/pyflakes/mod.rs
+---
+<filename>:7:26: F841 [*] Local variable `x` is assigned to but never used
+  |
+5 |     try:
+6 |         pass
+7 |     except ValueError as x:
+  |                          ^ F841
+8 |         pass
+  |
+  = help: Remove assignment to unused variable `x`
+
+ℹ Fix
+4 4 | def f():
+5 5 |     try:
+6 6 |         pass
+7   |-    except ValueError as x:
+  7 |+    except ValueError:
+8 8 |         pass
+9 9 | 
+10 10 |     # This should resolve to the `x` in `x = 1`.
+
+

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__load_after_unbind_from_nested_module_scope.snap

@@ -0,0 +1,44 @@
+---
+source: crates/ruff/src/rules/pyflakes/mod.rs
+---
+<filename>:7:26: F841 [*] Local variable `x` is assigned to but never used
+  |
+5 |     try:
+6 |         pass
+7 |     except ValueError as x:
+  |                          ^ F841
+8 |         pass
+  |
+  = help: Remove assignment to unused variable `x`
+
+ℹ Fix
+4 4 | def f():
+5 5 |     try:
+6 6 |         pass
+7   |-    except ValueError as x:
+  7 |+    except ValueError:
+8 8 |         pass
+9 9 | 
+10 10 |     def g():
+
+<filename>:13:30: F841 [*] Local variable `x` is assigned to but never used
+   |
+11 |         try:
+12 |             pass
+13 |         except ValueError as x:
+   |                              ^ F841
+14 |             pass
+   |
+   = help: Remove assignment to unused variable `x`
+
+ℹ Fix
+10 10 |     def g():
+11 11 |         try:
+12 12 |             pass
+13    |-        except ValueError as x:
+   13 |+        except ValueError:
+14 14 |             pass
+15 15 | 
+16 16 |         # This should resolve to the `x` in `x = 1`.
+
+