From b78135c66a1a6a7eda29bf3409d329470084e5f6 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Thu, 21 Nov 2024 21:14:37 +0300 Subject: [PATCH] fix(linter): add proper support for findIndex and findLastIndex for `unicorn/prefer-array-some` --- .../src/rules/unicorn/prefer_array_some.rs | 134 +++++++++++++++++- .../src/snapshots/prefer_array_some.snap | 131 +++++++++++++++-- 2 files changed, 251 insertions(+), 14 deletions(-) diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_array_some.rs b/crates/oxc_linter/src/rules/unicorn/prefer_array_some.rs index d52097d4b8f2a..d0c1ae474cfaa 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_array_some.rs +++ b/crates/oxc_linter/src/rules/unicorn/prefer_array_some.rs @@ -1,5 +1,5 @@ use oxc_ast::{ - ast::{Argument, CallExpression, Expression}, + ast::{Argument, CallExpression, Expression, UnaryOperator}, AstKind, }; use oxc_diagnostics::OxcDiagnostic; @@ -16,7 +16,7 @@ use crate::{ }; fn over_method(span: Span) -> OxcDiagnostic { - OxcDiagnostic::warn("Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.").with_label(span) + OxcDiagnostic::warn("Prefer `.some(…)` over `.find(…)` or `.findLast(…)`.").with_label(span) } fn non_zero_filter(span: Span) -> OxcDiagnostic { @@ -24,6 +24,11 @@ fn non_zero_filter(span: Span) -> OxcDiagnostic { .with_label(span) } +fn negative_one_or_zero_filter(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`.") + .with_label(span) +} + #[derive(Debug, Default, Clone)] pub struct PreferArraySome; @@ -55,12 +60,15 @@ declare_oxc_lint!( impl Rule for PreferArraySome { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { + // `.find(…)` + // `.findLast(…)` AstKind::CallExpression(call_expr) => { if !is_method_call(call_expr, None, Some(&["find", "findLast"]), Some(1), Some(2)) { return; } let is_compare = is_checking_undefined(node, call_expr, ctx); + if !is_compare && !is_boolean_node(node, ctx) { return; } @@ -87,6 +95,97 @@ impl Rule for PreferArraySome { ); } AstKind::BinaryExpression(bin_expr) => { + // `.{findIndex,findLastIndex}(…) !== -1` + // `.{findIndex,findLastIndex}(…) != -1` + // `.{findIndex,findLastIndex}(…) > -1` + // `.{findIndex,findLastIndex}(…) === -1` + // `.{findIndex,findLastIndex}(…) == -1` + // `.{findIndex,findLastIndex}(…) >= 0` + // `.{findIndex,findLastIndex}(…) < 0` + let with_negative_one = matches!( + bin_expr.operator, + BinaryOperator::StrictInequality + | BinaryOperator::Inequality + | BinaryOperator::GreaterThan + | BinaryOperator::StrictEquality + | BinaryOperator::Equality + ) && matches!( + bin_expr.right.without_parentheses(), + Expression::UnaryExpression(_) + ); + + let matches_against_zero = matches!( + bin_expr.operator, + BinaryOperator::GreaterEqualThan | BinaryOperator::LessThan + ); + + if with_negative_one { + if let Expression::UnaryExpression(right_unary_expr) = + &bin_expr.right.without_parentheses() + { + if matches!(right_unary_expr.operator, UnaryOperator::UnaryNegation) + && right_unary_expr.argument.is_number_literal() + && right_unary_expr.argument.is_number_value(1_f64) + { + let Expression::CallExpression(left_call_expr) = + &bin_expr.left.without_parentheses() + else { + return; + }; + + let Some(argument) = left_call_expr.arguments.first() else { + return; + }; + + if matches!(argument, Argument::SpreadElement(_)) { + return; + } + + if is_method_call( + left_call_expr, + None, + Some(&["findIndex", "findLastIndex"]), + None, + Some(1), + ) { + // TODO: fixer + ctx.diagnostic(negative_one_or_zero_filter( + call_expr_method_callee_info(left_call_expr).unwrap().0, + )); + } + } + } + } + + if matches_against_zero { + let Expression::NumericLiteral(right_num_lit) = &bin_expr.right else { + return; + }; + + let Expression::CallExpression(left_call_expr) = + &bin_expr.left.without_parentheses() + else { + return; + }; + + if right_num_lit.raw == "0" + && is_method_call( + left_call_expr, + None, + Some(&["findIndex", "findLastIndex"]), + None, + Some(1), + ) + { + // TODO: fixer + ctx.diagnostic(negative_one_or_zero_filter( + call_expr_method_callee_info(left_call_expr).unwrap().0, + )); + } + } + + // `.filter(…).length > 0` + // `.filter(…).length !== 0` if !matches!( bin_expr.operator, BinaryOperator::GreaterThan | BinaryOperator::StrictInequality @@ -281,6 +380,17 @@ fn test() { r"foo.find(fn) >= undefined", r"foo.find(fn) instanceof undefined", r#"typeof foo.find(fn) === "undefined""#, + // findIndex: negative one + r"foo.notMatchedMethod(bar) !== -1", + r"new foo.findIndex(bar) !== -1", + r"foo.findIndex(bar, extraArgument) !== -1", + r"foo.findIndex(bar) instanceof -1", + r"foo.findIndex(...bar) !== -1", + // findLastIndex: negative one + r"new foo.findLastIndex(bar) !== -1", + r"foo.findLastIndex(bar, extraArgument) !== -1", + r"foo.findLastIndex(bar) instanceof -1", + r"foo.findLastIndex(...bar) !== -1", ]; let fail = vec![ @@ -297,6 +407,26 @@ fn test() { r"foo.find(fn) != undefined", r"foo.find(fn) !== undefined", r#"a = (( ((foo.find(fn))) == ((null)) )) ? "no" : "yes";"#, + // findIndex: negative one || ( >= || < ) 0 + r"foo.findIndex(bar) !== -1", + r"foo.findIndex(bar) != -1", + r"foo.findIndex(bar) > - 1", + r"foo.findIndex(bar) === -1", + r"foo.findIndex(bar) == - 1", + r"foo.findIndex(bar) >= 0", + r"foo.findIndex(bar) < 0", + r"foo.findIndex(bar) !== (( - 1 ))", + r"foo.findIndex(element => element.bar === 1) !== (( - 1 ))", + // findLastIndex: negative one || ( >= || < ) 0 + r"foo.findLastIndex(bar) !== -1", + r"foo.findLastIndex(bar) != -1", + r"foo.findLastIndex(bar) > - 1", + r"foo.findLastIndex(bar) === -1", + r"foo.findLastIndex(bar) == - 1", + r"foo.findLastIndex(bar) >= 0", + r"foo.findLastIndex(bar) < 0", + r"foo.findLastIndex(bar) !== (( - 1 ))", + r"foo.findLastIndex(element => element.bar === 1) !== (( - 1 ))", ]; let fix = vec![ diff --git a/crates/oxc_linter/src/snapshots/prefer_array_some.snap b/crates/oxc_linter/src/snapshots/prefer_array_some.snap index dc838d5c118f8..c019faaff61d5 100644 --- a/crates/oxc_linter/src/snapshots/prefer_array_some.snap +++ b/crates/oxc_linter/src/snapshots/prefer_array_some.snap @@ -1,29 +1,28 @@ --- source: crates/oxc_linter/src/tester.rs -snapshot_kind: text --- - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:9] 1 │ if (foo.find(fn)) {} · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:9] 1 │ if (foo.findLast(fn)) {} · ──────── ╰──── help: Replace `findLast` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:11] 1 │ if (array.find(element => element === "🦄")) {} · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:19] 1 │ const foo = array.find(element => element === "🦄") ? bar : baz; · ──── @@ -44,51 +43,159 @@ snapshot_kind: text ╰──── help: Replace `filter` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:5] 1 │ foo.find(fn) == null · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:5] 1 │ foo.find(fn) == undefined · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:5] 1 │ foo.find(fn) === undefined · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:5] 1 │ foo.find(fn) != null · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:5] 1 │ foo.find(fn) != undefined · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:5] 1 │ foo.find(fn) !== undefined · ──── ╰──── help: Replace `find` with `some`. - ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`. + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)` or `.findLast(…)`. ╭─[prefer_array_some.tsx:1:14] 1 │ a = (( ((foo.find(fn))) == ((null)) )) ? "no" : "yes"; · ──── ╰──── help: Replace `find` with `some`. + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) !== -1 + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) != -1 + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) > - 1 + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) === -1 + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) == - 1 + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) >= 0 + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) < 0 + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(bar) !== (( - 1 )) + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findIndex(element => element.bar === 1) !== (( - 1 )) + · ───────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) !== -1 + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) != -1 + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) > - 1 + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) === -1 + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) == - 1 + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) >= 0 + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) < 0 + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(bar) !== (( - 1 )) + · ───────────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.findIndex(…)` or `.findLastIndex(…)`. + ╭─[prefer_array_some.tsx:1:5] + 1 │ foo.findLastIndex(element => element.bar === 1) !== (( - 1 )) + · ───────────── + ╰────