Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Fix and Edit API #4198

Merged
merged 11 commits into from
May 8, 2023
14 changes: 7 additions & 7 deletions crates/ruff/src/autofix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub mod actions;
pub fn fix_file(diagnostics: &[Diagnostic], locator: &Locator) -> Option<(String, FixTable)> {
let mut with_fixes = diagnostics
.iter()
.filter(|diag| !diag.fix.is_empty())
.filter(|diag| diag.fix.is_some())
.peekable();

if with_fixes.peek().is_none() {
Expand All @@ -38,11 +38,10 @@ fn apply_fixes<'a>(

for (rule, fix) in diagnostics
.filter_map(|diagnostic| {
if diagnostic.fix.is_empty() {
None
} else {
Some((diagnostic.kind.rule(), &diagnostic.fix))
}
diagnostic
.fix
.as_ref()
.map(|fix| (diagnostic.kind.rule(), fix))
})
.sorted_by(|(rule1, fix1), (rule2, fix2)| cmp_fix(*rule1, *rule2, fix1, fix2))
{
Expand Down Expand Up @@ -103,6 +102,7 @@ mod tests {

use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Edit;
use ruff_diagnostics::Fix;
use ruff_python_ast::source_code::Locator;

use crate::autofix::apply_fixes;
Expand All @@ -114,7 +114,7 @@ mod tests {
// The choice of rule here is arbitrary.
kind: MissingNewlineAtEndOfFile.into(),
range: edit.range(),
fix: edit.into(),
fix: Some(Fix::unspecified(edit)),
parent: None,
})
.collect()
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/checkers/logical_lines.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_text_size::TextRange;
use rustpython_parser::lexer::LexResult;

use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix};
use ruff_diagnostics::{Diagnostic, DiagnosticKind};
use ruff_python_ast::source_code::{Locator, Stylist};
use ruff_python_ast::token_kind::TokenKind;

Expand Down Expand Up @@ -144,7 +144,7 @@ impl<'a> LogicalLinesContext<'a> {
self.diagnostics.push(Diagnostic {
kind,
range,
fix: Fix::empty(),
fix: None,
parent: None,
});
}
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff/src/checkers/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use itertools::Itertools;
use ruff_text_size::{TextLen, TextRange, TextSize};

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

use crate::noqa;
Expand Down Expand Up @@ -178,10 +178,10 @@ pub fn check_noqa(
locator,
));
} else {
diagnostic.set_fix(Edit::range_replacement(
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
format!("# noqa: {}", valid_codes.join(", ")),
*range,
));
)));
}
}
diagnostics.push(diagnostic);
Expand Down
12 changes: 4 additions & 8 deletions crates/ruff/src/message/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,10 @@ pub(super) struct Diff<'a> {

impl<'a> Diff<'a> {
pub fn from_message(message: &'a Message) -> Option<Diff> {
if message.fix.is_empty() {
None
} else {
Some(Diff {
source_code: &message.file,
fix: &message.fix,
})
}
message.fix.as_ref().map(|fix| Diff {
source_code: &message.file,
fix,
})
}
}

Expand Down
14 changes: 6 additions & 8 deletions crates/ruff/src/message/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,12 @@ impl Serialize for ExpandedMessages<'_> {
for message in self.messages {
let source_code = message.file.to_source_code();

let fix = if message.fix.is_empty() {
None
} else {
Some(json!({
"message": message.kind.suggestion.as_deref(),
"edits": &ExpandedEdits { edits: message.fix.edits(), source_code: &source_code },
}))
};
let fix = message.fix.as_ref().map(|fix| {
json!({
"message": message.kind.suggestion.as_deref(),
"edits": &ExpandedEdits { edits: fix.edits(), source_code: &source_code },
})
});

let start_location = source_code.source_location(message.start());
let end_location = source_code.source_location(message.end());
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff/src/message/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use ruff_python_ast::source_code::{SourceFile, SourceLocation};
pub struct Message {
pub kind: DiagnosticKind,
pub range: TextRange,
pub fix: Fix,
pub fix: Option<Fix>,
pub file: SourceFile,
pub noqa_offset: TextSize,
}
Expand Down Expand Up @@ -191,10 +191,10 @@ def fibonacci(n):
},
TextRange::new(TextSize::from(94), TextSize::from(95)),
)
.with_fix(Fix::new(vec![Edit::deletion(
.with_fix(Fix::unspecified(Edit::deletion(
TextSize::from(94),
TextSize::from(99),
)]));
)));

let file_2 = r#"if a == 1: pass"#;

Expand Down
6 changes: 4 additions & 2 deletions crates/ruff/src/rules/eradicate/rules.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ruff_text_size::TextRange;

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::source_code::Locator;

Expand Down Expand Up @@ -58,7 +58,9 @@ pub fn commented_out_code(
if is_standalone_comment(line) && comment_contains_code(line, &settings.task_tags[..]) {
let mut diagnostic = Diagnostic::new(CommentedOutCode, range);
if autofix.into() && settings.rules.should_fix(Rule::CommentedOutCode) {
diagnostic.set_fix(Edit::range_deletion(locator.full_lines_range(range)));
diagnostic.set_fix(Fix::unspecified(Edit::range_deletion(
locator.full_lines_range(range),
)));
}
Some(diagnostic)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn fix_abstractmethod_missing(
),
stmt.range().start(),
);
Ok(Fix::from_iter([import_edit, reference_edit]))
Ok(Fix::unspecified_edits(import_edit, [reference_edit]))
}

/// B024
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff/src/rules/flake8_bugbear/rules/assert_false.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_text_size::TextSize;
use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::unparse_stmt;

Expand Down Expand Up @@ -63,10 +63,10 @@ pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: Option

let mut diagnostic = Diagnostic::new(AssertFalse, test.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::range_replacement(
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
unparse_stmt(&assertion_error(msg), checker.stylist),
stmt.range(),
));
)));
}
checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
use rustpython_parser::ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Violation};
use ruff_diagnostics::{Diagnostic, Edit};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path;
use ruff_python_ast::call_path::CallPath;
Expand Down Expand Up @@ -96,14 +96,14 @@ fn duplicate_handler_exceptions<'a>(
expr.range(),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::range_replacement(
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
if unique_elts.len() == 1 {
unparse_expr(unique_elts[0], checker.stylist)
} else {
unparse_expr(&type_pattern(unique_elts), checker.stylist)
},
expr.range(),
));
)));
}
checker.diagnostics.push(diagnostic);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_text_size::TextSize;
use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::unparse_expr;
use ruff_python_stdlib::identifiers::{is_identifier, is_mangled_private};
Expand Down Expand Up @@ -64,10 +64,10 @@ pub fn getattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range());

if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::range_replacement(
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
unparse_expr(&attribute(obj, value), checker.stylist),
expr.range(),
));
)));
}
checker.diagnostics.push(diagnostic);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustpython_parser::ast::{Excepthandler, ExcepthandlerKind, ExprKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::unparse_expr;

Expand Down Expand Up @@ -47,10 +47,10 @@ pub fn redundant_tuple_in_exception_handler(checker: &mut Checker, handlers: &[E
type_.range(),
);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::range_replacement(
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
unparse_expr(elt, checker.stylist),
type_.range(),
));
)));
}
checker.diagnostics.push(diagnostic);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_text_size::TextSize;
use rustpython_parser::ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::unparse_stmt;
use ruff_python_ast::source_code::Stylist;
Expand Down Expand Up @@ -79,10 +79,10 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar
let mut diagnostic = Diagnostic::new(SetAttrWithConstant, expr.range());

if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Edit::range_replacement(
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
assignment(obj, name, value, checker.stylist),
expr.range(),
));
)));
}
checker.diagnostics.push(diagnostic);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc_hash::FxHashMap;
use rustpython_parser::ast::{Expr, ExprKind, Stmt};
use serde::{Deserialize, Serialize};

use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Violation};
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::RefEquality;
use ruff_python_ast::visitor::Visitor;
Expand Down Expand Up @@ -176,7 +176,10 @@ pub fn unused_loop_control_variable(
if let Some(binding) = binding {
if binding.kind.is_loop_var() {
if !binding.used() {
diagnostic.set_fix(Edit::range_replacement(rename, expr.range()));
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
rename,
expr.range(),
)));
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff/src/rules/flake8_commas/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustpython_parser::lexer::{LexResult, Spanned};
use rustpython_parser::Tok;

use ruff_diagnostics::{AlwaysAutofixableViolation, Violation};
use ruff_diagnostics::{Diagnostic, Edit};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::source_code::Locator;

Expand Down Expand Up @@ -333,7 +333,7 @@ pub fn trailing_commas(
let comma = prev.spanned.unwrap();
let mut diagnostic = Diagnostic::new(ProhibitedTrailingComma, comma.1);
if autofix.into() && settings.rules.should_fix(Rule::ProhibitedTrailingComma) {
diagnostic.set_fix(Edit::range_deletion(diagnostic.range()));
diagnostic.set_fix(Fix::unspecified(Edit::range_deletion(diagnostic.range())));
}
diagnostics.push(diagnostic);
}
Expand Down Expand Up @@ -373,10 +373,10 @@ pub fn trailing_commas(
// removing any brackets in the same linter pass - doing both at the same time could
// lead to a syntax error.
let contents = locator.slice(missing_comma.1);
diagnostic.set_fix(Edit::range_replacement(
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
format!("{contents},"),
missing_comma.1,
));
)));
}
diagnostics.push(diagnostic);
}
Expand Down
9 changes: 6 additions & 3 deletions crates/ruff/src/rules/flake8_errmsg/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ fn generate_fix(stylist: &Stylist, stmt: &Stmt, exc_arg: &Expr, indentation: &st
}),
stylist,
);
Fix::from_iter([
Fix::unspecified_edits(
Edit::insertion(
format!(
"{}{}{}",
Expand All @@ -213,8 +213,11 @@ fn generate_fix(stylist: &Stylist, stmt: &Stmt, exc_arg: &Expr, indentation: &st
),
stmt.start(),
),
Edit::range_replacement(String::from("msg"), exc_arg.range()),
])
[Edit::range_replacement(
String::from("msg"),
exc_arg.range(),
)],
)
}

/// EM101, EM102, EM103
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use ruff_text_size::{TextRange, TextSize};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};

use crate::rules::flake8_executable::helpers::ShebangDirective;
Expand Down Expand Up @@ -32,10 +32,10 @@ pub fn shebang_whitespace(
TextRange::at(range.start(), *n_spaces),
);
if autofix {
diagnostic.set_fix(Edit::range_deletion(TextRange::at(
diagnostic.set_fix(Fix::unspecified(Edit::range_deletion(TextRange::at(
range.start(),
*n_spaces,
)));
))));
}
Some(diagnostic)
} else {
Expand Down
Loading