Content-Length: 12058 | pFad | http://github.com/typescript-eslint/typescript-eslint/pull/4620.patch

thub.com From 78892551647e86329de9a4c57920c604d350f61e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 1 Mar 2022 15:25:03 -0500 Subject: [PATCH 1/4] fix(eslint-plugin): [no-misused-promises] factor thenable returning function overload signatures --- .../src/rules/no-misused-promises.ts | 52 +++++++++++++++---- .../tests/rules/no-misused-promises.test.ts | 42 +++++++++++++++ 2 files changed, 85 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 7f08402334cc..ad4842084401 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); + // TODO(file bug on TypeScript): checker.getResolvedSignature prefers a () => void over a () => Promise + // See https://github.com/typescript-eslint/typescript-eslint/issues/4609 (todo: comment in PR) + for (const subType of tsutils.unionTypeParts(type)) { // Standard function calls and `new` have two different types of signatures const signatures = ts.isCallExpression(node) @@ -408,13 +412,22 @@ 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; } @@ -424,21 +437,42 @@ function isVoidReturningFunctionType( 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; + } + } + } + return false; +} + +/** + * @returns Whether type is a void-returning function. + */ +function isThenableReturningFunctionType( + checker: ts.TypeChecker, + node: ts.Node, + type: ts.Type, +): boolean { + 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 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 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 961a21d4c892..df786e886b4d 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,30 @@ 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 () => {}); + `, + }, ], invalid: [ @@ -687,5 +711,23 @@ 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', + }, + ], + }, ], }); From 0425fe816d53b114ba77a1d37747cf8cf82e682d Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 1 Mar 2022 16:17:51 -0500 Subject: [PATCH 2/4] chore: mention TypeScript issue --- packages/eslint-plugin/src/rules/no-misused-promises.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index ad4842084401..455600290e6c 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -398,8 +398,8 @@ function voidFunctionParams( const voidReturnIndices = new Set(); const type = checker.getTypeAtLocation(node.expression); - // TODO(file bug on TypeScript): checker.getResolvedSignature prefers a () => void over a () => Promise - // See https://github.com/typescript-eslint/typescript-eslint/issues/4609 (todo: comment in PR) + // 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 From 1911f45ac5819c04222ad2fb294e213cac65221a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 1 Mar 2022 16:26:06 -0500 Subject: [PATCH 3/4] chore: dedup and clean and fix comments --- .../src/rules/no-misused-promises.ts | 60 ++++++++++++------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 455600290e6c..b50ad53901ef 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -431,34 +431,45 @@ function voidFunctionParams( return voidReturnIndices; } -// Returns true if given type is a void-returning function. -function isVoidReturningFunctionType( +/** + * @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 subType of tsutils.unionTypeParts(type)) { - for (const signature of subType.getCallSignatures()) { - const returnType = signature.getReturnType(); + for (const signature of type.getCallSignatures()) { + const returnType = signature.getReturnType(); + if (tsutils.isThenableType(checker, node, returnType)) { + return true; + } + } - // 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; - } + return false; +} - if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) { - return true; - } +/** + * @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 isThenableReturningFunctionType( +function isVoidReturningFunctionType( checker: ts.TypeChecker, node: ts.Node, type: ts.Type, @@ -466,26 +477,29 @@ function isThenableReturningFunctionType( 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)) { return true; } } } - 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; From 252e3d9d49ef480c2131be94c8544f7ffe671dda Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Mar 2022 11:08:44 -0500 Subject: [PATCH 4/4] test: add a few more test cases --- .../tests/rules/no-misused-promises.test.ts | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) 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 df786e886b4d..1cc0576fb300 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -273,6 +273,34 @@ interface ItLike { 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 () => {}); `, }, @@ -729,5 +757,45 @@ it('', async () => {}); }, ], }, + { + 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', + }, + ], + }, ], });

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!