diff --git a/src/com/google/javascript/rhino/jstype/JSType.java b/src/com/google/javascript/rhino/jstype/JSType.java index 0ae7d91ae02..9d7aaacccc0 100644 --- a/src/com/google/javascript/rhino/jstype/JSType.java +++ b/src/com/google/javascript/rhino/jstype/JSType.java @@ -90,6 +90,10 @@ static final boolean areIdentical(JSType a, JSType b) { private static final ImmutableSet BIVARIANT_TYPES = ImmutableSet.of("Object", "IArrayLike", "Array"); + // NB: IThenable and all subtypes are also covariant; however, they are handled differently. + private static final ImmutableSet 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; } /** diff --git a/src/com/google/javascript/rhino/jstype/JSTypeNative.java b/src/com/google/javascript/rhino/jstype/JSTypeNative.java index 746f657918d..0c09dcc65ec 100644 --- a/src/com/google/javascript/rhino/jstype/JSTypeNative.java +++ b/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, diff --git a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java index e93a810396a..d5cc82a6d57 100644 --- a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java +++ b/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; + } } diff --git a/src/com/google/javascript/rhino/jstype/ObjectType.java b/src/com/google/javascript/rhino/jstype/ObjectType.java index 486ea5002ad..bfbea425451 100644 --- a/src/com/google/javascript/rhino/jstype/ObjectType.java +++ b/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; diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java index 9c91758212d..9ba7bcf2650 100644 --- a/test/com/google/javascript/jscomp/TypeCheckTest.java +++ b/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 */ arr) {", + " f(arr);", + "}")); + } + + @Test + public void testLocalShadowOfIterableNotCovariant() { + testTypesWithCommonExterns( + lines( + "/** @template T */", + "class Iterable {}", + "function f(/** !Iterable<(number|string)>*/ x) {};", + "function g(/** !Iterable */ arr) {", + " f(arr);", + "}", + "export {};"), + lines( + "actual parameter 1 of f does not match formal parameter", + "found : Iterable", + "required: Iterable<(number|string)>")); + } + + @Test + public void testIterableNotContravariant() { + testTypesWithCommonExterns( + lines( + "function f(/** !Iterable*/ 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")); + } + + @Test + public void testIterableCovariantWhenComparingToSubtype() { + testTypesWithExtraExterns( + lines( + "/** @constructor", + " * @implements {Iterable}", + " * @template T", + " */", + "function Set() {}"), + lines( + "function f(/** !Iterable<(number|string)>*/ x){};", + "function g(/** !Set */ arr) {", + " f(arr);", + "}")); + } + + @Test + public void testIteratorCovariant() { + testTypesWithCommonExterns( + lines( + "function f(/** !Iterator<(string|number)>*/ x){};", + "function g(/** !Iterator */ arr) {", + " f(arr);", + "}")); + } + + @Test + public void testGeneratorCovariant() { + testTypesWithCommonExterns( + lines( + "function f(/** !Generator<(string|number)>*/ x){};", + "function g(/** !Generator */ arr) {", + " f(arr);", + "}")); + } + + @Test + public void testIterableImplementorInvariant() { + testTypesWithExtraExterns( + lines( + "/** @constructor", + " * @implements {Iterable}", + " * @template T", + " */", + "function Set() {}"), + lines( + "function f(/** !Set<(string|number)>*/ x){};", + "function g(/** !Set */ arr) {", + " f(arr);", + "}"), + lines( + "actual parameter 1 of f does not match formal parameter", + "found : Set", + "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*/ x){};", + "function g(/** !IArrayLike<(string|number)> */ arr) {", + " f(arr);", + "}")); + } + @Test public void testIArrayLikeStructuralMatch1() { testTypesWithCommonExterns( @@ -22639,6 +22746,21 @@ public void testCovariantIThenableNonThenable4() { "required: IThenable")); } + @Test + public void testCovariantIThenableNonNativeSubclass() { + testTypes( + lines( + "/**", + " * @implements {IThenable}", + " * @template T", + " */", + "class CustomPromise {}", + "/** @type {!CustomPromise<(number|string)>} */ var x;", + "function fn(/** !CustomPromise */ a ) {", + " x = a;", + "}")); + } + @Test public void testPropertyReferenceOnShadowingParameter() { testTypes(