diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 5832b4d85..41bfc8a84 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -182,6 +182,7 @@ mod unicorn { pub mod prefer_dom_node_append; pub mod prefer_dom_node_dataset; pub mod prefer_event_target; + pub mod prefer_includes; pub mod prefer_logical_operator_over_ternary; pub mod prefer_math_trunc; pub mod prefer_optional_catch_binding; @@ -348,6 +349,7 @@ oxc_macros::declare_all_lint_rules! { unicorn::prefer_dom_node_append, unicorn::prefer_dom_node_dataset, unicorn::prefer_event_target, + unicorn::prefer_includes, unicorn::prefer_logical_operator_over_ternary, unicorn::prefer_math_trunc, unicorn::prefer_optional_catch_binding, diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_includes.rs b/crates/oxc_linter/src/rules/unicorn/prefer_includes.rs new file mode 100644 index 000000000..a6b36526d --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/prefer_includes.rs @@ -0,0 +1,145 @@ +use oxc_ast::{ast::Expression, AstKind}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use oxc_syntax::operator::{BinaryOperator, UnaryOperator}; + +use crate::{ + ast_util::{call_expr_method_callee_info, is_method_call}, + context::LintContext, + rule::Rule, + AstNode, +}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence.")] +#[diagnostic(severity(warning))] +struct PreferIncludesDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct PreferIncludes; + +declare_oxc_lint!( + /// ### What it does + /// + /// Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + /// + /// All built-ins have `.includes()` in addition to `.indexOf()`. + /// + /// ### Why is this bad? + /// + /// The `.includes()` method is more readable and less error-prone than `.indexOf()`. + /// + /// ### Example + /// ```javascript + /// // bad + /// if (str.indexOf('foo') !== -1) { } + /// + /// // good + /// if (str.includes('foo')) { } + /// ``` + PreferIncludes, + style +); + +impl Rule for PreferIncludes { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::BinaryExpression(bin_expr) = node.kind() else { + return; + }; + + let Expression::CallExpression(left_call_expr) = &bin_expr.left.without_parenthesized() + else { + return; + }; + + if !is_method_call(left_call_expr, None, Some(&["indexOf"]), None, Some(2)) { + return; + } + + if matches!( + bin_expr.operator, + BinaryOperator::StrictInequality + | BinaryOperator::Inequality + | BinaryOperator::GreaterThan + | BinaryOperator::StrictEquality + | BinaryOperator::Equality + ) { + if !is_negative_one(bin_expr.right.without_parenthesized()) { + return; + } + + ctx.diagnostic(PreferIncludesDiagnostic( + call_expr_method_callee_info(left_call_expr).unwrap().0, + )); + } + + if matches!(bin_expr.operator, BinaryOperator::GreaterEqualThan | BinaryOperator::LessThan) + { + let Expression::NumberLiteral(num_lit) = bin_expr.right.without_parenthesized() else { + return; + }; + + if num_lit.raw != "0" { + return; + } + ctx.diagnostic(PreferIncludesDiagnostic( + call_expr_method_callee_info(left_call_expr).unwrap().0, + )); + } + } +} + +fn is_negative_one(expr: &Expression) -> bool { + let Expression::UnaryExpression(unary_expr) = expr else { + return false; + }; + + if unary_expr.operator != UnaryOperator::UnaryNegation { + return false; + } + + let Expression::NumberLiteral(num_lit) = unary_expr.argument.without_parenthesized() else { + return false; + }; + + num_lit.raw == "1" +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + r"str.indexOf('foo') !== -n", + r"str.indexOf('foo') !== 1", + r"str.indexOf('foo') === -2", + r"!str.indexOf('foo') === 1", + r"!str.indexOf('foo') === -n", + r"str.includes('foo')", + r"'foobar'.includes('foo')", + r"[1,2,3].includes(4)", + r"null.indexOf('foo') !== 1", + r"f(0) < 0", + r"something.indexOf(foo, 0, another) !== -1", + ]; + + let fail = vec![ + r"'foobar'.indexOf('foo') !== -1", + r"str.indexOf('foo') != -1", + r"str.indexOf('foo') > -1", + r"str.indexOf('foo') == -1", + r"'foobar'.indexOf('foo') >= 0", + r"[1,2,3].indexOf(4) !== -1", + r"str.indexOf('foo') < 0", + r"''.indexOf('foo') < 0", + r"(a || b).indexOf('foo') === -1", + r"foo.indexOf(bar, 0) !== -1", + r"foo.indexOf(bar, 1) !== -1", + ]; + + Tester::new_without_config(PreferIncludes::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/prefer_includes.snap b/crates/oxc_linter/src/snapshots/prefer_includes.snap new file mode 100644 index 000000000..d6dc73ed3 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/prefer_includes.snap @@ -0,0 +1,71 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: prefer_includes +--- + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ 'foobar'.indexOf('foo') !== -1 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ str.indexOf('foo') != -1 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ str.indexOf('foo') > -1 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ str.indexOf('foo') == -1 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ 'foobar'.indexOf('foo') >= 0 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ [1,2,3].indexOf(4) !== -1 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ str.indexOf('foo') < 0 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ ''.indexOf('foo') < 0 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ (a || b).indexOf('foo') === -1 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ foo.indexOf(bar, 0) !== -1 + · ─────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-includes): Prefer `includes()` over `indexOf()` when checking for existence or non-existence. + ╭─[prefer_includes.tsx:1:1] + 1 │ foo.indexOf(bar, 1) !== -1 + · ─────── + ╰──── + +