feat(rome_js_analyze): noAsyncPromiseExecutor (#2733)

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

crates/rome_js_analyze/src/analyzers/no_async_promise_executor.rs

@@ -0,0 +1,99 @@
+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,
+    }
+}

crates/rome_js_analyze/tests/specs/noAsyncPromiseExecutor.js

@@ -0,0 +1,9 @@
+// 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 () => {})))))

crates/rome_js_analyze/tests/specs/noAsyncPromiseExecutor.js.snap

@@ -0,0 +1,50 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 98
+expression: noAsyncPromiseExecutor.js
+---
+# Input
+```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 () => {})))))
+```
+
+# Diagnostics
+```
+warning[noAsyncPromiseExecutor]: Promise executor functions should not be `async`.
+  ┌─ noAsyncPromiseExecutor.js:7:13
+  │
+7 │ new Promise(async function foo(resolve, reject) {})
+  │             --------------------------------------
+
+
+```
+
+```
+warning[noAsyncPromiseExecutor]: Promise executor functions should not be `async`.
+  ┌─ noAsyncPromiseExecutor.js:8:13
+  │
+8 │ new Promise(async (resolve, reject) => {})
+  │             -----------------------------
+
+
+```
+
+```
+warning[noAsyncPromiseExecutor]: Promise executor functions should not be `async`.
+  ┌─ noAsyncPromiseExecutor.js:9:17
+  │
+9 │ new Promise(((((async () => {})))))
+  │                 --------------
+
+
+```
+
+

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>

website/src/docs/lint/rules/noAsyncPromiseExecutor.md

@@ -0,0 +1,61 @@
+---
+title: Lint Rule noAsyncPromiseExecutor
+layout: layouts/rule.liquid
+---
+
+# noAsyncPromiseExecutor
+
+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
+
+```jsx
+  new Promise((resolve, reject) => {})
+  new Promise((resolve, reject) => {}, async function unrelated() {})
+  new Foo(async (resolve, reject) => {})
+  new Foo((( (resolve, reject) => {} )))
+```
+
+### Invalid
+
+```jsx
+new Promise(async function foo(resolve, reject) {})
+```
+
+{% raw %}<pre class="language-text"><code class="language-text"><span style="color: Orange;">warning</span><span style="color: Orange;">[</span><span style="color: Orange;">noAsyncPromiseExecutor</span><span style="color: Orange;">]</span><em>: </em><em>Promise executor functions should not be `async`.</em>
+  <span style="color: rgb(38, 148, 255);">┌</span><span style="color: rgb(38, 148, 255);">─</span> noAsyncPromiseExecutor.js:1:13
+  <span style="color: rgb(38, 148, 255);">│</span>
+<span style="color: rgb(38, 148, 255);">1</span> <span style="color: rgb(38, 148, 255);">│</span> new Promise(async function foo(resolve, reject) {})
+  <span style="color: rgb(38, 148, 255);">│</span>             <span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span>
+
+</code></pre>{% endraw %}
+
+```jsx
+  new Promise(async (resolve, reject) => {})
+```
+
+{% raw %}<pre class="language-text"><code class="language-text"><span style="color: Orange;">warning</span><span style="color: Orange;">[</span><span style="color: Orange;">noAsyncPromiseExecutor</span><span style="color: Orange;">]</span><em>: </em><em>Promise executor functions should not be `async`.</em>
+  <span style="color: rgb(38, 148, 255);">┌</span><span style="color: rgb(38, 148, 255);">─</span> noAsyncPromiseExecutor.js:1:15
+  <span style="color: rgb(38, 148, 255);">│</span>
+<span style="color: rgb(38, 148, 255);">1</span> <span style="color: rgb(38, 148, 255);">│</span>   new Promise(async (resolve, reject) =&gt; {})
+  <span style="color: rgb(38, 148, 255);">│</span>               <span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span>
+
+</code></pre>{% endraw %}
+
+```jsx
+  new Promise(((((async () => {})))))
+```
+
+{% raw %}<pre class="language-text"><code class="language-text"><span style="color: Orange;">warning</span><span style="color: Orange;">[</span><span style="color: Orange;">noAsyncPromiseExecutor</span><span style="color: Orange;">]</span><em>: </em><em>Promise executor functions should not be `async`.</em>
+  <span style="color: rgb(38, 148, 255);">┌</span><span style="color: rgb(38, 148, 255);">─</span> noAsyncPromiseExecutor.js:1:19
+  <span style="color: rgb(38, 148, 255);">│</span>
+<span style="color: rgb(38, 148, 255);">1</span> <span style="color: rgb(38, 148, 255);">│</span>   new Promise(((((async () =&gt; {})))))
+  <span style="color: rgb(38, 148, 255);">│</span>                   <span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span><span style="color: rgb(38, 148, 255);">-</span>
+
+</code></pre>{% endraw %}
+