diff --git a/crates/ruff/src/autofix/mod.rs b/crates/ruff/src/autofix/mod.rs index 777f225a7a..ab7b85f56d 100644 --- a/crates/ruff/src/autofix/mod.rs +++ b/crates/ruff/src/autofix/mod.rs @@ -1,6 +1,6 @@ +use itertools::Itertools; use std::collections::BTreeSet; -use itertools::Itertools; use ruff_text_size::{TextLen, TextRange, TextSize}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -59,35 +59,30 @@ fn apply_fixes<'a>( }) .sorted_by(|(rule1, fix1), (rule2, fix2)| cmp_fix(*rule1, *rule2, fix1, fix2)) { - // If we already applied an identical fix as part of another correction, skip - // any re-application. - if fix.edits().iter().all(|edit| applied.contains(edit)) { - *fixed.entry(rule).or_default() += 1; - continue; - } + let mut edits = fix + .edits() + .iter() + .filter(|edit| !applied.contains(edit)) + .peekable(); - // Best-effort approach: if this fix overlaps with a fix we've already applied, - // skip it. - if last_pos.map_or(false, |last_pos| { - fix.min_start() - .map_or(false, |fix_location| last_pos >= fix_location) - }) { - continue; - } + // If the fix contains at least one new edit, enforce isolation and positional requirements. + if let Some(first) = edits.peek() { + // If this fix requires isolation, and we've already applied another fix in the + // same isolation group, skip it. + if let IsolationLevel::Group(id) = fix.isolation() { + if !isolated.insert(id) { + continue; + } + } - // If this fix requires isolation, and we've already applied another fix in the - // same isolation group, skip it. - if let IsolationLevel::Group(id) = fix.isolation() { - if !isolated.insert(id) { + // If this fix overlaps with a fix we've already applied, skip it. + if last_pos.map_or(false, |last_pos| last_pos >= first.start()) { continue; } } - for edit in fix - .edits() - .iter() - .sorted_unstable_by_key(|edit| edit.start()) - { + let mut applied_edits = Vec::with_capacity(fix.edits().len()); + for edit in edits { // Add all contents from `last_pos` to `fix.location`. let slice = locator.slice(TextRange::new(last_pos.unwrap_or_default(), edit.start())); output.push_str(slice); @@ -103,9 +98,10 @@ fn apply_fixes<'a>( // Track that the edit was applied. last_pos = Some(edit.end()); - applied.insert(edit); + applied_edits.push(edit); } + applied.extend(applied_edits.drain(..)); *fixed.entry(rule).or_default() += 1; } diff --git a/crates/ruff/src/message/diff.rs b/crates/ruff/src/message/diff.rs index a3a03db244..ca113b9969 100644 --- a/crates/ruff/src/message/diff.rs +++ b/crates/ruff/src/message/diff.rs @@ -2,7 +2,7 @@ use std::fmt::{Display, Formatter}; use std::num::NonZeroUsize; use colored::{Color, ColoredString, Colorize, Styles}; -use itertools::Itertools; + use ruff_text_size::{TextRange, TextSize}; use similar::{ChangeTag, TextDiff}; @@ -38,12 +38,7 @@ impl Display for Diff<'_> { let mut output = String::with_capacity(self.source_code.source_text().len()); let mut last_end = TextSize::default(); - for edit in self - .fix - .edits() - .iter() - .sorted_unstable_by_key(|edit| edit.start()) - { + for edit in self.fix.edits() { output.push_str( self.source_code .slice(TextRange::new(last_end, edit.start())), diff --git a/crates/ruff/src/test.rs b/crates/ruff/src/test.rs index 4961f3f10d..9e72487231 100644 --- a/crates/ruff/src/test.rs +++ b/crates/ruff/src/test.rs @@ -85,7 +85,7 @@ pub fn test_snippet(contents: &str, settings: &Settings) -> Vec { } thread_local! { - static MAX_ITERATIONS: std::cell::Cell = std::cell::Cell::new(30); + static MAX_ITERATIONS: std::cell::Cell = std::cell::Cell::new(8); } pub fn set_max_iterations(max: usize) { diff --git a/crates/ruff_cli/src/diagnostics.rs b/crates/ruff_cli/src/diagnostics.rs index 4721700298..54ce93a57d 100644 --- a/crates/ruff_cli/src/diagnostics.rs +++ b/crates/ruff_cli/src/diagnostics.rs @@ -285,7 +285,9 @@ pub(crate) fn lint_path( .enumerate() .zip(dest_notebook.cells().iter()) { - let (Cell::Code(src_code_cell), Cell::Code(dest_code_cell)) = (src_cell, dest_cell) else { + let (Cell::Code(src_code_cell), Cell::Code(dest_code_cell)) = + (src_cell, dest_cell) + else { continue; }; TextDiff::from_lines( diff --git a/crates/ruff_diagnostics/src/edit.rs b/crates/ruff_diagnostics/src/edit.rs index 5da2dc590a..c26f566de6 100644 --- a/crates/ruff_diagnostics/src/edit.rs +++ b/crates/ruff_diagnostics/src/edit.rs @@ -1,9 +1,10 @@ use std::cmp::Ordering; -use ruff_text_size::{TextRange, TextSize}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; +use ruff_text_size::{TextRange, TextSize}; + /// A text edit to be applied to a source file. Inserts, deletes, or replaces /// content at a given location. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/crates/ruff_diagnostics/src/fix.rs b/crates/ruff_diagnostics/src/fix.rs index c96b428c8d..8b37e0b8fe 100644 --- a/crates/ruff_diagnostics/src/fix.rs +++ b/crates/ruff_diagnostics/src/fix.rs @@ -1,7 +1,8 @@ -use ruff_text_size::TextSize; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; +use ruff_text_size::TextSize; + use crate::edit::Edit; /// Indicates confidence in the correctness of a suggested fix. @@ -42,8 +43,11 @@ pub enum IsolationLevel { #[derive(Debug, PartialEq, Eq, Clone)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct Fix { + /// The [`Edit`] elements to be applied, sorted by [`Edit::start`] in ascending order. edits: Vec, + /// The [`Applicability`] of the fix. applicability: Applicability, + /// The [`IsolationLevel`] of the fix. isolation_level: IsolationLevel, } @@ -83,8 +87,10 @@ impl Fix { /// Create a new [`Fix`] with [automatic applicability](Applicability::Automatic) from multiple [`Edit`] elements. pub fn automatic_edits(edit: Edit, rest: impl IntoIterator) -> Self { + let mut edits: Vec = std::iter::once(edit).chain(rest).collect(); + edits.sort_by_key(Edit::start); Self { - edits: std::iter::once(edit).chain(rest).collect(), + edits, applicability: Applicability::Automatic, isolation_level: IsolationLevel::default(), } @@ -101,8 +107,10 @@ impl Fix { /// Create a new [`Fix`] with [suggested applicability](Applicability::Suggested) from multiple [`Edit`] elements. pub fn suggested_edits(edit: Edit, rest: impl IntoIterator) -> Self { + let mut edits: Vec = std::iter::once(edit).chain(rest).collect(); + edits.sort_by_key(Edit::start); Self { - edits: std::iter::once(edit).chain(rest).collect(), + edits, applicability: Applicability::Suggested, isolation_level: IsolationLevel::default(), } @@ -119,8 +127,10 @@ impl Fix { /// Create a new [`Fix`] with [manual applicability](Applicability::Manual) from multiple [`Edit`] elements. pub fn manual_edits(edit: Edit, rest: impl IntoIterator) -> Self { + let mut edits: Vec = std::iter::once(edit).chain(rest).collect(); + edits.sort_by_key(Edit::start); Self { - edits: std::iter::once(edit).chain(rest).collect(), + edits, applicability: Applicability::Manual, isolation_level: IsolationLevel::default(), } @@ -128,18 +138,14 @@ impl Fix { /// Return the [`TextSize`] of the first [`Edit`] in the [`Fix`]. pub fn min_start(&self) -> Option { - self.edits.iter().map(Edit::start).min() + self.edits.first().map(Edit::start) } - /// Return a slice of the [`Edit`] elements in the [`Fix`]. + /// Return a slice of the [`Edit`] elements in the [`Fix`], sorted by [`Edit::start`] in ascending order. pub fn edits(&self) -> &[Edit] { &self.edits } - pub fn into_edits(self) -> Vec { - self.edits - } - /// Return the [`Applicability`] of the [`Fix`]. pub fn applicability(&self) -> Applicability { self.applicability diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 443e296afe..c3f48a9f28 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -245,8 +245,8 @@ impl Workspace { fix: message.fix.map(|fix| ExpandedFix { message: message.kind.suggestion, edits: fix - .into_edits() - .into_iter() + .edits() + .iter() .map(|edit| ExpandedEdit { location: source_code.source_location(edit.start()), end_location: source_code.source_location(edit.end()),