feat(linter) eslint plugin unicorn: prefer array some (#1467)

crates/oxc_linter/src/ast_util.rs

@@ -2,7 +2,7 @@ use std::hash::{Hash, Hasher};
 
 use oxc_ast::AstKind;
 use oxc_semantic::AstNode;
-use oxc_span::{Atom, GetSpan};
+use oxc_span::{Atom, GetSpan, Span};
 use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator};
 use rustc_hash::FxHasher;
 
@@ -313,3 +313,46 @@ pub fn extract_regex_flags<'a>(
     }
     Some(flags)
 }
+
+pub fn is_method_call<'a>(
+    call_expr: &CallExpression<'a>,
+    methods: &[&'a str],
+    min_arg_count: Option<usize>,
+    max_arg_count: Option<usize>,
+) -> bool {
+    if let Some(min_arg_count) = min_arg_count {
+        if call_expr.arguments.len() < min_arg_count {
+            return false;
+        }
+    }
+
+    if let Some(max_arg_count) = max_arg_count {
+        if call_expr.arguments.len() > max_arg_count {
+            return false;
+        }
+    }
+
+    let Expression::MemberExpression(member_expr) = &call_expr.callee.without_parenthesized()
+    else {
+        return false;
+    };
+
+    let Some(static_property_name) = member_expr.static_property_name() else { return false };
+
+    if !methods.contains(&static_property_name) {
+        return false;
+    }
+
+    true
+}
+
+pub fn call_expr_method_callee_info<'a>(
+    call_expr: &'a CallExpression<'a>,
+) -> Option<(Span, &'a str)> {
+    let Expression::MemberExpression(member_expr) = &call_expr.callee.without_parenthesized()
+    else {
+        return None;
+    };
+
+    member_expr.static_property_info()
+}

crates/oxc_linter/src/rules.rs

@@ -175,6 +175,7 @@ mod unicorn {
     pub mod number_literal_case;
     pub mod prefer_add_event_listener;
     pub mod prefer_array_flat_map;
+    pub mod prefer_array_some;
     pub mod prefer_blob_reading_methods;
     pub mod prefer_code_point;
     pub mod prefer_date_now;
@@ -312,6 +313,7 @@ oxc_macros::declare_all_lint_rules! {
     jest::valid_title,
     unicorn::catch_error_name,
     unicorn::empty_brace_spaces,
+    unicorn::prefer_array_some,
     unicorn::error_message,
     unicorn::filename_case,
     unicorn::new_for_builtins,

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

@@ -0,0 +1,308 @@
+use oxc_ast::{
+    ast::{Argument, CallExpression, 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, LogicalOperator, UnaryOperator};
+
+use crate::{
+    ast_util::{call_expr_method_callee_info, is_method_call, outermost_paren_parent},
+    context::LintContext,
+    rule::Rule,
+    AstNode,
+};
+
+#[derive(Debug, Error, Diagnostic)]
+enum PreferArraySomeDiagnostic {
+    #[error("eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.")]
+    #[diagnostic(severity(warning))]
+    OverMethod(#[label] Span),
+    #[error("eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over non-zero length check from `.filter(…)`.")]
+    #[diagnostic(severity(warning))]
+    NonZeroFilter(#[label] Span),
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct PreferArraySome;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Prefers using [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find), [`Array#findLast()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLast) and a non-zero length check on the result of [`Array#filter()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter)
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Using `.some()` is more idiomatic and easier to read.
+    ///
+    /// ### Example
+    /// ```javascript
+    /// // Bad
+    /// const foo = array.find(fn) ? bar : baz;
+    ///
+    /// // Good
+    /// const foo = array.some(fn) ? bar : baz;
+    /// ```
+    PreferArraySome,
+    pedantic
+);
+
+impl Rule for PreferArraySome {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        match node.kind() {
+            AstKind::CallExpression(call_expr) => {
+                if !is_method_call(call_expr, &["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;
+                }
+
+                ctx.diagnostic(PreferArraySomeDiagnostic::OverMethod(
+                    // SAFETY: `call_expr_method_callee_info` returns `Some` if `is_method_call` returns `true`.
+                    call_expr_method_callee_info(call_expr).unwrap().0,
+                ));
+            }
+            AstKind::BinaryExpression(bin_expr) => {
+                if !matches!(
+                    bin_expr.operator,
+                    BinaryOperator::GreaterThan | BinaryOperator::StrictInequality
+                ) {
+                    return;
+                }
+
+                let Expression::NumberLiteral(right_num_lit) = &bin_expr.right else { return };
+
+                if right_num_lit.raw != "0" {
+                    return;
+                }
+
+                let Expression::MemberExpression(left_member_expr) =
+                    &bin_expr.left.without_parenthesized()
+                else {
+                    return;
+                };
+
+                let Some(static_property_name) = left_member_expr.static_property_name() else {
+                    return;
+                };
+
+                if !matches!(static_property_name, "length") {
+                    return;
+                }
+
+                let Expression::CallExpression(left_call_expr) =
+                    &left_member_expr.object().without_parenthesized()
+                else {
+                    return;
+                };
+
+                if !is_method_call(left_call_expr, &["filter"], None, None) {
+                    return;
+                }
+
+                let Some(Argument::Expression(first_filter_call_arg)) =
+                    left_call_expr.arguments.first()
+                else {
+                    return;
+                };
+
+                if is_node_value_not_function(first_filter_call_arg) {
+                    return;
+                }
+
+                ctx.diagnostic(PreferArraySomeDiagnostic::NonZeroFilter(
+                    // SAFETY: `call_expr_method_callee_info` returns `Some` if `is_method_call` returns `true`.
+                    call_expr_method_callee_info(left_call_expr).unwrap().0,
+                ));
+            }
+            _ => {}
+        }
+    }
+}
+
+fn is_node_value_not_function(expr: &Expression) -> bool {
+    if matches!(
+        expr,
+        Expression::ArrayExpression(_)
+            | Expression::BinaryExpression(_)
+            | Expression::ClassExpression(_)
+            | Expression::ObjectExpression(_)
+            | Expression::TemplateLiteral(_)
+            | Expression::UnaryExpression(_)
+            | Expression::UpdateExpression(_)
+    ) {
+        return true;
+    }
+    if expr.is_literal() {
+        return true;
+    }
+    if matches!(
+        expr,
+        Expression::AssignmentExpression(_)
+            | Expression::AwaitExpression(_)
+            | Expression::LogicalExpression(_)
+            | Expression::NewExpression(_)
+            | Expression::TaggedTemplateExpression(_)
+            | Expression::ThisExpression(_)
+    ) {
+        return true;
+    }
+    if expr.is_undefined() {
+        return true;
+    }
+
+    false
+}
+
+fn is_checking_undefined<'a, 'b>(
+    node: &'b AstNode<'a>,
+    _call_expr: &'b CallExpression<'a>,
+    ctx: &'b LintContext<'a>,
+) -> bool {
+    let Some(parent) = outermost_paren_parent(node, ctx) else { return false };
+
+    let AstKind::BinaryExpression(bin_expr) = parent.kind() else { return false };
+
+    let right_without_paren = bin_expr.right.without_parenthesized();
+
+    if matches!(
+        bin_expr.operator,
+        BinaryOperator::Inequality
+            | BinaryOperator::Equality
+            | BinaryOperator::StrictInequality
+            | BinaryOperator::StrictEquality
+    ) && right_without_paren.without_parenthesized().is_undefined()
+    {
+        return true;
+    }
+
+    if matches!(bin_expr.operator, BinaryOperator::Inequality | BinaryOperator::Equality)
+        && right_without_paren.is_null()
+    {
+        return true;
+    }
+
+    false
+}
+
+fn is_logic_not(node: &AstKind) -> bool {
+    let AstKind::UnaryExpression(logic_expr) = node else { return false };
+    logic_expr.operator == UnaryOperator::UnaryNegation
+}
+
+fn is_boolean_call_argument(node: &AstKind) -> bool {
+    let AstKind::CallExpression(call_expr) = node else { return false };
+    let Expression::Identifier(ident) = &call_expr.callee else { return false };
+    ident.name == "Boolean" && call_expr.arguments.len() == 1
+}
+
+fn is_logical_expression(node: &AstNode) -> bool {
+    let AstKind::LogicalExpression(logical_expr) = node.kind() else { return false };
+
+    matches!(logical_expr.operator, LogicalOperator::And | LogicalOperator::Or)
+}
+
+fn is_boolean_node<'a, 'b>(node: &'b AstNode<'a>, ctx: &'b LintContext<'a>) -> bool {
+    let kind = node.kind();
+
+    if is_logic_not(&kind) || is_boolean_call_argument(&kind) {
+        return true;
+    }
+
+    let Some(parent) = outermost_paren_parent(node, ctx) else { return false };
+
+    if matches!(
+        parent.kind(),
+        AstKind::IfStatement(_)
+            | AstKind::ConditionalExpression(_)
+            | AstKind::WhileStatement(_)
+            | AstKind::DoWhileStatement(_)
+            | AstKind::ForStatement(_)
+    ) {
+        return true;
+    }
+
+    if is_logical_expression(parent) {
+        return is_boolean_node(parent, ctx);
+    }
+
+    false
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        r"const bar = foo.find(fn)",
+        r"const bar = foo.find(fn) || baz",
+        r"if (foo.find(fn) ?? bar) {}",
+        r"array.filter(fn).length > 0.",
+        r"array.filter(fn).length > .0",
+        r"array.filter(fn).length > 0.0",
+        r"array.filter(fn).length > 0x00",
+        r"array.filter(fn).length < 0",
+        r"array.filter(fn).length >= 0",
+        r"0 > array.filter(fn).length",
+        r"array.filter(fn).length !== 0.",
+        r"array.filter(fn).length !== .0",
+        r"array.filter(fn).length !== 0.0",
+        r"array.filter(fn).length !== 0x00",
+        r"array.filter(fn).length != 0",
+        r"array.filter(fn).length === 0",
+        r"array.filter(fn).length == 0",
+        r"array.filter(fn).length = 0",
+        r"0 !== array.filter(fn).length",
+        r"array.filter(fn).length >= 1",
+        r"array.filter(fn).length >= 1.",
+        r"array.filter(fn).length >= 1.0",
+        r"array.filter(fn).length >= 0x1",
+        r"array.filter(fn).length > 1",
+        r"array.filter(fn).length < 1",
+        r"array.filter(fn).length = 1",
+        r"array.filter(fn).length += 1",
+        r"1 >= array.filter(fn).length",
+        r"array.filter(fn)?.length > 0",
+        r"array.filter(fn)[length] > 0",
+        r"array.filter(fn).notLength > 0",
+        r"array.filter(fn).length() > 0",
+        r"+array.filter(fn).length >= 1",
+        r"array.filter?.(fn).length > 0",
+        r"array?.filter(fn).length > 0",
+        r"array.notFilter(fn).length > 0",
+        r"array.filter.length > 0",
+        r#"$element.filter(":visible").length > 0"#,
+        r"foo.find(fn) == 0",
+        r#"foo.find(fn) != """#,
+        r"foo.find(fn) === null",
+        r#"foo.find(fn) !== "null""#,
+        r"foo.find(fn) >= undefined",
+        r"foo.find(fn) instanceof undefined",
+        r#"typeof foo.find(fn) === "undefined""#,
+    ];
+
+    let fail = vec![
+        r"if (foo.find(fn)) {}",
+        r"if (foo.findLast(fn)) {}",
+        r#"if (array.find(element => element === "🦄")) {}"#,
+        r#"const foo = array.find(element => element === "🦄") ? bar : baz;"#,
+        r"array.filter(fn).length > 0",
+        r"array.filter(fn).length !== 0",
+        r"foo.find(fn) == null",
+        r"foo.find(fn) == undefined",
+        r"foo.find(fn) === undefined",
+        r"foo.find(fn) != null",
+        r"foo.find(fn) != undefined",
+        r"foo.find(fn) !== undefined",
+        r#"a = (( ((foo.find(fn))) == ((null)) )) ? "no" : "yes";"#,
+    ];
+
+    Tester::new_without_config(PreferArraySome::NAME, pass, fail).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/prefer_array_some.snap

@@ -0,0 +1,83 @@
+---
+source: crates/oxc_linter/src/tester.rs
+expression: prefer_array_some
+---
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ if (foo.find(fn)) {}
+   ·         ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ if (foo.findLast(fn)) {}
+   ·         ────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ if (array.find(element => element === "🦄")) {}
+   ·           ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ const foo = array.find(element => element === "🦄") ? bar : baz;
+   ·                   ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over non-zero length check from `.filter(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ array.filter(fn).length > 0
+   ·       ──────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over non-zero length check from `.filter(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ array.filter(fn).length !== 0
+   ·       ──────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ foo.find(fn) == null
+   ·     ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ foo.find(fn) == undefined
+   ·     ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ foo.find(fn) === undefined
+   ·     ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ foo.find(fn) != null
+   ·     ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ foo.find(fn) != undefined
+   ·     ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ foo.find(fn) !== undefined
+   ·     ────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-array-some): Prefer `.some(…)` over `.find(…)`or `.findLast(…)`.
+   ╭─[prefer_array_some.tsx:1:1]
+ 1 │ a = (( ((foo.find(fn))) == ((null)) )) ? "no" : "yes";
+   ·              ────
+   ╰────
+
+