feat(linter) eslint-plugin-unicorn no single promise in promise methods

crates/oxc_linter/src/rules.rs

@@ -241,6 +241,7 @@ mod unicorn {
     pub mod no_null;
     pub mod no_object_as_default_parameter;
     pub mod no_process_exit;
+    pub mod no_single_promise_in_promise_methods;
     pub mod no_static_only_class;
     pub mod no_thenable;
     pub mod no_this_assignment;
@@ -516,16 +517,15 @@ oxc_macros::declare_all_lint_rules! {
     jest::valid_expect,
     jest::valid_title,
     unicorn::catch_error_name,
-    unicorn::prefer_node_protocol,
     unicorn::empty_brace_spaces,
     unicorn::error_message,
     unicorn::escape_case,
     unicorn::explicit_length_check,
     unicorn::filename_case,
     unicorn::new_for_builtins,
     unicorn::no_abusive_eslint_disable,
-    unicorn::no_array_reduce,
     unicorn::no_array_for_each,
+    unicorn::no_array_reduce,
     unicorn::no_await_expression_member,
     unicorn::no_console_spaces,
     unicorn::no_document_cookie,
@@ -541,16 +541,19 @@ oxc_macros::declare_all_lint_rules! {
     unicorn::no_null,
     unicorn::no_object_as_default_parameter,
     unicorn::no_process_exit,
+    unicorn::no_single_promise_in_promise_methods,
     unicorn::no_static_only_class,
     unicorn::no_thenable,
     unicorn::no_this_assignment,
     unicorn::no_typeof_undefined,
     unicorn::no_unnecessary_await,
     unicorn::no_unreadable_array_destructuring,
+    unicorn::prefer_node_protocol,
     unicorn::no_unreadable_iife,
     unicorn::no_useless_fallback_in_spread,
     unicorn::no_useless_length_check,
     unicorn::no_useless_promise_resolve_reject,
+    unicorn::no_useless_spread,
     unicorn::no_useless_switch_case,
     unicorn::no_zero_fractions,
     unicorn::number_literal_case,
@@ -573,7 +576,6 @@ oxc_macros::declare_all_lint_rules! {
     unicorn::prefer_modern_dom_apis,
     unicorn::prefer_modern_math_apis,
     unicorn::prefer_native_coercion_functions,
-    unicorn::no_useless_spread,
     unicorn::prefer_number_properties,
     unicorn::prefer_optional_catch_binding,
     unicorn::prefer_prototype_methods,

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

@@ -0,0 +1,191 @@
+use oxc_ast::{
+    ast::{Argument, ArrayExpressionElement, CallExpression, Expression},
+    AstKind,
+};
+use oxc_diagnostics::{
+    miette::{self, Diagnostic},
+    thiserror::{self, Error},
+};
+use oxc_macros::declare_oxc_lint;
+use oxc_span::Span;
+
+use crate::{ast_util::is_method_call, context::LintContext, rule::Rule, AstNode};
+
+#[derive(Debug, Error, Diagnostic)]
+#[error("Wrapping single-element array with `Promise.{1}()` is unnecessary.")]
+#[diagnostic(
+    severity(warning),
+    help("Either use the value directly, or switch to `Promise.resolve(…)`.")
+)]
+struct NoSinglePromiseInPromiseMethodsDiagnostic(#[label] Span, pub String);
+
+#[derive(Debug, Default, Clone)]
+pub struct NoSinglePromiseInPromiseMethods;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Disallow passing single-element arrays to Promise methods
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Passing a single-element array to `Promise.all()`, `Promise.any()`, or `Promise.race()` is likely a mistake.
+    ///
+    ///
+    /// ### Example
+    ///
+    /// Bad
+    /// ```js
+    /// const foo = await Promise.all([promise]);
+    /// const foo = await Promise.any([promise]);
+    /// const foo = await Promise.race([promise]);
+    /// const promise = Promise.all([nonPromise]);
+    /// ```
+    ///
+    /// Good
+    /// ```js
+    /// const foo = await promise;
+    /// const promise = Promise.resolve(nonPromise);
+    /// const foo = await Promise.all(promises);
+    /// const foo = await Promise.any([promise, anotherPromise]);
+    /// const [{ value: foo, reason: error }] = await Promise.allSettled([promise]);
+    /// ```
+    ///
+    NoSinglePromiseInPromiseMethods,
+    correctness
+);
+
+impl Rule for NoSinglePromiseInPromiseMethods {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        let AstKind::CallExpression(call_expr) = node.kind() else {
+            return;
+        };
+
+        if !is_promise_method_with_single_element_array(call_expr) {
+            return;
+        }
+
+        let info = call_expr
+            .callee
+            .get_member_expr()
+            .expect("callee is a member expression")
+            .static_property_info()
+            .expect("callee is a static property");
+
+        ctx.diagnostic(NoSinglePromiseInPromiseMethodsDiagnostic(info.0, info.1.to_string()));
+    }
+}
+
+fn is_promise_method_with_single_element_array(call_expr: &CallExpression) -> bool {
+    if !is_method_call(
+        call_expr,
+        Some(&["Promise"]),
+        Some(&["all", "any", "race"]),
+        Some(1),
+        Some(1),
+    ) {
+        return false;
+    }
+
+    let Some(Argument::Expression(first_argument)) = call_expr.arguments.first() else {
+        return false;
+    };
+    let first_argument = first_argument.without_parenthesized();
+    let Expression::ArrayExpression(first_argument_array_expr) = first_argument else {
+        return false;
+    };
+
+    if first_argument_array_expr.elements.len() != 1 {
+        return false;
+    }
+
+    matches!(first_argument_array_expr.elements[0], ArrayExpressionElement::Expression(_))
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        "Promise.all([promise, anotherPromise])",
+        "Promise.all(notArrayLiteral)",
+        "Promise.all([...promises])",
+        "Promise.any([promise, anotherPromise])",
+        "Promise.race([promise, anotherPromise])",
+        "Promise.notListedMethod([promise])",
+        "Promise[all]([promise])",
+        "Promise.all([,])",
+        "NotPromise.all([promise])",
+        "Promise.all(...[promise])",
+        "Promise.all([promise], extraArguments)",
+        "Promise.all()",
+        "new Promise.all([promise])",
+        "globalThis.Promise.all([promise])",
+        "Promise.allSettled([promise])",
+    ];
+
+    let fail = vec![
+        "await Promise.all([(0, promise)])",
+        "async function * foo() {await Promise.all([yield promise])}",
+        "async function * foo() {await Promise.all([yield* promise])}",
+        "await Promise.all([() => promise,],)",
+        "await Promise.all([a ? b : c,],)",
+        "await Promise.all([x ??= y,],)",
+        "await Promise.all([x ||= y,],)",
+        "await Promise.all([x &&= y,],)",
+        "await Promise.all([x |= y,],)",
+        "await Promise.all([x ^= y,],)",
+        "await Promise.all([x ??= y,],)",
+        "await Promise.all([x ||= y,],)",
+        "await Promise.all([x &&= y,],)",
+        "await Promise.all([x | y,],)",
+        "await Promise.all([x ^ y,],)",
+        "await Promise.all([x & y,],)",
+        "await Promise.all([x !== y,],)",
+        "await Promise.all([x == y,],)",
+        "await Promise.all([x in y,],)",
+        "await Promise.all([x >>> y,],)",
+        "await Promise.all([x + y,],)",
+        "await Promise.all([x / y,],)",
+        "await Promise.all([x ** y,],)",
+        "await Promise.all([promise,],)",
+        "await Promise.all([getPromise(),],)",
+        "await Promise.all([promises[0],],)",
+        "await Promise.all([await promise])",
+        "await Promise.any([promise])",
+        "await Promise.race([promise])",
+        "await Promise.all([new Promise(() => {})])",
+        "+await Promise.all([+1])",
+        "
+        await Promise.all([(x,y)])
+        [0].toString()
+		",
+        "Promise.all([promise,],)",
+        "
+		foo
+		Promise.all([(0, promise),],)
+		",
+        "
+        foo
+        Promise.all([[array][0],],)
+		",
+        "Promise.all([promise]).then()",
+        "Promise.all([1]).then()",
+        "Promise.all([1.]).then()",
+        "Promise.all([.1]).then()",
+        "Promise.all([(0, promise)]).then()",
+        "const _ = () => Promise.all([ a ?? b ,],)",
+        "Promise.all([ {a} = 1 ,],)",
+        "Promise.all([ function () {} ,],)",
+        "Promise.all([ class {} ,],)",
+        "Promise.all([ new Foo ,],).then()",
+        "Promise.all([ new Foo ,],).toString",
+        "foo(Promise.all([promise]))",
+        "Promise.all([promise]).foo = 1",
+        "Promise.all([promise])[0] ||= 1",
+        "Promise.all([undefined]).then()",
+        "Promise.all([null]).then()",
+    ];
+
+    Tester::new(NoSinglePromiseInPromiseMethods::NAME, pass, fail).test_and_snapshot();
+}