Skip to content

Commit

Permalink
feat(linter) eslint-plugin-unicorn no single promise in promise methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@camc314
camc314 committed Apr 13, 2024
1 parent 6561392 commit 9edbf65
Show file tree
Hide file tree
Showing 3 changed files with 562 additions and 3 deletions.
8 changes: 5 additions & 3 deletions crates/oxc_linter/src/rules.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
191 changes: 191 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/no_single_promise_in_promise_methods.rs
View file
Original file line number Diff line number Diff line change
@@ -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();
}
Loading

0 comments on commit 9edbf65

Please sign in to comment.