feat(eslint-plugin): Check 'rest' parameters in no-misused-promises (#…

…5731)

* feat(eslint-plugin): Check 'rest' parameters in no-misused-promises

Fixes #4015

This extends 'no-misued-promises' with support for checking
variadic arguments passed to a 'rest' parameter. If a function
is declared with an argument like '...handlers: Array<() => void>',
we now check if the type argument to `Array` is a void-returning function,
and if so, check if any of the variadic arguments return a Promise.

* Address review comments

* Fix spelling

* Address additional review comments

* nit: split up tests a bit, and add comments about ()

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

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

@@ -213,13 +213,13 @@ export default util.createRule<Options, MessageId>({
       node: TSESTree.CallExpression | TSESTree.NewExpression,
     ): void {
       const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
-      const voidParams = voidFunctionParams(checker, tsNode);
-      if (voidParams.size === 0) {
+      const voidArgs = voidFunctionArguments(checker, tsNode);
+      if (voidArgs.size === 0) {
         return;
       }
 
       for (const [index, argument] of node.arguments.entries()) {
-        if (!voidParams.has(index)) {
+        if (!voidArgs.has(index)) {
           continue;
         }
 
@@ -486,13 +486,40 @@ function isFunctionParam(
   return false;
 }
 
-// Get the positions of parameters which are void functions (and not also
+function checkThenableOrVoidArgument(
+  checker: ts.TypeChecker,
+  node: ts.CallExpression | ts.NewExpression,
+  type: ts.Type,
+  index: number,
+  thenableReturnIndices: Set<number>,
+  voidReturnIndices: Set<number>,
+): void {
+  if (isThenableReturningFunctionType(checker, node.expression, type)) {
+    thenableReturnIndices.add(index);
+  } else if (isVoidReturningFunctionType(checker, node.expression, type)) {
+    // If a certain argument accepts both thenable and void returns,
+    // a promise-returning function is valid
+    if (!thenableReturnIndices.has(index)) {
+      voidReturnIndices.add(index);
+    }
+  }
+}
+
+// Get the positions of arguments which are void functions (and not also
 // thenable functions). These are the candidates for the void-return check at
 // the current call site.
-function voidFunctionParams(
+// If the function parameters end with a 'rest' parameter, then we consider
+// the array type parameter (e.g. '...args:Array<SomeType>') when determining
+// if trailing arguments are candidates.
+function voidFunctionArguments(
   checker: ts.TypeChecker,
   node: ts.CallExpression | ts.NewExpression,
 ): Set<number> {
+  // 'new' can be used without any arguments, as in 'let b = new Object;'
+  // In this case, there are no argument positions to check, so return early.
+  if (!node.arguments) {
+    return new Set<number>();
+  }
   const thenableReturnIndices = new Set<number>();
   const voidReturnIndices = new Set<number>();
   const type = checker.getTypeAtLocation(node.expression);
@@ -507,17 +534,56 @@ function voidFunctionParams(
       : subType.getConstructSignatures();
     for (const signature of signatures) {
       for (const [index, parameter] of signature.parameters.entries()) {
-        const type = checker.getTypeOfSymbolAtLocation(
+        const decl = parameter.valueDeclaration;
+        let type = checker.getTypeOfSymbolAtLocation(
           parameter,
           node.expression,
         );
-        if (isThenableReturningFunctionType(checker, node.expression, type)) {
-          thenableReturnIndices.add(index);
-        } else if (
-          !thenableReturnIndices.has(index) &&
-          isVoidReturningFunctionType(checker, node.expression, type)
-        ) {
-          voidReturnIndices.add(index);
+
+        // If this is a array 'rest' parameter, check all of the argument indices
+        // from the current argument to the end.
+        // Note - we currently do not support 'spread' arguments - adding support for them
+        // is tracked in https://github.com/typescript-eslint/typescript-eslint/issues/5744
+        if (decl && ts.isParameter(decl) && decl.dotDotDotToken) {
+          if (checker.isArrayType(type)) {
+            // Unwrap 'Array<MaybeVoidFunction>' to 'MaybeVoidFunction',
+            // so that we'll handle it in the same way as a non-rest
+            // 'param: MaybeVoidFunction'
+            type = checker.getTypeArguments(type)[0];
+            for (let i = index; i < node.arguments.length; i++) {
+              checkThenableOrVoidArgument(
+                checker,
+                node,
+                type,
+                i,
+                thenableReturnIndices,
+                voidReturnIndices,
+              );
+            }
+          } else if (checker.isTupleType(type)) {
+            // Check each type in the tuple - for example, [boolean, () => void] would
+            // add the index of the second tuple parameter to 'voidReturnIndices'
+            const typeArgs = checker.getTypeArguments(type);
+            for (let i = index; i < node.arguments.length; i++) {
+              checkThenableOrVoidArgument(
+                checker,
+                node,
+                typeArgs[i - index],
+                i,
+                thenableReturnIndices,
+                voidReturnIndices,
+              );
+            }
+          }
+        } else {
+          checkThenableOrVoidArgument(
+            checker,
+            node,
+            type,
+            index,
+            thenableReturnIndices,
+            voidReturnIndices,
+          );
         }
       }
     }

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

@@ -373,6 +373,52 @@ console.log([...Promise.resolve(42)]);
       `,
       options: [{ checksSpreads: false }],
     },
+    `
+function spreadAny(..._args: any): void {}
+
+spreadAny(
+  true,
+  () => Promise.resolve(1),
+  () => Promise.resolve(false),
+);
+    `,
+    `
+function spreadArrayAny(..._args: Array<any>): void {}
+
+spreadArrayAny(
+  true,
+  () => Promise.resolve(1),
+  () => Promise.resolve(false),
+);
+    `,
+    `
+function spreadArrayUnknown(..._args: Array<unknown>): void {}
+
+spreadArrayUnknown(() => Promise.resolve(true), 1, 2);
+
+function spreadArrayFuncPromise(
+  ..._args: Array<() => Promise<undefined>>
+): void {}
+
+spreadArrayFuncPromise(
+  () => Promise.resolve(undefined),
+  () => Promise.resolve(undefined),
+);
+    `,
+    // Prettier adds a () but this tests arguments being undefined, not []
+    // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting
+    `
+class TakeCallbacks {
+  constructor(...callbacks: Array<() => void>) {}
+}
+
+new TakeCallbacks;
+new TakeCallbacks();
+new TakeCallbacks(
+  () => 1,
+  () => true,
+);
+    `,
   ],
 
   invalid: [
@@ -970,5 +1016,88 @@ console.log({ ...(condition ? Promise.resolve({ key: 42 }) : {}) });
         { line: 7, messageId: 'spread' },
       ],
     },
+    {
+      code: `
+function restPromises(first: Boolean, ...callbacks: Array<() => void>): void {}
+
+restPromises(
+  true,
+  () => Promise.resolve(true),
+  () => Promise.resolve(null),
+  () => true,
+  () => Promise.resolve('Hello'),
+);
+      `,
+      errors: [
+        { line: 6, messageId: 'voidReturnArgument' },
+        { line: 7, messageId: 'voidReturnArgument' },
+        { line: 9, messageId: 'voidReturnArgument' },
+      ],
+    },
+    {
+      code: `
+type MyUnion = (() => void) | boolean;
+
+function restUnion(first: string, ...callbacks: Array<MyUnion>): void {}
+restUnion('Testing', false, () => Promise.resolve(true));
+      `,
+      errors: [{ line: 5, messageId: 'voidReturnArgument' }],
+    },
+    {
+      code: `
+function restTupleOne(first: string, ...callbacks: [() => void]): void {}
+restTupleOne('My string', () => Promise.resolve(1));
+      `,
+      errors: [{ line: 3, messageId: 'voidReturnArgument' }],
+    },
+    {
+      code: `
+function restTupleTwo(
+  first: boolean,
+  ...callbacks: [undefined, () => void, undefined]
+): void {}
+
+restTupleTwo(true, undefined, () => Promise.resolve(true), undefined);
+      `,
+      errors: [{ line: 7, messageId: 'voidReturnArgument' }],
+    },
+    {
+      code: `
+function restTupleFour(
+  first: number,
+  ...callbacks: [() => void, boolean, () => void, () => void]
+): void;
+
+restTupleFour(
+  1,
+  () => Promise.resolve(true),
+  false,
+  () => {},
+  () => Promise.resolve(1),
+);
+      `,
+      errors: [
+        { line: 9, messageId: 'voidReturnArgument' },
+        { line: 12, messageId: 'voidReturnArgument' },
+      ],
+    },
+    {
+      // Prettier adds a () but this tests arguments being undefined, not []
+      // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting
+      code: `
+class TakesVoidCb {
+  constructor(first: string, ...args: Array<() => void>);
+}
+
+new TakesVoidCb;
+new TakesVoidCb();
+new TakesVoidCb(
+  'Testing',
+  () => {},
+  () => Promise.resolve(true),
+);
+      `,
+      errors: [{ line: 11, messageId: 'voidReturnArgument' }],
+    },
   ],
 });