Skip to content

Commit

Permalink
Allow diagnostics to generate multi-edit fixes (#3709)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh authored Mar 26, 2023
1 parent 32be63f commit e603382
Show file tree
Hide file tree
Showing 731 changed files with 17,319 additions and 13,447 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 32 additions & 26 deletions crates/ruff/src/autofix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use itertools::Itertools;
use rustc_hash::FxHashMap;
use rustpython_parser::ast::Location;

use ruff_diagnostics::{Diagnostic, Edit};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_python_ast::source_code::Locator;
use ruff_python_ast::types::Range;

Expand All @@ -15,7 +15,7 @@ pub mod helpers;

/// Auto-fix errors in a file, and write the fixed source code to disk.
pub fn fix_file(diagnostics: &[Diagnostic], locator: &Locator) -> Option<(String, FixTable)> {
if diagnostics.iter().all(|check| check.fix.is_none()) {
if diagnostics.iter().all(|check| check.fix.is_empty()) {
None
} else {
Some(apply_fixes(diagnostics.iter(), locator))
Expand All @@ -34,36 +34,43 @@ fn apply_fixes<'a>(

for (rule, fix) in diagnostics
.filter_map(|diagnostic| {
diagnostic
.fix
.as_ref()
.map(|fix| (diagnostic.kind.rule(), fix))
if diagnostic.fix.is_empty() {
None
} else {
Some((diagnostic.kind.rule(), &diagnostic.fix))
}
})
.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 applied.contains(&fix) {
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| last_pos >= fix.location) {
if last_pos.map_or(false, |last_pos| {
fix.location()
.map_or(false, |fix_location| last_pos >= fix_location)
}) {
continue;
}

// Add all contents from `last_pos` to `fix.location`.
let slice = locator.slice(Range::new(last_pos.unwrap_or_default(), fix.location));
output.push_str(slice);
for edit in fix.edits() {
// Add all contents from `last_pos` to `fix.location`.
let slice = locator.slice(Range::new(last_pos.unwrap_or_default(), edit.location));
output.push_str(slice);

// Add the patch itself.
output.push_str(&fix.content);
// Add the patch itself.
output.push_str(&edit.content);

// Track that the edit was applied.
last_pos = Some(edit.end_location);
applied.insert(edit);
}

// Track that the fix was applied.
last_pos = Some(fix.end_location);
applied.insert(fix);
*fixed.entry(rule).or_default() += 1;
}

Expand Down Expand Up @@ -93,9 +100,9 @@ pub(crate) fn apply_fix(fix: &Edit, locator: &Locator) -> String {
}

/// Compare two fixes.
fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Edit, fix2: &Edit) -> std::cmp::Ordering {
fix1.location
.cmp(&fix2.location)
fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering {
fix1.location()
.cmp(&fix2.location())
.then_with(|| match (&rule1, &rule2) {
// Apply `EndsInPeriod` fixes before `NewLineAfterLastParagraph` fixes.
(Rule::EndsInPeriod, Rule::NewLineAfterLastParagraph) => std::cmp::Ordering::Less,
Expand All @@ -115,15 +122,14 @@ mod tests {
use crate::autofix::{apply_fix, apply_fixes};
use crate::rules::pycodestyle::rules::MissingNewlineAtEndOfFile;

fn create_diagnostics(fixes: impl IntoIterator<Item = Edit>) -> Vec<Diagnostic> {
fixes
.into_iter()
.map(|fix| Diagnostic {
fn create_diagnostics(edit: impl IntoIterator<Item = Edit>) -> Vec<Diagnostic> {
edit.into_iter()
.map(|edit| Diagnostic {
// The choice of rule here is arbitrary.
kind: MissingNewlineAtEndOfFile.into(),
location: fix.location,
end_location: fix.end_location,
fix: Some(fix),
location: edit.location,
end_location: edit.end_location,
fix: edit.into(),
parent: None,
})
.collect()
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2455,7 +2455,7 @@ where
pyupgrade::rules::replace_universal_newlines(self, func, keywords);
}
if self.settings.rules.enabled(Rule::ReplaceStdoutStderr) {
pyupgrade::rules::replace_stdout_stderr(self, expr, func, keywords);
pyupgrade::rules::replace_stdout_stderr(self, expr, func, args, keywords);
}
if self.settings.rules.enabled(Rule::OSErrorAlias) {
pyupgrade::rules::os_error_alias_call(self, func);
Expand Down
18 changes: 9 additions & 9 deletions crates/ruff/src/checkers/logical_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use itertools::Itertools;
use rustpython_parser::ast::Location;
use rustpython_parser::lexer::LexResult;

use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::{Diagnostic, Fix};
use ruff_python_ast::source_code::{Locator, Stylist};
use ruff_python_ast::types::Range;

Expand Down Expand Up @@ -75,7 +75,7 @@ pub fn check_logical_lines(
kind,
location,
end_location: location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand All @@ -93,7 +93,7 @@ pub fn check_logical_lines(
kind,
location,
end_location: location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand All @@ -108,7 +108,7 @@ pub fn check_logical_lines(
kind,
location,
end_location: location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand All @@ -120,7 +120,7 @@ pub fn check_logical_lines(
kind,
location,
end_location: location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand All @@ -133,7 +133,7 @@ pub fn check_logical_lines(
kind,
location: range.location,
end_location: range.end_location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand All @@ -148,7 +148,7 @@ pub fn check_logical_lines(
kind,
location,
end_location: location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand All @@ -159,7 +159,7 @@ pub fn check_logical_lines(
kind,
location,
end_location: location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand Down Expand Up @@ -210,7 +210,7 @@ pub fn check_logical_lines(
kind,
location,
end_location: location,
fix: None,
fix: Fix::empty(),
parent: None,
});
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::cmp::Ordering;
pub use rustpython_parser::ast::Location;
use serde::{Deserialize, Serialize};

use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit};
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix};
use ruff_python_ast::source_code::Locator;
use ruff_python_ast::types::Range;

Expand All @@ -12,7 +12,7 @@ pub struct Message {
pub kind: DiagnosticKind,
pub location: Location,
pub end_location: Location,
pub fix: Option<Edit>,
pub fix: Fix,
pub filename: String,
pub source: Option<Source>,
pub noqa_row: usize,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ expression: diagnostics
row: 1
column: 10
fix:
content: ""
location:
row: 1
column: 0
end_location:
row: 2
column: 0
edits:
- content: ""
location:
row: 1
column: 0
end_location:
row: 2
column: 0
parent: ~
- kind:
name: CommentedOutCode
Expand All @@ -34,13 +35,14 @@ expression: diagnostics
row: 2
column: 22
fix:
content: ""
location:
row: 2
column: 0
end_location:
row: 3
column: 0
edits:
- content: ""
location:
row: 2
column: 0
end_location:
row: 3
column: 0
parent: ~
- kind:
name: CommentedOutCode
Expand All @@ -54,13 +56,14 @@ expression: diagnostics
row: 3
column: 6
fix:
content: ""
location:
row: 3
column: 0
end_location:
row: 4
column: 0
edits:
- content: ""
location:
row: 3
column: 0
end_location:
row: 4
column: 0
parent: ~
- kind:
name: CommentedOutCode
Expand All @@ -74,13 +77,14 @@ expression: diagnostics
row: 5
column: 13
fix:
content: ""
location:
row: 5
column: 0
end_location:
row: 6
column: 0
edits:
- content: ""
location:
row: 5
column: 0
end_location:
row: 6
column: 0
parent: ~
- kind:
name: CommentedOutCode
Expand All @@ -94,12 +98,13 @@ expression: diagnostics
row: 12
column: 16
fix:
content: ""
location:
row: 12
column: 0
end_location:
row: 13
column: 0
edits:
- content: ""
location:
row: 12
column: 0
end_location:
row: 13
column: 0
parent: ~

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ expression: diagnostics
end_location:
row: 6
column: 17
fix: ~
fix:
edits: []
parent: ~
- kind:
name: SysVersionSlice3
Expand All @@ -26,7 +27,8 @@ expression: diagnostics
end_location:
row: 7
column: 13
fix: ~
fix:
edits: []
parent: ~
- kind:
name: SysVersionSlice3
Expand All @@ -39,6 +41,7 @@ expression: diagnostics
end_location:
row: 8
column: 7
fix: ~
fix:
edits: []
parent: ~

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ expression: diagnostics
end_location:
row: 4
column: 22
fix: ~
fix:
edits: []
parent: ~
- kind:
name: SysVersion2
Expand All @@ -26,6 +27,7 @@ expression: diagnostics
end_location:
row: 5
column: 18
fix: ~
fix:
edits: []
parent: ~

Loading

0 comments on commit e603382

Please sign in to comment.