From 166b9d18eea59eaf26cb715085ba73bc4f0fee56 Mon Sep 17 00:00:00 2001 From: Christian Aguilera Date: Sun, 28 Sep 2025 17:38:23 +0100 Subject: [PATCH] [`ruff`] Ability to specify `--range` multiple times. ## Summary Text ranges that overlap (or are adjacent) will be collapsed. This is done after text ranges have been adjusted to their enclosing nodes in the tree. Formatting of each range is then performed starting from the bottom of the file, ensuring that the following range still applies to the lines that the user intended to format. Implements #12800. ## Test Plan - Pre-existing unit tests ensure no regression or unexpected behavior change in regular executions with a single `--range` argument. - New unit tests have been added to cover the feature enhancement. --- crates/ruff/src/args.rs | 8 +- crates/ruff/src/cache.rs | 3 +- crates/ruff/src/commands/format.rs | 126 +++++++++++----- crates/ruff/src/commands/format_stdin.rs | 6 +- crates/ruff/tests/cli/format.rs | 172 ++++++++++++++++++++++ crates/ruff_python_formatter/src/lib.rs | 2 +- crates/ruff_python_formatter/src/range.rs | 77 ++++++++++ crates/ruff_text_size/src/range.rs | 37 +++++ 8 files changed, 386 insertions(+), 45 deletions(-) diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index eb4bcd0a92..eb843497fc 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -532,8 +532,10 @@ pub struct FormatCommand { /// - The start position is optional. You can write `--range=-3` to format the first three lines of the document. /// /// The option can only be used when formatting a single file. Range formatting of notebooks is unsupported. + /// + /// The option can be specified multiple times to format more than one block. #[clap(long, help_heading = "Editor options", verbatim_doc_comment)] - pub range: Option, + pub range: Option>, /// Exit with a non-zero status code if any files were modified via format, even if all files were formatted successfully. #[arg(long, help_heading = "Miscellaneous", alias = "exit-non-zero-on-fix")] @@ -780,7 +782,7 @@ impl FormatCommand { files: self.files, no_cache: self.no_cache, stdin_filename: self.stdin_filename, - range: self.range, + ranges: self.range.unwrap_or_default(), exit_non_zero_on_format: self.exit_non_zero_on_format, }; @@ -1071,7 +1073,7 @@ pub struct FormatArguments { pub diff: bool, pub files: Vec, pub stdin_filename: Option, - pub range: Option, + pub ranges: Vec, pub exit_non_zero_on_format: bool, } diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 6b696b55d3..7badecb53f 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -1021,12 +1021,13 @@ mod tests { cache: &Cache, ) -> Result { let file_path = self.package_root.join(path); + let ranges = Vec::new(); format_path( &file_path, &self.settings.formatter, PySourceType::Python, FormatMode::Write, - None, + &ranges, Some(cache), ) } diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 1f79e59339..c4ed6de46f 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -32,7 +32,9 @@ use ruff_linter::rules::flake8_quotes::settings::Quote; use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_linter::warn_user_once; use ruff_python_ast::{PySourceType, SourceType}; -use ruff_python_formatter::{FormatModuleError, QuoteStyle, format_module_source, format_range}; +use ruff_python_formatter::{ + FormatModuleError, QuoteStyle, expand_and_collapse_ranges, format_module_source, format_range, +}; use ruff_source_file::{LineIndex, LineRanges, OneIndexed, SourceFileBuilder}; use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_workspace::FormatterSettings; @@ -84,7 +86,7 @@ pub(crate) fn format( return Ok(ExitStatus::Success); } - if cli.range.is_some() && paths.len() > 1 { + if !cli.ranges.is_empty() && paths.len() > 1 { return Err(anyhow::anyhow!( "The `--range` option is only supported when formatting a single file but the specified paths resolve to {} files.", paths.len() @@ -160,7 +162,7 @@ pub(crate) fn format( &settings.formatter, source_type, mode, - cli.range, + &cli.ranges, cache, ) }) { @@ -263,7 +265,7 @@ pub(crate) fn format_path( settings: &FormatterSettings, source_type: PySourceType, mode: FormatMode, - range: Option, + ranges: &[FormatRange], cache: Option<&Cache>, ) -> Result { if let Some(cache) = cache { @@ -289,20 +291,37 @@ pub(crate) fn format_path( }; // Don't write back to the cache if formatting a range. - let cache = cache.filter(|_| range.is_none()); + let cache = cache.filter(|_| ranges.is_empty()); // Format the source. - let format_result = match format_source(&unformatted, source_type, Some(path), settings, range)? - { - FormattedSource::Formatted(formatted) => match mode { - FormatMode::Write => { - let mut writer = File::create(path).map_err(|err| { - FormatCommandError::Write(Some(path.to_path_buf()), err.into()) - })?; - formatted - .write(&mut writer) - .map_err(|err| FormatCommandError::Write(Some(path.to_path_buf()), err))?; + let format_result = + match format_source(&unformatted, source_type, Some(path), settings, ranges)? { + FormattedSource::Formatted(formatted) => match mode { + FormatMode::Write => { + let mut writer = File::create(path).map_err(|err| { + FormatCommandError::Write(Some(path.to_path_buf()), err.into()) + })?; + formatted + .write(&mut writer) + .map_err(|err| FormatCommandError::Write(Some(path.to_path_buf()), err))?; + if let Some(cache) = cache { + if let Ok(cache_key) = FileCacheKey::from_path(path) { + let relative_path = cache + .relative_path(path) + .expect("wrong package cache for file"); + cache.set_formatted(relative_path.to_path_buf(), &cache_key); + } + } + + FormatResult::Formatted + } + FormatMode::Check | FormatMode::Diff => FormatResult::Diff { + unformatted, + formatted, + }, + }, + FormattedSource::Unchanged => { if let Some(cache) = cache { if let Ok(cache_key) = FileCacheKey::from_path(path) { let relative_path = cache @@ -312,26 +331,9 @@ pub(crate) fn format_path( } } - FormatResult::Formatted + FormatResult::Unchanged } - FormatMode::Check | FormatMode::Diff => FormatResult::Diff { - unformatted, - formatted, - }, - }, - FormattedSource::Unchanged => { - if let Some(cache) = cache { - if let Ok(cache_key) = FileCacheKey::from_path(path) { - let relative_path = cache - .relative_path(path) - .expect("wrong package cache for file"); - cache.set_formatted(relative_path.to_path_buf(), &cache_key); - } - } - - FormatResult::Unchanged - } - }; + }; Ok(format_result) } @@ -360,13 +362,14 @@ pub(crate) fn format_source( source_type: PySourceType, path: Option<&Path>, settings: &FormatterSettings, - range: Option, + ranges: &[FormatRange], ) -> Result { match &source_kind { SourceKind::Python(unformatted) => { let options = settings.to_format_options(source_type, unformatted, path); - let formatted = if let Some(range) = range { + let formatted = if ranges.len() == 1 { + let range = &ranges[0]; let line_index = LineIndex::from_source_text(unformatted); let byte_range = range.to_text_range(unformatted, &line_index); format_range(unformatted, byte_range, options).map(|formatted_range| { @@ -378,6 +381,55 @@ pub(crate) fn format_source( formatted }) + } else if !ranges.is_empty() { + // Convert to text ranges, and expand and collapse. + let line_index = LineIndex::from_source_text(unformatted); + let byte_ranges: Vec = ranges + .iter() + .map(|range| range.to_text_range(unformatted, &line_index)) + .collect(); + let treated_ranges: Vec = + expand_and_collapse_ranges(unformatted, &byte_ranges, options.clone()) + .map_err(|err| { + if let FormatModuleError::ParseError(err) = err { + DisplayParseError::from_source_kind( + err, + path.map(Path::to_path_buf), + source_kind, + ) + .into() + } else { + FormatCommandError::Format(path.map(Path::to_path_buf), err) + } + })?; + + let mut result = Ok(String::new()); + + let mut unformatted = unformatted.clone(); + + // Format in each text range starting from the bottom towards the top. + for byte_range in treated_ranges.iter().rev() { + result = format_range(&unformatted, *byte_range, options.clone()).map( + |formatted_range| { + let mut formatted = unformatted.to_string(); + formatted.replace_range( + std::ops::Range::::from(formatted_range.source_range()), + formatted_range.as_code(), + ); + + unformatted.clone_from(&formatted); + + formatted + }, + ); + + // Formatting will be halted on first error. + if result.is_err() { + break; + } + } + + result } else { // Using `Printed::into_code` requires adding `ruff_formatter` as a direct dependency, and I suspect that Rust can optimize the closure away regardless. #[expect(clippy::redundant_closure_for_method_calls)] @@ -408,7 +460,7 @@ pub(crate) fn format_source( return Ok(FormattedSource::Unchanged); } - if range.is_some() { + if !ranges.is_empty() { return Err(FormatCommandError::RangeFormatNotebook( path.map(Path::to_path_buf), )); diff --git a/crates/ruff/src/commands/format_stdin.rs b/crates/ruff/src/commands/format_stdin.rs index 015f32fba9..7f32c11894 100644 --- a/crates/ruff/src/commands/format_stdin.rs +++ b/crates/ruff/src/commands/format_stdin.rs @@ -65,7 +65,7 @@ pub(crate) fn format_stdin( }; // Format the file. - match format_source_code(path, cli.range, settings, source_type, mode) { + match format_source_code(path, &cli.ranges, settings, source_type, mode) { Ok(result) => match mode { FormatMode::Write => Ok(ExitStatus::Success), FormatMode::Check | FormatMode::Diff => { @@ -86,7 +86,7 @@ pub(crate) fn format_stdin( /// Format source code read from `stdin`. fn format_source_code( path: Option<&Path>, - range: Option, + ranges: &[FormatRange], settings: &FormatterSettings, source_type: PySourceType, mode: FormatMode, @@ -104,7 +104,7 @@ fn format_source_code( }; // Format the source. - let formatted = format_source(&source_kind, source_type, path, settings, range)?; + let formatted = format_source(&source_kind, source_type, path, settings, ranges)?; match &formatted { FormattedSource::Formatted(formatted) => match mode { diff --git a/crates/ruff/tests/cli/format.rs b/crates/ruff/tests/cli/format.rs index 7ab9e59f26..c54522e329 100644 --- a/crates/ruff/tests/cli/format.rs +++ b/crates/ruff/tests/cli/format.rs @@ -2158,6 +2158,178 @@ def foo(arg1, arg2,): Ok(()) } +#[test] +fn multiple_ranges() -> Result<()> { + let test = CliTest::new()?; + assert_cmd_snapshot!(test.format_command() + .args(["--isolated", "--stdin-filename", "test.py", "--range=7-9", "--range=2-3", "--range=4-6"]) + .arg("-") + .pass_stdin(r#" +a = [2 ] +b = [3 ] +c = [4 ] +d = [5 ] +e = [6 ] +f = [7 ] +g = [8 ] +h = [9 ] +"#), @r#" + success: true + exit_code: 0 + ----- stdout ----- + + a = [2] + b = [3 ] + c = [4] + d = [5] + e = [6 ] + f = [7] + g = [8] + h = [9 ] + + ----- stderr ----- + "#); + Ok(()) +} + +#[test] +fn multiple_overlapping_ranges() -> Result<()> { + let test = CliTest::new()?; + assert_cmd_snapshot!(test.format_command() + .args(["--isolated", "--stdin-filename", "test.py", "--range=5-9", "--range=5-8", "--range=4-6"]) + .arg("-") + .pass_stdin(r#" +a = [2 ] +b = [3 ] +c = [4 ] +d = [5 ] +e = [6 ] +f = [7 ] +g = [8 ] +h = [9 ] +"#), @r#" + success: true + exit_code: 0 + ----- stdout ----- + + a = [2 ] + b = [3 ] + c = [4] + d = [5] + e = [6] + f = [7] + g = [8] + h = [9 ] + + ----- stderr ----- + "#); + Ok(()) +} + +#[test] +fn multiple_ranges_in_same_statement() -> Result<()> { + let test = CliTest::new()?; + assert_cmd_snapshot!(test.format_command() + .args(["--isolated", "--stdin-filename", "test.py", "--range=3-4", "--range=6-7"]) + .arg("-") + .pass_stdin(r#" +a = (2, + 3 , + 4, +5, + 6 , + 7, + 8, + 9 +) +b = ('a', 'b', 'c', 'd',) +"#), @r#" + success: true + exit_code: 0 + ----- stdout ----- + + a = (2, 3, 4, 5, 6, 7, 8, 9) + b = ('a', 'b', 'c', 'd',) + + ----- stderr ----- + "#); + Ok(()) +} + +#[test] +fn multiple_overlapping_ranges_in_adjacent_statement() -> Result<()> { + let test = CliTest::new()?; + assert_cmd_snapshot!(test.format_command() + .args(["--isolated", "--stdin-filename", "test.py", "--range=2-3", "--range=4-7", "--range=11-12:35", "--range=12:74-17", "--range=21-22", "--range=29-30", "--range=25-30"]) + .arg("-") + .pass_stdin(r#" +def partition(array, begin, + end): + pivot = begin + for i in range( begin+1, + + end + 1): + if array[i] <= array[begin]: + pivot += 1 + array[i], array[ pivot ] = array[ + pivot], array[ i] + array[pivot], array[begin ] = array[begin + + ], array[ + + pivot + + + + + + + ] + return pivot + + + +def quicksort(array, begin=0, end=None): + if end is None: + end = len( array) - 1 + def _quicksort(array, begin , end): + if begin >= end: + return + pivot = partition(array, begin, end) + _quicksort(array, begin, pivot-1) + _quicksort(array, pivot+1, end) + return _quicksort(array, begin, end) +"#), @r#" + success: true + exit_code: 0 + ----- stdout ----- + + def partition(array, begin, end): + pivot = begin + for i in range(begin + 1, end + 1): + if array[i] <= array[begin]: + pivot += 1 + array[i], array[pivot] = array[pivot], array[i] + array[pivot], array[begin] = array[begin], array[pivot] + return pivot + + + def quicksort(array, begin=0, end=None): + if end is None: + end = len( array) - 1 + def _quicksort(array, begin , end): + if begin >= end: + return + pivot = partition(array, begin, end) + _quicksort(array, begin, pivot-1) + _quicksort(array, pivot+1, end) + return _quicksort(array, begin, end) + + ----- stderr ----- + "#); + Ok(()) +} + #[test] fn range_missing_line() -> Result<()> { let test = CliTest::new()?; diff --git a/crates/ruff_python_formatter/src/lib.rs b/crates/ruff_python_formatter/src/lib.rs index e6b2f9e7b8..d30352d493 100644 --- a/crates/ruff_python_formatter/src/lib.rs +++ b/crates/ruff_python_formatter/src/lib.rs @@ -5,7 +5,7 @@ use ruff_db::source::source_text; use thiserror::Error; use tracing::Level; -pub use range::format_range; +pub use range::{expand_and_collapse_ranges, format_range}; use ruff_formatter::prelude::*; use ruff_formatter::{FormatError, Formatted, PrintError, Printed, SourceCode, format, write}; use ruff_python_ast::{AnyNodeRef, Mod}; diff --git a/crates/ruff_python_formatter/src/range.rs b/crates/ruff_python_formatter/src/range.rs index 436c1a12c2..dfd02995f7 100644 --- a/crates/ruff_python_formatter/src/range.rs +++ b/crates/ruff_python_formatter/src/range.rs @@ -117,6 +117,83 @@ pub fn format_range( Ok(printed.slice_range(narrowed_range, source)) } +/// Expands the text ranges into their enclosing nodes, and collapses those that overlap or touch. +pub fn expand_and_collapse_ranges( + source: &str, + ranges: &[TextRange], + options: PyFormatOptions, +) -> Result, FormatModuleError> { + for range in ranges { + // Error if the specified range lies outside of the source file. + if source.text_len() < range.end() { + return Err(FormatModuleError::FormatError(FormatError::RangeError { + input: *range, + tree: TextRange::up_to(source.text_len()), + })); + } + + // If any range covers the entire file, that becomes the only range and no processing is + // required. + if *range == TextRange::up_to(source.text_len()) { + return Ok(vec![*range]); + } + } + + let parsed = parse(source, ParseOptions::from(options.source_type()))?; + let source_code = SourceCode::new(source); + let comment_ranges = CommentRanges::from(parsed.tokens()); + let comments = Comments::from_ast(parsed.syntax(), source_code, &comment_ranges); + + let context = PyFormatContext::new( + options.with_source_map_generation(SourceMapGeneration::Enabled), + source, + comments, + parsed.tokens(), + ); + + let mut expanded_ranges: Vec = Vec::new(); + for range in ranges { + if range.is_empty() { + continue; + } + + let enclosing_node = + match find_enclosing_node(*range, AnyNodeRef::from(parsed.syntax()), &context) { + EnclosingNode::Node { + node, + indent_level: _, + } => node, + EnclosingNode::Suppressed => { + // The entire range falls into a suppressed range. There's nothing to format. + continue; + } + }; + + let narrowed_range = narrow_range(*range, enclosing_node, &context); + assert_valid_char_boundaries(narrowed_range, source); + + expanded_ranges.push(narrowed_range); + } + if expanded_ranges.is_empty() { + return Ok(expanded_ranges); + } + expanded_ranges.sort(); + + let mut collapsed_ranges: Vec = Vec::new(); + collapsed_ranges.push(expanded_ranges[0]); + for range in expanded_ranges { + let back_range_index = collapsed_ranges.len() - 1; + let back_range = &collapsed_ranges[back_range_index]; + if back_range.overlap_or_touch(&range) { + collapsed_ranges[back_range_index] = back_range.joined(&range); + } else { + collapsed_ranges.push(range); + } + } + + Ok(collapsed_ranges) +} + /// Finds the node with the minimum covering range of `range`. /// /// It traverses the tree and returns the deepest node that fully encloses `range`. diff --git a/crates/ruff_text_size/src/range.rs b/crates/ruff_text_size/src/range.rs index 302568e8e3..56e2d3ede7 100644 --- a/crates/ruff_text_size/src/range.rs +++ b/crates/ruff_text_size/src/range.rs @@ -269,6 +269,30 @@ impl TextRange { self.cover(TextRange::empty(offset)) } + /// Checks if the text ranges overlap or touch each other. + #[must_use] + pub fn overlap_or_touch(&self, other: &Self) -> bool { + (self.start <= other.start && other.start <= self.end) + || (other.start <= self.start && self.start <= other.end) + } + + /// Joins the two text ranges. + #[must_use] + pub fn joined(&self, other: &Self) -> Self { + Self { + start: if self.start < other.start { + self.start + } else { + other.start + }, + end: if self.end > other.end { + self.end + } else { + other.end + }, + } + } + /// Add an offset to this range. /// /// Note that this is not appropriate for changing where a `TextRange` is @@ -441,6 +465,19 @@ impl TextRange { } } +impl PartialOrd for TextRange { + #[inline] + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for TextRange { + fn cmp(&self, other: &Self) -> Ordering { + self.start.cmp(&other.start).then(self.end.cmp(&other.end)) + } +} + impl Index for str { type Output = str; #[inline]