diff --git a/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx b/packages/eslint-plugin/docs/rules/no-unsafe-return.mdx index c5ca28700e6a..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` or `any[]` 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,6 +56,10 @@ const foo10 = () => [] as any[]; const foo11 = (): string[] => [1, 2, 3] as any[]; +async function foo13() { + return Promise.resolve({} as any); +} + // generic position examples function assignability1(): Set { return new Set([1]); @@ -78,6 +82,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/src/rules/no-unsafe-return.ts b/packages/eslint-plugin/src/rules/no-unsafe-return.ts index d9c7cd60cc0f..1b89ffbf8568 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-return.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-return.ts @@ -6,11 +6,11 @@ import * as ts from 'typescript'; import { AnyType, createRule, + discriminateAnyType, getConstrainedTypeAtLocation, getContextualType, getParserServices, getThisExpression, - isAnyOrAnyArrayTypeDiscriminated, isTypeAnyType, isTypeFlagSet, isTypeUnknownArrayType, @@ -28,9 +28,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: @@ -78,7 +78,14 @@ export default createRule({ reportingNode: TSESTree.Node = returnNode, ): void { const tsNode = services.esTreeNodeToTSNodeMap.get(returnNode); - const anyType = isAnyOrAnyArrayTypeDiscriminated(tsNode, checker); + const type = checker.getTypeAtLocation(tsNode); + + const anyType = discriminateAnyType( + type, + checker, + services.program, + tsNode, + ); const functionNode = getParentFunctionNode(returnNode); /* istanbul ignore if */ if (!functionNode) { return; @@ -100,27 +107,46 @@ 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 ( - returnNodeType === signature.getReturnType() || + returnNodeType === signatureReturnType || isTypeFlagSet( - signature.getReturnType(), + signatureReturnType, ts.TypeFlags.Any | ts.TypeFlags.Unknown, ) ) { return; } + if (functionNode.async) { + const awaitedSignatureReturnType = + checker.getAwaitedType(signatureReturnType); + + const awaitedReturnNodeType = + checker.getAwaitedType(returnNodeType); + if ( + awaitedReturnNodeType === awaitedSignatureReturnType || + (awaitedSignatureReturnType && + isTypeFlagSet( + awaitedSignatureReturnType, + ts.TypeFlags.Any | ts.TypeFlags.Unknown, + )) + ) { + return; + } + } } } 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 && @@ -134,6 +160,18 @@ export default createRule({ ) { return; } + const awaitedType = checker.getAwaitedType(functionReturnType); + if ( + awaitedType && + anyType === AnyType.PromiseAny && + isTypeUnknownType(awaitedType) + ) { + return; + } + } + + if (anyType === AnyType.PromiseAny && !functionNode.async) { + return; } let messageId: 'unsafeReturn' | 'unsafeReturnThis' = 'unsafeReturn'; @@ -161,7 +199,9 @@ export default createRule({ ? 'error' : anyType === AnyType.Any ? '`any`' - : '`any[]`', + : anyType === AnyType.PromiseAny + ? '`Promise`' + : '`any[]`', }, }); } 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..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,44 +5,49 @@ 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. + ~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`any[]\`. + +async function foo13() { + return Promise.resolve({} as any); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Unsafe return of a value of type \`Promise\`. +} // generic position examples function assignability1(): Set { @@ -68,6 +73,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/rules/no-unsafe-return.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts index c30c6a2b0bf1..f6343404e743 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-return.test.ts @@ -64,6 +64,21 @@ function foo(): any[] { ` function foo(): Set { return new Set(); +} + `, + ` +async function foo(): Promise { + return Promise.resolve({} as any); +} + `, + ` +async function foo(): Promise { + return {} as any; +} + `, + ` +function foo(): object { + return Promise.resolve({} as any); } `, // TODO - this should error, but it's hard to detect, as the type references are different @@ -110,6 +125,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 { @@ -127,6 +152,12 @@ function foo(): Set { return [] as any[]; } `, + ` + declare const value: Promise; + function foo() { + return value; + } + `, 'const foo: (() => void) | undefined = () => 1;', ], invalid: [ @@ -460,5 +491,233 @@ function example() { }, ], }, + { + code: ` +declare const value: any; +async function foo() { + return value; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 4, + column: 3, + data: { + type: '`any`', + }, + }, + ], + }, + { + code: ` +declare const value: Promise; +async function foo(): Promise { + return value; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 4, + column: 3, + data: { + type: '`Promise`', + }, + }, + ], + }, + { + code: ` +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, + data: { + type: '`any`', + }, + }, + ], + }, + { + code: ` +function foo(): Promise { + return {} as any; +} + `, + errors: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: '`any`', + }, + }, + ], + }, + { + code: ` +async function foo(): Promise { + return Promise.resolve({}); +} + `, + 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`', + }, + }, + ], + }, + { + 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: [ + { + messageId: 'unsafeReturn', + line: 3, + column: 3, + data: { + type: '`Promise`', + }, + }, + ], + }, + { + code: ` +interface Alias extends Promise { + foo: 'bar'; +} + +declare const value: Alias; +async 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..3579a04dc205 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -20,5 +20,12 @@ declare module 'typescript' { * - `readonly [foo]` */ 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/src/predicates.ts b/packages/type-utils/src/predicates.ts index 26ef59355781..83ef77f4c7e1 100644 --- a/packages/type-utils/src/predicates.ts +++ b/packages/type-utils/src/predicates.ts @@ -123,24 +123,43 @@ export function isTypeUnknownArrayType( export enum AnyType { Any, + PromiseAny, AnyArray, 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 isAnyOrAnyArrayTypeDiscriminated( - node: ts.Node, +export function discriminateAnyType( + type: ts.Type, checker: ts.TypeChecker, + program: ts.Program, + tsNode: ts.Node, ): AnyType { - const type = checker.getTypeAtLocation(node); if (isTypeAnyType(type)) { return AnyType.Any; } if (isTypeAnyArrayType(type, checker)) { return AnyType.AnyArray; } + for (const part of tsutils.typeParts(type)) { + 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; + } + } + } + } + return AnyType.Safe; } diff --git a/packages/type-utils/typings/typescript.d.ts b/packages/type-utils/typings/typescript.d.ts index 335b12b86fe5..77df1f1fe5e9 100644 --- a/packages/type-utils/typings/typescript.d.ts +++ b/packages/type-utils/typings/typescript.d.ts @@ -22,6 +22,13 @@ 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. + * + * TODO: Remove when it's exposed as a public API. + * https://github.com/microsoft/TypeScript/issues/59256 + */ + getAwaitedType(type: Type): Type | undefined; } interface Type {