diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_includes.rs b/crates/oxc_linter/src/rules/unicorn/prefer_includes.rs index 4ab71bdeb3e7c..0a2281f9aaff1 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_includes.rs +++ b/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 = 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::>() + .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(); } diff --git a/crates/oxc_linter/src/snapshots/unicorn_prefer_includes.snap b/crates/oxc_linter/src/snapshots/unicorn_prefer_includes.snap index 82abce56de6b6..0b2ed2ee2d7b6 100644 --- a/crates/oxc_linter/src/snapshots/unicorn_prefer_includes.snap +++ b/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`.