feat(linter): implement fixer for unicorn/prefer-includes

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

@@ -1,7 +1,10 @@
-use oxc_ast::{AstKind, ast::Expression};
+use oxc_ast::{
+    AstKind,
+    ast::{ChainElement, Expression},
+};
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
-use oxc_span::Span;
+use oxc_span::{GetSpan, Span};
 use oxc_syntax::operator::{BinaryOperator, UnaryOperator};
 
 use crate::{
@@ -21,6 +24,16 @@ fn prefer_includes_diagnostic(span: Span) -> OxcDiagnostic {
 #[derive(Debug, Default, Clone)]
 pub struct PreferIncludes;
 
+#[derive(Debug, Clone, Copy)]
+enum ComparisonKind {
+    // `indexOf(...) != -1` / `!== -1`
+    ExistsOrUndefined,
+    // `indexOf(...) > -1` / `>= 0`
+    ExistsOnly,
+    // `indexOf(...) == -1` / `=== -1` / `< 0`
+    NotExistsOnly,
+}
+
 declare_oxc_lint!(
     /// ### What it does
     ///
@@ -45,7 +58,7 @@ declare_oxc_lint!(
     PreferIncludes,
     unicorn,
     style,
-    pending
+    suggestion
 );
 
 impl Rule for PreferIncludes {
@@ -54,45 +67,112 @@ impl Rule for PreferIncludes {
             return;
         };
 
-        let Expression::CallExpression(left_call_expr) = &bin_expr.left.without_parentheses()
-        else {
-            return;
+        let left_call_expr = match bin_expr.left.without_parentheses() {
+            Expression::CallExpression(call_expr) => call_expr,
+            Expression::ChainExpression(chain_expr) => {
+                let ChainElement::CallExpression(call_expr) = &chain_expr.expression else {
+                    return;
+                };
+                call_expr
+            }
+            _ => return,
         };
 
         if !is_method_call(left_call_expr, None, Some(&["indexOf"]), None, Some(2)) {
             return;
         }
 
-        if matches!(
+        let comparison_kind: Option<ComparisonKind> = if matches!(
             bin_expr.operator,
-            BinaryOperator::StrictInequality
-                | BinaryOperator::Inequality
-                | BinaryOperator::GreaterThan
-                | BinaryOperator::StrictEquality
-                | BinaryOperator::Equality
+            BinaryOperator::StrictInequality | BinaryOperator::Inequality
         ) {
-            if !is_negative_one(bin_expr.right.without_parentheses()) {
-                return;
+            if is_negative_one(bin_expr.right.without_parentheses()) {
+                Some(ComparisonKind::ExistsOrUndefined)
+            } else {
+                None
             }
-
-            ctx.diagnostic(prefer_includes_diagnostic(
-                call_expr_method_callee_info(left_call_expr).unwrap().0,
-            ));
-        }
-
-        if matches!(bin_expr.operator, BinaryOperator::GreaterEqualThan | BinaryOperator::LessThan)
-        {
+        } else if bin_expr.operator == BinaryOperator::GreaterThan {
+            if is_negative_one(bin_expr.right.without_parentheses()) {
+                Some(ComparisonKind::ExistsOnly)
+            } else {
+                None
+            }
+        } else if matches!(
+            bin_expr.operator,
+            BinaryOperator::StrictEquality | BinaryOperator::Equality
+        ) {
+            if is_negative_one(bin_expr.right.without_parentheses()) {
+                Some(ComparisonKind::NotExistsOnly)
+            } else {
+                None
+            }
+        } else if bin_expr.operator == BinaryOperator::GreaterEqualThan {
             let Expression::NumericLiteral(num_lit) = bin_expr.right.without_parentheses() else {
                 return;
             };
-
-            if num_lit.raw.as_ref().unwrap() != "0" {
+            if num_lit.raw.as_ref().unwrap() == "0" {
+                Some(ComparisonKind::ExistsOnly)
+            } else {
+                None
+            }
+        } else if bin_expr.operator == BinaryOperator::LessThan {
+            let Expression::NumericLiteral(num_lit) = bin_expr.right.without_parentheses() else {
                 return;
+            };
+            if num_lit.raw.as_ref().unwrap() == "0" {
+                Some(ComparisonKind::NotExistsOnly)
+            } else {
+                None
             }
-            ctx.diagnostic(prefer_includes_diagnostic(
-                call_expr_method_callee_info(left_call_expr).unwrap().0,
-            ));
-        }
+        } else {
+            None
+        };
+
+        let Some(comparison_kind) = comparison_kind else {
+            return;
+        };
+
+        let callee_info = call_expr_method_callee_info(left_call_expr).unwrap();
+        let callee_span = callee_info.0;
+
+        // Get the object (receiver) text.
+        let Some(member_expr) = left_call_expr.callee.get_inner_expression().as_member_expression()
+        else {
+            return;
+        };
+
+        let object_text = ctx.source_range(member_expr.object().span()).to_string();
+        let member_operator = if member_expr.optional() { "?." } else { "." };
+        let call_operator = if left_call_expr.optional { "?.(" } else { "(" };
+        let has_optional_chain = left_call_expr.optional || member_expr.optional();
+
+        // Get arguments text
+        let args_text = left_call_expr
+            .arguments
+            .iter()
+            .map(|arg| ctx.source_range(arg.span()))
+            .collect::<Vec<_>>()
+            .join(", ");
+
+        let fix_span = bin_expr.span;
+        ctx.diagnostic_with_suggestion(prefer_includes_diagnostic(callee_span), |fixer| {
+            let includes_call =
+                format!("{object_text}{member_operator}includes{call_operator}{args_text})");
+
+            let replacement = if has_optional_chain {
+                match comparison_kind {
+                    ComparisonKind::ExistsOrUndefined => format!("{includes_call} !== false"),
+                    ComparisonKind::ExistsOnly => format!("{includes_call} === true"),
+                    ComparisonKind::NotExistsOnly => format!("{includes_call} === false"),
+                }
+            } else {
+                match comparison_kind {
+                    ComparisonKind::NotExistsOnly => format!("!{includes_call}"),
+                    ComparisonKind::ExistsOrUndefined | ComparisonKind::ExistsOnly => includes_call,
+                }
+            };
+            fixer.replace(fix_span, replacement)
+        });
     }
 }
 
@@ -142,7 +222,34 @@ fn test() {
         r"(a || b).indexOf('foo') === -1",
         r"foo.indexOf(bar, 0) !== -1",
         r"foo.indexOf(bar, 1) !== -1",
+        r"foo?.indexOf('x') !== -1",
+        r"foo.indexOf?.('x') === -1",
+        r"foo?.indexOf?.('x') == -1",
+        r"foo?.indexOf('x') > -1",
+        r"foo?.indexOf('x') >= 0",
+        r"foo?.indexOf('x') < 0",
+    ];
+
+    let fix = vec![
+        (r"'foobar'.indexOf('foo') !== -1", r"'foobar'.includes('foo')"),
+        (r"str.indexOf('foo') != -1", r"str.includes('foo')"),
+        (r"str.indexOf('foo') > -1", r"str.includes('foo')"),
+        (r"str.indexOf('foo') == -1", r"!str.includes('foo')"),
+        (r"'foobar'.indexOf('foo') >= 0", r"'foobar'.includes('foo')"),
+        (r"[1,2,3].indexOf(4) !== -1", r"[1,2,3].includes(4)"),
+        (r"str.indexOf('foo') < 0", r"!str.includes('foo')"),
+        (r"''.indexOf('foo') < 0", r"!''.includes('foo')"),
+        (r"foo.indexOf(bar, 0) !== -1", r"foo.includes(bar, 0)"),
+        (r"foo.indexOf(bar, 1) !== -1", r"foo.includes(bar, 1)"),
+        (r"foo?.indexOf('x') !== -1", r"foo?.includes('x') !== false"),
+        (r"foo.indexOf?.('x') === -1", r"foo.includes?.('x') === false"),
+        (r"foo?.indexOf?.('x') == -1", r"foo?.includes?.('x') === false"),
+        (r"foo?.indexOf('x') > -1", r"foo?.includes('x') === true"),
+        (r"foo?.indexOf('x') >= 0", r"foo?.includes('x') === true"),
+        (r"foo?.indexOf('x') < 0", r"foo?.includes('x') === false"),
     ];
 
-    Tester::new(PreferIncludes::NAME, PreferIncludes::PLUGIN, pass, fail).test_and_snapshot();
+    Tester::new(PreferIncludes::NAME, PreferIncludes::PLUGIN, pass, fail)
+        .expect_fix(fix)
+        .test_and_snapshot();
 }

crates/oxc_linter/src/snapshots/unicorn_prefer_includes.snap

@@ -7,63 +7,116 @@ source: crates/oxc_linter/src/tester.rs
  1 │ 'foobar'.indexOf('foo') !== -1
    ·          ───────
    ╰────
+  help: Replace `'foobar'.indexOf('foo') !== -1` with `'foobar'.includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:5]
  1 │ str.indexOf('foo') != -1
    ·     ───────
    ╰────
+  help: Replace `str.indexOf('foo') != -1` with `str.includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:5]
  1 │ str.indexOf('foo') > -1
    ·     ───────
    ╰────
+  help: Replace `str.indexOf('foo') > -1` with `str.includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:5]
  1 │ str.indexOf('foo') == -1
    ·     ───────
    ╰────
+  help: Replace `str.indexOf('foo') == -1` with `!str.includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:10]
  1 │ 'foobar'.indexOf('foo') >= 0
    ·          ───────
    ╰────
+  help: Replace `'foobar'.indexOf('foo') >= 0` with `'foobar'.includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:9]
  1 │ [1,2,3].indexOf(4) !== -1
    ·         ───────
    ╰────
+  help: Replace `[1,2,3].indexOf(4) !== -1` with `[1,2,3].includes(4)`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:5]
  1 │ str.indexOf('foo') < 0
    ·     ───────
    ╰────
+  help: Replace `str.indexOf('foo') < 0` with `!str.includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:4]
  1 │ ''.indexOf('foo') < 0
    ·    ───────
    ╰────
+  help: Replace `''.indexOf('foo') < 0` with `!''.includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:10]
  1 │ (a || b).indexOf('foo') === -1
    ·          ───────
    ╰────
+  help: Replace `(a || b).indexOf('foo') === -1` with `!(a || b).includes('foo')`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:5]
  1 │ foo.indexOf(bar, 0) !== -1
    ·     ───────
    ╰────
+  help: Replace `foo.indexOf(bar, 0) !== -1` with `foo.includes(bar, 0)`.
 
   ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
    ╭─[prefer_includes.tsx:1:5]
  1 │ foo.indexOf(bar, 1) !== -1
    ·     ───────
    ╰────
+  help: Replace `foo.indexOf(bar, 1) !== -1` with `foo.includes(bar, 1)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
+   ╭─[prefer_includes.tsx:1:6]
+ 1 │ foo?.indexOf('x') !== -1
+   ·      ───────
+   ╰────
+  help: Replace `foo?.indexOf('x') !== -1` with `foo?.includes('x') !== false`.
+
+  ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
+   ╭─[prefer_includes.tsx:1:5]
+ 1 │ foo.indexOf?.('x') === -1
+   ·     ───────
+   ╰────
+  help: Replace `foo.indexOf?.('x') === -1` with `foo.includes?.('x') === false`.
+
+  ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
+   ╭─[prefer_includes.tsx:1:6]
+ 1 │ foo?.indexOf?.('x') == -1
+   ·      ───────
+   ╰────
+  help: Replace `foo?.indexOf?.('x') == -1` with `foo?.includes?.('x') === false`.
+
+  ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
+   ╭─[prefer_includes.tsx:1:6]
+ 1 │ foo?.indexOf('x') > -1
+   ·      ───────
+   ╰────
+  help: Replace `foo?.indexOf('x') > -1` with `foo?.includes('x') === true`.
+
+  ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
+   ╭─[prefer_includes.tsx:1:6]
+ 1 │ foo?.indexOf('x') >= 0
+   ·      ───────
+   ╰────
+  help: Replace `foo?.indexOf('x') >= 0` with `foo?.includes('x') === true`.
+
+  ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.
+   ╭─[prefer_includes.tsx:1:6]
+ 1 │ foo?.indexOf('x') < 0
+   ·      ───────
+   ╰────
+  help: Replace `foo?.indexOf('x') < 0` with `foo?.includes('x') === false`.