feat(linter): add fixer for no-useless-fallback-in-spread rule (#3544)

DonIsaac · web-flow · commit 1fb9d234a6e9 · 2024-06-06T13:59:42.000+08:00
diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_fallback_in_spread.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_fallback_in_spread.rs
@@ -2,15 +2,17 @@ use oxc_ast::{ast::Expression, AstKind};
 use oxc_diagnostics::OxcDiagnostic;
 
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
+use oxc_span::{GetSpan, Span};
 use oxc_syntax::operator::LogicalOperator;
 
-use crate::{ast_util::outermost_paren_parent, context::LintContext, rule::Rule, AstNode};
+use crate::{
+    ast_util::outermost_paren_parent, context::LintContext, fixer::Fix, rule::Rule, AstNode,
+};
 
-fn no_useless_fallback_in_spread_diagnostic(span0: Span) -> OxcDiagnostic {
+fn no_useless_fallback_in_spread_diagnostic(span: Span) -> OxcDiagnostic {
     OxcDiagnostic::warn("eslint-plugin-unicorn(no-useless-fallback-in-spread): Disallow useless fallback when spreading in object literals")
         .with_help("Spreading falsy values in object literals won't add any unexpected properties, so it's unnecessary to add an empty object as fallback.")
-        .with_labels([span0.into()])
+        .with_labels([span.into()])
 }
 
 #[derive(Debug, Default, Clone)]
@@ -75,7 +77,29 @@ impl Rule for NoUselessFallbackInSpread {
             return;
         }
 
-        ctx.diagnostic(no_useless_fallback_in_spread_diagnostic(spread_element.span));
+        let diagnostic = no_useless_fallback_in_spread_diagnostic(spread_element.span);
+
+        if can_fix(&logical_expression.left) {
+            ctx.diagnostic_with_fix(diagnostic, || {
+                let left_text = logical_expression.left.span().source_text(ctx.source_text());
+                Fix::new(format!("...{left_text}"), spread_element.span)
+            });
+        } else {
+            ctx.diagnostic(diagnostic);
+        }
+    }
+}
+
+fn can_fix(left: &Expression<'_>) -> bool {
+    const BANNED_IDENTIFIERS: [&str; 3] = ["undefined", "NaN", "Infinity"];
+    match left.without_parenthesized() {
+        Expression::Identifier(ident) => !BANNED_IDENTIFIERS.contains(&ident.name.as_str()),
+        Expression::LogicalExpression(expr) => can_fix(&expr.left),
+        Expression::ObjectExpression(_)
+        | Expression::CallExpression(_)
+        | Expression::StaticMemberExpression(_)
+        | Expression::ComputedMemberExpression(_) => true,
+        _ => false,
     }
 }
 
@@ -131,5 +155,15 @@ fn test() {
         r"const object = {...(document.all || {})}",
     ];
 
-    Tester::new(NoUselessFallbackInSpread::NAME, pass, fail).test_and_snapshot();
+    let fix = vec![
+        //
+        (r"const object = {...(foo || {})}", r"const object = {...foo}"),
+        (r"const object = {...(foo() || {})}", r"const object = {...foo()}"),
+        (r"const object = {...((foo && {}) || {})}", "const object = {...(foo && {})}"),
+        (r"const object = {...(0 || {})}", r"const object = {...(0 || {})}"),
+        (r"const object = {...(NaN || {})}", r"const object = {...(NaN || {})}"),
+        (r"const object = {...(Infinity || {})}", r"const object = {...(Infinity || {})}"),
+    ];
+
+    Tester::new(NoUselessFallbackInSpread::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
 }
diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs
@@ -65,12 +65,30 @@ impl From<(&str, Option<Value>, Option<Value>, Option<PathBuf>)> for TestCase {
     }
 }
 
+#[derive(Debug, Clone)]
+pub struct ExpectFix {
+    /// Source code being tested
+    source: String,
+    /// Expected source code after fix has been applied
+    expected: String,
+    rule_config: Option<Value>,
+}
+impl<S: Into<String>> From<(S, S, Option<Value>)> for ExpectFix {
+    fn from(value: (S, S, Option<Value>)) -> Self {
+        Self { source: value.0.into(), expected: value.1.into(), rule_config: value.2 }
+    }
+}
+impl<S: Into<String>> From<(S, S)> for ExpectFix {
+    fn from(value: (S, S)) -> Self {
+        Self { source: value.0.into(), expected: value.1.into(), rule_config: None }
+    }
+}
 pub struct Tester {
     rule_name: &'static str,
     rule_path: PathBuf,
     expect_pass: Vec<TestCase>,
     expect_fail: Vec<TestCase>,
-    expect_fix: Vec<(String, String, Option<Value>)>,
+    expect_fix: Vec<ExpectFix>,
     snapshot: String,
     current_working_directory: Box<Path>,
     import_plugin: bool,
@@ -138,9 +156,8 @@ impl Tester {
         self
     }
 
-    pub fn expect_fix<S: Into<String>>(mut self, expect_fix: Vec<(S, S, Option<Value>)>) -> Self {
-        self.expect_fix =
-            expect_fix.into_iter().map(|(s1, s2, r)| (s1.into(), s2.into(), r)).collect::<Vec<_>>();
+    pub fn expect_fix<F: Into<ExpectFix>>(mut self, expect_fix: Vec<F>) -> Self {
+        self.expect_fix = expect_fix.into_iter().map(std::convert::Into::into).collect::<Vec<_>>();
         self
     }
 
@@ -179,8 +196,9 @@ impl Tester {
     }
 
     fn test_fix(&mut self) {
-        for (test, expected, config) in self.expect_fix.clone() {
-            let result = self.run(&test, config, &None, None, true);
+        for fix in self.expect_fix.clone() {
+            let ExpectFix { source, expected, rule_config: config } = fix;
+            let result = self.run(&source, config, &None, None, true);
             if let TestResult::Fixed(fixed_str) = result {
                 assert_eq!(expected, fixed_str);
             } else {
