feat(eslint-plugin): [no-floating-promises] flag result of .map(async) (

#7897)

* flag arrays of promises

* remove comment

* trying to trigger CI

* Substantial simplification

* add some grody test cases for generics

* flip if/else

* add detail to test case

* switch to isArrayLikeType

* Revert "switch to isArrayLikeType"

This reverts commit 2afe794.

* add checks for tuples

* One more test case!

* else if

* Remove unnecessary whitespace

* Remove invalid test case

* Add to docs

* Add test cases around array that's fine but tuple that isn't

* tweaks

packages/eslint-plugin/docs/rules/no-floating-promises.md

@@ -17,6 +17,13 @@ Valid ways of handling a Promise-valued statement include:
 - Calling its `.then()` with two arguments
 - Calling its `.catch()` with one argument
 
+This rule also reports when an Array containing Promises is created and not properly handled. The main way to resolve this is by using one of the Promise concurrency methods to create a single Promise, then handling that according to the procedure above. These methods include:
+
+- `Promise.all()`,
+- `Promise.allSettled()`,
+- `Promise.any()`
+- `Promise.race()`
+
 :::tip
 `no-floating-promises` only detects unhandled Promise _statements_.
 See [`no-misused-promises`](./no-misused-promises.md) for detecting code that provides Promises to _logical_ locations such as if statements.
@@ -40,6 +47,8 @@ returnsPromise().then(() => {});
 Promise.reject('value').catch();
 
 Promise.reject('value').finally();
+
+[1, 2, 3].map(async x => x + 1);
 ```
 
 ### ✅ Correct
@@ -59,6 +68,8 @@ returnsPromise().then(
 Promise.reject('value').catch(() => {});
 
 await Promise.reject('value').finally(() => {});
+
+await Promise.all([1, 2, 3].map(async x => x + 1));
 ```
 
 ## Options
@@ -106,3 +117,7 @@ You might consider using `void`s and/or [ESLint disable comments](https://eslint
 ## Related To
 
 - [`no-misused-promises`](./no-misused-promises.md)
+
+## Further Reading
+
+- ["Using Promises" MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises). Note especially the sections on [Promise rejection events](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises#promise_rejection_events) and [Composition](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises#composition).

packages/eslint-plugin/src/rules/no-floating-promises.ts

@@ -23,7 +23,9 @@ type MessageId =
   | 'floatingUselessRejectionHandler'
   | 'floatingUselessRejectionHandlerVoid'
   | 'floatingFixAwait'
-  | 'floatingFixVoid';
+  | 'floatingFixVoid'
+  | 'floatingPromiseArray'
+  | 'floatingPromiseArrayVoid';
 
 const messageBase =
   'Promises must be awaited, end with a call to .catch, or end with a call to .then with a rejection handler.';
@@ -35,6 +37,13 @@ const messageBaseVoid =
 const messageRejectionHandler =
   'A rejection handler that is not a function will be ignored.';
 
+const messagePromiseArray =
+  "An array of Promises may be unintentional. Consider handling the promises' fulfillment or rejection with Promise.all or similar.";
+
+const messagePromiseArrayVoid =
+  "An array of Promises may be unintentional. Consider handling the promises' fulfillment or rejection with Promise.all or similar," +
+  ' or explicitly marking the expression as ignored with the `void` operator.';
+
 export default createRule<Options, MessageId>({
   name: 'no-floating-promises',
   meta: {
@@ -54,6 +63,8 @@ export default createRule<Options, MessageId>({
         messageBase + ' ' + messageRejectionHandler,
       floatingUselessRejectionHandlerVoid:
         messageBaseVoid + ' ' + messageRejectionHandler,
+      floatingPromiseArray: messagePromiseArray,
+      floatingPromiseArrayVoid: messagePromiseArrayVoid,
     },
     schema: [
       {
@@ -97,13 +108,18 @@ export default createRule<Options, MessageId>({
           expression = expression.expression;
         }
 
-        const { isUnhandled, nonFunctionHandler } = isUnhandledPromise(
-          checker,
-          expression,
-        );
+        const { isUnhandled, nonFunctionHandler, promiseArray } =
+          isUnhandledPromise(checker, expression);
 
         if (isUnhandled) {
-          if (options.ignoreVoid) {
+          if (promiseArray) {
+            context.report({
+              node,
+              messageId: options.ignoreVoid
+                ? 'floatingPromiseArrayVoid'
+                : 'floatingPromiseArray',
+            });
+          } else if (options.ignoreVoid) {
             context.report({
               node,
               messageId: nonFunctionHandler
@@ -205,7 +221,11 @@ export default createRule<Options, MessageId>({
     function isUnhandledPromise(
       checker: ts.TypeChecker,
       node: TSESTree.Node,
-    ): { isUnhandled: boolean; nonFunctionHandler?: boolean } {
+    ): {
+      isUnhandled: boolean;
+      nonFunctionHandler?: boolean;
+      promiseArray?: boolean;
+    } {
       // First, check expressions whose resulting types may not be promise-like
       if (node.type === AST_NODE_TYPES.SequenceExpression) {
         // Any child in a comma expression could return a potentially unhandled
@@ -228,8 +248,16 @@ export default createRule<Options, MessageId>({
         return isUnhandledPromise(checker, node.argument);
       }
 
+      const tsNode = services.esTreeNodeToTSNodeMap.get(node);
+
       // Check the type. At this point it can't be unhandled if it isn't a promise
-      if (!isPromiseLike(checker, services.esTreeNodeToTSNodeMap.get(node))) {
+      // or array thereof.
+
+      if (isPromiseArray(checker, tsNode)) {
+        return { isUnhandled: true, promiseArray: true };
+      }
+
+      if (!isPromiseLike(checker, tsNode)) {
         return { isUnhandled: false };
       }
 
@@ -295,12 +323,39 @@ export default createRule<Options, MessageId>({
   },
 });
 
+function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean {
+  const type = checker.getTypeAtLocation(node);
+  for (const ty of tsutils
+    .unionTypeParts(type)
+    .map(t => checker.getApparentType(t))) {
+    if (checker.isArrayType(ty)) {
+      const arrayType = checker.getTypeArguments(ty)[0];
+      if (isPromiseLike(checker, node, arrayType)) {
+        return true;
+      }
+    }
+
+    if (checker.isTupleType(ty)) {
+      for (const tupleElementType of checker.getTypeArguments(ty)) {
+        if (isPromiseLike(checker, node, tupleElementType)) {
+          return true;
+        }
+      }
+    }
+  }
+  return false;
+}
+
 // Modified from tsutils.isThenable() to only consider thenables which can be
 // rejected/caught via a second parameter. Original source (MIT licensed):
 //
 //   https://github.com/ajafff/tsutils/blob/49d0d31050b44b81e918eae4fbaf1dfe7b7286af/util/type.ts#L95-L125
-function isPromiseLike(checker: ts.TypeChecker, node: ts.Node): boolean {
-  const type = checker.getTypeAtLocation(node);
+function isPromiseLike(
+  checker: ts.TypeChecker,
+  node: ts.Node,
+  type?: ts.Type,
+): boolean {
+  type ??= checker.getTypeAtLocation(node);
   for (const ty of tsutils.unionTypeParts(checker.getApparentType(type))) {
     const then = ty.getProperty('then');
     if (then === undefined) {

packages/eslint-plugin/tests/rules/no-floating-promises.test.ts

@@ -222,7 +222,7 @@ async function test() {
     `
 async function test() {
   class Thenable {
-    then(callback: () => {}): Thenable {
+    then(callback: () => void): Thenable {
       return new Thenable();
     }
   }
@@ -264,7 +264,7 @@ async function test() {
     `
 async function test() {
   class CatchableThenable {
-    then(callback: () => {}, callback: () => {}): CatchableThenable {
+    then(callback: () => void, callback: () => void): CatchableThenable {
       return new CatchableThenable();
     }
   }
@@ -475,6 +475,35 @@ Promise.reject()
   .finally(() => {});
       `,
     },
+    {
+      code: `
+await Promise.all([Promise.resolve(), Promise.resolve()]);
+      `,
+    },
+    {
+      code: `
+declare const promiseArray: Array<Promise<unknown>>;
+void promiseArray;
+      `,
+    },
+    {
+      code: `
+[Promise.reject(), Promise.reject()].then(() => {});
+      `,
+    },
+    {
+      // Expressions aren't checked by this rule, so this just becomes an array
+      // of number | undefined, which is fine regardless of the ignoreVoid setting.
+      code: `
+[1, 2, void Promise.reject(), 3];
+      `,
+      options: [{ ignoreVoid: false }],
+    },
+    {
+      code: `
+['I', 'am', 'just', 'an', 'array'];
+      `,
+    },
   ],
 
   invalid: [
@@ -997,7 +1026,7 @@ async function test() {
       code: `
 async function test() {
   class CatchableThenable {
-    then(callback: () => {}, callback: () => {}): CatchableThenable {
+    then(callback: () => void, callback: () => void): CatchableThenable {
       return new CatchableThenable();
     }
   }
@@ -1651,5 +1680,131 @@ Promise.reject(new Error('message')).finally(() => {});
       `,
       errors: [{ line: 2, messageId: 'floatingVoid' }],
     },
+    {
+      code: `
+function _<T, S extends Array<T | Promise<T>>>(
+  maybePromiseArray: S | undefined,
+): void {
+  maybePromiseArray?.[0];
+}
+      `,
+      errors: [{ line: 5, messageId: 'floatingVoid' }],
+    },
+    {
+      code: `
+[1, 2, 3].map(() => Promise.reject());
+      `,
+      errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+declare const array: unknown[];
+array.map(() => Promise.reject());
+      `,
+      errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+declare const promiseArray: Array<Promise<unknown>>;
+void promiseArray;
+      `,
+      options: [{ ignoreVoid: false }],
+      errors: [{ line: 3, messageId: 'floatingPromiseArray' }],
+    },
+    {
+      code: `
+[1, 2, Promise.reject(), 3];
+      `,
+      errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+[1, 2, Promise.reject().catch(() => {}), 3];
+      `,
+      errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+const data = ['test'];
+data.map(async () => {
+  await new Promise((_res, rej) => setTimeout(rej, 1000));
+});
+      `,
+      errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+function _<T, S extends Array<T | Array<T | Promise<T>>>>(
+  maybePromiseArrayArray: S | undefined,
+): void {
+  maybePromiseArrayArray?.[0];
+}
+      `,
+      errors: [{ line: 5, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+function f<T extends Array<Promise<number>>>(a: T): void {
+  a;
+}
+      `,
+      errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+declare const a: Array<Promise<number>> | undefined;
+a;
+      `,
+      errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+function f<T extends Array<Promise<number>>>(a: T | undefined): void {
+  a;
+}
+      `,
+      errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+[Promise.reject()] as const;
+      `,
+      errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+declare function cursed(): [Promise<number>, Promise<string>];
+cursed();
+      `,
+      errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+[
+  'Type Argument number ',
+  1,
+  'is not',
+  Promise.resolve(),
+  'but it still is flagged',
+] as const;
+      `,
+      errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+        declare const arrayOrPromiseTuple:
+          | Array<number>
+          | [number, number, Promise<unknown>, string];
+        arrayOrPromiseTuple;
+      `,
+      errors: [{ line: 5, messageId: 'floatingPromiseArrayVoid' }],
+    },
+    {
+      code: `
+        declare const okArrayOrPromiseArray: Array<number> | Array<Promise<unknown>>;
+        okArrayOrPromiseArray;
+      `,
+      errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }],
+    },
   ],
 });