From 5816985ecd51a29bc17bc1b7553ff659ad1b9172 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Wed, 17 Sep 2025 10:40:54 -0400 Subject: [PATCH] [ruff] Remove `Locator` from `Importer` It seems like we'd like to remove `Locator` since it's a bit awkward in how it works: https://github.com/astral-sh/ruff/pull/20439#discussion_r2354683797 It looked pretty easy to rip it out of the `Importer`, so that's one less thing using it. --- Cargo.lock | 1 + crates/ruff_linter/src/checkers/ast/mod.rs | 2 +- crates/ruff_linter/src/fix/codemods.rs | 5 +-- crates/ruff_linter/src/importer/mod.rs | 34 +++++++++---------- .../rules/isort/rules/add_required_imports.rs | 3 +- crates/ty_ide/Cargo.toml | 1 + 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 25bbf438fa..d5481193fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4234,6 +4234,7 @@ dependencies = [ "rayon", "regex", "ruff_db", + "ruff_diagnostics", "ruff_index", "ruff_memory_usage", "ruff_python_ast", diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 8dfe10a0ac..2434ae19cb 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -277,7 +277,7 @@ impl<'a> Checker<'a> { locator, stylist, indexer, - importer: Importer::new(parsed, locator, stylist), + importer: Importer::new(parsed, locator.contents(), stylist), semantic, visit: deferred::Visit::default(), analyze: deferred::Analyze::default(), diff --git a/crates/ruff_linter/src/fix/codemods.rs b/crates/ruff_linter/src/fix/codemods.rs index 29db53fed1..aca0a8f0c1 100644 --- a/crates/ruff_linter/src/fix/codemods.rs +++ b/crates/ruff_linter/src/fix/codemods.rs @@ -14,6 +14,7 @@ use unicode_normalization::UnicodeNormalization; use ruff_python_ast::Stmt; use ruff_python_ast::name::UnqualifiedName; use ruff_python_codegen::Stylist; +use ruff_text_size::Ranged; use crate::Locator; use crate::cst::matchers::match_statement; @@ -127,10 +128,10 @@ pub(crate) fn remove_imports<'a>( pub(crate) fn retain_imports( member_names: &[&str], stmt: &Stmt, - locator: &Locator, + contents: &str, stylist: &Stylist, ) -> Result { - let module_text = locator.slice(stmt); + let module_text = &contents[stmt.range()]; let mut tree = match_statement(module_text)?; let Statement::Simple(body) = &mut tree else { diff --git a/crates/ruff_linter/src/importer/mod.rs b/crates/ruff_linter/src/importer/mod.rs index 9b056ea770..ec8745e0d7 100644 --- a/crates/ruff_linter/src/importer/mod.rs +++ b/crates/ruff_linter/src/importer/mod.rs @@ -19,7 +19,6 @@ use ruff_python_semantic::{ use ruff_python_trivia::textwrap::indent; use ruff_text_size::{Ranged, TextSize}; -use crate::Locator; use crate::cst::matchers::{match_aliases, match_import_from, match_statement}; use crate::fix; use crate::fix::codemods::CodegenStylist; @@ -29,8 +28,8 @@ pub(crate) struct Importer<'a> { python_ast: &'a [Stmt], /// The tokens representing the Python AST. tokens: &'a Tokens, - /// The [`Locator`] for the Python AST. - locator: &'a Locator<'a>, + /// The source code text for `python_ast`. + source: &'a str, /// The [`Stylist`] for the Python AST. stylist: &'a Stylist<'a>, /// The list of visited, top-level runtime imports in the Python AST. @@ -42,13 +41,13 @@ pub(crate) struct Importer<'a> { impl<'a> Importer<'a> { pub(crate) fn new( parsed: &'a Parsed, - locator: &'a Locator<'a>, + source: &'a str, stylist: &'a Stylist<'a>, ) -> Self { Self { python_ast: parsed.suite(), tokens: parsed.tokens(), - locator, + source, stylist, runtime_imports: Vec::default(), type_checking_blocks: Vec::default(), @@ -74,11 +73,10 @@ impl<'a> Importer<'a> { let required_import = import.to_string(); if let Some(stmt) = self.preceding_import(at) { // Insert after the last top-level import. - Insertion::end_of_statement(stmt, self.locator, self.stylist) - .into_edit(&required_import) + Insertion::end_of_statement(stmt, self.source, self.stylist).into_edit(&required_import) } else { // Insert at the start of the file. - Insertion::start_of_file(self.python_ast, self.locator, self.stylist) + Insertion::start_of_file(self.python_ast, self.source, self.stylist) .into_edit(&required_import) } } @@ -97,17 +95,17 @@ impl<'a> Importer<'a> { let content = fix::codemods::retain_imports( &import.names, import.statement, - self.locator, + self.source, self.stylist, )?; // Add the import to the top-level. let insertion = if let Some(stmt) = self.preceding_import(at) { // Insert after the last top-level import. - Insertion::end_of_statement(stmt, self.locator, self.stylist) + Insertion::end_of_statement(stmt, self.source, self.stylist) } else { // Insert at the start of the file. - Insertion::start_of_file(self.python_ast, self.locator, self.stylist) + Insertion::start_of_file(self.python_ast, self.source, self.stylist) }; let add_import_edit = insertion.into_edit(&content); @@ -129,7 +127,7 @@ impl<'a> Importer<'a> { let content = fix::codemods::retain_imports( &import.names, import.statement, - self.locator, + self.source, self.stylist, )?; @@ -153,7 +151,7 @@ impl<'a> Importer<'a> { None } else { Some(Edit::range_replacement( - self.locator.slice(statement.range()).to_string(), + self.source[statement.range()].to_string(), statement.range(), )) } @@ -184,7 +182,7 @@ impl<'a> Importer<'a> { None } else { Some(Edit::range_replacement( - self.locator.slice(type_checking.range()).to_string(), + self.source[type_checking.range()].to_string(), type_checking.range(), )) }; @@ -360,7 +358,7 @@ impl<'a> Importer<'a> { // By adding this no-op edit, we force the `unused-imports` fix to conflict with the // `sys-exit-alias` fix, and thus will avoid applying both fixes in the same pass. let import_edit = Edit::range_replacement( - self.locator.slice(imported_name.range()).to_string(), + self.source[imported_name.range()].to_string(), imported_name.range(), ); Ok(Some((import_edit, imported_name.into_name()))) @@ -467,7 +465,7 @@ impl<'a> Importer<'a> { /// Add the given member to an existing `Stmt::ImportFrom` statement. fn add_member(&self, stmt: &Stmt, member: &str) -> Result { - let mut statement = match_statement(self.locator.slice(stmt))?; + let mut statement = match_statement(&self.source[stmt.range()])?; let import_from = match_import_from(&mut statement)?; let aliases = match_aliases(import_from)?; aliases.push(cst::ImportAlias { @@ -489,10 +487,10 @@ impl<'a> Importer<'a> { fn add_type_checking_block(&self, content: &str, at: TextSize) -> Result { let insertion = if let Some(stmt) = self.preceding_import(at) { // Insert after the last top-level import. - Insertion::end_of_statement(stmt, self.locator, self.stylist) + Insertion::end_of_statement(stmt, self.source, self.stylist) } else { // Insert at the start of the file. - Insertion::start_of_file(self.python_ast, self.locator, self.stylist) + Insertion::start_of_file(self.python_ast, self.source, self.stylist) }; if insertion.is_inline() { Err(anyhow::anyhow!( diff --git a/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs b/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs index bff7e42137..e674c8fe9a 100644 --- a/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs +++ b/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs @@ -125,7 +125,8 @@ fn add_required_import( TextRange::default(), ); diagnostic.set_fix(Fix::safe_edit( - Importer::new(parsed, locator, stylist).add_import(required_import, TextSize::default()), + Importer::new(parsed, locator.contents(), stylist) + .add_import(required_import, TextSize::default()), )); } diff --git a/crates/ty_ide/Cargo.toml b/crates/ty_ide/Cargo.toml index e0bedc8ee2..3cbb298f9d 100644 --- a/crates/ty_ide/Cargo.toml +++ b/crates/ty_ide/Cargo.toml @@ -13,6 +13,7 @@ license = { workspace = true } [dependencies] bitflags = { workspace = true } ruff_db = { workspace = true } +ruff_diagnostics = { workspace = true } ruff_index = { workspace = true } ruff_memory_usage = { workspace = true } ruff_python_ast = { workspace = true }