Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Skip partial duplicates when applying multi-edit fixes (#6144) 

## Summary

Right now, if we have two fixes that have an overlapping edit, but not
an _identical_ set of edits, they'll conflict, causing us to do another
linter traversal. Here, I've enabled the fixer to support partially
overlapping edits, which (as an example) let's us greatly reduce the
number of iterations required in the test suite.

The most common case here is that in which a bunch of edits need to
import some symbol, and then use that symbol, but in different ways. In
that case, all edits will have a common fix (to import the symbol), but
deviate in some way. With this change, we can do all of those edits in
one pass.

Note that the simplest way to enable this was to store sorted edits on
`Fix`. We don't allow modifying the edits on `Fix` once it's
constructed, so this is an easy change, and allows us to avoid a bunch
of clones and traversals later on.

Closes #5800.
This commit is contained in:
Charlie Marsh 2023-07-29 08:11:57 -04:00 committed by GitHub
parent badbfb2d3e
commit 4231ed2fc3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 47 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
crates/ruff/src/autofix/mod.rs
View File

@ -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,22 +59,14 @@ 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;
}
// 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;
}
let mut edits = fix
.edits()
.iter()
.filter(|edit| !applied.contains(edit))
.peekable();
// 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() {
@ -83,11 +75,14 @@ fn apply_fixes<'a>(
}
}
for edit in fix
.edits()
.iter()
.sorted_unstable_by_key(|edit| edit.start())
{
// 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;
}
}
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;
}

9
crates/ruff/src/message/diff.rs
View File

@ -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())),

2
crates/ruff/src/test.rs
View File

@ -85,7 +85,7 @@ pub fn test_snippet(contents: &str, settings: &Settings) -> Vec<Message> {
}
thread_local! {
static MAX_ITERATIONS: std::cell::Cell<usize> = std::cell::Cell::new(30);
static MAX_ITERATIONS: std::cell::Cell<usize> = std::cell::Cell::new(8);
}
pub fn set_max_iterations(max: usize) {

4
crates/ruff_cli/src/diagnostics.rs
View File

@ -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(

3
crates/ruff_diagnostics/src/edit.rs
View File

@ -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)]

26
crates/ruff_diagnostics/src/fix.rs
View File

@ -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<Edit>,
/// 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<Item = Edit>) -> Self {
let mut edits: Vec<Edit> = 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<Item = Edit>) -> Self {
let mut edits: Vec<Edit> = 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<Item = Edit>) -> Self {
let mut edits: Vec<Edit> = 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<TextSize> {
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<Edit> {
self.edits
}
/// Return the [`Applicability`] of the [`Fix`].
pub fn applicability(&self) -> Applicability {
self.applicability

4
crates/ruff_wasm/src/lib.rs
View File

@ -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()),