Move unused deletion tracking to deferred analysis (#5852)

## Summary

This PR moves the "unused exception" rule out of the visitor and into a
deferred check. When we can base rules solely on the semantic model, we
probably should, as it greatly simplifies the `Checker` itself.

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

@@ -4057,7 +4057,7 @@ where
         }
 
         // Step 2: Binding
-        let bindings = match except_handler {
+        let binding = match except_handler {
             ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
                 type_: _,
                 name,
@@ -4066,17 +4066,17 @@ where
             }) => {
                 if let Some(name) = name {
                     // Store the existing binding, if any.
-                    let existing_id = self.semantic.lookup_symbol(name.as_str());
+                    let binding_id = self.semantic.lookup_symbol(name.as_str());
 
                     // Add the bound exception name to the scope.
-                    let binding_id = self.add_binding(
+                    self.add_binding(
                         name.as_str(),
                         name.range(),
-                        BindingKind::Assignment,
+                        BindingKind::BoundException,
                         BindingFlags::empty(),
                     );
 
-                    Some((name, existing_id, binding_id))
+                    Some((name, binding_id))
                 } else {
                     None
                 }
@@ -4087,30 +4087,11 @@ where
         walk_except_handler(self, except_handler);
 
         // Step 4: Clean-up
-        if let Some((name, existing_id, binding_id)) = bindings {
-            // If the exception name wasn't used in the scope, emit a diagnostic.
-            if !self.semantic.is_used(binding_id) {
-                if self.enabled(Rule::UnusedVariable) {
-                    let mut diagnostic = Diagnostic::new(
-                        pyflakes::rules::UnusedVariable {
-                            name: name.to_string(),
-                        },
-                        name.range(),
-                    );
-                    if self.patch(Rule::UnusedVariable) {
-                        diagnostic.try_set_fix(|| {
-                            pyflakes::fixes::remove_exception_handler_assignment(name, self.locator)
-                                .map(Fix::automatic)
-                        });
-                    }
-                    self.diagnostics.push(diagnostic);
-                }
-            }
-
+        if let Some((name, binding_id)) = binding {
             self.add_binding(
                 name.as_str(),
                 name.range(),
-                BindingKind::UnboundException(existing_id),
+                BindingKind::UnboundException(binding_id),
                 BindingFlags::empty(),
             );
         }
@@ -4797,6 +4778,37 @@ impl<'a> Checker<'a> {
         }
     }
 
+    /// Run any lint rules that operate over a single [`Binding`].
+    fn check_bindings(&mut self) {
+        if !self.any_enabled(&[Rule::UnusedVariable]) {
+            return;
+        }
+
+        for binding in self.semantic.bindings.iter() {
+            // F841
+            if self.enabled(Rule::UnusedVariable) {
+                if binding.kind.is_bound_exception() && !binding.is_used() {
+                    let mut diagnostic = Diagnostic::new(
+                        pyflakes::rules::UnusedVariable {
+                            name: binding.name(self.locator).to_string(),
+                        },
+                        binding.range,
+                    );
+                    if self.patch(Rule::UnusedVariable) {
+                        diagnostic.try_set_fix(|| {
+                            pyflakes::fixes::remove_exception_handler_assignment(
+                                binding,
+                                self.locator,
+                            )
+                            .map(Fix::automatic)
+                        });
+                    }
+                    self.diagnostics.push(diagnostic);
+                }
+            }
+        }
+    }
+
     fn check_deferred_scopes(&mut self) {
         if !self.any_enabled(&[
             Rule::GlobalVariableNotAssigned,
@@ -5475,6 +5487,7 @@ pub(crate) fn check_ast(
     checker.semantic.scope_id = ScopeId::global();
     checker.deferred.scopes.push(ScopeId::global());
     checker.check_deferred_scopes();
+    checker.check_bindings();
 
     checker.diagnostics
 }

crates/ruff/src/renamer.rs

@@ -245,6 +245,7 @@ impl Renamer {
             | BindingKind::NamedExprAssignment
             | BindingKind::UnpackedAssignment
             | BindingKind::Assignment
+            | BindingKind::BoundException
             | BindingKind::LoopVar
             | BindingKind::Global
             | BindingKind::Nonlocal(_)

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

@@ -1,9 +1,10 @@
 use anyhow::{Context, Ok, Result};
 use ruff_text_size::TextRange;
-use rustpython_parser::ast::{Expr, Identifier, Ranged};
+use rustpython_parser::ast::{Expr, Ranged};
 
 use ruff_diagnostics::Edit;
 use ruff_python_ast::source_code::{Locator, Stylist};
+use ruff_python_semantic::Binding;
 use ruff_python_whitespace::{SimpleTokenizer, TokenKind};
 
 use crate::autofix::codemods::CodegenStylist;
@@ -90,12 +91,12 @@ pub(crate) fn remove_unused_positional_arguments_from_format_call(
 
 /// Generate a [`Edit`] to remove the binding from an exception handler.
 pub(crate) fn remove_exception_handler_assignment(
-    bound_exception: &Identifier,
+    bound_exception: &Binding,
     locator: &Locator,
 ) -> Result<Edit> {
     // Lex backwards, to the token just before the `as`.
     let mut tokenizer =
-        SimpleTokenizer::up_to(bound_exception.start(), locator.contents()).skip_trivia();
+        SimpleTokenizer::up_to(bound_exception.range.start(), locator.contents()).skip_trivia();
 
     // Eat the `as` token.
     let preceding = tokenizer
@@ -109,7 +110,7 @@ pub(crate) fn remove_exception_handler_assignment(
         .context("expected the exception name to be preceded by a token")?;
 
     // Lex forwards, to the `:` token.
-    let following = SimpleTokenizer::starts_at(bound_exception.end(), locator.contents())
+    let following = SimpleTokenizer::starts_at(bound_exception.range.end(), locator.contents())
         .next()
         .context("expected the exception name to be followed by a colon")?;
     debug_assert!(matches!(following.kind, TokenKind::Colon));

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

@@ -2115,7 +2115,7 @@ mod tests {
         try: pass
         except Exception as fu: pass
         "#,
-            &[Rule::UnusedVariable, Rule::RedefinedWhileUnused],
+            &[Rule::RedefinedWhileUnused, Rule::UnusedVariable],
         );
     }
 

crates/ruff_python_semantic/src/binding.rs

@@ -5,6 +5,7 @@ use ruff_text_size::TextRange;
 use rustpython_parser::ast::Ranged;
 
 use ruff_index::{newtype_index, IndexSlice, IndexVec};
+use ruff_python_ast::source_code::Locator;
 
 use crate::context::ExecutionContext;
 use crate::model::SemanticModel;
@@ -163,6 +164,11 @@ impl<'a> Binding<'a> {
         }
     }
 
+    /// Returns the name of the binding (e.g., `x` in `x = 1`).
+    pub fn name<'b>(&self, locator: &'b Locator) -> &'b str {
+        locator.slice(self.range)
+    }
+
     /// Returns the range of the binding's parent.
     pub fn parent_range(&self, semantic: &SemanticModel) -> Option<TextRange> {
         self.source
@@ -417,7 +423,16 @@ pub enum BindingKind<'a> {
     /// ```
     Deletion,
 
-    /// A binding to unbind the local variable, like `x` in:
+    /// A binding to bind an exception to a local variable, like `x` in:
+    /// ```python
+    /// try:
+    ///    ...
+    /// except Exception as x:
+    ///   ...
+    /// ```
+    BoundException,
+
+    /// A binding to unbind a bound local exception, like `x` in:
     /// ```python
     /// try:
     ///    ...
@@ -428,7 +443,6 @@ pub enum BindingKind<'a> {
     /// After the `except` block, `x` is unbound, despite the lack
     /// of an explicit `del` statement.
     ///
-    ///
     /// Stores the ID of the binding that was shadowed in the enclosing
     /// scope, if any.
     UnboundException(Option<BindingId>),