feat(linter): accept multiple fixes when fix code

[mysteryven]

Want to add this because of there are bunch of fix codes in consistent-type-import are not easy to port.

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[mysteryven]

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

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mysteryven and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #3842 will not alter performance

_{Comparing 06-23-feat_linter_add_StringFixer_to_help_modify_fixer_text (63b98bd) with main (a471e62)}

Summary

✅ 28 untouched benchmarks

[Boshen]

What's the difference between this and

oxc/crates/oxc_linter/src/fixer.rs

Line 29 in d76f34b

pub struct RuleFixer<'c, 'a: 'c> {

?

Ideally we should only have 1 - a port from eslint.

[mysteryven]

I need to compose multi Fix at one time(one line code may have to delete and replace). And have three thoughts at first, one is shown as this PR, and another is change our Fix

#[derive(Debug, Clone, Default)]
	pub struct Fix<'a> {
-		pub contents: Cow<'a, str>,
+		pub contents: Vec<Cow<'a, str>>,
-		pub spans: Span,
+		pub spans: Vec<Span>,
}

 ctx.diagnostic_with_fix(
      no_import_type_side_effects_diagnostic(import_decl.span),
      |fixer| {
          let raw = ctx.source_range(import_decl.span);
          let mut string_fixer = StringFixer::new(raw);

          let mut fix = fixer.new_fix();
          fix.replace(span, "abc");
          fix.delete(span, "abc");
		fix
 })

I checked rule-fixer.js, and found change Fix is not keep same with ESLint, ESLint produces one fix in RuleFixer's method, so I didn't change RuleFixer

The third option is let the fixer return Vec<fix>, I thought it will influence many files that have a fixer.

#[derive(Clone)]
pub struct Message<'a> {
    pub error: OxcDiagnostic,
    pub start: u32,
    pub end: u32,
+ pub fixs: Option<Vec<Fix<'a>>>,
- pub fix: Option<Fix<'a>>,
    fixed: bool,
}

Finally I chose option 1. I thought this way only need to change the least things =.=

Now, looks like option 3 is better, are we to accept it? or would you like to give me some advice?

[Boshen]

My question is always how does eslint do it? Can we align our implementation with theirs?

[Boshen]

Are there unit tests from eslint? We should port them over.

[mysteryven]

Are there unit tests from eslint? We should port them over.

Added now, and it really exposed my bug 😥

[graphite-app]

Merge activity

• Jun 26, 9:28 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Jun 26, 9:29 AM EDT: Boshen added this pull request to the Graphite merge queue.
• Jun 26, 9:33 AM EDT: Boshen merged this pull request with the Graphite merge queue.