Make Iterable, Iterator, Generator, and their async equivalents covar…

…iant

Classes that implement Iterable, like Map or Set, are still invariant by default.

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

src/com/google/javascript/rhino/jstype/JSType.java

@@ -90,6 +90,10 @@ static final boolean areIdentical(JSType a, JSType b) {
   private static final ImmutableSet<String> BIVARIANT_TYPES =
       ImmutableSet.of("Object", "IArrayLike", "Array");
 
+  // NB: IThenable and all subtypes are also covariant; however, they are handled differently.
+  private static final ImmutableSet<String> COVARIANT_TYPES =
+      ImmutableSet.of("Iterable", "Iterator", "Generator", "AsyncIterator", "AsyncIterable");
+
   final JSTypeRegistry registry;
 
   JSType(JSTypeRegistry registry) {
@@ -1574,19 +1578,17 @@ static boolean isSubtypeHelper(JSType thisType, JSType thatType,
 
       templateMatch = thisElement.isSubtype(thatElement, implicitImplCache, subtypingMode)
           || thatElement.isSubtype(thisElement, implicitImplCache, subtypingMode);
-    } else if (isIThenableSubtype(thatType)) {
-      // NOTE: special case IThenable subclass (Promise, etc).  These classes are expected to be
-      // covariant.
-      // Also note that this ignores any additional template type arguments that might be defined
-      // on subtypes.  If we expand this to other types, a more correct solution will be needed.
-      TemplateType key = thisType.registry.getThenableValueKey();
-      JSType thisElement = thisTypeParams.getResolvedTemplateType(key);
-      JSType thatElement = thatTypeParams.getResolvedTemplateType(key);
-
-      templateMatch = thisElement.isSubtype(thatElement, implicitImplCache, subtypingMode);
     } else {
-      templateMatch = thisTypeParams.checkEquivalenceHelper(
-          thatTypeParams, EquivalenceMethod.INVARIANT, subtypingMode);
+      TemplateType covariantKey = getTemplateKeyIfCovariantType(thatType);
+      if (covariantKey != null) {
+        JSType thisElement = thisTypeParams.getResolvedTemplateType(covariantKey);
+        JSType thatElement = thatTypeParams.getResolvedTemplateType(covariantKey);
+        templateMatch = thisElement.isSubtype(thatElement, implicitImplCache, subtypingMode);
+      } else {
+        templateMatch =
+            thisTypeParams.checkEquivalenceHelper(
+                thatTypeParams, EquivalenceMethod.INVARIANT, subtypingMode);
+      }
     }
     if (!templateMatch) {
       return false;
@@ -1620,26 +1622,43 @@ static boolean isSubtypeHelper(JSType thisType, JSType thatType,
   }
 
   /**
-   * Determines if the supplied type should be checked as a bivariant
-   * templatized type rather the standard invariant templatized type
-   * rules.
+   * Determines if the supplied type should be checked as a bivariant templatized type rather the
+   * standard invariant templatized type rules.
    */
   static boolean isBivariantType(JSType type) {
-    ObjectType objType = type.toObjectType();
-    return objType != null
-        // && objType.isNativeObjectType()
-        && BIVARIANT_TYPES.contains(objType.getReferenceName());
+    ObjectType unwrapped = getObjectTypeIfNative(type);
+    return unwrapped != null && BIVARIANT_TYPES.contains(unwrapped.getReferenceName());
   }
 
   /**
-   * Determines if the specified type is exempt from standard invariant templatized typing rules.
+   * Determines if the specified type should be checked as covariant rather than the standard
+   * invariant type. If so, returns the template type to check covariantly.
    */
-  static boolean isIThenableSubtype(JSType type) {
+  @Nullable
+  static TemplateType getTemplateKeyIfCovariantType(JSType type) {
     if (type.isTemplatizedType()) {
+      // Unlike other covariant/bivariant types, even non-native subtypes of IThenable are
+      // covariant, so IThenable is special-cased here.
       TemplatizedType ttype = type.toMaybeTemplatizedType();
-      return ttype.getTemplateTypeMap().hasTemplateKey(ttype.registry.getThenableValueKey());
+      if (ttype.getTemplateTypeMap().hasTemplateKey(ttype.registry.getIThenableTemplate())) {
+        return ttype.registry.getIThenableTemplate();
+      }
     }
-    return false;
+    ObjectType unwrapped = getObjectTypeIfNative(type);
+    if (unwrapped != null && COVARIANT_TYPES.contains(unwrapped.getReferenceName())) {
+      // Note: this code only works because the currently covariant types only have a single
+      // @template. A different solution would be needed to generalize variance for arbitrary types.
+      return Iterables.getOnlyElement(
+          unwrapped.getConstructor().getTemplateTypeMap().getTemplateKeys());
+    }
+    return null;
+  }
+
+  @Nullable
+  private static ObjectType getObjectTypeIfNative(JSType type) {
+    ObjectType objType = type.toObjectType();
+    ObjectType unwrapped = ObjectType.deeplyUnwrap(objType);
+    return unwrapped != null && unwrapped.isNativeObjectType() ? unwrapped : null;
   }
 
   /**

src/com/google/javascript/rhino/jstype/JSTypeNative.java

@@ -104,6 +104,9 @@ public enum JSTypeNative {
   ITERATOR_FUNCTION_TYPE,
   ITERATOR_TYPE,
 
+  I_ARRAY_LIKE_FUNCTION_TYPE,
+  I_ARRAY_LIKE_TYPE,
+
   I_TEMPLATE_ARRAY_TYPE,
 
   I_OBJECT_FUNCTION_TYPE,

src/com/google/javascript/rhino/jstype/JSTypeRegistry.java

@@ -258,15 +258,6 @@ public TemplateType getObjectIndexKey() {
     return iObjectIndexTemplateKey;
   }
 
-  /**
-   * @return The template variable corresponding to the
-   * property key type of the built-in Javascript object.
-   */
-  public TemplateType getThenableValueKey() {
-    checkNotNull(iThenableTemplateKey);
-    return iThenableTemplateKey;
-  }
-
   /**
    * @return The template variable for the Iterable interface.
    */
@@ -353,6 +344,7 @@ private void initializeBuiltInTypes() {
     iObjectIndexTemplateKey = new TemplateType(this, "IObject#KEY1");
     iObjectElementTemplateKey = new TemplateType(this, I_OBJECT_ELEMENT_TEMPLATE);
     // These should match the template type name in externs files.
+    TemplateType iArrayLikeTemplate = new TemplateType(this, "VALUE2");
     arrayElementTemplateKey = new TemplateType(this, "T");
     iteratorTemplate = new TemplateType(this, "VALUE");
     iiterableResultTemplate = new TemplateType(this, "VALUE");
@@ -431,6 +423,13 @@ private void initializeBuiltInTypes() {
     ObjectType iiterableResultType = iiterableResultFunctionType.getInstanceType();
     registerNativeType(JSTypeNative.I_ITERABLE_RESULT_TYPE, iiterableResultType);
 
+    // IArrayLike.
+    // TODO(lharker): Should the native Array implement IArrayLike?
+    FunctionType iArrayLikeFunctionType = nativeRecord("IArrayLike", iArrayLikeTemplate);
+    registerNativeType(JSTypeNative.I_ARRAY_LIKE_FUNCTION_TYPE, iArrayLikeFunctionType);
+    ObjectType iArrayLikeType = iArrayLikeFunctionType.getInstanceType();
+    registerNativeType(JSTypeNative.I_ARRAY_LIKE_TYPE, iArrayLikeType);
+
     // Array
     FunctionType arrayFunctionType =
         nativeConstructorBuilder("Array")
@@ -734,6 +733,7 @@ private void initializeRegistry() {
     registerGlobalType(getNativeType(JSTypeNative.ASYNC_GENERATOR_TYPE));
     registerGlobalType(getNativeType(JSTypeNative.BOOLEAN_OBJECT_TYPE));
     registerGlobalType(getNativeType(JSTypeNative.BOOLEAN_TYPE));
+    registerGlobalType(getNativeType(JSTypeNative.I_ARRAY_LIKE_TYPE));
     registerGlobalType(getNativeType(JSTypeNative.ITERABLE_TYPE));
     registerGlobalType(getNativeType(JSTypeNative.ITERATOR_TYPE));
     registerGlobalType(getNativeType(JSTypeNative.GENERATOR_TYPE));
@@ -2337,4 +2337,10 @@ private FunctionType nativeInterface(String name, TemplateType... templateKeys)
     }
     return builder.build();
   }
+
+  private FunctionType nativeRecord(String name, TemplateType... templateKeys) {
+    FunctionType type = nativeInterface(name, templateKeys);
+    type.setImplicitMatch(true);
+    return type;
+  }
 }

src/com/google/javascript/rhino/jstype/ObjectType.java

@@ -765,16 +765,20 @@ final boolean isImplicitPrototype(ObjectType prototype) {
     return false;
   }
 
-  private static ObjectType deeplyUnwrap(ObjectType current) {
+  static ObjectType deeplyUnwrap(ObjectType original) {
+    ObjectType current = original;
     while (current instanceof ProxyObjectType) {
       if (current.isTemplatizedType()) {
         current = current.toMaybeTemplatizedType().getReferencedType();
-      }
-      if (current.isNamedType()) {
+      } else if (current.isNamedType()) {
         if (!current.isSuccessfullyResolved()) {
           break;
         }
         current = current.toMaybeNamedType().getReferencedObjTypeInternal();
+      } else {
+        // TODO(lharker): remove this case and instead fail. Only the Rhino unit tests are
+        // triggering this by creating new ProxyObjectTypes.
+        current = ((ProxyObjectType) current).getReferencedObjTypeInternal();
       }
     }
     return current;

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

@@ -18504,6 +18504,103 @@ public void testIArrayLike13() {
             "required: number"));
   }
 
+  @Test
+  public void testIterableCovariant() {
+    testTypesWithCommonExterns(
+        lines(
+            "function f(/** !Iterable<(number|string)>*/ x){};",
+            "function g(/** !Iterable<number> */ arr) {",
+            "    f(arr);",
+            "}"));
+  }
+
+  @Test
+  public void testLocalShadowOfIterableNotCovariant() {
+    testTypesWithCommonExterns(
+        lines(
+            "/** @template T */",
+            "class Iterable {}",
+            "function f(/** !Iterable<(number|string)>*/ x) {};",
+            "function g(/** !Iterable<number> */ arr) {",
+            "    f(arr);",
+            "}",
+            "export {};"),
+        lines(
+            "actual parameter 1 of f does not match formal parameter",
+            "found   : Iterable<number>",
+            "required: Iterable<(number|string)>"));
+  }
+
+  @Test
+  public void testIterableNotContravariant() {
+    testTypesWithCommonExterns(
+        lines(
+            "function f(/** !Iterable<number>*/ x){};",
+            "function g(/** !Iterable<(number|string)> */ arr) {",
+            "    f(arr);",
+            "}"),
+        lines(
+            "actual parameter 1 of f does not match formal parameter",
+            "found   : Iterable<(number|string)>",
+            "required: Iterable<number>"));
+  }
+
+  @Test
+  public void testIterableCovariantWhenComparingToSubtype() {
+    testTypesWithExtraExterns(
+        lines(
+            "/** @constructor",
+            " * @implements {Iterable<T>}",
+            " * @template T",
+            " */",
+            "function Set() {}"),
+        lines(
+            "function f(/** !Iterable<(number|string)>*/ x){};",
+            "function g(/** !Set<number> */ arr) {",
+            "    f(arr);",
+            "}"));
+  }
+
+  @Test
+  public void testIteratorCovariant() {
+    testTypesWithCommonExterns(
+        lines(
+            "function f(/** !Iterator<(string|number)>*/ x){};",
+            "function g(/** !Iterator<number> */ arr) {",
+            "    f(arr);",
+            "}"));
+  }
+
+  @Test
+  public void testGeneratorCovariant() {
+    testTypesWithCommonExterns(
+        lines(
+            "function f(/** !Generator<(string|number)>*/ x){};",
+            "function g(/** !Generator<number> */ arr) {",
+            "    f(arr);",
+            "}"));
+  }
+
+  @Test
+  public void testIterableImplementorInvariant() {
+    testTypesWithExtraExterns(
+        lines(
+            "/** @constructor",
+            " * @implements {Iterable<T>}",
+            " * @template T",
+            " */",
+            "function Set() {}"),
+        lines(
+            "function f(/** !Set<(string|number)>*/ x){};",
+            "function g(/** !Set<number> */ arr) {",
+            "    f(arr);",
+            "}"),
+        lines(
+            "actual parameter 1 of f does not match formal parameter",
+            "found   : Set<number>",
+            "required: Set<(number|string)>"));
+  }
+
   @Test
   public void testIArrayLikeCovariant1() {
     testTypesWithCommonExterns(
@@ -18524,6 +18621,16 @@ public void testIArrayLikeCovariant2() {
             "}"));
   }
 
+  @Test
+  public void testIArrayLikeBivaraint() {
+    testTypesWithCommonExterns(
+        lines(
+            "function f(/** !IArrayLike<number>*/ x){};",
+            "function g(/** !IArrayLike<(string|number)> */ arr) {",
+            "    f(arr);",
+            "}"));
+  }
+
   @Test
   public void testIArrayLikeStructuralMatch1() {
     testTypesWithCommonExterns(
@@ -22639,6 +22746,21 @@ public void testCovariantIThenableNonThenable4() {
             "required: IThenable<string>"));
   }
 
+  @Test
+  public void testCovariantIThenableNonNativeSubclass() {
+    testTypes(
+        lines(
+            "/**",
+            " * @implements {IThenable<T>}",
+            " * @template T",
+            " */",
+            "class CustomPromise {}",
+            "/** @type {!CustomPromise<(number|string)>} */ var x;",
+            "function fn(/** !CustomPromise<string> */ a ) {",
+            "  x = a;",
+            "}"));
+  }
+
   @Test
   public void testPropertyReferenceOnShadowingParameter() {
     testTypes(