feat(linter): accept multiple fixes when fix code (#3842)

Want to add this because of there are bunch of fix codes in [consistent-type-import](https://github.com/typescript-eslint/typescript-eslint/blob/e408b93e48e5199d83a8d99d1e27126d2dd8bc8f/packages/eslint-plugin/src/rules/consistent-type-imports.ts#L610-L945) are not easy to port.

crates/oxc_linter/src/context.rs

@@ -9,7 +9,7 @@ use oxc_syntax::module_record::ModuleRecord;
 use crate::{
     config::OxlintRules,
     disable_directives::{DisableDirectives, DisableDirectivesBuilder},
-    fixer::{Fix, Message, RuleFixer},
+    fixer::{CompositeFix, Message, RuleFixer},
     javascript_globals::GLOBALS,
     AllowWarnDeny, OxlintConfig, OxlintEnv, OxlintGlobals, OxlintSettings,
 };
@@ -171,14 +171,16 @@ impl<'a> LintContext<'a> {
     }
 
     /// Report a lint rule violation and provide an automatic fix.
-    pub fn diagnostic_with_fix<F: FnOnce(RuleFixer<'_, 'a>) -> Fix<'a>>(
-        &self,
-        diagnostic: OxcDiagnostic,
-        fix: F,
-    ) {
+    pub fn diagnostic_with_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
+    where
+        C: Into<CompositeFix<'a>>,
+        F: FnOnce(RuleFixer<'_, 'a>) -> C,
+    {
         if self.fix {
             let fixer = RuleFixer::new(self);
-            self.add_diagnostic(Message::new(diagnostic, Some(fix(fixer))));
+            let composite_fix: CompositeFix = fix(fixer).into();
+            let fix = composite_fix.normalize_fixes(self.source_text());
+            self.add_diagnostic(Message::new(diagnostic, Some(fix)));
         } else {
             self.diagnostic(diagnostic);
         }

crates/oxc_linter/src/fixer.rs

@@ -22,6 +22,92 @@ impl<'a> Fix<'a> {
     }
 }
 
+pub enum CompositeFix<'a> {
+    Single(Fix<'a>),
+    Multiple(Vec<Fix<'a>>),
+}
+
+impl<'a> From<Fix<'a>> for CompositeFix<'a> {
+    fn from(fix: Fix<'a>) -> Self {
+        CompositeFix::Single(fix)
+    }
+}
+
+impl<'a> From<Vec<Fix<'a>>> for CompositeFix<'a> {
+    fn from(fixes: Vec<Fix<'a>>) -> Self {
+        CompositeFix::Multiple(fixes)
+    }
+}
+
+impl<'a> CompositeFix<'a> {
+    /// Gets one fix from the fixes. If we retrieve multiple fixes, this merges those into one.
+    /// <https://github.com/eslint/eslint/blob/main/lib/linter/report-translator.js#L181-L203>
+    pub fn normalize_fixes(self, source_text: &str) -> Fix<'a> {
+        match self {
+            CompositeFix::Single(fix) => fix,
+            CompositeFix::Multiple(fixes) => Self::merge_fixes(fixes, source_text),
+        }
+    }
+    // Merges multiple fixes to one, returns an `Fix::default`(which will not fix anything) if:
+    // 1. `fixes` is empty
+    // 2. contains overlapped ranges
+    // 3. contains negative ranges (span.start > span.end)
+    // <https://github.com/eslint/eslint/blob/main/lib/linter/report-translator.js#L147-L179>
+    fn merge_fixes(fixes: Vec<Fix<'a>>, source_text: &str) -> Fix<'a> {
+        let mut fixes = fixes;
+        let empty_fix = Fix::default();
+        if fixes.is_empty() {
+            // Do nothing
+            return empty_fix;
+        }
+        if fixes.len() == 1 {
+            return fixes.pop().unwrap();
+        }
+
+        fixes.sort_by(|a, b| a.span.cmp(&b.span));
+
+        // safe, as fixes.len() > 1
+        let start = fixes[0].span.start;
+        let end = fixes[fixes.len() - 1].span.end;
+        let mut last_pos = start;
+        let mut output = String::new();
+
+        for fix in fixes {
+            let Fix { ref content, span } = fix;
+            // negative range or overlapping ranges is invalid
+            if span.start > span.end {
+                debug_assert!(false, "Negative range is invalid: {span:?}");
+                return empty_fix;
+            }
+            if last_pos > span.start {
+                debug_assert!(
+                    false,
+                    "Fix must not be overlapped, last_pos: {}, span.start: {}",
+                    last_pos, span.start
+                );
+                return empty_fix;
+            }
+
+            let Some(before) = source_text.get((last_pos) as usize..span.start as usize) else {
+                debug_assert!(false, "Invalid range: {}, {}", last_pos, span.start);
+                return empty_fix;
+            };
+
+            output.push_str(before);
+            output.push_str(content);
+            last_pos = span.end;
+        }
+
+        let Some(after) = source_text.get(last_pos as usize..end as usize) else {
+            debug_assert!(false, "Invalid range: {:?}", last_pos as usize..end as usize);
+            return empty_fix;
+        };
+
+        output.push_str(after);
+        Fix::new(output, Span::new(start, end))
+    }
+}
+
 /// Inspired by ESLint's [`RuleFixer`].
 ///
 /// [`RuleFixer`]: https://github.com/eslint/eslint/blob/main/lib/linter/rule-fixer.js
@@ -174,7 +260,7 @@ mod test {
     use oxc_diagnostics::OxcDiagnostic;
     use oxc_span::Span;
 
-    use super::{Fix, FixResult, Fixer, Message};
+    use super::{CompositeFix, Fix, FixResult, Fixer, Message};
 
     fn insert_at_end() -> OxcDiagnostic {
         OxcDiagnostic::warn("End")
@@ -448,4 +534,118 @@ mod test {
         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);
+        let start = fix.span.start as usize;
+        let end = fix.span.end as usize;
+        source_text.replace_range(start..end, fix.content.to_string().as_str());
+        assert_eq!(source_text, expected);
+    }
+
+    #[test]
+    fn merge_fixes_in_composite_fix() {
+        let source_text = "foo bar baz";
+        let fixes = vec![Fix::new("quux", Span::new(0, 3)), Fix::new("qux", Span::new(4, 7))];
+        let composite_fix = CompositeFix::Multiple(fixes);
+        assert_fixed_corrected(source_text, "quux qux baz", composite_fix);
+    }
+
+    #[test]
+    fn one_fix_in_composite_fix() {
+        let source_text = "foo bar baz";
+        let fix = Fix::new("quxx", Span::new(4, 7));
+        let composite_fix = CompositeFix::Single(fix.clone());
+        assert_fixed_corrected(source_text, "foo quxx baz", composite_fix);
+
+        let composite_fix = CompositeFix::Multiple(vec![fix]);
+        assert_fixed_corrected(source_text, "foo quxx baz", composite_fix);
+    }
+
+    #[test]
+    fn zero_fixes_in_composite_fix() {
+        let source_text = "foo bar baz";
+        let composite_fix = CompositeFix::Multiple(vec![]);
+        assert_fixed_corrected(source_text, source_text, composite_fix);
+    }
+
+    #[test]
+    #[should_panic(expected = "Fix must not be overlapped, last_pos: 3, span.start: 2")]
+    fn overlapping_ranges_in_composite_fix() {
+        let source_text = "foo bar baz";
+        let fixes = vec![Fix::new("quux", Span::new(0, 3)), Fix::new("qux", Span::new(2, 5))];
+        let composite_fix = CompositeFix::Multiple(fixes);
+        assert_fixed_corrected(source_text, source_text, composite_fix);
+    }
+
+    #[test]
+    #[should_panic(expected = "Negative range is invalid: Span { start: 5, end: 2 }")]
+    fn negative_ranges_in_composite_fix() {
+        let source_text = "foo bar baz";
+        let fixes = vec![Fix::new("quux", Span::new(0, 3)), Fix::new("qux", Span::new(5, 2))];
+        let composite_fix = CompositeFix::Multiple(fixes);
+        assert_fixed_corrected(source_text, source_text, composite_fix);
+    }
+
+    fn assert_fixes_merged(fixes: Vec<Fix>, fix: &Fix, source_text: &str) {
+        let composite_fix = CompositeFix::from(fixes);
+        let merged_fix = composite_fix.normalize_fixes(source_text);
+        assert_eq!(merged_fix.content, fix.content);
+        assert_eq!(merged_fix.span, fix.span);
+    }
+
+    // Remain test caces picked from eslint
+    // <https://github.com/eslint/eslint/blob/main/tests/lib/linter/report-translator.js>
+    // 1. Combining autofixes
+    #[test]
+    fn merge_fixes_into_one() {
+        let source_text = "foo\nbar";
+        let fixes = vec![Fix::new("foo", Span::new(1, 2)), Fix::new("bar", Span::new(4, 5))];
+        assert_fixes_merged(fixes, &Fix::new("fooo\nbar", Span::new(1, 5)), source_text);
+    }
+
+    #[test]
+    fn respect_ranges_of_empty_insertions() {
+        let source_text = "foo\nbar";
+        let fixes = vec![
+            Fix::new("cd", Span::new(4, 5)),
+            Fix::new("", Span::new(2, 2)),
+            Fix::new("", Span::new(7, 7)),
+        ];
+        assert_fixes_merged(fixes, &Fix::new("o\ncdar", Span::new(2, 7)), source_text);
+    }
+
+    #[test]
+    fn pass_through_fixes_if_only_one_present() {
+        let source_text = "foo\nbar";
+        let fix = Fix::new("foo", Span::new(1, 2));
+        assert_fixes_merged(vec![fix.clone()], &fix, source_text);
+    }
+
+    #[test]
+    #[should_panic(expected = "Fix must not be overlapped, last_pos: 3, span.start: 2")]
+    fn throw_error_when_ranges_overlap() {
+        let source_text = "foo\nbar";
+        let fixes = vec![Fix::new("foo", Span::new(0, 3)), Fix::new("x", Span::new(2, 5))];
+        assert_fixes_merged(fixes, &Fix::default(), source_text);
+    }
+
+    // 2. unique `fix` and `fix.range` objects
+    #[test]
+    fn return_new_fix_when_fixes_is_one() {
+        let source_text = "foo\nbar";
+        let fix = Fix::new("baz", Span::new(0, 3));
+        let fixes = vec![fix.clone()];
+
+        assert_fixes_merged(fixes, &fix, source_text);
+    }
+
+    #[test]
+    fn create_new_fix_with_new_range_when_fixes_is_multiple() {
+        let source_text = "foo\nbar";
+        let fixes = vec![Fix::new("baz", Span::new(0, 3)), Fix::new("qux", Span::new(4, 7))];
+
+        assert_fixes_merged(fixes, &Fix::new("baz\nqux", Span::new(0, 7)), source_text);
+    }
 }

crates/oxc_linter/src/rules/typescript/no_import_type_side_effects.rs

@@ -6,7 +6,7 @@ use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_span::{GetSpan, Span};
 
-use crate::{context::LintContext, rule::Rule, AstNode};
+use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
 
 fn no_import_type_side_effects_diagnostic(span0: Span) -> OxcDiagnostic {
     OxcDiagnostic::warn("typescript-eslint(no-import-type-side-effects): TypeScript will only remove the inline type specifiers which will leave behind a side effect import at runtime.")
@@ -83,45 +83,29 @@ impl Rule for NoImportTypeSideEffects {
         // `import { type A, type B } from 'foo.js'`
         ctx.diagnostic_with_fix(
             no_import_type_side_effects_diagnostic(import_decl.span),
-            |fixer| {
-                let mut delete_ranges = vec![];
+            |_fixer| {
+                let raw = ctx.source_range(import_decl.span);
+                let mut fixes = vec![];
+
+                // import type A from 'foo.js'
+                //        ^^^^ add
+                if raw.starts_with("import") {
+                    fixes.push(Fix::new(
+                        "import type",
+                        Span::new(import_decl.span.start, import_decl.span.start + 6),
+                    ));
+                }
 
                 for specifier in type_specifiers {
                     // import { type    A } from 'foo.js'
                     //          ^^^^^^^^
-                    delete_ranges
-                        .push(Span::new(specifier.span.start, specifier.imported.span().start));
-                }
-
-                let mut output = String::new();
-                let mut last_pos = import_decl.span.start;
-                for range in delete_ranges {
-                    // import      { type A } from 'foo.js'
-                    // ^^^^^^^^^^^^^^^
-                    // |             |
-                    // [last_pos      range.start)
-                    output.push_str(ctx.source_range(Span::new(last_pos, range.start)));
-                    // import      { type A } from 'foo.js'
-                    //                    ^
-                    //                    |
-                    //                    last_pos
-                    last_pos = range.end;
+                    fixes.push(Fix::delete(Span::new(
+                        specifier.span.start,
+                        specifier.imported.span().start,
+                    )));
                 }
 
-                // import      { type A } from 'foo.js'
-                //                    ^^^^^^^^^^^^^^^^^^
-                //                    ^                ^
-                //                    |                |
-                //                    [last_pos        import_decl_span.end)
-                output.push_str(ctx.source_range(Span::new(last_pos, import_decl.span.end)));
-
-                if let Some(output) = output.strip_prefix("import ") {
-                    let output = format!("import type {output}");
-                    fixer.replace(import_decl.span, output)
-                } else {
-                    // Do not do anything, this should never happen
-                    fixer.replace(import_decl.span, ctx.source_range(import_decl.span))
-                }
+                fixes
             },
         );
     }

crates/oxc_linter/src/rules/unicorn/no_null.rs

@@ -10,7 +10,11 @@ use oxc_span::{GetSpan, Span};
 use oxc_syntax::operator::BinaryOperator;
 
 use crate::{
-    ast_util::is_method_call, context::LintContext, fixer::RuleFixer, rule::Rule, AstNode, Fix,
+    ast_util::is_method_call,
+    context::LintContext,
+    fixer::{Fix, RuleFixer},
+    rule::Rule,
+    AstNode,
 };
 
 fn replace_null_diagnostic(span0: Span) -> OxcDiagnostic {