feat(linter): handle arrays of Promises in noFloatingPromises (#6512)

Author: arendjr

.changeset/angry-asses-attack.md

@@ -0,0 +1,19 @@
+---
+"@biomejs/biome": minor
+---
+
+The rule [`noFloatingPromises`](https://biomejs.dev/linter/rules/no-misused-promises/)
+can now detect floating arrays of `Promise`s.
+
+## Invalid examples
+
+```ts
+// This gets flagged because the Promises are not handled.
+[1, 2, 3].map(async (x) => x + 1);
+```
+
+## Valid examples
+
+```ts
+await Promise.all([1, 2, 3].map(async (x) => x + 1));
+```

crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs

@@ -3,7 +3,9 @@ use biome_analyze::{
 };
 use biome_console::markup;
 use biome_js_factory::make;
-use biome_js_syntax::{AnyJsExpression, JsExpressionStatement, JsSyntaxKind};
+use biome_js_syntax::{
+    AnyJsCallArgument, AnyJsExpression, AnyJsName, JsExpressionStatement, JsSyntaxKind, T,
+};
 use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, TriviaPieceKind};
 
 use crate::{JsRuleAction, ast_utils::is_in_async_function, services::typed::Typed};
@@ -164,9 +166,14 @@ declare_lint_rule! {
     }
 }
 
+pub enum NoFloatingPromisesState {
+    ArrayOfPromises,
+    UnhandledPromise,
+}
+
 impl Rule for NoFloatingPromises {
     type Query = Typed<JsExpressionStatement>;
-    type State = ();
+    type State = NoFloatingPromisesState;
     type Signals = Option<Self::State>;
     type Options = ();
 
@@ -177,6 +184,10 @@ impl Rule for NoFloatingPromises {
 
         // Uncomment the following line for debugging convenience:
         //let printed = format!("type of {expression:?} = {ty:?}");
+        if ty.is_array_of(|ty| ty.is_promise_instance()) {
+            return Some(NoFloatingPromisesState::ArrayOfPromises);
+        }
+
         let is_maybe_promise =
             ty.is_promise_instance() || ty.has_variant(|ty| ty.is_promise_instance());
         if !is_maybe_promise {
@@ -187,26 +198,48 @@ impl Rule for NoFloatingPromises {
             return None;
         }
 
-        Some(())
+        Some(NoFloatingPromisesState::UnhandledPromise)
     }
 
-    fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
+    fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
         let node = ctx.query();
-        Some(
-            RuleDiagnostic::new(
-                rule_category!(),
-                node.range(),
-                markup! {
-                    "A \"floating\" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior."
-                },
-            )
-            .note(markup! {
-                "This happens when a Promise is not awaited, lacks a `.catch` or `.then` rejection handler, or is not explicitly ignored using the `void` operator."
-            })
-        )
+        match state {
+            NoFloatingPromisesState::ArrayOfPromises => Some(
+                RuleDiagnostic::new(
+                    rule_category!(),
+                    node.range(),
+                    markup! {
+                        "An array of Promises was found, meaning they are not "
+                        "properly handled and could lead to ignored errors or "
+                        "unexpected behavior."
+                    },
+                )
+                .note(markup! {
+                    "This happens when an array of Promises is not wrapped "
+                    "with Promise.all() or a similar method, and is not "
+                    "explicitly ignored using the `void` operator."
+                }),
+            ),
+            NoFloatingPromisesState::UnhandledPromise => Some(
+                RuleDiagnostic::new(
+                    rule_category!(),
+                    node.range(),
+                    markup! {
+                        "A \"floating\" Promise was found, meaning it is not "
+                        "properly handled and could lead to ignored errors or "
+                        "unexpected behavior."
+                    },
+                )
+                .note(markup! {
+                    "This happens when a Promise is not awaited, lacks a "
+                    "`.catch` or `.then` rejection handler, or is not "
+                    "explicitly ignored using the `void` operator."
+                }),
+            ),
+        }
     }
 
-    fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
+    fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
         let node = ctx.query();
 
         if !is_in_async_function(node.syntax()) {
@@ -215,19 +248,66 @@ impl Rule for NoFloatingPromises {
 
         let expression = node.expression().ok()?;
         let mut mutation = ctx.root().begin();
-        let await_expression = AnyJsExpression::JsAwaitExpression(make::js_await_expression(
-            make::token(JsSyntaxKind::AWAIT_KW)
-                .with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
-            expression.clone().trim_leading_trivia()?,
-        ));
+        match state {
+            NoFloatingPromisesState::ArrayOfPromises => {
+                let callee_expression =
+                    AnyJsExpression::JsStaticMemberExpression(make::js_static_member_expression(
+                        AnyJsExpression::JsIdentifierExpression(make::js_identifier_expression(
+                            make::js_reference_identifier(make::ident("Promise")),
+                        )),
+                        make::token(T![.]),
+                        AnyJsName::JsName(make::js_name(make::ident("all"))),
+                    ));
+
+                let call_expression = AnyJsExpression::JsCallExpression(
+                    make::js_call_expression(
+                        callee_expression,
+                        make::js_call_arguments(
+                            make::token(T!['(']),
+                            make::js_call_argument_list(
+                                [AnyJsCallArgument::AnyJsExpression(
+                                    expression.clone().trim_trivia()?,
+                                )],
+                                [],
+                            ),
+                            make::token(T![')']),
+                        ),
+                    )
+                    .build(),
+                );
+
+                let await_expression =
+                    AnyJsExpression::JsAwaitExpression(make::js_await_expression(
+                        make::token(JsSyntaxKind::AWAIT_KW)
+                            .with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
+                        call_expression,
+                    ));
 
-        mutation.replace_node(expression, await_expression);
-        Some(JsRuleAction::new(
-            ctx.metadata().action_category(ctx.category(), ctx.group()),
-            ctx.metadata().applicability(),
-            markup! { "Add await operator." }.to_owned(),
-            mutation,
-        ))
+                mutation.replace_node(expression, await_expression);
+                Some(JsRuleAction::new(
+                    ctx.metadata().action_category(ctx.category(), ctx.group()),
+                    ctx.metadata().applicability(),
+                    markup! { "Wrap in Promise.all() and add await operator." }.to_owned(),
+                    mutation,
+                ))
+            }
+            NoFloatingPromisesState::UnhandledPromise => {
+                let await_expression =
+                    AnyJsExpression::JsAwaitExpression(make::js_await_expression(
+                        make::token(JsSyntaxKind::AWAIT_KW)
+                            .with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
+                        expression.clone().trim_leading_trivia()?,
+                    ));
+
+                mutation.replace_node(expression, await_expression);
+                Some(JsRuleAction::new(
+                    ctx.metadata().action_category(ctx.category(), ctx.group()),
+                    ctx.metadata().applicability(),
+                    markup! { "Add await operator." }.to_owned(),
+                    mutation,
+                ))
+            }
+        }
     }
 }
 

crates/biome_js_analyze/src/lint/nursery/no_misused_promises.rs

@@ -94,9 +94,9 @@ declare_lint_rule! {
 
 pub enum NoMisusedPromisesState {
     Conditional,
-    ExpectedConditionalReturn,
-    ExpectedVoidReturn,
+    ConditionalReturn,
     Spread,
+    VoidReturn,
 }
 
 impl Rule for NoMisusedPromises {
@@ -133,7 +133,7 @@ impl Rule for NoMisusedPromises {
                     "You may have intended to `await` the Promise instead."
                 }),
             ),
-            NoMisusedPromisesState::ExpectedConditionalReturn => Some(
+            NoMisusedPromisesState::ConditionalReturn => Some(
                 RuleDiagnostic::new(
                     rule_category!(),
                     node.range(),
@@ -150,7 +150,7 @@ impl Rule for NoMisusedPromises {
                     "does not work inside a synchronous callback."
                 }),
             ),
-            NoMisusedPromisesState::ExpectedVoidReturn => Some(
+            NoMisusedPromisesState::VoidReturn => Some(
                 RuleDiagnostic::new(
                     rule_category!(),
                     node.range(),
@@ -182,7 +182,7 @@ impl Rule for NoMisusedPromises {
 
     fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
         use NoMisusedPromisesState::*;
-        if matches!(state, ExpectedConditionalReturn | ExpectedVoidReturn) {
+        if matches!(state, ConditionalReturn | VoidReturn) {
             return None; // These cannot be automatically fixed.
         }
 
@@ -296,9 +296,9 @@ fn find_misused_promise_returning_callback(
     };
 
     if argument_ty.is_function_with_return_type(|ty| ty.is_conditional()) {
-        Some(NoMisusedPromisesState::ExpectedConditionalReturn)
+        Some(NoMisusedPromisesState::ConditionalReturn)
     } else if argument_ty.is_function_with_return_type(|ty| ty.is_void()) {
-        Some(NoMisusedPromisesState::ExpectedVoidReturn)
+        Some(NoMisusedPromisesState::VoidReturn)
     } else {
         None
     }

crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts

@@ -345,3 +345,9 @@ function returnMaybePromise(): Promise<void> | undefined {
 }
 
 returnMaybePromise();
+
+[1, 2, 3].map(async (x) => x + 1);
+
+async function floatingArray() {
+	[1, 2, 3].map((x) => Promise.resolve(x + 1));
+}

crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts.snap

@@ -352,6 +352,12 @@ function returnMaybePromise(): Promise<void> | undefined {
 
 returnMaybePromise();
 
+[1, 2, 3].map(async (x) => x + 1);
+
+async function floatingArray() {
+	[1, 2, 3].map((x) => Promise.resolve(x + 1));
+}
+
 ```
 
 # Diagnostics
@@ -1631,8 +1637,46 @@ invalid.ts:347:1 lint/nursery/noFloatingPromises ━━━━━━━━━━
   > 347 │ returnMaybePromise();
         │ ^^^^^^^^^^^^^^^^^^^^^
     348 │ 
+    349 │ [1, 2, 3].map(async (x) => x + 1);
   
   i This happens when a Promise is not awaited, lacks a `.catch` or `.then` rejection handler, or is not explicitly ignored using the `void` operator.
   
 
 ```
+
+```
+invalid.ts:349:1 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  i An array of Promises was found, meaning they are not properly handled and could lead to ignored errors or unexpected behavior.
+  
+    347 │ returnMaybePromise();
+    348 │ 
+  > 349 │ [1, 2, 3].map(async (x) => x + 1);
+        │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    350 │ 
+    351 │ async function floatingArray() {
+  
+  i This happens when an array of Promises is not wrapped with Promise.all() or a similar method, and is not explicitly ignored using the `void` operator.
+  
+
+```
+
+```
+invalid.ts:352:2 lint/nursery/noFloatingPromises  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  i An array of Promises was found, meaning they are not properly handled and could lead to ignored errors or unexpected behavior.
+  
+    351 │ async function floatingArray() {
+  > 352 │ 	[1, 2, 3].map((x) => Promise.resolve(x + 1));
+        │ 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    353 │ }
+    354 │ 
+  
+  i This happens when an array of Promises is not wrapped with Promise.all() or a similar method, and is not explicitly ignored using the `void` operator.
+  
+  i Unsafe fix: Wrap in Promise.all() and add await operator.
+  
+    352 │ → await·Promise.all([1,·2,·3].map((x)·=>·Promise.resolve(x·+·1)));
+        │   ++++++++++++++++++                                            + 
+
+```

crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts

@@ -248,3 +248,9 @@ async function testDestructuringAndCallingReturnsPromiseFromRest({
 }: Props) {
   rest.returnsPromise().catch(() => {}).finally(() => {});
 }
+
+void [1, 2, 3].map(async (x) => x + 1);
+
+async function floatingArray() {
+  await Promise.all([1, 2, 3].map((x) => Promise.resolve(x + 1)));
+}

crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts.snap

@@ -255,4 +255,10 @@ async function testDestructuringAndCallingReturnsPromiseFromRest({
   rest.returnsPromise().catch(() => {}).finally(() => {});
 }
 
+void [1, 2, 3].map(async (x) => x + 1);
+
+async function floatingArray() {
+  await Promise.all([1, 2, 3].map((x) => Promise.resolve(x + 1)));
+}
+
 ```