From 0562de3ef638b6cf590e6c1f9077c4f737a816ff Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 30 Sep 2022 13:47:23 -0500 Subject: [PATCH] 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. --- .../src/rules/no-misused-promises.ts | 45 +++++++++++++++---- .../tests/rules/no-misused-promises.test.ts | 24 ++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 2c7f1eeb2599..fdf13f14ed66 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -213,13 +213,13 @@ export default util.createRule({ 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,10 +486,13 @@ function isFunctionParam( return false; } -// Get the positions of parameters which are void functions (and not also +// 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') when determining +// if trailing arguments are candidates. +function voidFunctionArguments( checker: ts.TypeChecker, node: ts.CallExpression | ts.NewExpression, ): Set { @@ -507,17 +510,41 @@ function voidFunctionParams( : subType.getConstructSignatures(); for (const signature of signatures) { for (const [index, parameter] of signature.parameters.entries()) { - const type = checker.getTypeOfSymbolAtLocation( + const decl = parameter.getDeclarations()?.[0]; + let type = checker.getTypeOfSymbolAtLocation( parameter, node.expression, ); + + const indices = [index]; + // If this is a array 'rest' parameter, add all of the remaining parameter indices. + // Note - we currently do not support 'spread' arguments + if ( + decl && + ts.isParameter(decl) && + decl.dotDotDotToken && + checker.isArrayType(type) && + node.arguments + ) { + for (let i = index + 1; i < node.arguments.length; i++) { + indices.push(i); + } + // Unwrap 'Array' to 'MaybeVoidFunction', + // so that we'll handle it in the same way as a non-rest + // 'param: MaybeVoidFunction' + type = checker.getTypeArguments(type)[0]; + } + if (isThenableReturningFunctionType(checker, node.expression, type)) { - thenableReturnIndices.add(index); + indices.forEach(index => thenableReturnIndices.add(index)); } else if ( - !thenableReturnIndices.has(index) && isVoidReturningFunctionType(checker, node.expression, type) ) { - voidReturnIndices.add(index); + indices.forEach(index => { + if (!thenableReturnIndices.has(index)) { + voidReturnIndices.add(index); + } + }); } } } diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index 55ba429cdb09..340f64bb898b 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -970,5 +970,29 @@ console.log({ ...(condition ? Promise.resolve({ key: 42 }) : {}) }); { line: 7, messageId: 'spread' }, ], }, + { + code: ` +type MyUnion = (() => void) | boolean; + +function restPromises(first: Boolean, ...callbacks: Array<() => void>): void {} +function restUnion(first: string, ...callbacks: Array): void {} + +restPromises( + true, + () => Promise.resolve(true), + () => Promise.resolve(null), + () => true, + () => Promise.resolve('Hello'), +); + +restUnion('Testing', false, () => Promise.resolve(true)); + `, + errors: [ + { line: 9, messageId: 'voidReturnArgument' }, + { line: 10, messageId: 'voidReturnArgument' }, + { line: 12, messageId: 'voidReturnArgument' }, + { line: 15, messageId: 'voidReturnArgument' }, + ], + }, ], });