diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index d8a7d174508dc..a378dc9e18da5 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -35,7 +35,6 @@ use crate::fs::relativize_path; use crate::importer::Importer; use crate::noqa::NoqaMapping; use crate::registry::Rule; -use crate::rules::flake8_builtins::helpers::AnyShadowing; use crate::rules::{ airflow, flake8_2020, flake8_annotations, flake8_async, flake8_bandit, flake8_blind_except, @@ -614,7 +613,7 @@ where self, class_def, name, - AnyShadowing::from(stmt), + name.range(), ); } } else { @@ -622,7 +621,7 @@ where flake8_builtins::rules::builtin_variable_shadowing( self, name, - AnyShadowing::from(stmt), + name.range(), ); } } @@ -753,11 +752,7 @@ where flake8_bugbear::rules::f_string_docstring(self, body); } if self.enabled(Rule::BuiltinVariableShadowing) { - flake8_builtins::rules::builtin_variable_shadowing( - self, - name, - AnyShadowing::from(stmt), - ); + flake8_builtins::rules::builtin_variable_shadowing(self, name, name.range()); } if self.enabled(Rule::DuplicateBases) { pylint::rules::duplicate_bases(self, name, bases); @@ -837,7 +832,7 @@ where flake8_builtins::rules::builtin_variable_shadowing( self, asname, - AnyShadowing::from(stmt), + asname.range(), ); } } @@ -1091,7 +1086,7 @@ where flake8_builtins::rules::builtin_variable_shadowing( self, asname, - AnyShadowing::from(stmt), + asname.range(), ); } } @@ -2265,7 +2260,7 @@ where } } } - Expr::Name(ast::ExprName { id, ctx, range: _ }) => { + Expr::Name(ast::ExprName { id, ctx, range }) => { match ctx { ExprContext::Load => { self.handle_node_load(expr); @@ -2338,18 +2333,13 @@ where if let ScopeKind::Class(class_def) = self.semantic.scope().kind { if self.enabled(Rule::BuiltinAttributeShadowing) { flake8_builtins::rules::builtin_attribute_shadowing( - self, - class_def, - id, - AnyShadowing::from(expr), + self, class_def, id, *range, ); } } else { if self.enabled(Rule::BuiltinVariableShadowing) { flake8_builtins::rules::builtin_variable_shadowing( - self, - id, - AnyShadowing::from(expr), + self, id, *range, ); } } @@ -3880,7 +3870,6 @@ where body, range: _, }) => { - let name = name.as_deref(); if self.enabled(Rule::BareExcept) { if let Some(diagnostic) = pycodestyle::rules::bare_except( type_.as_deref(), @@ -3892,17 +3881,25 @@ where } } if self.enabled(Rule::RaiseWithoutFromInsideExcept) { - flake8_bugbear::rules::raise_without_from_inside_except(self, name, body); + flake8_bugbear::rules::raise_without_from_inside_except( + self, + name.as_deref(), + body, + ); } if self.enabled(Rule::BlindExcept) { - flake8_blind_except::rules::blind_except(self, type_.as_deref(), name, body); + flake8_blind_except::rules::blind_except( + self, + type_.as_deref(), + name.as_deref(), + body, + ); } if self.enabled(Rule::TryExceptPass) { flake8_bandit::rules::try_except_pass( self, except_handler, type_.as_deref(), - name, body, self.settings.flake8_bandit.check_typed_exception, ); @@ -3912,7 +3909,6 @@ where self, except_handler, type_.as_deref(), - name, body, self.settings.flake8_bandit.check_typed_exception, ); @@ -3929,66 +3925,66 @@ where if self.enabled(Rule::BinaryOpException) { pylint::rules::binary_op_exception(self, except_handler); } - match name { - Some(name) => { - let range = except_handler.try_identifier().unwrap(); - if self.enabled(Rule::AmbiguousVariableName) { - if let Some(diagnostic) = - pycodestyle::rules::ambiguous_variable_name(name, range) - { - self.diagnostics.push(diagnostic); - } - } - if self.enabled(Rule::BuiltinVariableShadowing) { - flake8_builtins::rules::builtin_variable_shadowing( - self, - name, - AnyShadowing::from(except_handler), - ); + if let Some(name) = name { + if self.enabled(Rule::AmbiguousVariableName) { + if let Some(diagnostic) = + pycodestyle::rules::ambiguous_variable_name(name.as_str(), name.range()) + { + self.diagnostics.push(diagnostic); } - - // Store the existing binding, if any. - let existing_id = self.semantic.lookup_symbol(name); - - // Add the bound exception name to the scope. - let binding_id = self.add_binding( + } + if self.enabled(Rule::BuiltinVariableShadowing) { + flake8_builtins::rules::builtin_variable_shadowing( + self, name, - range, - BindingKind::Assignment, - BindingFlags::empty(), + name.range(), ); + } - walk_except_handler(self, except_handler); + // Store the existing binding, if any. + let existing_id = self.semantic.lookup_symbol(name.as_str()); - // 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.into() }, - range, - ); - if self.patch(Rule::UnusedVariable) { - diagnostic.try_set_fix(|| { - pyflakes::fixes::remove_exception_handler_assignment( - except_handler, - self.locator, - ) - .map(Fix::automatic) - }); - } - self.diagnostics.push(diagnostic); + // Add the bound exception name to the scope. + let binding_id = self.add_binding( + name.as_str(), + name.range(), + BindingKind::Assignment, + BindingFlags::empty(), + ); + + walk_except_handler(self, except_handler); + + // 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( + except_handler, + self.locator, + ) + .map(Fix::automatic) + }); } + self.diagnostics.push(diagnostic); } - - self.add_binding( - name, - range, - BindingKind::UnboundException(existing_id), - BindingFlags::empty(), - ); } - None => walk_except_handler(self, except_handler), + + self.add_binding( + name.as_str(), + name.range(), + BindingKind::UnboundException(existing_id), + BindingFlags::empty(), + ); + } else { + walk_except_handler(self, except_handler); } } } diff --git a/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs b/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs index 82cc379cfb145..a153c99845bc4 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/try_except_continue.rs @@ -55,16 +55,14 @@ pub(crate) fn try_except_continue( checker: &mut Checker, except_handler: &ExceptHandler, type_: Option<&Expr>, - _name: Option<&str>, body: &[Stmt], check_typed_exception: bool, ) { - if body.len() == 1 - && body[0].is_continue_stmt() - && (check_typed_exception || is_untyped_exception(type_, checker.semantic())) - { - checker - .diagnostics - .push(Diagnostic::new(TryExceptContinue, except_handler.range())); + if matches!(body, [Stmt::Continue(_)]) { + if check_typed_exception || is_untyped_exception(type_, checker.semantic()) { + checker + .diagnostics + .push(Diagnostic::new(TryExceptContinue, except_handler.range())); + } } } diff --git a/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs b/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs index ba4e1d713da05..2febb189367bd 100644 --- a/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs +++ b/crates/ruff/src/rules/flake8_bandit/rules/try_except_pass.rs @@ -51,16 +51,14 @@ pub(crate) fn try_except_pass( checker: &mut Checker, except_handler: &ExceptHandler, type_: Option<&Expr>, - _name: Option<&str>, body: &[Stmt], check_typed_exception: bool, ) { - if body.len() == 1 - && body[0].is_pass_stmt() - && (check_typed_exception || is_untyped_exception(type_, checker.semantic())) - { - checker - .diagnostics - .push(Diagnostic::new(TryExceptPass, except_handler.range())); + if matches!(body, [Stmt::Pass(_)]) { + if check_typed_exception || is_untyped_exception(type_, checker.semantic()) { + checker + .diagnostics + .push(Diagnostic::new(TryExceptPass, except_handler.range())); + } } } diff --git a/crates/ruff/src/rules/flake8_builtins/helpers.rs b/crates/ruff/src/rules/flake8_builtins/helpers.rs index 1fe69e0cb4d32..2521dec2e28a6 100644 --- a/crates/ruff/src/rules/flake8_builtins/helpers.rs +++ b/crates/ruff/src/rules/flake8_builtins/helpers.rs @@ -1,44 +1,5 @@ -use ruff_text_size::TextRange; -use rustpython_parser::ast::{ExceptHandler, Expr, Ranged, Stmt}; - -use ruff_python_ast::identifier::{Identifier, TryIdentifier}; use ruff_python_stdlib::builtins::BUILTINS; pub(super) fn shadows_builtin(name: &str, ignorelist: &[String]) -> bool { BUILTINS.contains(&name) && ignorelist.iter().all(|ignore| ignore != name) } - -#[derive(Debug, Copy, Clone, PartialEq)] -pub(crate) enum AnyShadowing<'a> { - Expression(&'a Expr), - Statement(&'a Stmt), - ExceptHandler(&'a ExceptHandler), -} - -impl Identifier for AnyShadowing<'_> { - fn identifier(&self) -> TextRange { - match self { - AnyShadowing::Expression(expr) => expr.range(), - AnyShadowing::Statement(stmt) => stmt.identifier(), - AnyShadowing::ExceptHandler(handler) => handler.try_identifier().unwrap(), - } - } -} - -impl<'a> From<&'a Stmt> for AnyShadowing<'a> { - fn from(value: &'a Stmt) -> Self { - AnyShadowing::Statement(value) - } -} - -impl<'a> From<&'a Expr> for AnyShadowing<'a> { - fn from(value: &'a Expr) -> Self { - AnyShadowing::Expression(value) - } -} - -impl<'a> From<&'a ExceptHandler> for AnyShadowing<'a> { - fn from(value: &'a ExceptHandler) -> Self { - AnyShadowing::ExceptHandler(value) - } -} diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs index 45edcb2ef7d6b..6f523d76f9b89 100644 --- a/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs +++ b/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs @@ -1,12 +1,12 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast; + use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::identifier::Identifier; -use rustpython_parser::ast; use crate::checkers::ast::Checker; - -use super::super::helpers::{shadows_builtin, AnyShadowing}; +use crate::rules::flake8_builtins::helpers::shadows_builtin; /// ## What it does /// Checks for any class attributes that use the same name as a builtin. @@ -67,7 +67,7 @@ pub(crate) fn builtin_attribute_shadowing( checker: &mut Checker, class_def: &ast::StmtClassDef, name: &str, - shadowing: AnyShadowing, + range: TextRange, ) { if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { // Ignore shadowing within `TypedDict` definitions, since these are only accessible through @@ -84,7 +84,7 @@ pub(crate) fn builtin_attribute_shadowing( BuiltinAttributeShadowing { name: name.to_string(), }, - shadowing.identifier(), + range, )); } } diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs index d61aff2e57738..74aa7c2e25f0a 100644 --- a/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs +++ b/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs @@ -1,11 +1,11 @@ +use ruff_text_size::TextRange; + use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::identifier::Identifier; use crate::checkers::ast::Checker; - -use super::super::helpers::{shadows_builtin, AnyShadowing}; +use crate::rules::flake8_builtins::helpers::shadows_builtin; /// ## What it does /// Checks for variable (and function) assignments that use the same name @@ -59,17 +59,13 @@ impl Violation for BuiltinVariableShadowing { } /// A001 -pub(crate) fn builtin_variable_shadowing( - checker: &mut Checker, - name: &str, - shadowing: AnyShadowing, -) { +pub(crate) fn builtin_variable_shadowing(checker: &mut Checker, name: &str, range: TextRange) { if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { checker.diagnostics.push(Diagnostic::new( BuiltinVariableShadowing { name: name.to_string(), }, - shadowing.identifier(), + range, )); } } diff --git a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py.snap b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py.snap index 4aa57c119ff5a..4ae541d0941be 100644 --- a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py.snap +++ b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py.snap @@ -1,28 +1,28 @@ --- source: crates/ruff/src/rules/flake8_builtins/mod.rs --- -A001.py:1:1: A001 Variable `sum` is shadowing a Python builtin +A001.py:1:16: A001 Variable `sum` is shadowing a Python builtin | 1 | import some as sum - | ^^^^^^^^^^^^^^^^^^ A001 + | ^^^ A001 2 | from some import other as int 3 | from directory import new as dir | -A001.py:2:1: A001 Variable `int` is shadowing a Python builtin +A001.py:2:27: A001 Variable `int` is shadowing a Python builtin | 1 | import some as sum 2 | from some import other as int - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ A001 + | ^^^ A001 3 | from directory import new as dir | -A001.py:3:1: A001 Variable `dir` is shadowing a Python builtin +A001.py:3:30: A001 Variable `dir` is shadowing a Python builtin | 1 | import some as sum 2 | from some import other as int 3 | from directory import new as dir - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ A001 + | ^^^ A001 4 | 5 | print = 1 | diff --git a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py_builtins_ignorelist.snap b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py_builtins_ignorelist.snap index 591ee6781fa16..ce715c0d0aaa8 100644 --- a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py_builtins_ignorelist.snap +++ b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A001_A001.py_builtins_ignorelist.snap @@ -1,19 +1,19 @@ --- source: crates/ruff/src/rules/flake8_builtins/mod.rs --- -A001.py:1:1: A001 Variable `sum` is shadowing a Python builtin +A001.py:1:16: A001 Variable `sum` is shadowing a Python builtin | 1 | import some as sum - | ^^^^^^^^^^^^^^^^^^ A001 + | ^^^ A001 2 | from some import other as int 3 | from directory import new as dir | -A001.py:2:1: A001 Variable `int` is shadowing a Python builtin +A001.py:2:27: A001 Variable `int` is shadowing a Python builtin | 1 | import some as sum 2 | from some import other as int - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ A001 + | ^^^ A001 3 | from directory import new as dir | diff --git a/crates/ruff_python_ast/src/identifier.rs b/crates/ruff_python_ast/src/identifier.rs index fb801f098eca7..9e381195152a6 100644 --- a/crates/ruff_python_ast/src/identifier.rs +++ b/crates/ruff_python_ast/src/identifier.rs @@ -138,22 +138,6 @@ impl TryIdentifier for Pattern { } } -impl TryIdentifier for ExceptHandler { - /// Return the [`TextRange`] of a named exception in an [`ExceptHandler`]. - /// - /// For example, return the range of `e` in: - /// ```python - /// try: - /// ... - /// except ValueError as e: - /// ... - /// ``` - fn try_identifier(&self) -> Option { - let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { name, .. }) = self; - name.as_ref().map(Ranged::range) - } -} - /// Return the [`TextRange`] of the `except` token in an [`ExceptHandler`]. pub fn except(handler: &ExceptHandler, locator: &Locator) -> TextRange { IdentifierTokenizer::new(locator.contents(), handler.range())