feat(eslint-plugin): [no-unsafe-return] check promise any (#8693)

* feat(eslint-plugin): [no-unsafe-return] check promise any * add testcases * apply reviews * fix lint errors * refactor * add type * improve messages * update comment * apply review * fix * fix tests * update docs * apply review * fix tests * change to use internal getAwaitedType * add todo comment * Update packages/eslint-plugin/docs/rules/no-unsafe-return.mdx Co-authored-by: Josh Goldberg ✨ <[email protected]> * apply reviews --------- Co-authored-by: Josh Goldberg ✨ <[email protected]>
typescript-eslint · Aug 12, 2024 · c20bd2f · c20bd2f
1 parent 3d9ae44
commit c20bd2f
Show file tree

Hide file tree

Showing 7 changed files with 375 additions and 26 deletions.
diff --git 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<any>` 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<any>` from a function declared as returning `Set<string>`.
@@ -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<string> {
   return new Set<any>([1]);
@@ -78,6 +82,10 @@ function foo2() {
 const foo3 = () => [];
 const foo4 = () => ['a'];
 
+async function foo5() {
+  return Promise.resolve(1);
+}
+
 function assignability1(): Set<string> {
   return new Set<string>(['foo']);
 }

diff --git 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>`'
+                  : '`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