mirror of
				https://github.com/astral-sh/ruff.git
				synced 2025-10-31 12:05:57 +00:00 
			
		
		
		
	[ruff] Move Insertion abstraction out of ruff_linter
				
					
				
			This refactors the importer abstraction to use a shared `Insertion`. This is mostly just moving some code around with some slight tweaks. The plan here is to keep the rest of the importing code in `ruff_linter` and then write something ty-specific on top of `Insertion`. This ends up sharing some code, but not as much as would be ideal. In particular, the `ruff_linter` imported is pretty tightly coupled with ruff's semantic model. So to share the code, we'd need to abstract over that.
This commit is contained in:
		
							parent
							
								
									ec2720c814
								
							
						
					
					
						commit
						880a867696
					
				
					 7 changed files with 81 additions and 41 deletions
				
			
		
							
								
								
									
										15
									
								
								Cargo.lock
									
										
									
										generated
									
									
									
								
							
							
						
						
									
										15
									
								
								Cargo.lock
									
										
									
										generated
									
									
									
								
							|  | @ -3016,6 +3016,7 @@ dependencies = [ | ||||||
|  "ruff_notebook", |  "ruff_notebook", | ||||||
|  "ruff_python_ast", |  "ruff_python_ast", | ||||||
|  "ruff_python_codegen", |  "ruff_python_codegen", | ||||||
|  |  "ruff_python_importer", | ||||||
|  "ruff_python_index", |  "ruff_python_index", | ||||||
|  "ruff_python_literal", |  "ruff_python_literal", | ||||||
|  "ruff_python_parser", |  "ruff_python_parser", | ||||||
|  | @ -3166,6 +3167,20 @@ dependencies = [ | ||||||
|  "tracing", |  "tracing", | ||||||
| ] | ] | ||||||
| 
 | 
 | ||||||
|  | [[package]] | ||||||
|  | name = "ruff_python_importer" | ||||||
|  | version = "0.0.0" | ||||||
|  | dependencies = [ | ||||||
|  |  "anyhow", | ||||||
|  |  "ruff_diagnostics", | ||||||
|  |  "ruff_python_ast", | ||||||
|  |  "ruff_python_codegen", | ||||||
|  |  "ruff_python_parser", | ||||||
|  |  "ruff_python_trivia", | ||||||
|  |  "ruff_source_file", | ||||||
|  |  "ruff_text_size", | ||||||
|  | ] | ||||||
|  | 
 | ||||||
| [[package]] | [[package]] | ||||||
| name = "ruff_python_index" | name = "ruff_python_index" | ||||||
| version = "0.0.0" | version = "0.0.0" | ||||||
|  |  | ||||||
|  | @ -29,6 +29,7 @@ ruff_options_metadata = { path = "crates/ruff_options_metadata" } | ||||||
| ruff_python_ast = { path = "crates/ruff_python_ast" } | ruff_python_ast = { path = "crates/ruff_python_ast" } | ||||||
| ruff_python_codegen = { path = "crates/ruff_python_codegen" } | ruff_python_codegen = { path = "crates/ruff_python_codegen" } | ||||||
| ruff_python_formatter = { path = "crates/ruff_python_formatter" } | ruff_python_formatter = { path = "crates/ruff_python_formatter" } | ||||||
|  | ruff_python_importer = { path = "crates/ruff_python_importer" } | ||||||
| ruff_python_index = { path = "crates/ruff_python_index" } | ruff_python_index = { path = "crates/ruff_python_index" } | ||||||
| ruff_python_literal = { path = "crates/ruff_python_literal" } | ruff_python_literal = { path = "crates/ruff_python_literal" } | ||||||
| ruff_python_parser = { path = "crates/ruff_python_parser" } | ruff_python_parser = { path = "crates/ruff_python_parser" } | ||||||
|  |  | ||||||
|  | @ -20,6 +20,7 @@ ruff_notebook = { workspace = true } | ||||||
| ruff_macros = { workspace = true } | ruff_macros = { workspace = true } | ||||||
| ruff_python_ast = { workspace = true, features = ["serde", "cache"] } | ruff_python_ast = { workspace = true, features = ["serde", "cache"] } | ||||||
| ruff_python_codegen = { workspace = true } | ruff_python_codegen = { workspace = true } | ||||||
|  | ruff_python_importer = { workspace = true } | ||||||
| ruff_python_index = { workspace = true } | ruff_python_index = { workspace = true } | ||||||
| ruff_python_literal = { workspace = true } | ruff_python_literal = { workspace = true } | ||||||
| ruff_python_semantic = { workspace = true } | ruff_python_semantic = { workspace = true } | ||||||
|  |  | ||||||
|  | @ -8,8 +8,10 @@ use std::error::Error; | ||||||
| use anyhow::Result; | use anyhow::Result; | ||||||
| use libcst_native as cst; | use libcst_native as cst; | ||||||
| 
 | 
 | ||||||
|  | use ruff_diagnostics::Edit; | ||||||
| use ruff_python_ast::{self as ast, Expr, ModModule, Stmt}; | use ruff_python_ast::{self as ast, Expr, ModModule, Stmt}; | ||||||
| use ruff_python_codegen::Stylist; | use ruff_python_codegen::Stylist; | ||||||
|  | use ruff_python_importer::Insertion; | ||||||
| use ruff_python_parser::{Parsed, Tokens}; | use ruff_python_parser::{Parsed, Tokens}; | ||||||
| use ruff_python_semantic::{ | use ruff_python_semantic::{ | ||||||
|     ImportedName, MemberNameImport, ModuleNameImport, NameImport, SemanticModel, |     ImportedName, MemberNameImport, ModuleNameImport, NameImport, SemanticModel, | ||||||
|  | @ -17,14 +19,10 @@ use ruff_python_semantic::{ | ||||||
| use ruff_python_trivia::textwrap::indent; | use ruff_python_trivia::textwrap::indent; | ||||||
| use ruff_text_size::{Ranged, TextSize}; | use ruff_text_size::{Ranged, TextSize}; | ||||||
| 
 | 
 | ||||||
| use crate::Edit; |  | ||||||
| use crate::Locator; | use crate::Locator; | ||||||
| use crate::cst::matchers::{match_aliases, match_import_from, match_statement}; | use crate::cst::matchers::{match_aliases, match_import_from, match_statement}; | ||||||
| use crate::fix; | use crate::fix; | ||||||
| use crate::fix::codemods::CodegenStylist; | use crate::fix::codemods::CodegenStylist; | ||||||
| use crate::importer::insertion::Insertion; |  | ||||||
| 
 |  | ||||||
| mod insertion; |  | ||||||
| 
 | 
 | ||||||
| pub(crate) struct Importer<'a> { | pub(crate) struct Importer<'a> { | ||||||
|     /// The Python AST to which we are adding imports.
 |     /// The Python AST to which we are adding imports.
 | ||||||
|  | @ -507,7 +505,7 @@ impl<'a> Importer<'a> { | ||||||
| 
 | 
 | ||||||
|     /// Add an import statement to an existing `TYPE_CHECKING` block.
 |     /// Add an import statement to an existing `TYPE_CHECKING` block.
 | ||||||
|     fn add_to_type_checking_block(&self, content: &str, at: TextSize) -> Edit { |     fn add_to_type_checking_block(&self, content: &str, at: TextSize) -> Edit { | ||||||
|         Insertion::start_of_block(at, self.locator, self.stylist, self.tokens).into_edit(content) |         Insertion::start_of_block(at, self.source, self.stylist, self.tokens).into_edit(content) | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     /// Return the import statement that precedes the given position, if any.
 |     /// Return the import statement that precedes the given position, if any.
 | ||||||
|  |  | ||||||
							
								
								
									
										29
									
								
								crates/ruff_python_importer/Cargo.toml
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										29
									
								
								crates/ruff_python_importer/Cargo.toml
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,29 @@ | ||||||
|  | [package] | ||||||
|  | name = "ruff_python_importer" | ||||||
|  | version = "0.0.0" | ||||||
|  | publish = false | ||||||
|  | authors = { workspace = true } | ||||||
|  | edition = { workspace = true } | ||||||
|  | rust-version = { workspace = true } | ||||||
|  | homepage = { workspace = true } | ||||||
|  | documentation = { workspace = true } | ||||||
|  | repository = { workspace = true } | ||||||
|  | license = { workspace = true } | ||||||
|  | 
 | ||||||
|  | [dependencies] | ||||||
|  | ruff_diagnostics = { workspace = true } | ||||||
|  | ruff_python_ast = { workspace = true } | ||||||
|  | ruff_python_codegen = { workspace = true } | ||||||
|  | ruff_python_parser = { workspace = true } | ||||||
|  | ruff_python_trivia = { workspace = true } | ||||||
|  | ruff_source_file = { workspace = true, features = ["serde"] } | ||||||
|  | ruff_text_size = { workspace = true } | ||||||
|  | 
 | ||||||
|  | anyhow = { workspace = true } | ||||||
|  | 
 | ||||||
|  | [dev-dependencies] | ||||||
|  | 
 | ||||||
|  | [features] | ||||||
|  | 
 | ||||||
|  | [lints] | ||||||
|  | workspace = true | ||||||
|  | @ -1,6 +1,8 @@ | ||||||
| //! Insert statements into Python code.
 | //! Insert statements into Python code.
 | ||||||
|  | 
 | ||||||
| use std::ops::Add; | use std::ops::Add; | ||||||
| 
 | 
 | ||||||
|  | use ruff_diagnostics::Edit; | ||||||
| use ruff_python_ast::Stmt; | use ruff_python_ast::Stmt; | ||||||
| use ruff_python_ast::helpers::is_docstring_stmt; | use ruff_python_ast::helpers::is_docstring_stmt; | ||||||
| use ruff_python_codegen::Stylist; | use ruff_python_codegen::Stylist; | ||||||
|  | @ -10,9 +12,6 @@ use ruff_python_trivia::{PythonWhitespace, textwrap::indent}; | ||||||
| use ruff_source_file::{LineRanges, UniversalNewlineIterator}; | use ruff_source_file::{LineRanges, UniversalNewlineIterator}; | ||||||
| use ruff_text_size::{Ranged, TextSize}; | use ruff_text_size::{Ranged, TextSize}; | ||||||
| 
 | 
 | ||||||
| use crate::Edit; |  | ||||||
| use crate::Locator; |  | ||||||
| 
 |  | ||||||
| #[derive(Debug, Clone, PartialEq, Eq)] | #[derive(Debug, Clone, PartialEq, Eq)] | ||||||
| pub(super) enum Placement<'a> { | pub(super) enum Placement<'a> { | ||||||
|     /// The content will be inserted inline with the existing code (i.e., within semicolon-delimited
 |     /// The content will be inserted inline with the existing code (i.e., within semicolon-delimited
 | ||||||
|  | @ -25,7 +24,7 @@ pub(super) enum Placement<'a> { | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| #[derive(Debug, Clone, PartialEq, Eq)] | #[derive(Debug, Clone, PartialEq, Eq)] | ||||||
| pub(super) struct Insertion<'a> { | pub struct Insertion<'a> { | ||||||
|     /// The content to add before the insertion.
 |     /// The content to add before the insertion.
 | ||||||
|     prefix: &'a str, |     prefix: &'a str, | ||||||
|     /// The location at which to insert.
 |     /// The location at which to insert.
 | ||||||
|  | @ -50,33 +49,31 @@ impl<'a> Insertion<'a> { | ||||||
|     ///
 |     ///
 | ||||||
|     /// The insertion returned will begin at the start of the `import os` statement, and will
 |     /// The insertion returned will begin at the start of the `import os` statement, and will
 | ||||||
|     /// include a trailing newline.
 |     /// include a trailing newline.
 | ||||||
|     pub(super) fn start_of_file( |     pub fn start_of_file(body: &[Stmt], contents: &str, stylist: &Stylist) -> Insertion<'static> { | ||||||
|         body: &[Stmt], |  | ||||||
|         locator: &Locator, |  | ||||||
|         stylist: &Stylist, |  | ||||||
|     ) -> Insertion<'static> { |  | ||||||
|         // Skip over any docstrings.
 |         // Skip over any docstrings.
 | ||||||
|         let mut location = if let Some(mut location) = match_docstring_end(body) { |         let mut location = if let Some(mut location) = match_docstring_end(body) { | ||||||
|             // If the first token after the docstring is a semicolon, insert after the semicolon as
 |             // If the first token after the docstring is a semicolon, insert after the semicolon as
 | ||||||
|             // an inline statement.
 |             // an inline statement.
 | ||||||
|             if let Some(offset) = match_semicolon(locator.after(location)) { |             if let Some(offset) = match_semicolon(&contents[location.to_usize()..]) { | ||||||
|                 return Insertion::inline(" ", location.add(offset).add(TextSize::of(';')), ";"); |                 return Insertion::inline(" ", location.add(offset).add(TextSize::of(';')), ";"); | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|             // While the first token after the docstring is a continuation character (i.e. "\"), advance
 |             // While the first token after the docstring is a continuation character (i.e. "\"), advance
 | ||||||
|             // additional rows to prevent inserting in the same logical line.
 |             // additional rows to prevent inserting in the same logical line.
 | ||||||
|             while match_continuation(locator.after(location)).is_some() { |             while match_continuation(&contents[location.to_usize()..]).is_some() { | ||||||
|                 location = locator.full_line_end(location); |                 location = contents.full_line_end(location); | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|             // Otherwise, advance to the next row.
 |             // Otherwise, advance to the next row.
 | ||||||
|             locator.full_line_end(location) |             contents.full_line_end(location) | ||||||
|         } else { |         } else { | ||||||
|             locator.bom_start_offset() |             contents.bom_start_offset() | ||||||
|         }; |         }; | ||||||
| 
 | 
 | ||||||
|         // Skip over commented lines, with whitespace separation.
 |         // Skip over commented lines, with whitespace separation.
 | ||||||
|         for line in UniversalNewlineIterator::with_offset(locator.after(location), location) { |         for line in | ||||||
|  |             UniversalNewlineIterator::with_offset(&contents[location.to_usize()..], location) | ||||||
|  |         { | ||||||
|             let trimmed_line = line.trim_whitespace_start(); |             let trimmed_line = line.trim_whitespace_start(); | ||||||
|             if trimmed_line.is_empty() { |             if trimmed_line.is_empty() { | ||||||
|                 continue; |                 continue; | ||||||
|  | @ -111,17 +108,13 @@ impl<'a> Insertion<'a> { | ||||||
|     /// in this case is the line after `import math`, and will include a trailing newline.
 |     /// in this case is the line after `import math`, and will include a trailing newline.
 | ||||||
|     ///
 |     ///
 | ||||||
|     /// The statement itself is assumed to be at the top-level of the module.
 |     /// The statement itself is assumed to be at the top-level of the module.
 | ||||||
|     pub(super) fn end_of_statement( |     pub fn end_of_statement(stmt: &Stmt, contents: &str, stylist: &Stylist) -> Insertion<'static> { | ||||||
|         stmt: &Stmt, |  | ||||||
|         locator: &Locator, |  | ||||||
|         stylist: &Stylist, |  | ||||||
|     ) -> Insertion<'static> { |  | ||||||
|         let location = stmt.end(); |         let location = stmt.end(); | ||||||
|         if let Some(offset) = match_semicolon(locator.after(location)) { |         if let Some(offset) = match_semicolon(&contents[location.to_usize()..]) { | ||||||
|             // If the first token after the statement is a semicolon, insert after the semicolon as
 |             // If the first token after the statement is a semicolon, insert after the semicolon as
 | ||||||
|             // an inline statement.
 |             // an inline statement.
 | ||||||
|             Insertion::inline(" ", location.add(offset).add(TextSize::of(';')), ";") |             Insertion::inline(" ", location.add(offset).add(TextSize::of(';')), ";") | ||||||
|         } else if match_continuation(locator.after(location)).is_some() { |         } else if match_continuation(&contents[location.to_usize()..]).is_some() { | ||||||
|             // If the first token after the statement is a continuation, insert after the statement
 |             // If the first token after the statement is a continuation, insert after the statement
 | ||||||
|             // with a semicolon.
 |             // with a semicolon.
 | ||||||
|             Insertion::inline("; ", location, "") |             Insertion::inline("; ", location, "") | ||||||
|  | @ -129,7 +122,7 @@ impl<'a> Insertion<'a> { | ||||||
|             // Otherwise, insert on the next line.
 |             // Otherwise, insert on the next line.
 | ||||||
|             Insertion::own_line( |             Insertion::own_line( | ||||||
|                 "", |                 "", | ||||||
|                 locator.full_line_end(location), |                 contents.full_line_end(location), | ||||||
|                 stylist.line_ending().as_str(), |                 stylist.line_ending().as_str(), | ||||||
|             ) |             ) | ||||||
|         } |         } | ||||||
|  | @ -149,9 +142,9 @@ impl<'a> Insertion<'a> { | ||||||
|     /// include a trailing newline.
 |     /// include a trailing newline.
 | ||||||
|     ///
 |     ///
 | ||||||
|     /// The block itself is assumed to be at the top-level of the module.
 |     /// The block itself is assumed to be at the top-level of the module.
 | ||||||
|     pub(super) fn start_of_block( |     pub fn start_of_block( | ||||||
|         mut location: TextSize, |         mut location: TextSize, | ||||||
|         locator: &Locator<'a>, |         contents: &'a str, | ||||||
|         stylist: &Stylist, |         stylist: &Stylist, | ||||||
|         tokens: &Tokens, |         tokens: &Tokens, | ||||||
|     ) -> Insertion<'a> { |     ) -> Insertion<'a> { | ||||||
|  | @ -204,7 +197,7 @@ impl<'a> Insertion<'a> { | ||||||
|                             "", |                             "", | ||||||
|                             token.start(), |                             token.start(), | ||||||
|                             stylist.line_ending().as_str(), |                             stylist.line_ending().as_str(), | ||||||
|                             locator.slice(token), |                             &contents[token.range()], | ||||||
|                         ); |                         ); | ||||||
|                     } |                     } | ||||||
|                     _ => { |                     _ => { | ||||||
|  | @ -220,7 +213,7 @@ impl<'a> Insertion<'a> { | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     /// Convert this [`Insertion`] into an [`Edit`] that inserts the given content.
 |     /// Convert this [`Insertion`] into an [`Edit`] that inserts the given content.
 | ||||||
|     pub(super) fn into_edit(self, content: &str) -> Edit { |     pub fn into_edit(self, content: &str) -> Edit { | ||||||
|         let Insertion { |         let Insertion { | ||||||
|             prefix, |             prefix, | ||||||
|             location, |             location, | ||||||
|  | @ -240,7 +233,7 @@ impl<'a> Insertion<'a> { | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     /// Returns `true` if this [`Insertion`] is inline.
 |     /// Returns `true` if this [`Insertion`] is inline.
 | ||||||
|     pub(super) fn is_inline(&self) -> bool { |     pub fn is_inline(&self) -> bool { | ||||||
|         matches!(self.placement, Placement::Inline) |         matches!(self.placement, Placement::Inline) | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  | @ -323,17 +316,14 @@ mod tests { | ||||||
|     use ruff_source_file::LineEnding; |     use ruff_source_file::LineEnding; | ||||||
|     use ruff_text_size::TextSize; |     use ruff_text_size::TextSize; | ||||||
| 
 | 
 | ||||||
|     use crate::Locator; |  | ||||||
| 
 |  | ||||||
|     use super::Insertion; |     use super::Insertion; | ||||||
| 
 | 
 | ||||||
|     #[test] |     #[test] | ||||||
|     fn start_of_file() -> Result<()> { |     fn start_of_file() -> Result<()> { | ||||||
|         fn insert(contents: &str) -> Result<Insertion<'_>> { |         fn insert(contents: &str) -> Result<Insertion<'_>> { | ||||||
|             let parsed = parse_module(contents)?; |             let parsed = parse_module(contents)?; | ||||||
|             let locator = Locator::new(contents); |             let stylist = Stylist::from_tokens(parsed.tokens(), contents); | ||||||
|             let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); |             Ok(Insertion::start_of_file(parsed.suite(), contents, &stylist)) | ||||||
|             Ok(Insertion::start_of_file(parsed.suite(), &locator, &stylist)) |  | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         let contents = ""; |         let contents = ""; | ||||||
|  | @ -463,9 +453,8 @@ x = 1 | ||||||
|     fn start_of_block() { |     fn start_of_block() { | ||||||
|         fn insert(contents: &str, offset: TextSize) -> Insertion<'_> { |         fn insert(contents: &str, offset: TextSize) -> Insertion<'_> { | ||||||
|             let parsed = parse_module(contents).unwrap(); |             let parsed = parse_module(contents).unwrap(); | ||||||
|             let locator = Locator::new(contents); |             let stylist = Stylist::from_tokens(parsed.tokens(), contents); | ||||||
|             let stylist = Stylist::from_tokens(parsed.tokens(), locator.contents()); |             Insertion::start_of_block(offset, contents, &stylist, parsed.tokens()) | ||||||
|             Insertion::start_of_block(offset, &locator, &stylist, parsed.tokens()) |  | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         let contents = "if True: pass"; |         let contents = "if True: pass"; | ||||||
							
								
								
									
										7
									
								
								crates/ruff_python_importer/src/lib.rs
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										7
									
								
								crates/ruff_python_importer/src/lib.rs
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,7 @@ | ||||||
|  | /*! | ||||||
|  | Low-level helpers for manipulating Python import statements. | ||||||
|  | */ | ||||||
|  | 
 | ||||||
|  | pub use self::insertion::Insertion; | ||||||
|  | 
 | ||||||
|  | mod insertion; | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andrew Gallant
						Andrew Gallant