From 56a09e98f171662d25ae2692be703a8bbbd3a3a5 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Mar 2022 13:16:15 -0500 Subject: [PATCH] fix(eslint-plugin): [no-misused-promises] factor thenable returning function overload signatures (#4620) * fix(eslint-plugin): [no-misused-promises] factor thenable returning function overload signatures * chore: mention TypeScript issue * chore: dedup and clean and fix comments * test: add a few more test cases --- .../src/rules/no-misused-promises.ts | 84 ++++++++++--- .../tests/rules/no-misused-promises.test.ts | 110 ++++++++++++++++++ 2 files changed, 176 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 7f08402334c..b50ad53901e 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -394,9 +394,13 @@ function voidFunctionParams( checker: ts.TypeChecker, node: ts.CallExpression | ts.NewExpression, ): Set { + const thenableReturnIndices = new Set(); const voidReturnIndices = new Set(); const type = checker.getTypeAtLocation(node.expression); + // We can't use checker.getResolvedSignature because it prefers an early '() => void' over a later '() => Promise' + // See https://github.com/microsoft/TypeScript/issues/48077 + for (const subType of tsutils.unionTypeParts(type)) { // Standard function calls and `new` have two different types of signatures const signatures = ts.isCallExpression(node) @@ -408,50 +412,94 @@ function voidFunctionParams( parameter, node.expression, ); - if (isVoidReturningFunctionType(checker, node.expression, type)) { + if (isThenableReturningFunctionType(checker, node.expression, type)) { + thenableReturnIndices.add(index); + } else if ( + !thenableReturnIndices.has(index) && + isVoidReturningFunctionType(checker, node.expression, type) + ) { voidReturnIndices.add(index); } } } } + for (const index of thenableReturnIndices) { + voidReturnIndices.delete(index); + } + return voidReturnIndices; } -// Returns true if given type is a void-returning function. +/** + * @returns Whether any call signature of the type has a thenable return type. + */ +function anySignatureIsThenableType( + checker: ts.TypeChecker, + node: ts.Node, + type: ts.Type, +): boolean { + for (const signature of type.getCallSignatures()) { + const returnType = signature.getReturnType(); + if (tsutils.isThenableType(checker, node, returnType)) { + return true; + } + } + + return false; +} + +/** + * @returns Whether type is a thenable-returning function. + */ +function isThenableReturningFunctionType( + checker: ts.TypeChecker, + node: ts.Node, + type: ts.Type, +): boolean { + for (const subType of tsutils.unionTypeParts(type)) { + if (anySignatureIsThenableType(checker, node, subType)) { + return true; + } + } + + return false; +} + +/** + * @returns Whether type is a void-returning function. + */ function isVoidReturningFunctionType( checker: ts.TypeChecker, node: ts.Node, type: ts.Type, ): boolean { - let hasVoidReturningFunction = false; - let hasThenableReturningFunction = false; for (const subType of tsutils.unionTypeParts(type)) { for (const signature of subType.getCallSignatures()) { const returnType = signature.getReturnType(); + + // If a certain positional argument accepts both thenable and void returns, + // a promise-returning function is valid + if (tsutils.isThenableType(checker, node, returnType)) { + return false; + } + if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) { - hasVoidReturningFunction = true; - } else if (tsutils.isThenableType(checker, node, returnType)) { - hasThenableReturningFunction = true; + return true; } } } - // If a certain positional argument accepts both thenable and void returns, - // a promise-returning function is valid - return hasVoidReturningFunction && !hasThenableReturningFunction; + return false; } -// Returns true if the expression is a function that returns a thenable +/** + * @returns Whether expression is a function that returns a thenable. + */ function returnsThenable(checker: ts.TypeChecker, node: ts.Node): boolean { const type = checker.getApparentType(checker.getTypeAtLocation(node)); - for (const subType of tsutils.unionTypeParts(type)) { - for (const signature of subType.getCallSignatures()) { - const returnType = signature.getReturnType(); - if (tsutils.isThenableType(checker, node, returnType)) { - return true; - } - } + if (anySignatureIsThenableType(checker, node, type)) { + return true; } return false; 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 961a21d4c89..1cc0576fb30 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -252,6 +252,58 @@ const Component: any = () => null; `, filename: 'react.tsx', }, + { + code: ` +interface ItLike { + (name: string, callback: () => Promise): void; + (name: string, callback: () => void): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + }, + { + code: ` +interface ItLike { + (name: string, callback: () => void): void; + (name: string, callback: () => Promise): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + }, + { + code: ` +interface ItLike { + (name: string, callback: () => void): void; +} +interface ItLike { + (name: string, callback: () => Promise): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + }, + { + code: ` +interface ItLike { + (name: string, callback: () => Promise): void; +} +interface ItLike { + (name: string, callback: () => void): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + }, ], invalid: [ @@ -687,5 +739,63 @@ const Component = (obj: O) => null; }, ], }, + { + code: ` +interface ItLike { + (name: string, callback: () => number): void; + (name: string, callback: () => void): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + errors: [ + { + line: 9, + messageId: 'voidReturnArgument', + }, + ], + }, + { + code: ` +interface ItLike { + (name: string, callback: () => number): void; +} +interface ItLike { + (name: string, callback: () => void): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + errors: [ + { + line: 11, + messageId: 'voidReturnArgument', + }, + ], + }, + { + code: ` +interface ItLike { + (name: string, callback: () => void): void; +} +interface ItLike { + (name: string, callback: () => number): void; +} + +declare const it: ItLike; + +it('', async () => {}); + `, + errors: [ + { + line: 11, + messageId: 'voidReturnArgument', + }, + ], + }, ], });