diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index d979952140..95ada177cc 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/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 } diff --git a/crates/ruff/src/renamer.rs b/crates/ruff/src/renamer.rs index 14aec66ef7..9274018a79 100644 --- a/crates/ruff/src/renamer.rs +++ b/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(_) diff --git a/crates/ruff/src/rules/pyflakes/fixes.rs b/crates/ruff/src/rules/pyflakes/fixes.rs index c6cdee0f5a..bad191b555 100644 --- a/crates/ruff/src/rules/pyflakes/fixes.rs +++ b/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 { // 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)); diff --git a/crates/ruff/src/rules/pyflakes/mod.rs b/crates/ruff/src/rules/pyflakes/mod.rs index 1f13edf973..885b4baf6b 100644 --- a/crates/ruff/src/rules/pyflakes/mod.rs +++ b/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], ); } diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 862be2b617..f66da2a80a 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/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 { 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),