feat(linter/language_server): add second editor suggestion for react/forward-ref-uses-ref

crates/oxc_language_server/fixtures/linter/multiple_suggestions/.oxlintrc.json

@@ -0,0 +1,9 @@
+{
+  "plugins": ["react"],
+  "categories": {
+    "correctness": "off"
+  },
+  "rules": {
+    "react/forward-ref-uses-ref": "error"
+  }
+}

crates/oxc_language_server/fixtures/linter/multiple_suggestions/forward_ref.ts

@@ -0,0 +1 @@
+forwardRef((props) => {});

crates/oxc_language_server/fixtures/linter/multiple_suggestions/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "react": "latest"
+  }
+}

crates/oxc_language_server/src/linter/server_linter.rs

@@ -350,4 +350,19 @@ mod test {
         Tester::new("fixtures/linter/cross_module_extended_config", None)
             .test_and_snapshot_single_file("dep-a.ts");
     }
+
+    #[test]
+    fn test_multiple_suggestions() {
+        Tester::new(
+            "fixtures/linter/multiple_suggestions",
+            Some(Options {
+                flags: FxHashMap::from_iter([(
+                    "fix_kind".to_string(),
+                    "safe_fix_or_suggestion".to_string(),
+                )]),
+                ..Options::default()
+            }),
+        )
+        .test_and_snapshot_single_file("forward_ref.ts");
+    }
 }

crates/oxc_language_server/src/snapshots/fixtures_linter_multiple_suggestions@forward_ref.ts.snap

@@ -0,0 +1,15 @@
+---
+source: crates/oxc_language_server/src/tester.rs
+input_file: crates/oxc_language_server/fixtures/linter/multiple_suggestions/forward_ref.ts
+---
+code: "eslint-plugin-react(forward-ref-uses-ref)"
+code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/react/forward-ref-uses-ref.html"
+message: "Components wrapped with `forwardRef` must have a `ref` parameter\nhelp: Add a `ref` parameter, or remove `forwardRef`"
+range: Range { start: Position { line: 0, character: 11 }, end: Position { line: 0, character: 24 } }
+related_information[0].message: ""
+related_information[0].location.uri: "file://<variable>/fixtures/linter/multiple_suggestions/forward_ref.ts"
+related_information[0].location.range: Range { start: Position { line: 0, character: 11 }, end: Position { line: 0, character: 24 } }
+severity: Some(Error)
+source: Some("oxc")
+tags: None
+fixed: Multiple([FixedContent { message: Some("remove `forwardRef` wrapper"), code: "(props) => {}", range: Range { start: Position { line: 0, character: 0 }, end: Position { line: 0, character: 25 } } }, FixedContent { message: Some("add `ref` parameter"), code: "(props, ref)", range: Range { start: Position { line: 0, character: 11 }, end: Position { line: 0, character: 18 } } }])

crates/oxc_linter/src/fixer/mod.rs

@@ -311,11 +311,21 @@ impl GetSpan for Message<'_> {
 pub struct Fixer<'a> {
     source_text: &'a str,
     messages: Vec<Message<'a>>,
+
+    // To test different fixes, we need to override the default behavior.
+    // The behavior is oriented by `oxlint` where only one PossibleFixes is applied.
+    fix_index: u8,
 }
 
 impl<'a> Fixer<'a> {
     pub fn new(source_text: &'a str, messages: Vec<Message<'a>>) -> Self {
-        Self { source_text, messages }
+        Self { source_text, messages, fix_index: 0 }
+    }
+
+    #[cfg(test)]
+    pub fn with_fix_index(mut self, fix_index: u8) -> Self {
+        self.fix_index = fix_index;
+        self
     }
 
     /// # Panics
@@ -343,7 +353,7 @@ impl<'a> Fixer<'a> {
                 PossibleFixes::Single(fix) => Some(fix),
                 // For multiple fixes, we take the first one as a representative fix.
                 // Applying all possible fixes at once is not possible in this context.
-                PossibleFixes::Multiple(multiple) => multiple.first(),
+                PossibleFixes::Multiple(multiple) => multiple.get(self.fix_index as usize),
             };
             let Some(Fix { content, span, .. }) = fix else {
                 filtered_messages.push(m);

crates/oxc_linter/src/rules/react/forward_ref_uses_ref.rs

@@ -6,7 +6,7 @@ use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_span::Span;
 
-use crate::{AstNode, context::LintContext, rule::Rule};
+use crate::{AstNode, context::LintContext, fixer::RuleFixer, rule::Rule};
 
 fn forward_ref_uses_ref_diagnostic(span: Span) -> OxcDiagnostic {
     OxcDiagnostic::warn("Components wrapped with `forwardRef` must have a `ref` parameter")
@@ -60,9 +60,14 @@ declare_oxc_lint!(
     react,
     correctness,
     suggestion
+    suggestion
 );
 
-fn check_forward_ref_inner(exp: &Expression, call_expr: &CallExpression, ctx: &LintContext) {
+fn check_forward_ref_inner<'a>(
+    exp: &Expression,
+    call_expr: &CallExpression,
+    ctx: &LintContext<'a>,
+) {
     let (params, span) = match exp {
         Expression::ArrowFunctionExpression(f) => (&f.params, f.span),
         Expression::FunctionExpression(f) => (&f.params, f.span),
@@ -71,9 +76,26 @@ fn check_forward_ref_inner(exp: &Expression, call_expr: &CallExpression, ctx: &L
     if params.parameters_count() != 1 || params.rest.is_some() {
         return;
     }
-    ctx.diagnostic_with_suggestion(forward_ref_uses_ref_diagnostic(span), |fixer| {
-        fixer.replace_with(call_expr, exp)
-    });
+
+    ctx.diagnostics_with_multiple_fixes(
+        forward_ref_uses_ref_diagnostic(span),
+        (FixKind::Suggestion, |fixer: RuleFixer<'_, 'a>| {
+            fixer.replace_with(call_expr, exp).with_message("remove `forwardRef` wrapper")
+        }),
+        (FixKind::Suggestion, |fixer: RuleFixer<'_, 'a>| {
+            let fixed = ctx.source_range(params.span);
+            // remove the trailing `)`, `` and `,` if they exist
+            let fixed = fixed.strip_suffix(')').unwrap_or(fixed).trim_end();
+            let mut fixed = fixed.strip_suffix(',').unwrap_or(fixed).to_string();
+
+            if !fixed.starts_with('(') {
+                fixed.insert(0, '(');
+            }
+            fixed.push_str(", ref)");
+
+            fixer.replace(params.span, fixed).with_message("add `ref` parameter")
+        }),
+    );
 }
 
 impl Rule for ForwardRefUsesRef {
@@ -192,11 +214,15 @@ fn test() {
     ];
 
     let fix = vec![
-        ("forwardRef((a) => {})", "(a) => {}"),
-        ("forwardRef(a => {})", "a => {}"),
-        ("forwardRef(function (a) {})", "function (a) {}"),
-        ("forwardRef(function(a,) {})", "function(a,) {}"),
-        ("React.forwardRef(function(a) {})", "function(a) {}"),
+        ("forwardRef((a) => {})", ("(a) => {}", "forwardRef((a, ref) => {})")),
+        ("forwardRef(a => {})", ("a => {}", "forwardRef((a, ref) => {})")),
+        ("forwardRef(function (a) {})", ("function (a) {}", "forwardRef(function (a, ref) {})")),
+        ("forwardRef(function(a,) {})", ("function(a,) {}", "forwardRef(function(a, ref) {})")),
+        ("forwardRef(function(a, ) {})", ("function(a, ) {}", "forwardRef(function(a, ref) {})")),
+        (
+            "React.forwardRef(function(a) {})",
+            ("function(a) {}", "React.forwardRef(function(a, ref) {})"),
+        ),
     ];
 
     Tester::new(ForwardRefUsesRef::NAME, ForwardRefUsesRef::PLUGIN, pass, fail)

crates/oxc_linter/src/tester.rs

@@ -123,46 +123,62 @@ impl From<Option<FixKind>> for ExpectFixKind {
 }
 
 #[derive(Debug, Clone)]
-pub struct ExpectFix {
+pub struct ExpectFixTestCase {
     /// Source code being tested
     source: String,
+    expected: Vec<ExpectFix>,
+    rule_config: Option<Value>,
+}
+
+#[derive(Debug, Clone)]
+struct ExpectFix {
     /// Expected source code after fix has been applied
     expected: String,
     kind: ExpectFixKind,
-    rule_config: Option<Value>,
 }
 
-impl<S: Into<String>> From<(S, S, Option<Value>)> for ExpectFix {
+impl<S: Into<String>> From<(S, S, Option<Value>)> for ExpectFixTestCase {
     fn from(value: (S, S, Option<Value>)) -> Self {
         Self {
             source: value.0.into(),
-            expected: value.1.into(),
-            kind: ExpectFixKind::Any,
+            expected: vec![ExpectFix { expected: value.1.into(), kind: ExpectFixKind::Any }],
             rule_config: value.2,
         }
     }
 }
 
-impl<S: Into<String>> From<(S, S)> for ExpectFix {
+impl<S: Into<String>> From<(S, S)> for ExpectFixTestCase {
     fn from(value: (S, S)) -> Self {
         Self {
             source: value.0.into(),
-            expected: value.1.into(),
-            kind: ExpectFixKind::Any,
+            expected: vec![ExpectFix { expected: value.1.into(), kind: ExpectFixKind::Any }],
             rule_config: None,
         }
     }
 }
-impl<S, F> From<(S, S, Option<Value>, F)> for ExpectFix
+
+impl<S: Into<String>> From<(S, (S, S))> for ExpectFixTestCase {
+    fn from(value: (S, (S, S))) -> Self {
+        Self {
+            source: value.0.into(),
+            expected: vec![
+                ExpectFix { expected: value.1.0.into(), kind: ExpectFixKind::Any },
+                ExpectFix { expected: value.1.1.into(), kind: ExpectFixKind::Any },
+            ],
+            rule_config: None,
+        }
+    }
+}
+
+impl<S, F> From<(S, S, Option<Value>, F)> for ExpectFixTestCase
 where
     S: Into<String>,
     F: Into<ExpectFixKind>,
 {
     fn from((source, expected, config, kind): (S, S, Option<Value>, F)) -> Self {
         Self {
             source: source.into(),
-            expected: expected.into(),
-            kind: kind.into(),
+            expected: vec![ExpectFix { expected: expected.into(), kind: kind.into() }],
             rule_config: config,
         }
     }
@@ -206,7 +222,7 @@ pub struct Tester {
     ///
     /// Note that disabling this check should be done as little as possible, and
     /// never in bad faith (e.g. no `#[test]` functions have fixer cases at all).
-    expect_fix: Option<Vec<ExpectFix>>,
+    expect_fix: Option<Vec<ExpectFixTestCase>>,
     snapshot: String,
     /// Suffix added to end of snapshot name.
     ///
@@ -345,7 +361,7 @@ impl Tester {
     /// Tester::new("no-undef", pass, fail).expect_fix(fix).test();
     /// ```
     #[must_use]
-    pub fn expect_fix<F: Into<ExpectFix>>(mut self, expect_fix: Vec<F>) -> Self {
+    pub fn expect_fix<F: Into<ExpectFixTestCase>>(mut self, expect_fix: Vec<F>) -> Self {
         // prevent `expect_fix` abuse
         assert!(
             !expect_fix.is_empty(),
@@ -398,8 +414,14 @@ impl Tester {
 
     fn test_pass(&mut self) {
         for TestCase { source, rule_config, eslint_config, path } in self.expect_pass.clone() {
-            let result =
-                self.run(&source, rule_config.clone(), &eslint_config, path, ExpectFixKind::None);
+            let result = self.run(
+                &source,
+                rule_config.clone(),
+                &eslint_config,
+                path,
+                ExpectFixKind::None,
+                0,
+            );
             let passed = result == TestResult::Passed;
             let config = rule_config.map_or_else(
                 || "\n\n------------------------\n".to_string(),
@@ -420,8 +442,14 @@ impl Tester {
 
     fn test_fail(&mut self) {
         for TestCase { source, rule_config, eslint_config, path } in self.expect_fail.clone() {
-            let result =
-                self.run(&source, rule_config.clone(), &eslint_config, path, ExpectFixKind::None);
+            let result = self.run(
+                &source,
+                rule_config.clone(),
+                &eslint_config,
+                path,
+                ExpectFixKind::None,
+                0,
+            );
             let failed = result == TestResult::Failed;
             let config = rule_config.map_or_else(
                 || "\n\n------------------------".to_string(),
@@ -439,6 +467,7 @@ impl Tester {
         }
     }
 
+    #[expect(clippy::cast_possible_truncation)] // there are no rules with over 255 different possible fixes
     fn test_fix(&mut self) {
         // If auto-fixes are reported, make sure some fix test cases are provided
         let rule = self.find_rule();
@@ -453,15 +482,19 @@ impl Tester {
         };
 
         for fix in fix_test_cases {
-            let ExpectFix { source, expected, kind, rule_config: config } = fix;
-            let result = self.run(&source, config, &None, None, kind);
-            match result {
-                TestResult::Fixed(fixed_str) => assert_eq!(
-                    expected, fixed_str,
-                    r#"Expected "{source}" to be fixed into "{expected}""#
-                ),
-                TestResult::Passed => panic!("Expected a fix, but test passed: {source}"),
-                TestResult::Failed => panic!("Expected a fix, but test failed: {source}"),
+            let ExpectFixTestCase { source, expected, rule_config: config } = fix;
+            for (index, expect) in expected.iter().enumerate() {
+                let result =
+                    self.run(&source, config.clone(), &None, None, expect.kind, index as u8);
+                match result {
+                    TestResult::Fixed(fixed_str) => assert_eq!(
+                        expect.expected, fixed_str,
+                        r#"Expected "{source}" to be fixed into "{}""#,
+                        expect.expected
+                    ),
+                    TestResult::Passed => panic!("Expected a fix, but test passed: {source}"),
+                    TestResult::Failed => panic!("Expected a fix, but test failed: {source}"),
+                }
             }
         }
     }
@@ -473,7 +506,8 @@ impl Tester {
         rule_config: Option<Value>,
         eslint_config: &Option<Value>,
         path: Option<PathBuf>,
-        fix: ExpectFixKind,
+        fix_kind: ExpectFixKind,
+        fix_index: u8,
     ) -> TestResult {
         let allocator = Allocator::default();
         let rule = self.find_rule().read_json(rule_config.unwrap_or_default());
@@ -492,7 +526,7 @@ impl Tester {
                 FxHashMap::default(),
             ),
         )
-        .with_fix(fix.into());
+        .with_fix(fix_kind.into());
 
         let path_to_lint = if self.plugins.has_import() {
             assert!(path.is_none(), "import plugin does not support path");
@@ -520,8 +554,8 @@ impl Tester {
             return TestResult::Passed;
         }
 
-        if fix.is_some() {
-            let fix_result = Fixer::new(source_text, result).fix();
+        if fix_kind.is_some() {
+            let fix_result = Fixer::new(source_text, result).with_fix_index(fix_index).fix();
             return TestResult::Fixed(fix_result.fixed_code.to_string());
         }