From c48ff4275802c223c97f54aad1a31267ff365e18 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sun, 17 Mar 2024 16:51:11 +0900 Subject: [PATCH 01/18] feat(eslint-plugin): [no-unsafe-return] check promise any --- .../src/rules/no-unsafe-return.ts | 17 +++++- .../tests/rules/no-unsafe-return.test.ts | 58 +++++++++++++++++++ packages/type-utils/src/predicates.ts | 3 +- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index fa301bafa652..4a2df0e8bd2b 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -73,12 +73,27 @@ export default createRule({ /* istanbul ignore next */ return null; } + function getAwaitedType(node: ts.Node): ts.Type { + const type = checker.getTypeAtLocation(node); + if ( + tsutils.isThenableType(checker, node, type) && + tsutils.isTypeReference(type) + ) { + const awaitedType = type.typeArguments?.[0]; + if (awaitedType) { + return awaitedType; + } + } + return type; + } + function checkReturn( returnNode: TSESTree.Node, reportingNode: TSESTree.Node = returnNode, ): void { const tsNode = services.esTreeNodeToTSNodeMap.get(returnNode); - const anyType = isAnyOrAnyArrayTypeDiscriminated(tsNode, checker); + const awaitedType = getAwaitedType(tsNode); + const anyType = isAnyOrAnyArrayTypeDiscriminated(awaitedType, checker); const functionNode = getParentFunctionNode(returnNode); /* istanbul ignore if */ if (!functionNode) { return; diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index 42a798ffe117..e57f7eebb381 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -56,6 +56,16 @@ function foo(): any[] { ` function foo(): Set { return new Set(); +} + `, + ` +async function foo(): any { + return {} as any; +} + `, + ` +async function foo(): Promise { + return Promise.resolve({} as any); } `, // TODO - this should error, but it's hard to detect, as the type references are different @@ -408,5 +418,53 @@ function bar() { }, ], }, + { + code: ` +declare const value: any; +async function foo() { + return value; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 4, + column: 3, + endColumn: 16, + }, + ], + }, + { + code: ` +declare const value: Promise; +function foo() { + return value; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 4, + column: 3, + endColumn: 16, + }, + ], + }, + { + code: ` +declare const value: Promise; +async function foo(): Promise { + return value; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 4, + column: 3, + endColumn: 16, + }, + ], + }, ], }); diff --git a/packages/type-utils/src/predicates.ts b/packages/type-utils/src/predicates.ts index 26ef59355781..51d2511b6a2d 100644 --- a/packages/type-utils/src/predicates.ts +++ b/packages/type-utils/src/predicates.ts @@ -131,10 +131,9 @@ export enum AnyType { * otherwise it returns `AnyType.Safe`. */ export function isAnyOrAnyArrayTypeDiscriminated( - node: ts.Node, + type: ts.Type, checker: ts.TypeChecker, ): AnyType { - const type = checker.getTypeAtLocation(node); if (isTypeAnyType(type)) { return AnyType.Any; } From 2cf600419bb66225f27a84be303c02680284a713 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Mon, 18 Mar 2024 00:43:04 +0900 Subject: [PATCH 02/18] add testcases --- .../src/rules/no-unsafe-return.ts | 9 ++--- .../tests/rules/no-unsafe-return.test.ts | 40 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 4a2df0e8bd2b..d1e313cdd69a 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -73,8 +73,7 @@ export default createRule({ /* istanbul ignore next */ return null; } - function getAwaitedType(node: ts.Node): ts.Type { - const type = checker.getTypeAtLocation(node); + function getAwaitedType(node: ts.Node, type: ts.Type): ts.Type { if ( tsutils.isThenableType(checker, node, type) && tsutils.isTypeReference(type) @@ -92,7 +91,8 @@ export default createRule({ reportingNode: TSESTree.Node = returnNode, ): void { const tsNode = services.esTreeNodeToTSNodeMap.get(returnNode); - const awaitedType = getAwaitedType(tsNode); + const returnNodeType = getConstrainedTypeAtLocation(services, returnNode); + const awaitedType = getAwaitedType(tsNode, returnNodeType); const anyType = isAnyOrAnyArrayTypeDiscriminated(awaitedType, checker); const functionNode = getParentFunctionNode(returnNode); /* istanbul ignore if */ if (!functionNode) { @@ -100,7 +100,6 @@ export default createRule({ } // function has an explicit return type, so ensure it's a safe return - const returnNodeType = getConstrainedTypeAtLocation(services, returnNode); const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); // function expressions will not have their return type modified based on receiver typing @@ -123,7 +122,7 @@ export default createRule({ if ( returnNodeType === signature.getReturnType() || isTypeFlagSet( - signature.getReturnType(), + getAwaitedType(functionTSNode, signature.getReturnType()), ts.TypeFlags.Any | ts.TypeFlags.Unknown, ) ) { diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index e57f7eebb381..cf2ba2f65fb7 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -112,6 +112,16 @@ function foo(): Set { return x as Set; } `, + ` + async function fn(x: T): Promise { + return x as any; + } + `, + ` + function fn(x: T): Promise { + return Promise.resolve(x as any); + } + `, // https://github.com/typescript-eslint/typescript-eslint/issues/2109 ` function test(): Map { @@ -466,5 +476,35 @@ async function foo(): Promise { }, ], }, + { + code: ` +async function foo(arg: number) { + return arg as any; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + endColumn: 21, + }, + ], + }, + { + code: ` +async function foo(arg: number) { + return arg as Promise; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + endColumn: 30, + }, + ], + }, ], }); From e84d6ab030d28ee83e5fda2de59a9913f6cec7e2 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 1 May 2024 00:50:13 +0900 Subject: [PATCH 03/18] apply reviews --- .../src/rules/no-unsafe-return.ts | 73 ++++++--- .../tests/rules/no-unsafe-return.test.ts | 153 ++++++++++++++++-- packages/type-utils/src/getAwaitedType.ts | 13 ++ packages/type-utils/src/index.ts | 1 + packages/type-utils/src/predicates.ts | 17 +- .../type-utils/tests/getAwaitedType.test.ts | 66 ++++++++ 6 files changed, 290 insertions(+), 33 deletions(-) create mode 100644 packages/type-utils/src/getAwaitedType.ts create mode 100644 packages/type-utils/tests/getAwaitedType.test.ts diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 47a8fbaee5be..2a6ac3591c6a 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -6,16 +6,18 @@ import * as ts from 'typescript'; import { AnyType, createRule, + discriminateAnyType, getConstrainedTypeAtLocation, getContextualType, getParserServices, getThisExpression, - isAnyOrAnyArrayTypeDiscriminated, + isPromiseLike, isTypeAnyType, isTypeFlagSet, isTypeUnknownArrayType, isTypeUnknownType, isUnsafeAssignment, + getAwaitedType, } from '../util'; export default createRule({ @@ -73,33 +75,20 @@ export default createRule({ /* istanbul ignore next */ return null; } - function getAwaitedType(node: ts.Node, type: ts.Type): ts.Type { - if ( - tsutils.isThenableType(checker, node, type) && - tsutils.isTypeReference(type) - ) { - const awaitedType = type.typeArguments?.[0]; - if (awaitedType) { - return awaitedType; - } - } - return type; - } - function checkReturn( returnNode: TSESTree.Node, reportingNode: TSESTree.Node = returnNode, ): void { const tsNode = services.esTreeNodeToTSNodeMap.get(returnNode); - const returnNodeType = getConstrainedTypeAtLocation(services, returnNode); - const awaitedType = getAwaitedType(tsNode, returnNodeType); - const anyType = isAnyOrAnyArrayTypeDiscriminated(awaitedType, checker); + const type = checker.getTypeAtLocation(tsNode); + const anyType = discriminateAnyType(type, checker, services.program); const functionNode = getParentFunctionNode(returnNode); /* istanbul ignore if */ if (!functionNode) { return; } // function has an explicit return type, so ensure it's a safe return + const returnNodeType = getConstrainedTypeAtLocation(services, returnNode); const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); // function expressions will not have their return type modified based on receiver typing @@ -119,15 +108,36 @@ export default createRule({ // function return type, we shouldn't complain (it's intentional, even if unsafe) if (functionTSNode.type) { for (const signature of tsutils.getCallSignaturesOfType(functionType)) { + const signatureReturnType = signature.getReturnType(); + if ( - returnNodeType === signature.getReturnType() || + returnNodeType === signatureReturnType || isTypeFlagSet( - getAwaitedType(functionTSNode, signature.getReturnType()), + signatureReturnType, ts.TypeFlags.Any | ts.TypeFlags.Unknown, ) ) { return; } + if (functionNode.async) { + const awaitedSignatureReturnType = getAwaitedType( + services.program, + signatureReturnType, + ); + const awaitedReturnNodeType = getAwaitedType( + services.program, + returnNodeType, + ); + if ( + awaitedReturnNodeType === awaitedSignatureReturnType || + isTypeFlagSet( + awaitedSignatureReturnType, + ts.TypeFlags.Any | ts.TypeFlags.Unknown, + ) + ) { + return; + } + } } } @@ -148,6 +158,24 @@ export default createRule({ ) { return; } + if ( + anyType === AnyType.PromiseAny && + isPromiseLike(services.program, functionReturnType) && + isTypeUnknownType( + getAwaitedType(services.program, functionReturnType), + ) + ) { + return; + } + } + + if ( + anyType === AnyType.PromiseAny && + functionType + .getCallSignatures() + .every(sig => !isPromiseLike(services.program, sig.getReturnType())) + ) { + return; } let messageId: 'unsafeReturn' | 'unsafeReturnThis' = 'unsafeReturn'; @@ -170,7 +198,12 @@ export default createRule({ node: reportingNode, messageId, data: { - type: anyType === AnyType.Any ? 'any' : 'any[]', + type: + anyType === AnyType.Any + ? 'any' + : anyType === AnyType.AnyArray + ? 'any[]' + : 'Promise', }, }); } diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index 5ccd64d1b8b1..5995fdec2321 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -67,12 +67,17 @@ function foo(): Set { } `, ` -async function foo(): any { - return {} as any; +async function foo(): Promise { + return Promise.resolve({} as any); } `, ` async function foo(): Promise { + return {} as any; +} + `, + ` +function foo(): object { return Promise.resolve({} as any); } `, @@ -438,6 +443,20 @@ function bar() { }, { code: ` +declare function foo(arg: null | (() => any)): void; +foo(() => 'foo' as any); + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 11, + endColumn: 23, + }, + ], + }, + { + code: ` declare const value: any; async function foo() { return value; @@ -448,7 +467,6 @@ async function foo() { messageId: 'unsafeReturn', line: 4, column: 3, - endColumn: 16, }, ], }, @@ -464,7 +482,9 @@ function foo() { messageId: 'unsafeReturn', line: 4, column: 3, - endColumn: 16, + data: { + type: 'Promise', + }, }, ], }, @@ -480,36 +500,145 @@ async function foo(): Promise { messageId: 'unsafeReturn', line: 4, column: 3, - endColumn: 16, + data: { + type: 'Promise', + }, }, ], }, { code: ` -declare function foo(arg: null | (() => any)): void; -foo(() => 'foo' as any); +async function foo(arg: number) { + return arg as Promise; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: 'Promise', + }, + }, + ], + }, + { + code: ` +function foo(): Promise { + return {} as any; +} `, errors: [ { messageId: 'unsafeReturn', line: 3, column: 3, - endColumn: 21, + data: { + type: 'any', + }, }, ], }, { code: ` -async function foo(arg: number) { - return arg as Promise; +function foo(): Promise { + return {} as any; } `, errors: [ { messageId: 'unsafeReturn', line: 3, - column: 11, - endColumn: 23, + column: 3, + data: { + type: 'any', + }, + }, + ], + }, + { + code: ` +function foo(): Promise { + return Promise.resolve({}); +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: 'Promise', + }, + }, + ], + }, + { + code: ` +function foo(): Promise { + return Promise.resolve>>({} as any); +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: 'Promise', + }, + }, + ], + }, + { + code: ` +async function foo(): Promise { + return Promise.resolve>>({} as Promise); +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: 'Promise', + }, + }, + ], + }, + { + code: ` +async function foo(): Promise { + return {} as Promise>>>; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: 'Promise', + }, + }, + ], + }, + { + code: ` +async function foo() { + return {} as Promise>>>; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: 'Promise', + }, }, ], }, diff --git a/packages/type-utils/src/getAwaitedType.ts b/packages/type-utils/src/getAwaitedType.ts new file mode 100644 index 000000000000..db158b6b5a6a --- /dev/null +++ b/packages/type-utils/src/getAwaitedType.ts @@ -0,0 +1,13 @@ +import * as ts from 'typescript'; +import * as tsutils from 'ts-api-utils'; +import { isPromiseLike } from './builtinSymbolLikes'; + +export function getAwaitedType(program: ts.Program, type: ts.Type): ts.Type { + if (isPromiseLike(program, type) && tsutils.isTypeReference(type)) { + const awaitedType = type.typeArguments?.[0]; + if (awaitedType) { + return getAwaitedType(program, awaitedType); + } + } + return type; +} diff --git a/packages/type-utils/src/index.ts b/packages/type-utils/src/index.ts index 14d5b652099f..847def8433af 100644 --- a/packages/type-utils/src/index.ts +++ b/packages/type-utils/src/index.ts @@ -15,6 +15,7 @@ export * from './propertyTypes'; export * from './requiresQuoting'; export * from './TypeOrValueSpecifier'; export * from './typeFlagUtils'; +export * from './getAwaitedType'; export { getDecorators, getModifiers, diff --git a/packages/type-utils/src/predicates.ts b/packages/type-utils/src/predicates.ts index 51d2511b6a2d..b2f31a2a9b3f 100644 --- a/packages/type-utils/src/predicates.ts +++ b/packages/type-utils/src/predicates.ts @@ -2,6 +2,7 @@ import debug from 'debug'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; +import { isPromiseLike } from './builtinSymbolLikes'; import { isTypeFlagSet } from './typeFlagUtils'; const log = debug('typescript-eslint:eslint-plugin:utils:types'); @@ -123,6 +124,7 @@ export function isTypeUnknownArrayType( export enum AnyType { Any, + PromiseAny, AnyArray, Safe, } @@ -130,9 +132,10 @@ export enum AnyType { * @returns `AnyType.Any` if the type is `any`, `AnyType.AnyArray` if the type is `any[]` or `readonly any[]`, * otherwise it returns `AnyType.Safe`. */ -export function isAnyOrAnyArrayTypeDiscriminated( +export function discriminateAnyType( type: ts.Type, checker: ts.TypeChecker, + program: ts.Program, ): AnyType { if (isTypeAnyType(type)) { return AnyType.Any; @@ -140,6 +143,18 @@ export function isAnyOrAnyArrayTypeDiscriminated( if (isTypeAnyArrayType(type, checker)) { return AnyType.AnyArray; } + if (isPromiseLike(program, type) && tsutils.isTypeReference(type)) { + const awaitedType = type.typeArguments?.[0]; + if (awaitedType) { + const awaitedAnyType = discriminateAnyType(awaitedType, checker, program); + if ( + awaitedAnyType === AnyType.Any || + awaitedAnyType === AnyType.PromiseAny + ) { + return AnyType.PromiseAny; + } + } + } return AnyType.Safe; } diff --git a/packages/type-utils/tests/getAwaitedType.test.ts b/packages/type-utils/tests/getAwaitedType.test.ts new file mode 100644 index 000000000000..f9f6dce893b1 --- /dev/null +++ b/packages/type-utils/tests/getAwaitedType.test.ts @@ -0,0 +1,66 @@ +import { parseForESLint } from '@typescript-eslint/parser'; +import type { TSESTree } from '@typescript-eslint/utils'; +import path from 'path'; +import type * as ts from 'typescript'; + +import { getAwaitedType } from '../src/getAwaitedType'; +import { expectToHaveParserServices } from './test-utils/expectToHaveParserServices'; + +describe('getAwaitedType', () => { + const rootDir = path.join(__dirname, 'fixtures'); + + function getTypes( + code: string, + declarationIndex = 0, + ): { + program: ts.Program; + type: ts.Type; + checker: ts.TypeChecker; + } { + const { ast, services } = parseForESLint(code, { + project: './tsconfig.json', + filePath: path.join(rootDir, 'file.ts'), + tsconfigRootDir: rootDir, + }); + expectToHaveParserServices(services); + const checker = services.program.getTypeChecker(); + + const declaration = ast.body[ + declarationIndex + ] as TSESTree.VariableDeclaration; + const declarator = declaration.declarations[0]; + return { + program: services.program, + type: services.getTypeAtLocation(declarator.id), + checker, + }; + } + + function expectTypesAre( + result: ts.Type, + checker: ts.TypeChecker, + typeStr: string, + ): void { + expect(result).toBeTruthy(); + expect(checker.typeToString(result)).toBe(typeStr); + } + + it('non-promise', () => { + const { program, type, checker } = getTypes('const test: number = 1'); + expectTypesAre(getAwaitedType(program, type), checker, 'number'); + }); + + it('Promise<{{type}}> to {{type}}', () => { + const { program, type, checker } = getTypes( + 'const test = Promise.resolve(1)', + ); + expectTypesAre(getAwaitedType(program, type), checker, 'number'); + }); + + it('Promise> to {{type}}', () => { + const { program, type, checker } = getTypes( + 'const test: Promise> = Promise.resolve(Promise.resolve(1))', + ); + expectTypesAre(getAwaitedType(program, type), checker, 'number'); + }); +}); From f24b74643d52c54b1091a4355543b1c671a1e122 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 2 May 2024 23:47:14 +0900 Subject: [PATCH 04/18] fix lint errors --- packages/eslint-plugin/src/rules/no-unsafe-return.ts | 2 +- packages/type-utils/src/getAwaitedType.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 2a6ac3591c6a..49ac8dcbd831 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -7,6 +7,7 @@ import { AnyType, createRule, discriminateAnyType, + getAwaitedType, getConstrainedTypeAtLocation, getContextualType, getParserServices, @@ -17,7 +18,6 @@ import { isTypeUnknownArrayType, isTypeUnknownType, isUnsafeAssignment, - getAwaitedType, } from '../util'; export default createRule({ diff --git a/packages/type-utils/src/getAwaitedType.ts b/packages/type-utils/src/getAwaitedType.ts index db158b6b5a6a..d65750f50d25 100644 --- a/packages/type-utils/src/getAwaitedType.ts +++ b/packages/type-utils/src/getAwaitedType.ts @@ -1,5 +1,6 @@ -import * as ts from 'typescript'; import * as tsutils from 'ts-api-utils'; +import type * as ts from 'typescript'; + import { isPromiseLike } from './builtinSymbolLikes'; export function getAwaitedType(program: ts.Program, type: ts.Type): ts.Type { From 014cd48a6f85edb271a6cb6edf66c657e7550164 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Fri, 3 May 2024 01:01:01 +0900 Subject: [PATCH 05/18] refactor --- .../eslint-plugin/src/rules/no-unsafe-return.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 49ac8dcbd831..894a5b0983e2 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -103,11 +103,11 @@ export default createRule({ if (!functionType) { functionType = services.getTypeAtLocation(functionNode); } - + const callSignatures = tsutils.getCallSignaturesOfType(functionType); // If there is an explicit type annotation *and* that type matches the actual // function return type, we shouldn't complain (it's intentional, even if unsafe) if (functionTSNode.type) { - for (const signature of tsutils.getCallSignaturesOfType(functionType)) { + for (const signature of callSignatures) { const signatureReturnType = signature.getReturnType(); if ( @@ -144,7 +144,7 @@ export default createRule({ if (anyType !== AnyType.Safe) { // Allow cases when the declared return type of the function is either unknown or unknown[] // and the function is returning any or any[]. - for (const signature of functionType.getCallSignatures()) { + for (const signature of callSignatures) { const functionReturnType = signature.getReturnType(); if ( anyType === AnyType.Any && @@ -171,9 +171,10 @@ export default createRule({ if ( anyType === AnyType.PromiseAny && - functionType - .getCallSignatures() - .every(sig => !isPromiseLike(services.program, sig.getReturnType())) + callSignatures.every( + signature => + !isPromiseLike(services.program, signature.getReturnType()), + ) ) { return; } @@ -208,7 +209,7 @@ export default createRule({ }); } - for (const signature of functionType.getCallSignatures()) { + for (const signature of callSignatures) { const functionReturnType = signature.getReturnType(); const result = isUnsafeAssignment( returnNodeType, From 6390a955248cc4c71e0dd1f20bae7b217c86e3f3 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Fri, 3 May 2024 20:13:47 +0900 Subject: [PATCH 06/18] add type --- packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index 5995fdec2321..0cf57556d240 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -467,6 +467,9 @@ async function foo() { messageId: 'unsafeReturn', line: 4, column: 3, + data: { + type: 'any', + }, }, ], }, From 25daa682644fce65538dcd828fc8c05d2e3b67a9 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 12 Jun 2024 20:17:33 +0900 Subject: [PATCH 07/18] improve messages --- packages/eslint-plugin/src/rules/no-unsafe-return.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 894a5b0983e2..63f430e4519c 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -30,9 +30,9 @@ export default createRule({ requiresTypeChecking: true, }, messages: { - unsafeReturn: 'Unsafe return of an `{{type}}` typed value.', + unsafeReturn: 'Unsafe return of a value of type `{{type}}`.', unsafeReturnThis: [ - 'Unsafe return of an `{{type}}` typed value. `this` is typed as `any`.', + 'Unsafe return of a value of type `{{type}}`. `this` is typed as `any`.', 'You can try to fix this by turning on the `noImplicitThis` compiler option, or adding a `this` parameter to the function.', ].join('\n'), unsafeReturnAssignment: From 979bd7228fc1b727e360373e1f1b4cb6373335e8 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 12 Jun 2024 20:20:50 +0900 Subject: [PATCH 08/18] update comment --- packages/type-utils/src/predicates.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/type-utils/src/predicates.ts b/packages/type-utils/src/predicates.ts index b2f31a2a9b3f..7fed3b664471 100644 --- a/packages/type-utils/src/predicates.ts +++ b/packages/type-utils/src/predicates.ts @@ -129,7 +129,7 @@ export enum AnyType { Safe, } /** - * @returns `AnyType.Any` if the type is `any`, `AnyType.AnyArray` if the type is `any[]` or `readonly any[]`, + * @returns `AnyType.Any` if the type is `any`, `AnyType.AnyArray` if the type is `any[]` or `readonly any[]`, `AnyType.PromiseAny` if the type is `Promise`, * otherwise it returns `AnyType.Safe`. */ export function discriminateAnyType( From 4277c7089d96ffde9053feab051ae8bc23b465c2 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 12 Jun 2024 21:34:09 +0900 Subject: [PATCH 09/18] apply review --- .../eslint-plugin/src/rules/no-unsafe-return.ts | 10 ++++++---- packages/type-utils/src/getAwaitedType.ts | 15 ++++++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 63f430e4519c..5046123e9ac4 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -90,6 +90,7 @@ export default createRule({ // function has an explicit return type, so ensure it's a safe return const returnNodeType = getConstrainedTypeAtLocation(services, returnNode); const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); + const returnTSNode = services.esTreeNodeToTSNodeMap.get(returnNode); // function expressions will not have their return type modified based on receiver typing // so we have to use the contextual typing in these cases, i.e. @@ -121,12 +122,14 @@ export default createRule({ } if (functionNode.async) { const awaitedSignatureReturnType = getAwaitedType( - services.program, + checker, signatureReturnType, + functionTSNode, ); const awaitedReturnNodeType = getAwaitedType( - services.program, + checker, returnNodeType, + returnTSNode, ); if ( awaitedReturnNodeType === awaitedSignatureReturnType || @@ -160,9 +163,8 @@ export default createRule({ } if ( anyType === AnyType.PromiseAny && - isPromiseLike(services.program, functionReturnType) && isTypeUnknownType( - getAwaitedType(services.program, functionReturnType), + getAwaitedType(checker, functionReturnType, returnTSNode), ) ) { return; diff --git a/packages/type-utils/src/getAwaitedType.ts b/packages/type-utils/src/getAwaitedType.ts index d65750f50d25..d74260b018fa 100644 --- a/packages/type-utils/src/getAwaitedType.ts +++ b/packages/type-utils/src/getAwaitedType.ts @@ -1,13 +1,18 @@ import * as tsutils from 'ts-api-utils'; import type * as ts from 'typescript'; -import { isPromiseLike } from './builtinSymbolLikes'; - -export function getAwaitedType(program: ts.Program, type: ts.Type): ts.Type { - if (isPromiseLike(program, type) && tsutils.isTypeReference(type)) { +export function getAwaitedType( + checker: ts.TypeChecker, + type: ts.Type, + node: ts.Node, +): ts.Type { + if ( + tsutils.isThenableType(checker, node, type) && + tsutils.isTypeReference(type) + ) { const awaitedType = type.typeArguments?.[0]; if (awaitedType) { - return getAwaitedType(program, awaitedType); + return getAwaitedType(checker, awaitedType, node); } } return type; From c2d14e506be4cc1a1e30b340258fd8de832fefe1 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Tue, 9 Jul 2024 09:29:22 +0900 Subject: [PATCH 10/18] fix --- .../src/rules/no-unsafe-return.ts | 4 +++- .../tests/rules/no-unsafe-return.test.ts | 22 +++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 765f964355e2..cb09c9f058c8 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -206,7 +206,9 @@ export default createRule({ ? 'error' : anyType === AnyType.Any ? '`any`' - : '`any[]`', + : anyType === AnyType.PromiseAny + ? '`Promise`' + : '`any[]`', }, }); } diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index 95661984064d..165aa677e094 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -497,7 +497,7 @@ async function foo() { line: 4, column: 3, data: { - type: 'any', + type: '`any`', }, }, ], @@ -515,7 +515,7 @@ function foo() { line: 4, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], @@ -533,7 +533,7 @@ async function foo(): Promise { line: 4, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], @@ -550,7 +550,7 @@ async function foo(arg: number) { line: 3, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], @@ -567,7 +567,7 @@ function foo(): Promise { line: 3, column: 3, data: { - type: 'any', + type: '`any`', }, }, ], @@ -584,7 +584,7 @@ function foo(): Promise { line: 3, column: 3, data: { - type: 'any', + type: '`any`', }, }, ], @@ -601,7 +601,7 @@ function foo(): Promise { line: 3, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], @@ -618,7 +618,7 @@ function foo(): Promise { line: 3, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], @@ -635,7 +635,7 @@ async function foo(): Promise { line: 3, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], @@ -652,7 +652,7 @@ async function foo(): Promise { line: 3, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], @@ -669,7 +669,7 @@ async function foo() { line: 3, column: 3, data: { - type: 'Promise', + type: '`Promise`', }, }, ], From d2c014577fd03783c7e95f36329bb3a7a933e19a Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Tue, 9 Jul 2024 22:55:46 +0900 Subject: [PATCH 11/18] fix tests --- packages/type-utils/tests/getAwaitedType.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/type-utils/tests/getAwaitedType.test.ts b/packages/type-utils/tests/getAwaitedType.test.ts index f9f6dce893b1..ebe31017333c 100644 --- a/packages/type-utils/tests/getAwaitedType.test.ts +++ b/packages/type-utils/tests/getAwaitedType.test.ts @@ -16,6 +16,7 @@ describe('getAwaitedType', () => { program: ts.Program; type: ts.Type; checker: ts.TypeChecker; + node: ts.Node; } { const { ast, services } = parseForESLint(code, { project: './tsconfig.json', @@ -32,6 +33,7 @@ describe('getAwaitedType', () => { return { program: services.program, type: services.getTypeAtLocation(declarator.id), + node: services.esTreeNodeToTSNodeMap.get(declarator.id), checker, }; } @@ -46,21 +48,19 @@ describe('getAwaitedType', () => { } it('non-promise', () => { - const { program, type, checker } = getTypes('const test: number = 1'); - expectTypesAre(getAwaitedType(program, type), checker, 'number'); + const { type, checker, node } = getTypes('const test: number = 1'); + expectTypesAre(getAwaitedType(checker, type, node), checker, 'number'); }); it('Promise<{{type}}> to {{type}}', () => { - const { program, type, checker } = getTypes( - 'const test = Promise.resolve(1)', - ); - expectTypesAre(getAwaitedType(program, type), checker, 'number'); + const { type, checker, node } = getTypes('const test = Promise.resolve(1)'); + expectTypesAre(getAwaitedType(checker, type, node), checker, 'number'); }); it('Promise> to {{type}}', () => { - const { program, type, checker } = getTypes( + const { type, checker, node } = getTypes( 'const test: Promise> = Promise.resolve(Promise.resolve(1))', ); - expectTypesAre(getAwaitedType(program, type), checker, 'number'); + expectTypesAre(getAwaitedType(checker, type, node), checker, 'number'); }); }); From 7f61819fba7b03ec0e4dce956a0826700751bc69 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Wed, 10 Jul 2024 23:58:07 +0900 Subject: [PATCH 12/18] update docs --- .../eslint-plugin/docs/rules/no-unsafe-return.mdx | 14 +++++++++++++- .../no-unsafe-return.shot | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx b/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx index c5ca28700e6a..491bfb6bdcfc 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx +++ b/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx @@ -15,7 +15,7 @@ Using `any` disables many type checking rules and is generally best used only as Despite your best intentions, the `any` type can sometimes leak into your codebase. Returning an an `any`-typed value from a function creates a potential type safety hole and source of bugs in your codebase. -This rule disallows returning `any` or `any[]` from a function. +This rule disallows returning `any`, `any[]` or `Promise` from a function. This rule also compares generic type argument types to ensure you don't return an unsafe `any` in a generic position to a function that's expecting a specific type. For example, it will error if you return `Set` from a function declared as returning `Set`. @@ -56,6 +56,14 @@ const foo10 = () => [] as any[]; const foo11 = (): string[] => [1, 2, 3] as any[]; +function foo12(): Promise { + return Promise.resolve({}); +} + +async function foo13() { + return Promise.resolve({} as any); +} + // generic position examples function assignability1(): Set { return new Set([1]); @@ -78,6 +86,10 @@ function foo2() { const foo3 = () => []; const foo4 = () => ['a']; +async function foo5() { + return Promise.resolve(1); +} + function assignability1(): Set { return new Set(['foo']); } diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot index e645fdcbb568..8efa006f8597 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot @@ -44,6 +44,16 @@ const foo10 = () => [] as any[]; const foo11 = (): string[] => [1, 2, 3] as any[]; ~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. +function foo12(): Promise { + return Promise.resolve({}); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`Promise\` typed value. +} + +async function foo13() { + return Promise.resolve({} as any); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`Promise\` typed value. +} + // generic position examples function assignability1(): Set { return new Set([1]); @@ -68,6 +78,10 @@ function foo2() { const foo3 = () => []; const foo4 = () => ['a']; +async function foo5() { + return Promise.resolve(1); +} + function assignability1(): Set { return new Set(['foo']); } From 74e16c424b8078063210fb5cbcd71bbb3456b837 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Tue, 16 Jul 2024 01:19:42 +0900 Subject: [PATCH 13/18] apply review --- .../src/rules/no-unsafe-return.ts | 17 +++++- .../tests/rules/no-unsafe-return.test.ts | 59 +++++++++++++++++++ packages/type-utils/src/getAwaitedType.ts | 10 +++- packages/type-utils/src/predicates.ts | 23 +++++--- 4 files changed, 96 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index cb09c9f058c8..56d50bbe7157 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -81,7 +81,12 @@ export default createRule({ ): void { const tsNode = services.esTreeNodeToTSNodeMap.get(returnNode); const type = checker.getTypeAtLocation(tsNode); - const anyType = discriminateAnyType(type, checker, services.program); + const anyType = discriminateAnyType( + type, + checker, + services.program, + tsNode, + ); const functionNode = getParentFunctionNode(returnNode); /* istanbul ignore if */ if (!functionNode) { return; @@ -122,11 +127,13 @@ export default createRule({ } if (functionNode.async) { const awaitedSignatureReturnType = getAwaitedType( + services.program, checker, signatureReturnType, functionTSNode, ); const awaitedReturnNodeType = getAwaitedType( + services.program, checker, returnNodeType, returnTSNode, @@ -161,10 +168,16 @@ export default createRule({ ) { return; } + if ( anyType === AnyType.PromiseAny && isTypeUnknownType( - getAwaitedType(checker, functionReturnType, returnTSNode), + getAwaitedType( + services.program, + checker, + functionReturnType, + returnTSNode, + ), ) ) { return; diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index 165aa677e094..49a4d68eeec4 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -152,6 +152,14 @@ function foo(): Set { return [] as any[]; } `, + ` + class PromiseLike extends Promise {} + async function foo() { + return new PromiseLike(resolve => { + resolve(42); + }); + } + `, ], invalid: [ { @@ -661,6 +669,57 @@ async function foo(): Promise { code: ` async function foo() { return {} as Promise>>>; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: '`Promise`', + }, + }, + ], + }, + { + code: ` +async function foo() { + return {} as Promise | Promise; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: '`Promise`', + }, + }, + ], + }, + { + code: ` +async function foo() { + return {} as Promise; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: '`Promise`', + }, + }, + ], + }, + { + code: ` +async function foo() { + return {} as Promise & { __brand: 'any' }; } `, errors: [ diff --git a/packages/type-utils/src/getAwaitedType.ts b/packages/type-utils/src/getAwaitedType.ts index d74260b018fa..e01dd14cfd1f 100644 --- a/packages/type-utils/src/getAwaitedType.ts +++ b/packages/type-utils/src/getAwaitedType.ts @@ -1,18 +1,24 @@ import * as tsutils from 'ts-api-utils'; import type * as ts from 'typescript'; +import { isSymbolFromDefaultLibrary } from './isSymbolFromDefaultLibrary'; + export function getAwaitedType( + program: ts.Program, checker: ts.TypeChecker, type: ts.Type, node: ts.Node, ): ts.Type { + const symbol = type.getSymbol(); if ( tsutils.isThenableType(checker, node, type) && - tsutils.isTypeReference(type) + tsutils.isTypeReference(type) && + isSymbolFromDefaultLibrary(program, symbol) && + symbol?.getName() === 'Promise' ) { const awaitedType = type.typeArguments?.[0]; if (awaitedType) { - return getAwaitedType(checker, awaitedType, node); + return getAwaitedType(program, checker, awaitedType, node); } } return type; diff --git a/packages/type-utils/src/predicates.ts b/packages/type-utils/src/predicates.ts index 7fed3b664471..3dccc6ceec21 100644 --- a/packages/type-utils/src/predicates.ts +++ b/packages/type-utils/src/predicates.ts @@ -2,7 +2,7 @@ import debug from 'debug'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; -import { isPromiseLike } from './builtinSymbolLikes'; +import { getAwaitedType } from './getAwaitedType'; import { isTypeFlagSet } from './typeFlagUtils'; const log = debug('typescript-eslint:eslint-plugin:utils:types'); @@ -136,6 +136,7 @@ export function discriminateAnyType( type: ts.Type, checker: ts.TypeChecker, program: ts.Program, + tsNode: ts.Node, ): AnyType { if (isTypeAnyType(type)) { return AnyType.Any; @@ -143,18 +144,22 @@ export function discriminateAnyType( if (isTypeAnyArrayType(type, checker)) { return AnyType.AnyArray; } - if (isPromiseLike(program, type) && tsutils.isTypeReference(type)) { - const awaitedType = type.typeArguments?.[0]; - if (awaitedType) { - const awaitedAnyType = discriminateAnyType(awaitedType, checker, program); - if ( - awaitedAnyType === AnyType.Any || - awaitedAnyType === AnyType.PromiseAny - ) { + + for (const part of tsutils.typeParts(type)) { + const awaitedType = getAwaitedType(program, checker, part, tsNode); + if (awaitedType !== type) { + const awaitedAnyType = discriminateAnyType( + awaitedType, + checker, + program, + tsNode, + ); + if (awaitedAnyType === AnyType.Any) { return AnyType.PromiseAny; } } } + return AnyType.Safe; } From 8e5a19b69cc62a80e1c9e7fbd4f6706f04e808ba Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Tue, 16 Jul 2024 01:33:00 +0900 Subject: [PATCH 14/18] fix tests --- .../type-utils/tests/getAwaitedType.test.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/type-utils/tests/getAwaitedType.test.ts b/packages/type-utils/tests/getAwaitedType.test.ts index ebe31017333c..9bc93ce844cc 100644 --- a/packages/type-utils/tests/getAwaitedType.test.ts +++ b/packages/type-utils/tests/getAwaitedType.test.ts @@ -48,19 +48,33 @@ describe('getAwaitedType', () => { } it('non-promise', () => { - const { type, checker, node } = getTypes('const test: number = 1'); - expectTypesAre(getAwaitedType(checker, type, node), checker, 'number'); + const { program, type, checker, node } = getTypes('const test: number = 1'); + expectTypesAre( + getAwaitedType(program, checker, type, node), + checker, + 'number', + ); }); it('Promise<{{type}}> to {{type}}', () => { - const { type, checker, node } = getTypes('const test = Promise.resolve(1)'); - expectTypesAre(getAwaitedType(checker, type, node), checker, 'number'); + const { program, type, checker, node } = getTypes( + 'const test = Promise.resolve(1)', + ); + expectTypesAre( + getAwaitedType(program, checker, type, node), + checker, + 'number', + ); }); it('Promise> to {{type}}', () => { - const { type, checker, node } = getTypes( + const { program, type, checker, node } = getTypes( 'const test: Promise> = Promise.resolve(Promise.resolve(1))', ); - expectTypesAre(getAwaitedType(checker, type, node), checker, 'number'); + expectTypesAre( + getAwaitedType(program, checker, type, node), + checker, + 'number', + ); }); }); From 0d0a86a2c63de8ef17ae17dc4f9adb4d0362fb8b Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Thu, 18 Jul 2024 01:02:43 +0900 Subject: [PATCH 15/18] change to use internal getAwaitedType --- .../src/rules/no-unsafe-return.ts | 41 ++++------ .../tests/rules/no-unsafe-return.test.ts | 30 +++++-- .../eslint-plugin/typings/typescript.d.ts | 4 + packages/type-utils/src/getAwaitedType.ts | 25 ------ packages/type-utils/src/index.ts | 1 - packages/type-utils/src/predicates.ts | 24 +++--- .../type-utils/tests/getAwaitedType.test.ts | 80 ------------------- packages/type-utils/typings/typescript.d.ts | 4 + 8 files changed, 56 insertions(+), 153 deletions(-) delete mode 100644 packages/type-utils/src/getAwaitedType.ts delete mode 100644 packages/type-utils/tests/getAwaitedType.test.ts diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index 56d50bbe7157..c9483bc50b83 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -7,7 +7,6 @@ import { AnyType, createRule, discriminateAnyType, - getAwaitedType, getConstrainedTypeAtLocation, getContextualType, getParserServices, @@ -81,6 +80,7 @@ export default createRule({ ): void { const tsNode = services.esTreeNodeToTSNodeMap.get(returnNode); const type = checker.getTypeAtLocation(tsNode); + const anyType = discriminateAnyType( type, checker, @@ -95,7 +95,6 @@ export default createRule({ // function has an explicit return type, so ensure it's a safe return const returnNodeType = getConstrainedTypeAtLocation(services, returnNode); const functionTSNode = services.esTreeNodeToTSNodeMap.get(functionNode); - const returnTSNode = services.esTreeNodeToTSNodeMap.get(returnNode); // function expressions will not have their return type modified based on receiver typing // so we have to use the contextual typing in these cases, i.e. @@ -126,24 +125,18 @@ export default createRule({ return; } if (functionNode.async) { - const awaitedSignatureReturnType = getAwaitedType( - services.program, - checker, - signatureReturnType, - functionTSNode, - ); - const awaitedReturnNodeType = getAwaitedType( - services.program, - checker, - returnNodeType, - returnTSNode, - ); + const awaitedSignatureReturnType = + checker.getAwaitedType(signatureReturnType); + + const awaitedReturnNodeType = + checker.getAwaitedType(returnNodeType); if ( awaitedReturnNodeType === awaitedSignatureReturnType || - isTypeFlagSet( - awaitedSignatureReturnType, - ts.TypeFlags.Any | ts.TypeFlags.Unknown, - ) + (awaitedSignatureReturnType && + isTypeFlagSet( + awaitedSignatureReturnType, + ts.TypeFlags.Any | ts.TypeFlags.Unknown, + )) ) { return; } @@ -168,17 +161,11 @@ export default createRule({ ) { return; } - + const awaitedType = checker.getAwaitedType(functionReturnType); if ( + awaitedType && anyType === AnyType.PromiseAny && - isTypeUnknownType( - getAwaitedType( - services.program, - checker, - functionReturnType, - returnTSNode, - ), - ) + isTypeUnknownType(awaitedType) ) { return; } diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index 49a4d68eeec4..ab7990c2819b 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -152,14 +152,6 @@ function foo(): Set { return [] as any[]; } `, - ` - class PromiseLike extends Promise {} - async function foo() { - return new PromiseLike(resolve => { - resolve(42); - }); - } - `, ], invalid: [ { @@ -733,5 +725,27 @@ async function foo() { }, ], }, + { + code: ` +interface Alias extends Promise { + foo: 'bar'; +} + +declare const value: Alias; +function foo() { + return value; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 8, + column: 3, + data: { + type: '`Promise`', + }, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index e08a8fb42267..c43c29934030 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -20,5 +20,9 @@ declare module 'typescript' { * - `readonly [foo]` */ isTupleType(type: Type): type is TupleTypeReference; + /** + * Return the awaited type of the given type. + */ + getAwaitedType(type: Type): Type | undefined; } } diff --git a/packages/type-utils/src/getAwaitedType.ts b/packages/type-utils/src/getAwaitedType.ts deleted file mode 100644 index e01dd14cfd1f..000000000000 --- a/packages/type-utils/src/getAwaitedType.ts +++ /dev/null @@ -1,25 +0,0 @@ -import * as tsutils from 'ts-api-utils'; -import type * as ts from 'typescript'; - -import { isSymbolFromDefaultLibrary } from './isSymbolFromDefaultLibrary'; - -export function getAwaitedType( - program: ts.Program, - checker: ts.TypeChecker, - type: ts.Type, - node: ts.Node, -): ts.Type { - const symbol = type.getSymbol(); - if ( - tsutils.isThenableType(checker, node, type) && - tsutils.isTypeReference(type) && - isSymbolFromDefaultLibrary(program, symbol) && - symbol?.getName() === 'Promise' - ) { - const awaitedType = type.typeArguments?.[0]; - if (awaitedType) { - return getAwaitedType(program, checker, awaitedType, node); - } - } - return type; -} diff --git a/packages/type-utils/src/index.ts b/packages/type-utils/src/index.ts index 847def8433af..14d5b652099f 100644 --- a/packages/type-utils/src/index.ts +++ b/packages/type-utils/src/index.ts @@ -15,7 +15,6 @@ export * from './propertyTypes'; export * from './requiresQuoting'; export * from './TypeOrValueSpecifier'; export * from './typeFlagUtils'; -export * from './getAwaitedType'; export { getDecorators, getModifiers, diff --git a/packages/type-utils/src/predicates.ts b/packages/type-utils/src/predicates.ts index 3dccc6ceec21..83ef77f4c7e1 100644 --- a/packages/type-utils/src/predicates.ts +++ b/packages/type-utils/src/predicates.ts @@ -2,7 +2,6 @@ import debug from 'debug'; import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; -import { getAwaitedType } from './getAwaitedType'; import { isTypeFlagSet } from './typeFlagUtils'; const log = debug('typescript-eslint:eslint-plugin:utils:types'); @@ -144,18 +143,19 @@ export function discriminateAnyType( if (isTypeAnyArrayType(type, checker)) { return AnyType.AnyArray; } - for (const part of tsutils.typeParts(type)) { - const awaitedType = getAwaitedType(program, checker, part, tsNode); - if (awaitedType !== type) { - const awaitedAnyType = discriminateAnyType( - awaitedType, - checker, - program, - tsNode, - ); - if (awaitedAnyType === AnyType.Any) { - return AnyType.PromiseAny; + if (tsutils.isThenableType(checker, tsNode, type)) { + const awaitedType = checker.getAwaitedType(part); + if (awaitedType) { + const awaitedAnyType = discriminateAnyType( + awaitedType, + checker, + program, + tsNode, + ); + if (awaitedAnyType === AnyType.Any) { + return AnyType.PromiseAny; + } } } } diff --git a/packages/type-utils/tests/getAwaitedType.test.ts b/packages/type-utils/tests/getAwaitedType.test.ts deleted file mode 100644 index 9bc93ce844cc..000000000000 --- a/packages/type-utils/tests/getAwaitedType.test.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { parseForESLint } from '@typescript-eslint/parser'; -import type { TSESTree } from '@typescript-eslint/utils'; -import path from 'path'; -import type * as ts from 'typescript'; - -import { getAwaitedType } from '../src/getAwaitedType'; -import { expectToHaveParserServices } from './test-utils/expectToHaveParserServices'; - -describe('getAwaitedType', () => { - const rootDir = path.join(__dirname, 'fixtures'); - - function getTypes( - code: string, - declarationIndex = 0, - ): { - program: ts.Program; - type: ts.Type; - checker: ts.TypeChecker; - node: ts.Node; - } { - const { ast, services } = parseForESLint(code, { - project: './tsconfig.json', - filePath: path.join(rootDir, 'file.ts'), - tsconfigRootDir: rootDir, - }); - expectToHaveParserServices(services); - const checker = services.program.getTypeChecker(); - - const declaration = ast.body[ - declarationIndex - ] as TSESTree.VariableDeclaration; - const declarator = declaration.declarations[0]; - return { - program: services.program, - type: services.getTypeAtLocation(declarator.id), - node: services.esTreeNodeToTSNodeMap.get(declarator.id), - checker, - }; - } - - function expectTypesAre( - result: ts.Type, - checker: ts.TypeChecker, - typeStr: string, - ): void { - expect(result).toBeTruthy(); - expect(checker.typeToString(result)).toBe(typeStr); - } - - it('non-promise', () => { - const { program, type, checker, node } = getTypes('const test: number = 1'); - expectTypesAre( - getAwaitedType(program, checker, type, node), - checker, - 'number', - ); - }); - - it('Promise<{{type}}> to {{type}}', () => { - const { program, type, checker, node } = getTypes( - 'const test = Promise.resolve(1)', - ); - expectTypesAre( - getAwaitedType(program, checker, type, node), - checker, - 'number', - ); - }); - - it('Promise> to {{type}}', () => { - const { program, type, checker, node } = getTypes( - 'const test: Promise> = Promise.resolve(Promise.resolve(1))', - ); - expectTypesAre( - getAwaitedType(program, checker, type, node), - checker, - 'number', - ); - }); -}); diff --git a/packages/type-utils/typings/typescript.d.ts b/packages/type-utils/typings/typescript.d.ts index 335b12b86fe5..9cbf6de460b9 100644 --- a/packages/type-utils/typings/typescript.d.ts +++ b/packages/type-utils/typings/typescript.d.ts @@ -22,6 +22,10 @@ declare module 'typescript' { * Return the type of the given property in the given type, or undefined if no such property exists */ getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined; + /** + * Return the awaited type of the given type. + */ + getAwaitedType(type: Type): Type | undefined; } interface Type { From fcca5ecc161696a97ee207db4839056e46c438eb Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Thu, 18 Jul 2024 11:54:24 +0900 Subject: [PATCH 16/18] add todo comment --- packages/eslint-plugin/typings/typescript.d.ts | 3 +++ packages/type-utils/typings/typescript.d.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index c43c29934030..3579a04dc205 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -22,6 +22,9 @@ declare module 'typescript' { isTupleType(type: Type): type is TupleTypeReference; /** * Return the awaited type of the given type. + * + * TODO: Remove when it's exposed as a public API. + * https://github.com/microsoft/TypeScript/issues/59256 */ getAwaitedType(type: Type): Type | undefined; } diff --git a/packages/type-utils/typings/typescript.d.ts b/packages/type-utils/typings/typescript.d.ts index 9cbf6de460b9..77df1f1fe5e9 100644 --- a/packages/type-utils/typings/typescript.d.ts +++ b/packages/type-utils/typings/typescript.d.ts @@ -24,6 +24,9 @@ declare module 'typescript' { getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined; /** * Return the awaited type of the given type. + * + * TODO: Remove when it's exposed as a public API. + * https://github.com/microsoft/TypeScript/issues/59256 */ getAwaitedType(type: Type): Type | undefined; } From 2542bd510cba4e6e29d658786fbff38a7d6694e7 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Sat, 20 Jul 2024 23:07:24 +0900 Subject: [PATCH 17/18] Update packages/eslint-plugin/docs/rules/no-unsafe-return.mdx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Josh Goldberg ✨ --- packages/eslint-plugin/docs/rules/no-unsafe-return.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx b/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx index 491bfb6bdcfc..814f5e4556ef 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx +++ b/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx @@ -15,7 +15,7 @@ Using `any` disables many type checking rules and is generally best used only as Despite your best intentions, the `any` type can sometimes leak into your codebase. Returning an an `any`-typed value from a function creates a potential type safety hole and source of bugs in your codebase. -This rule disallows returning `any`, `any[]` or `Promise` from a function. +This rule disallows returning `any`, `any[]`, or `Promise` from a function. This rule also compares generic type argument types to ensure you don't return an unsafe `any` in a generic position to a function that's expecting a specific type. For example, it will error if you return `Set` from a function declared as returning `Set`. From ccc2e97d454436e7c0cf7d687271cde0b5bbdb73 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Tue, 23 Jul 2024 00:15:26 +0900 Subject: [PATCH 18/18] apply reviews --- .../docs/rules/no-unsafe-return.mdx | 6 +-- .../src/rules/no-unsafe-return.ts | 11 +---- .../no-unsafe-return.shot | 29 +++++------- .../tests/rules/no-unsafe-return.test.ts | 45 ++++--------------- 4 files changed, 23 insertions(+), 68 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx b/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx index 814f5e4556ef..1b6170dcf887 100644 --- a/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx +++ b/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx @@ -15,7 +15,7 @@ Using `any` disables many type checking rules and is generally best used only as Despite your best intentions, the `any` type can sometimes leak into your codebase. Returning an an `any`-typed value from a function creates a potential type safety hole and source of bugs in your codebase. -This rule disallows returning `any`, `any[]`, or `Promise` from a function. +This rule disallows returning `any` or `any[]` from a function and returning `Promise` from an async function. This rule also compares generic type argument types to ensure you don't return an unsafe `any` in a generic position to a function that's expecting a specific type. For example, it will error if you return `Set` from a function declared as returning `Set`. @@ -56,10 +56,6 @@ const foo10 = () => [] as any[]; const foo11 = (): string[] => [1, 2, 3] as any[]; -function foo12(): Promise { - return Promise.resolve({}); -} - async function foo13() { return Promise.resolve({} as any); } diff --git a/packages/eslint-plugin/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index c9483bc50b83..e70fcf23963c 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -11,7 +11,6 @@ import { getContextualType, getParserServices, getThisExpression, - isPromiseLike, isTypeAnyType, isTypeFlagSet, isTypeUnknownArrayType, @@ -29,7 +28,7 @@ export default createRule({ requiresTypeChecking: true, }, messages: { - unsafeReturn: 'Unsafe return of an {{type}} typed value.', + unsafeReturn: 'Unsafe return of a value of type {{type}}.', unsafeReturnThis: [ 'Unsafe return of a value of type `{{type}}`. `this` is typed as `any`.', 'You can try to fix this by turning on the `noImplicitThis` compiler option, or adding a `this` parameter to the function.', @@ -171,13 +170,7 @@ export default createRule({ } } - if ( - anyType === AnyType.PromiseAny && - callSignatures.every( - signature => - !isPromiseLike(services.program, signature.getReturnType()), - ) - ) { + if (anyType === AnyType.PromiseAny && !functionNode.async) { return; } diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot index 8efa006f8597..c8651c683547 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unsafe-return.shot @@ -5,53 +5,48 @@ exports[`Validating rule docs no-unsafe-return.mdx code examples ESLint output 1 function foo1() { return 1 as any; - ~~~~~~~~~~~~~~~~ Unsafe return of an \`any\` typed value. + ~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any\`. } function foo2() { return Object.create(null); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any\` typed value. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any\`. } const foo3 = () => { return 1 as any; - ~~~~~~~~~~~~~~~~ Unsafe return of an \`any\` typed value. + ~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any\`. }; const foo4 = () => Object.create(null); - ~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any\` typed value. + ~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any\`. function foo5() { return [] as any[]; - ~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. + ~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. } function foo6() { return [] as Array; - ~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. + ~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. } function foo7() { return [] as readonly any[]; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. } function foo8() { return [] as Readonly; - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. } const foo9 = () => { return [] as any[]; - ~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. + ~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. }; const foo10 = () => [] as any[]; - ~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. + ~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. const foo11 = (): string[] => [1, 2, 3] as any[]; - ~~~~~~~~~~~~~~~~~~ Unsafe return of an \`any[]\` typed value. - -function foo12(): Promise { - return Promise.resolve({}); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`Promise\` typed value. -} + ~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. async function foo13() { return Promise.resolve({} as any); - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of an \`Promise\` typed value. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`Promise\`. } // generic position examples diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index ab7990c2819b..d3f9797d8ee1 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -152,6 +152,12 @@ function foo(): Set { return [] as any[]; } `, + ` + declare const value: Promise; + function foo() { + return value; + } + `, ], invalid: [ { @@ -505,24 +511,6 @@ async function foo() { { code: ` declare const value: Promise; -function foo() { - return value; -} - `, - errors: [ - { - messageId: 'unsafeReturn', - line: 4, - column: 3, - data: { - type: '`Promise`', - }, - }, - ], - }, - { - code: ` -declare const value: Promise; async function foo(): Promise { return value; } @@ -591,7 +579,7 @@ function foo(): Promise { }, { code: ` -function foo(): Promise { +async function foo(): Promise { return Promise.resolve({}); } `, @@ -608,23 +596,6 @@ function foo(): Promise { }, { code: ` -function foo(): Promise { - return Promise.resolve>>({} as any); -} - `, - errors: [ - { - messageId: 'unsafeReturn', - line: 3, - column: 3, - data: { - type: '`Promise`', - }, - }, - ], - }, - { - code: ` async function foo(): Promise { return Promise.resolve>>({} as Promise); } @@ -732,7 +703,7 @@ interface Alias extends Promise { } declare const value: Alias; -function foo() { +async function foo() { return value; } `,