diff --git a/Cargo.lock b/Cargo.lock index f87bd95bfc..fcc975f38b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2000,7 +2000,7 @@ dependencies = [ [[package]] name = "ruff_text_size" version = "0.0.0" -source = "git+https://github.com/RustPython/Parser.git?rev=e1f02fced740cdd99593ae4b14bcfcac725e9e02#e1f02fced740cdd99593ae4b14bcfcac725e9e02" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=335780aeeac1e6fcd85994ba001d7b8ce99fcf65#335780aeeac1e6fcd85994ba001d7b8ce99fcf65" dependencies = [ "schemars", "serde", @@ -2071,7 +2071,7 @@ dependencies = [ [[package]] name = "rustpython-ast" version = "0.2.0" -source = "git+https://github.com/RustPython/Parser.git?rev=e1f02fced740cdd99593ae4b14bcfcac725e9e02#e1f02fced740cdd99593ae4b14bcfcac725e9e02" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=335780aeeac1e6fcd85994ba001d7b8ce99fcf65#335780aeeac1e6fcd85994ba001d7b8ce99fcf65" dependencies = [ "is-macro", "num-bigint", @@ -2082,7 +2082,7 @@ dependencies = [ [[package]] name = "rustpython-format" version = "0.2.0" -source = "git+https://github.com/RustPython/Parser.git?rev=e1f02fced740cdd99593ae4b14bcfcac725e9e02#e1f02fced740cdd99593ae4b14bcfcac725e9e02" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=335780aeeac1e6fcd85994ba001d7b8ce99fcf65#335780aeeac1e6fcd85994ba001d7b8ce99fcf65" dependencies = [ "bitflags 2.3.1", "itertools", @@ -2094,7 +2094,7 @@ dependencies = [ [[package]] name = "rustpython-literal" version = "0.2.0" -source = "git+https://github.com/RustPython/Parser.git?rev=e1f02fced740cdd99593ae4b14bcfcac725e9e02#e1f02fced740cdd99593ae4b14bcfcac725e9e02" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=335780aeeac1e6fcd85994ba001d7b8ce99fcf65#335780aeeac1e6fcd85994ba001d7b8ce99fcf65" dependencies = [ "hexf-parse", "is-macro", @@ -2106,7 +2106,7 @@ dependencies = [ [[package]] name = "rustpython-parser" version = "0.2.0" -source = "git+https://github.com/RustPython/Parser.git?rev=e1f02fced740cdd99593ae4b14bcfcac725e9e02#e1f02fced740cdd99593ae4b14bcfcac725e9e02" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=335780aeeac1e6fcd85994ba001d7b8ce99fcf65#335780aeeac1e6fcd85994ba001d7b8ce99fcf65" dependencies = [ "anyhow", "is-macro", @@ -2129,7 +2129,7 @@ dependencies = [ [[package]] name = "rustpython-parser-core" version = "0.2.0" -source = "git+https://github.com/RustPython/Parser.git?rev=e1f02fced740cdd99593ae4b14bcfcac725e9e02#e1f02fced740cdd99593ae4b14bcfcac725e9e02" +source = "git+https://github.com/astral-sh/RustPython-Parser.git?rev=335780aeeac1e6fcd85994ba001d7b8ce99fcf65#335780aeeac1e6fcd85994ba001d7b8ce99fcf65" dependencies = [ "is-macro", "ruff_text_size", diff --git a/Cargo.toml b/Cargo.toml index 5544d5e34d..727613ca94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,10 +31,10 @@ proc-macro2 = { version = "1.0.51" } quote = { version = "1.0.23" } regex = { version = "1.7.1" } rustc-hash = { version = "1.1.0" } -ruff_text_size = { git = "https://github.com/RustPython/Parser.git", rev = "e1f02fced740cdd99593ae4b14bcfcac725e9e02" } -rustpython-format = { git = "https://github.com/RustPython/Parser.git", rev = "e1f02fced740cdd99593ae4b14bcfcac725e9e02" } -rustpython-literal = { git = "https://github.com/RustPython/Parser.git", rev = "e1f02fced740cdd99593ae4b14bcfcac725e9e02" } -rustpython-parser = { git = "https://github.com/RustPython/Parser.git", rev = "e1f02fced740cdd99593ae4b14bcfcac725e9e02", default-features = false, features = ["full-lexer", "all-nodes-with-ranges"] } +ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "335780aeeac1e6fcd85994ba001d7b8ce99fcf65" } +rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "335780aeeac1e6fcd85994ba001d7b8ce99fcf65" } +rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "335780aeeac1e6fcd85994ba001d7b8ce99fcf65" } +rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "335780aeeac1e6fcd85994ba001d7b8ce99fcf65", default-features = false, features = ["full-lexer", "all-nodes-with-ranges"] } schemars = { version = "0.8.12" } serde = { version = "1.0.152", features = ["derive"] } serde_json = { version = "1.0.93", features = ["preserve_order"] } diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index f6419c1e58..ab204814d5 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -41,6 +41,7 @@ use crate::fs::relativize_path; use crate::importer::Importer; use crate::noqa::NoqaMapping; use crate::registry::{AsRule, Rule}; +use crate::rules::flake8_builtins::rules::AnyShadowing; use crate::rules::{ flake8_2020, flake8_annotations, flake8_async, flake8_bandit, flake8_blind_except, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, @@ -626,11 +627,19 @@ where if self.semantic_model.scope().kind.is_class() { if self.settings.rules.enabled(Rule::BuiltinAttributeShadowing) { - flake8_builtins::rules::builtin_attribute_shadowing(self, name, stmt); + flake8_builtins::rules::builtin_attribute_shadowing( + self, + name, + AnyShadowing::from(stmt), + ); } } else { if self.settings.rules.enabled(Rule::BuiltinVariableShadowing) { - flake8_builtins::rules::builtin_variable_shadowing(self, name, stmt); + flake8_builtins::rules::builtin_variable_shadowing( + self, + name, + AnyShadowing::from(stmt), + ); } } } @@ -804,7 +813,11 @@ where } if self.settings.rules.enabled(Rule::BuiltinVariableShadowing) { - flake8_builtins::rules::builtin_variable_shadowing(self, name, stmt); + flake8_builtins::rules::builtin_variable_shadowing( + self, + name, + AnyShadowing::from(stmt), + ); } if self.settings.rules.enabled(Rule::DuplicateBases) { @@ -921,7 +934,9 @@ where if let Some(asname) = &alias.asname { if self.settings.rules.enabled(Rule::BuiltinVariableShadowing) { flake8_builtins::rules::builtin_variable_shadowing( - self, asname, stmt, + self, + asname, + AnyShadowing::from(stmt), ); } } @@ -1228,7 +1243,9 @@ where if let Some(asname) = &alias.asname { if self.settings.rules.enabled(Rule::BuiltinVariableShadowing) { flake8_builtins::rules::builtin_variable_shadowing( - self, asname, stmt, + self, + asname, + AnyShadowing::from(stmt), ); } } @@ -2526,11 +2543,19 @@ where if self.semantic_model.scope().kind.is_class() { if self.settings.rules.enabled(Rule::BuiltinAttributeShadowing) { - flake8_builtins::rules::builtin_attribute_shadowing(self, id, expr); + flake8_builtins::rules::builtin_attribute_shadowing( + self, + id, + AnyShadowing::from(expr), + ); } } else { if self.settings.rules.enabled(Rule::BuiltinVariableShadowing) { - flake8_builtins::rules::builtin_variable_shadowing(self, id, expr); + flake8_builtins::rules::builtin_variable_shadowing( + self, + id, + AnyShadowing::from(expr), + ); } } @@ -4323,7 +4348,7 @@ where flake8_builtins::rules::builtin_variable_shadowing( self, name, - excepthandler, + AnyShadowing::from(excepthandler), ); } @@ -4481,7 +4506,7 @@ where } if self.settings.rules.enabled(Rule::BuiltinArgumentShadowing) { - flake8_builtins::rules::builtin_argument_shadowing(self, &arg.arg, arg); + flake8_builtins::rules::builtin_argument_shadowing(self, arg); } } @@ -4686,26 +4711,18 @@ impl<'a> Checker<'a> { { if self.settings.rules.enabled(Rule::RedefinedWhileUnused) { #[allow(deprecated)] - let line = self.locator.compute_line_index(existing.range.start()); + let line = self.locator.compute_line_index( + existing + .trimmed_range(self.semantic_model(), self.locator) + .start(), + ); let mut diagnostic = Diagnostic::new( pyflakes::rules::RedefinedWhileUnused { name: name.to_string(), line, }, - matches!( - binding.kind, - BindingKind::ClassDefinition | BindingKind::FunctionDefinition - ) - .then(|| { - binding.source.map_or(binding.range, |source| { - helpers::identifier_range( - self.semantic_model.stmts[source], - self.locator, - ) - }) - }) - .unwrap_or(binding.range), + binding.trimmed_range(self.semantic_model(), self.locator), ); if let Some(parent) = binding.source { let parent = self.semantic_model.stmts[parent]; @@ -5466,27 +5483,18 @@ impl<'a> Checker<'a> { for index in indices { let rebound = &self.semantic_model.bindings[*index]; #[allow(deprecated)] - let line = self.locator.compute_line_index(binding.range.start()); + let line = self.locator.compute_line_index( + binding + .trimmed_range(self.semantic_model(), self.locator) + .start(), + ); let mut diagnostic = Diagnostic::new( pyflakes::rules::RedefinedWhileUnused { name: (*name).to_string(), line, }, - matches!( - rebound.kind, - BindingKind::ClassDefinition - | BindingKind::FunctionDefinition - ) - .then(|| { - rebound.source.map_or(rebound.range, |source| { - helpers::identifier_range( - self.semantic_model.stmts[source], - self.locator, - ) - }) - }) - .unwrap_or(rebound.range), + rebound.trimmed_range(self.semantic_model(), self.locator), ); if let Some(source) = rebound.source { let parent = &self.semantic_model.stmts[source]; diff --git a/crates/ruff/src/rules/flake8_builtins/rules.rs b/crates/ruff/src/rules/flake8_builtins/rules.rs index 52da75ceea..9879ef66fd 100644 --- a/crates/ruff/src/rules/flake8_builtins/rules.rs +++ b/crates/ruff/src/rules/flake8_builtins/rules.rs @@ -1,8 +1,11 @@ -use rustpython_parser::ast::Ranged; +use ruff_text_size::TextRange; +use rustpython_parser::ast::{Arg, Excepthandler, Expr, Ranged, Stmt}; use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Violation; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::identifier_range; +use ruff_python_ast::source_code::Locator; use ruff_python_stdlib::builtins::BUILTINS; use crate::checkers::ast::Checker; @@ -172,46 +175,83 @@ fn shadows_builtin(name: &str, ignorelist: &[String]) -> bool { } /// A001 -pub(crate) fn builtin_variable_shadowing(checker: &mut Checker, name: &str, attributed: &T) -where - T: Ranged, -{ +pub(crate) fn builtin_variable_shadowing( + checker: &mut Checker, + name: &str, + shadowing: AnyShadowing, +) { if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { checker.diagnostics.push(Diagnostic::new( BuiltinVariableShadowing { name: name.to_string(), }, - attributed.range(), + shadowing.range(checker.locator), )); } } /// A002 -pub(crate) fn builtin_argument_shadowing(checker: &mut Checker, name: &str, attributed: &T) -where - T: Ranged, -{ - if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { +pub(crate) fn builtin_argument_shadowing(checker: &mut Checker, argument: &Arg) { + if shadows_builtin( + argument.arg.as_str(), + &checker.settings.flake8_builtins.builtins_ignorelist, + ) { checker.diagnostics.push(Diagnostic::new( BuiltinArgumentShadowing { - name: name.to_string(), + name: argument.arg.to_string(), }, - attributed.range(), + argument.range(), )); } } /// A003 -pub(crate) fn builtin_attribute_shadowing(checker: &mut Checker, name: &str, attributed: &T) -where - T: Ranged, -{ +pub(crate) fn builtin_attribute_shadowing( + checker: &mut Checker, + name: &str, + shadowing: AnyShadowing, +) { if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { checker.diagnostics.push(Diagnostic::new( BuiltinAttributeShadowing { name: name.to_string(), }, - attributed.range(), + shadowing.range(checker.locator), )); } } + +#[derive(Debug, Copy, Clone, PartialEq)] +pub(crate) enum AnyShadowing<'a> { + Expression(&'a Expr), + Statement(&'a Stmt), + ExceptHandler(&'a Excepthandler), +} + +impl AnyShadowing<'_> { + fn range(self, locator: &Locator) -> TextRange { + match self { + AnyShadowing::Expression(expr) => expr.range(), + AnyShadowing::Statement(stmt) => identifier_range(stmt, locator), + AnyShadowing::ExceptHandler(handler) => handler.range(), + } + } +} + +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/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 1079c55bbe..d65e5fa476 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 @@ -104,26 +104,22 @@ A001.py:11:1: A001 Variable `id` is shadowing a Python builtin 15 | def bytes(): | -A001.py:13:1: A001 Variable `bytes` is shadowing a Python builtin +A001.py:13:5: A001 Variable `bytes` is shadowing a Python builtin | -13 | id = 4 -14 | -15 | / def bytes(): -16 | | pass - | |________^ A001 -17 | -18 | class slice: +13 | id = 4 +14 | +15 | def bytes(): + | ^^^^^ A001 +16 | pass | -A001.py:16:1: A001 Variable `slice` is shadowing a Python builtin +A001.py:16:7: A001 Variable `slice` is shadowing a Python builtin | -16 | pass -17 | -18 | / class slice: -19 | | pass - | |________^ A001 -20 | -21 | try: +16 | pass +17 | +18 | class slice: + | ^^^^^ A001 +19 | pass | A001.py:21:1: A001 Variable `ValueError` is shadowing a Python builtin 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 d3480ecbaa..a709ce3c92 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 @@ -84,26 +84,22 @@ A001.py:9:6: A001 Variable `max` is shadowing a Python builtin 13 | id = 4 | -A001.py:13:1: A001 Variable `bytes` is shadowing a Python builtin +A001.py:13:5: A001 Variable `bytes` is shadowing a Python builtin | -13 | id = 4 -14 | -15 | / def bytes(): -16 | | pass - | |________^ A001 -17 | -18 | class slice: +13 | id = 4 +14 | +15 | def bytes(): + | ^^^^^ A001 +16 | pass | -A001.py:16:1: A001 Variable `slice` is shadowing a Python builtin +A001.py:16:7: A001 Variable `slice` is shadowing a Python builtin | -16 | pass -17 | -18 | / class slice: -19 | | pass - | |________^ A001 -20 | -21 | try: +16 | pass +17 | +18 | class slice: + | ^^^^^ A001 +19 | pass | A001.py:21:1: A001 Variable `ValueError` is shadowing a Python builtin diff --git a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap index 590cda5054..1513b901fd 100644 --- a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap +++ b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py.snap @@ -29,14 +29,13 @@ A003.py:4:5: A003 Class attribute `dir` is shadowing a Python builtin 8 | def __init__(self): | -A003.py:11:5: A003 Class attribute `str` is shadowing a Python builtin +A003.py:11:9: A003 Class attribute `str` is shadowing a Python builtin | -11 | self.dir = "." -12 | -13 | def str(self): - | _____^ -14 | | pass - | |____________^ A003 +11 | self.dir = "." +12 | +13 | def str(self): + | ^^^ A003 +14 | pass | diff --git a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap index 6549678fb9..82c08824fc 100644 --- a/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap +++ b/crates/ruff/src/rules/flake8_builtins/snapshots/ruff__rules__flake8_builtins__tests__A003_A003.py_builtins_ignorelist.snap @@ -10,14 +10,13 @@ A003.py:2:5: A003 Class attribute `ImportError` is shadowing a Python builtin 5 | dir = "/" | -A003.py:11:5: A003 Class attribute `str` is shadowing a Python builtin +A003.py:11:9: A003 Class attribute `str` is shadowing a Python builtin | -11 | self.dir = "." -12 | -13 | def str(self): - | _____^ -14 | | pass - | |____________^ A003 +11 | self.dir = "." +12 | +13 | def str(self): + | ^^^ A003 +14 | pass | diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index d41e9ad4fb..352023a337 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1090,21 +1090,40 @@ pub fn match_parens(start: TextSize, locator: &Locator) -> Option { /// Specifically, this method returns the range of a function or class name, /// rather than that of the entire function or class body. pub fn identifier_range(stmt: &Stmt, locator: &Locator) -> TextRange { - if matches!( - stmt, - Stmt::ClassDef(_) | Stmt::FunctionDef(_) | Stmt::AsyncFunctionDef(_) - ) { - let contents = &locator.contents()[stmt.range()]; + match stmt { + Stmt::ClassDef(ast::StmtClassDef { + decorator_list, + range, + .. + }) + | Stmt::FunctionDef(ast::StmtFunctionDef { + decorator_list, + range, + .. + }) + | Stmt::AsyncFunctionDef(ast::StmtAsyncFunctionDef { + decorator_list, + range, + .. + }) => { + let header_range = decorator_list.last().map_or(*range, |last_decorator| { + TextRange::new(last_decorator.end(), range.end()) + }); - for (tok, range) in lexer::lex_starts_at(contents, Mode::Module, stmt.start()).flatten() { - if matches!(tok, Tok::Name { .. }) { - return range; - } + let contents = locator.slice(header_range); + + let mut tokens = + lexer::lex_starts_at(contents, Mode::Module, header_range.start()).flatten(); + tokens + .find_map(|(t, range)| t.is_name().then_some(range)) + .unwrap_or_else(|| { + error!("Failed to find identifier for {:?}", stmt); + + header_range + }) } - error!("Failed to find identifier for {:?}", stmt); + _ => stmt.range(), } - - stmt.range() } /// Return the ranges of [`Tok::Name`] tokens within a specified node. diff --git a/crates/ruff_python_formatter/src/cst/mod.rs b/crates/ruff_python_formatter/src/cst/mod.rs index ef398673b3..a6e353e917 100644 --- a/crates/ruff_python_formatter/src/cst/mod.rs +++ b/crates/ruff_python_formatter/src/cst/mod.rs @@ -953,12 +953,7 @@ impl From<(ast::Stmt, &Locator<'_>)> for Stmt { }; Stmt { - range: TextRange::new( - decorator_list - .first() - .map_or(range.start(), ast::Ranged::start), - body.end(), - ), + range: TextRange::new(range.start(), body.end()), node: StmtKind::FunctionDef { name: name.into(), args: Box::new((*args, locator).into()), @@ -999,12 +994,7 @@ impl From<(ast::Stmt, &Locator<'_>)> for Stmt { }; Stmt { - range: TextRange::new( - decorator_list - .first() - .map_or(range.start(), |expr| expr.range().start()), - body.end(), - ), + range: TextRange::new(range.start(), body.end()), node: StmtKind::AsyncFunctionDef { name: name.into(), args: Box::new((*args, locator).into()), diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 53092906c0..b8a8ad24c3 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -1,7 +1,10 @@ use std::num::TryFromIntError; use std::ops::{Deref, Index, IndexMut}; +use crate::model::SemanticModel; use bitflags::bitflags; +use ruff_python_ast::helpers; +use ruff_python_ast::source_code::Locator; use ruff_text_size::TextRange; use crate::node::NodeId; @@ -108,6 +111,18 @@ impl<'a> Binding<'a> { } existing.is_definition() } + + /// Returns the appropriate visual range for highlighting this binding. + pub fn trimmed_range(&self, semantic_model: &SemanticModel, locator: &Locator) -> TextRange { + match self.kind { + BindingKind::ClassDefinition | BindingKind::FunctionDefinition => { + self.source.map_or(self.range, |source| { + helpers::identifier_range(semantic_model.stmts[source], locator) + }) + } + _ => self.range, + } + } } /// ID uniquely identifying a [Binding] in a program.