mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
perf(linter): avoid diagnostic sorting after applying fixes (#25079)
`oxlint --fix` already presorts the messages by it `fixes` spans, before applying it into the source text.
Resorting the unapplied diagnostics after the operation is only useful for display usages.
I thought maybe the suppression manager needs the right order, but AI🤖 did scan the project is answered the following:
> So the line at mod.rs:413 is mainly for deterministic, source-order diagnostics after fixing. It is not required for correctness of applying fixes, but it is needed for stable and intuitive reporting order.
> Yes, for that specific final sort you highlighted, you can skip it if your only concern is suppression-file correctness.
> Why this is safe:
> Suppression counting is order-independent because it just increments counts by rule key while iterating messages: diff.rs:90.
Runtime suppression aggregation is also additive by rule and filename, not positional: mod.rs:30.
Suppression file output is deterministically sorted at serialization time (BTreeMap), independent of incoming message order: tracking.rs:35.
Important caveat:
> Keep the earlier sort used before applying fixes, since fix application logic depends on processing by fix span: mod.rs:364.
Only the final sort of filtered messages is optional for your case: mod.rs:413.
This commit is contained in:
1 parent
697c6294b0
commit
273d8675ac
1 file changed
-24
@@ -413,8 +413,6 @@ impl<'a> Fixer<'a> {
|
||||
|
||||
output.push_str(&source_text[last_pos as usize..]);
|
||||
|
||||
filtered_messages.sort_unstable_by_key(GetSpan::span);
|
||||
|
||||
#[cfg(debug_assertions)]
|
||||
if fixed && let Some(source_type) = self.source_type {
|
||||
use oxc_allocator::Allocator;
|
||||
@@ -497,14 +495,6 @@ mod test {
|
||||
OxcDiagnostic::warn("nofix").with_label(span)
|
||||
}
|
||||
|
||||
fn no_fix_1(span: Span) -> OxcDiagnostic {
|
||||
OxcDiagnostic::warn("nofix1").with_label(span)
|
||||
}
|
||||
|
||||
fn no_fix_2(span: Span) -> OxcDiagnostic {
|
||||
OxcDiagnostic::warn("nofix2").with_label(span)
|
||||
}
|
||||
|
||||
const TEST_CODE: &str = "var answer = 6 * 7;";
|
||||
const INSERT_AT_END: Fix = Fix {
|
||||
span: Span::new(19, 19),
|
||||
@@ -766,20 +756,6 @@ mod test {
|
||||
assert!(!result.fixed);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sort_no_fix_messages_correctly() {
|
||||
let result = get_fix_result(vec![
|
||||
create_message(replace_id(), PossibleFixes::Single(REPLACE_ID)),
|
||||
Message::new(no_fix_2(Span::new(1, 7)), PossibleFixes::None),
|
||||
Message::new(no_fix_1(Span::new(1, 3)), PossibleFixes::None),
|
||||
]);
|
||||
assert_eq!(result.fixed_code, TEST_CODE.cow_replace("answer", "foo"));
|
||||
assert_eq!(result.messages.len(), 2);
|
||||
assert_eq!(result.messages[0].error.to_string(), "nofix1");
|
||||
assert_eq!(result.messages[1].error.to_string(), "nofix2");
|
||||
assert!(result.fixed);
|
||||
}
|
||||
|
||||
fn assert_fixed_corrected(source_text: &str, expected: &str, composite_fix: CompositeFix) {
|
||||
let mut source_text = source_text.to_string();
|
||||
let fix = composite_fix.normalize_fixes(&source_text);
|
||||
|
||||
Reference in new issue
Block a user