Expand the typechecking of async function declared/inferred return ty…

…pes to allow union types; synthesize a more accurate upper bound for return expression types.

This change is motivated by the case where an async function is inferred to have a union return type (as can happen when one is passed as a callback to some other function that made the type declarations). Since this happens relatively often, and is safe, it was decided to improve checking to support it.

As as consequence, it was also necessary to improve the way we synthesize the return expression type bounds to be more accurately handle "synchronous" types; they can never actually returned. They are now illegal.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=220381088

src/com/google/javascript/jscomp/Promises.java

@@ -16,10 +16,13 @@
 
 package com.google.javascript.jscomp;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 
+import com.google.common.collect.ImmutableList;
 import com.google.javascript.rhino.jstype.JSType;
 import com.google.javascript.rhino.jstype.JSTypeNative;
 import com.google.javascript.rhino.jstype.JSTypeRegistry;
+import com.google.javascript.rhino.jstype.ObjectType;
 import com.google.javascript.rhino.jstype.TemplateTypeMap;
 import com.google.javascript.rhino.jstype.UnionTypeBuilder;
 
@@ -100,33 +103,44 @@ static final JSType getResolvedType(JSTypeRegistry registry, JSType type) {
 
   /**
    * Synthesizes a type representing the legal types of a return expression within async code
-   * (i.e.`Promise` callbacks, async functions).
+   * (i.e.`Promise` callbacks, async functions) based on the expected return type of that code.
    *
-   * <p>The return type will generally be a union but may not be, for example:
+   * <p>The return type will generally be a union but may not be in the case of top-like types. If
+   * the expected return type is a union, any synchronous elements will be dropped, since they can
+   * never occur. For example:
    *
    * <ul>
    *   <li>`!Promise<number>` => `number|!IThenable<number>`
-   *   <li>`number` => `number|!IThenable<number>`
+   *   <li>`number` => `?`
+   *   <li>`number|!Promise<string>` => `string|!IThenable<string>`
+   *   <li>`!IThenable<number>|!Promise<string>` => `number|string|!IThenable<number|string>`
+   *   <li>`!IThenable<number|string>` => `number|string|!IThenable<number|string>`
    *   <li>`?` => `?`
-   *   <li>`*` => `*`
-   *   <li>`number|!Foo` => `number|!Foo|!IThenable<number|!Foo>`
-   *   <li>`!Foo|!IThenable<!Foo>` => `Foo|!IThenable<!Foo>`
-   *   <li>`!Promise<!IThenable<!Foo>>` => `!Foo|!IThenable<!Foo>`
-   * </ul>
-   *
-   * Note that this method may create an incorrect (but not really dangerous) type when supplied
-   * with types that are nonsensical in an async context, for example:
-   *
-   * <ul>
-   *   <li>`number|!IThenable<string>` => `number|string|!IThenable<number|string>`
-   *   <li>`?IThenable<!Foo>` => `null|!Foo|!Thenable<?Foo>`
+   *   <li>`*` => `?`
    * </ul>
    */
   static final JSType createAsyncReturnableType(JSTypeRegistry registry, JSType maybeThenable) {
-    JSType parameterType = getResolvedType(registry, maybeThenable);
+    JSType unknownType = registry.getNativeType(JSTypeNative.UNKNOWN_TYPE);
+    ObjectType iThenableType = registry.getNativeObjectType(JSTypeNative.I_THENABLE_TYPE);
+
+    JSType iThenableOfUnknownType = registry.createTemplatizedType(iThenableType, unknownType);
+
+    ImmutableList<JSType> alternates =
+        maybeThenable.isUnionType()
+            ? maybeThenable.toMaybeUnionType().getAlternates()
+            : ImmutableList.of(maybeThenable);
+    ImmutableList<JSType> asyncTemplateAlternates =
+        alternates.stream()
+            .filter((t) -> t.isSubtypeOf(iThenableOfUnknownType)) // Discard "synchronous" types.
+            .map((t) -> getTemplateTypeOfThenable(registry, t)) // Unwrap "asynchronous" types.
+            .collect(toImmutableList());
+
+    if (asyncTemplateAlternates.isEmpty()) {
+      return unknownType;
+    }
+
+    JSType asyncTemplateUnion = registry.createUnionType(asyncTemplateAlternates);
     return registry.createUnionType(
-        parameterType,
-        registry.createTemplatizedType(
-            registry.getNativeObjectType(JSTypeNative.I_THENABLE_TYPE), parameterType));
+        asyncTemplateUnion, registry.createTemplatizedType(iThenableType, asyncTemplateUnion));
   }
 }

src/com/google/javascript/jscomp/TypeValidator.java

@@ -114,8 +114,7 @@ class TypeValidator implements Serializable {
   static final DiagnosticType INVALID_ASYNC_RETURN_TYPE =
       DiagnosticType.warning(
           "JSC_INVALID_ASYNC_RETURN_TYPE",
-          "The return type of an async function must be a non-union supertype of Promise\n"
-              + "found: {0}");
+          "The return type of an async function must be a supertype of Promise\n" + "found: {0}");
 
   static final DiagnosticType INVALID_OPERAND_TYPE =
       DiagnosticType.disabled("JSC_INVALID_OPERAND_TYPE", "{0}");
@@ -312,25 +311,18 @@ void expectGeneratorSupertype(NodeTraversal t, Node n, JSType type, String msg)
   }
 
   /**
-   * Expect the type to be a supertype of Promise.
+   * Expect the type to be a supertype of `Promise`.
    *
    * <p>`Promise` is the <em>lower</em> bound of the declared return type, since that's what async
    * functions always return; the user can't return an instance of a more specific type.
-   *
-   * <p>We forbid returning a union type because it complicates how we typecheck returns within
-   * async functions.
    */
   void expectValidAsyncReturnType(NodeTraversal t, Node n, JSType type) {
-    boolean isSupertypeOfPromise = promiseOfUnknownType.isSubtypeOf(type);
-    if (isSupertypeOfPromise && !type.isUnionType()) {
+    if (promiseOfUnknownType.isSubtypeOf(type)) {
       return;
     }
 
     JSError err = JSError.make(n, INVALID_ASYNC_RETURN_TYPE, type.toString());
-    if (!isSupertypeOfPromise) {
-      // Only subtyping issues should report mismatches, not all invalid return types.
-      registerMismatch(type, promiseOfUnknownType, err);
-    }
+    registerMismatch(type, promiseOfUnknownType, err);
     report(err);
   }
 

test/com/google/javascript/jscomp/TypeCheckNoTranspileTest.java

@@ -202,25 +202,49 @@ public void testArrowBlocklessCorrectInheritsArguments() {
   }
 
   @Test
-  public void testAsyncArrowWithCorrectBlocklessReturn() {
-    testTypes(
+  public void testAsyncArrow_withValidBlocklessReturn_isAllowed() {
+    testTypesWithCommonExterns(
         lines(
-            "function takesPromiseProvider(/** function(): !Promise<number> */ getPromise) {}",
+            "function takesPromiseProvider(/** function():!Promise<number> */ getPromise) {}",
             "takesPromiseProvider(async () => 1);"));
   }
 
   @Test
-  public void testAsyncArrowWithIncorrectBlocklessReturn() {
-    testTypes(
+  public void testAsyncArrow_withInvalidBlocklessReturn_isError() {
+    testTypesWithCommonExterns(
         lines(
-            "function takesPromiseProvider(/** function(): !Promise<string> */ getPromise) {}",
+            "function takesPromiseProvider(/** function():!Promise<string> */ getPromise) {}",
             "takesPromiseProvider(async () => 1);"),
         lines(
             "inconsistent return type", // preserve newline
             "found   : number",
             "required: (IThenable<string>|string)"));
   }
 
+  @Test
+  public void testAsyncArrow_withInferredReturnType_ofValidUnionType_isAllowed() {
+    testTypesWithCommonExterns(
+        lines(
+            "/** @param {function():(number|!Promise<string>)} getPromise */",
+            "function takesPromiseProvider(getPromise) {}",
+            "",
+            "takesPromiseProvider(async () => '');"));
+  }
+
+  @Test
+  public void testAsyncArrow_withInferredReturnType_ofInvalidUnionType_isError() {
+    testTypesWithCommonExterns(
+        lines(
+            "/** @param {function():(number|!Promise<string>)} getPromise */",
+            "function takesPromiseProvider(getPromise) {}",
+            "",
+            "takesPromiseProvider(async () => true);"),
+        lines(
+            "inconsistent return type", // preserve newline
+            "found   : boolean",
+            "required: (IThenable<string>|string)"));
+  }
+
   @Test
   public void testTypedefOfPropertyInBlock() {
     disableStrictMissingPropertyChecks();
@@ -4218,31 +4242,30 @@ public void testAsyncFunctionInferredToReturnPromise_withExplicitReturns() {
   }
 
   @Test
-  public void testAsyncFunctionCannotReturnNumber() {
+  public void testAsyncFunction_cannotDeclareReturnToBe_Number() {
     testTypesWithCommonExterns(
         "/** @return {number} */ async function f() {}",
         lines(
-            "The return type of an async function must be a non-union supertype of Promise",
+            "The return type of an async function must be a supertype of Promise",
             "found: number"));
   }
 
   @Test
-  public void testAsyncFunctionCannotReturnArray() {
+  public void testAsyncFunction_cannotDeclareReturnToBe_Array() {
     testTypesWithCommonExterns(
         "/** @return {!Array} */ async function f() {}",
         lines(
-            "The return type of an async function must be a non-union supertype of Promise",
-            "found: Array"));
+            "The return type of an async function must be a supertype of Promise", "found: Array"));
   }
 
   @Test
-  public void testAsyncFunctionCanReturnObject() {
-    testTypesWithCommonExterns("/** @return {!Object} */ async function f() {}");
+  public void testAsyncFunction_canDeclareReturnToBe_Object_andAccepts_undefined() {
+    testTypesWithCommonExterns("/** @return {!Object} */ async function f() { return undefined; }");
   }
 
   @Test
-  public void testAsyncFunctionCanReturnAllType() {
-    testTypesWithCommonExterns("/** @return {*} */ async function f() {}");
+  public void testAsyncFunction_canDeclareReturnToBe_allType_andAccepts_undefined() {
+    testTypesWithCommonExterns("/** @return {*} */ async function f() { return undefined; }");
   }
 
   @Test
@@ -4270,33 +4293,27 @@ public void testAsyncReturnsPromise2() {
   }
 
   @Test
-  public void testAsyncFunctionCannotReturnNullablePromise() {
+  public void testAsyncFunction_canDeclareReturnToBe_nullablePromise() {
     testTypesWithCommonExterns(
         lines(
             "/** @return {?Promise<string>} */",
             "async function getAString() {",
             "  return '';",
-            "}"),
-        lines(
-            "The return type of an async function must be a non-union supertype of Promise",
-            "found: (Promise<string>|null)"));
+            "}"));
   }
 
   @Test
-  public void testAsyncCannotReturnUnionOfPromiseAndNumber() {
+  public void testAsyncFunction_canDeclareReturnToBe_unionOfPromiseAndNumber() {
     testTypesWithCommonExterns(
         lines(
-            "/** @return {(number|!Promise<string>)} */",
+            "/** @return {(number|!Promise<number>)} */",
             "async function getAString() {",
             "  return 1;",
-            "}"),
-        lines(
-            "The return type of an async function must be a non-union supertype of Promise",
-            "found: (Promise<string>|number)"));
+            "}"));
   }
 
   @Test
-  public void testAsyncCannotReturnASubtypeOfPromise() {
+  public void testAsyncFunction_cannotDeclareReturnToBe_aSubtypeOfPromise() {
     testTypesWithCommonExterns(
         lines(
             "/** @extends {Promise<string>} */",
@@ -4307,12 +4324,12 @@ public void testAsyncCannotReturnASubtypeOfPromise() {
             "  return '';",
             "}"),
         lines(
-            "The return type of an async function must be a non-union supertype of Promise",
+            "The return type of an async function must be a supertype of Promise",
             "found: MyPromise"));
   }
 
   @Test
-  public void testAsyncCannotReturnASiblingOfPromise() {
+  public void testAsyncFunction_cannotDeclareReturnToBe_aSiblingOfPromise() {
     testTypesWithCommonExterns(
         lines(
             "/**",
@@ -4326,12 +4343,12 @@ public void testAsyncCannotReturnASiblingOfPromise() {
             "  return '';",
             "}"),
         lines(
-            "The return type of an async function must be a non-union supertype of Promise",
+            "The return type of an async function must be a supertype of Promise",
             "found: MyThenable"));
   }
 
   @Test
-  public void testAsyncCanReturnIThenable1() {
+  public void testAsyncFunction_canDeclareReturnToBe_IThenable1() {
     testTypesWithCommonExterns(
         lines(
             "/** @return {!IThenable<string>} */",
@@ -4344,6 +4361,21 @@ public void testAsyncCanReturnIThenable1() {
             "required: (IThenable<string>|string)"));
   }
 
+  @Test
+  public void testAsyncFunction_checksReturnExpressionType_againstCorrectUpperBound() {
+    testTypesWithCommonExterns(
+        lines(
+            "/** @return {string|!IThenable<boolean|undefined>|!Promise<null>} */",
+            "async function getAString() {",
+            "  return {};",
+            "}"),
+        lines(
+            "inconsistent return type", //
+            "found   : {}",
+            // We're specifically checking this type.
+            "required: (IThenable<(boolean|null|undefined)>|boolean|null|undefined)"));
+  }
+
   @Test
   public void testAsyncReturnStatementIsResolved() {
     // Test that we correctly handle resolving an "IThenable" return statement inside an async