feat(rome_js_analyze): noAsyncPromiseExecutor
MODIFY
crates/rome_js_analyze/src/analyzers.rs
crates/rome_js_analyze/src/analyzers.rs
//! Generated file, do not edit by hand, see `xtask/codegen`
+mod no_async_promise_executor;
+pub(crate) use no_async_promise_executor::NoAsyncPromiseExecutor;
mod no_compare_neg_zero;
pub(crate) use no_compare_neg_zero::NoCompareNegZero;
mod no_debugger;

ADD
crates/rome_js_analyze/src/analyzers/no_async_promise_executor.rs
+use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleCategory, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{JsAnyExpression, JsAnyFunction, JsNewExpression, JsNewExpressionFields};
+use rome_rowan::{AstNode, AstSeparatedList};
+
+declare_rule! {
+ /// Disallows using an async function as a Promise executor.
+ /// The executor function can also be an async function. However, this is usually a mistake, for a few reasons:
+ /// If an async executor function throws an error, the error will be lost and won't cause the newly-constructed `Promise` to reject. This could make it difficult to debug and handle some errors.
+ /// If a Promise executor function is using `await`, this is usually a sign that it is not actually necessary to use the `new Promise` constructor, or the scope of the `new Promise` constructor can be reduced.
+ ///
+ /// ## Examples
+ /// ### Valid
+ ///
+ /// ```js
+ /// new Promise((resolve, reject) => {})
+ /// new Promise((resolve, reject) => {}, async function unrelated() {})
+ /// new Foo(async (resolve, reject) => {})
+ /// new Foo((( (resolve, reject) => {} )))
+ /// ```
+ ///
+ /// ### Invalid
+ ///
+ /// ```js,expect_diagnostic
+ /// new Promise(async function foo(resolve, reject) {})
+ /// ```
+ ///
+ /// ```js,expect_diagnostic
+ /// new Promise(async (resolve, reject) => {})
+ /// ```
+ ///
+ /// ```js,expect_diagnostic
+ /// new Promise(((((async () => {})))))
+ /// ```
+ pub(crate) NoAsyncPromiseExecutor = "noAsyncPromiseExecutor"
+}
+
+impl Rule for NoAsyncPromiseExecutor {
+ const CATEGORY: RuleCategory = RuleCategory::Lint;
+
+ type Query = JsNewExpression;
+ type State = JsAnyFunction;
+
+ fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
+ let node = ctx.query();
+ let JsNewExpressionFields {
+ new_token: _,
+ callee,
+ type_arguments: _,
+ arguments,
+ } = node.as_fields();
+ let callee = callee.ok()?;
+ let is_promise_constructor = callee
+ .as_js_identifier_expression()
+ .and_then(|ident| ident.name().ok())
+ .map_or(false, |name| name.syntax().text_trimmed() == "Promise");
+ if !is_promise_constructor {
+ return None;
+ }
+
+ // get first argument of the `Promise` constructor
+ let first_arg = arguments?.args().iter().next()?.ok()?;
+
+ if let Some(expr) = first_arg.as_js_any_expression() {
+ get_async_function_expression_like(expr)
+ } else {
+ None
+ }
+ }
+
+ fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+ Some(RuleDiagnostic::warning(
+ state.range(),
+ markup! {
+ "Promise executor functions should not be `async`."
+ },
+ ))
+ }
+}
+
+/// Check if the expression is async function expression like, include the edge case
+/// ```js
+/// ((((((async function () {}))))))
+/// ```
+fn get_async_function_expression_like(expr: &JsAnyExpression) -> Option<JsAnyFunction> {
+ match expr {
+ JsAnyExpression::JsFunctionExpression(func) => func
+ .async_token()
+ .map(|_| JsAnyFunction::JsFunctionExpression(func.clone())),
+ JsAnyExpression::JsArrowFunctionExpression(func) => func
+ .async_token()
+ .map(|_| JsAnyFunction::JsArrowFunctionExpression(func.clone())),
+ JsAnyExpression::JsParenthesizedExpression(expr) => {
+ let inner_expression = expr.expression().ok()?;
+ get_async_function_expression_like(&inner_expression)
+ }
+ _ => None,
+ }
+}

MODIFY
crates/rome_js_analyze/src/registry.rs
crates/rome_js_analyze/src/registry.rs
@@ -5,6 +5,9 @@ use rome_analyze::{AnalysisFilter, RuleRegistry};
use rome_js_syntax::JsLanguage;
pub(crate) fn build_registry(filter: &AnalysisFilter) -> RuleRegistry<JsLanguage> {
let mut rules = RuleRegistry::empty();
+ if filter.match_rule::<NoAsyncPromiseExecutor>() {
+ rules.push::<NoAsyncPromiseExecutor>();
+ }
if filter.match_rule::<NoCompareNegZero>() {
rules.push::<NoCompareNegZero>();
}

ADD
crates/rome_js_analyze/tests/specs/noAsyncPromiseExecutor.js
+// valid
+new Promise((resolve, reject) => {})
+new Promise((resolve, reject) => {}, async function unrelated() {})
+new Foo(async (resolve, reject) => {})
+new Foo((( (resolve, reject) => {} )))
+// invalid
+new Promise(async function foo(resolve, reject) {})
+new Promise(async (resolve, reject) => {})
+new Promise(((((async () => {})))))
\ No newline at end of file

MODIFY
website/src/docs/lint/rules/index.md
website/src/docs/lint/rules/index.md
@@ -14,6 +14,16 @@ eleventyNavigation:
<section>
<h2>JavaScript</h2>
<div class="rule">
+<h3 data-toc-exclude id="noAsyncPromiseExecutor">
+ <a href="/docs/lint/rules/noAsyncPromiseExecutor">noAsyncPromiseExecutor</a>
+ <a class="header-anchor" href="#noAsyncPromiseExecutor"></a>
+</h3>
+Disallows using an async function as a Promise executor.
+The executor function can also be an async function. However, this is usually a mistake, for a few reasons:
+If an async executor function throws an error, the error will be lost and won't cause the newly-constructed <code>Promise</code> to reject. This could make it difficult to debug and handle some errors.
+If a Promise executor function is using <code>await</code>, this is usually a sign that it is not actually necessary to use the <code>new Promise</code> constructor, or the scope of the <code>new Promise</code> constructor can be reduced.
+</div>
+<div class="rule">
<h3 data-toc-exclude id="noCompareNegZero">
<a href="/docs/lint/rules/noCompareNegZero">noCompareNegZero</a>
<a class="header-anchor" href="#noCompareNegZero"></a>

feat(rome_js_analyze): noAsyncPromiseExecutor (#2733)

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>