Skip to content

Commit

Permalink
Update hashCode() on TemplatizedType to be equal to that of the c…
Browse files Browse the repository at this point in the history
…orresponding `IntanceObjectType` when all template parameter values are the unknown type. (i.e. `hashCode(Foo) == hashCode(Foo<?>)`)

This is being done because currently such pairs of types (a raw template type and it's specialization on unknown) are considered identity equal (`equals()`). Therefore, the old behaviour violated the contract of `hashCode()`.

In a better world we would rather have `Foo !== Foo<?>`, but that change has larger implications. This CL represents a quick fix to bring things to a less-bad state.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=200632664
  • Loading branch information
nreid260 authored and blickly committed Jun 15, 2018
1 parent 220e3ca commit b23a7f5
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 56 deletions.
29 changes: 19 additions & 10 deletions src/com/google/javascript/rhino/jstype/TemplatizedType.java
Expand Up @@ -54,26 +54,33 @@
public final class TemplatizedType extends ProxyObjectType { public final class TemplatizedType extends ProxyObjectType {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;


final ImmutableList<JSType> templateTypes; /** A cache of the type parameter values for this specialization. */
transient TemplateTypeMapReplacer replacer; private final ImmutableList<JSType> templateTypes;
/** Whether all type parameter values for this specialization are `?`. */
private final boolean isSpecializedOnlyWithUnknown;

private transient TemplateTypeMapReplacer replacer;


TemplatizedType( TemplatizedType(
JSTypeRegistry registry, ObjectType objectType, JSTypeRegistry registry, ObjectType objectType,
ImmutableList<JSType> templateTypes) { ImmutableList<JSType> templateTypes) {
super(registry, objectType, objectType.getTemplateTypeMap().addValues( super(registry, objectType, objectType.getTemplateTypeMap().addValues(
templateTypes)); templateTypes));


// Cache which template keys were filled, and what JSTypes they were filled
// with.
ImmutableList<TemplateType> filledTemplateKeys =
objectType.getTemplateTypeMap().getUnfilledTemplateKeys();
ImmutableList.Builder<JSType> builder = ImmutableList.builder(); ImmutableList.Builder<JSType> builder = ImmutableList.builder();
for (TemplateType filledTemplateKey : filledTemplateKeys) { boolean maybeIsSpecializedOnlyWithUnknown = true;
builder.add(getTemplateTypeMap().getResolvedTemplateType(filledTemplateKey)); for (TemplateType newlyFilledTemplateKey :
objectType.getTemplateTypeMap().getUnfilledTemplateKeys()) {
JSType resolvedType = getTemplateTypeMap().getResolvedTemplateType(newlyFilledTemplateKey);

builder.add(resolvedType);
maybeIsSpecializedOnlyWithUnknown =
maybeIsSpecializedOnlyWithUnknown && resolvedType.isUnknownType();
} }
this.templateTypes = builder.build(); this.templateTypes = builder.build();
this.isSpecializedOnlyWithUnknown = maybeIsSpecializedOnlyWithUnknown;


replacer = new TemplateTypeMapReplacer(registry, getTemplateTypeMap()); this.replacer = new TemplateTypeMapReplacer(registry, getTemplateTypeMap());
} }


// NOTE(dimvar): If getCtorImplementedInterfaces is implemented here, this is the // NOTE(dimvar): If getCtorImplementedInterfaces is implemented here, this is the
Expand Down Expand Up @@ -118,7 +125,9 @@ StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) {
@Override @Override
public int hashCode() { public int hashCode() {
int baseHash = super.hashCode(); int baseHash = super.hashCode();
if (templateTypes.isEmpty()) {
// TODO(b/110224889): This case can probably be removed if `equals()` is updated.
if (isSpecializedOnlyWithUnknown) {
return baseHash; return baseHash;
} }
return Objects.hash(templateTypes, baseHash); return Objects.hash(templateTypes, baseHash);
Expand Down
98 changes: 52 additions & 46 deletions test/com/google/javascript/rhino/jstype/TemplatizedTypeTest.java
Expand Up @@ -45,18 +45,6 @@


public class TemplatizedTypeTest extends BaseJSTypeTestCase { public class TemplatizedTypeTest extends BaseJSTypeTestCase {


@Override
public void setUp() throws Exception {
super.setUp();
}

/**
* Assert that a type can assign to itself.
*/
private void assertTypeCanAssignToItself(JSType type) {
assertTrue(type.isSubtypeOf(type));
}

/** /**
* Tests the behavior of variants type. * Tests the behavior of variants type.
*/ */
Expand Down Expand Up @@ -106,6 +94,12 @@ public void testPrint3() throws Exception {
assertEquals("Array<?>", arrOfUnknown.toString()); assertEquals("Array<?>", arrOfUnknown.toString());
} }


public void testPrintingRawType() throws Exception {
ObjectType rawType = createCustomTemplatizedType("Foo");

assertEquals("Foo", rawType.toString());
}

public void testDifferentRawTypes() throws Exception { public void testDifferentRawTypes() throws Exception {
TemplatizedType arrOfNumber = createTemplatizedType( TemplatizedType arrOfNumber = createTemplatizedType(
ARRAY_TYPE, NUMBER_TYPE); ARRAY_TYPE, NUMBER_TYPE);
Expand All @@ -115,45 +109,57 @@ public void testDifferentRawTypes() throws Exception {
assertFalse(objType.isSubtype(arrOfNumber)); assertFalse(objType.isSubtype(arrOfNumber));
} }


public void testCustomTemplatizedType() throws Exception { public void testSubtypingAndEquivalenceAmongCustomTemplatizedTypes() throws Exception {
FunctionType ctor = ObjectType rawType = createCustomTemplatizedType("Baz");

JSType templatizedStringNumber =
registry.createTemplatizedType(rawType, ImmutableList.of(STRING_TYPE, NUMBER_TYPE));
JSType templatizedStringAll =
registry.createTemplatizedType(rawType, ImmutableList.of(STRING_TYPE, ALL_TYPE));
JSType templatizedStringUnknown =
registry.createTemplatizedType(rawType, ImmutableList.of(STRING_TYPE, UNKNOWN_TYPE));
JSType templatizedUnknownUnknown =
registry.createTemplatizedType(rawType, ImmutableList.of(UNKNOWN_TYPE, UNKNOWN_TYPE));

assertTrue(templatizedStringNumber.isSubtypeOf(rawType));
assertTrue(templatizedStringAll.isSubtypeOf(rawType));
assertTrue(templatizedStringUnknown.isSubtypeOf(rawType));
assertTrue(templatizedUnknownUnknown.isSubtypeOf(rawType));

assertTypeNotEquals(templatizedStringNumber, rawType);
assertTypeNotEquals(templatizedStringAll, rawType);
assertTypeNotEquals(templatizedStringUnknown, rawType);

// TODO(b/110224889): This case should probably be `assertTypeNotEquals`.
assertTypeEquals(templatizedUnknownUnknown, rawType);

assertTrue(rawType.isSubtypeOf(templatizedStringNumber));
assertTrue(rawType.isSubtypeOf(templatizedStringAll));
assertTrue(rawType.isSubtypeOf(templatizedStringUnknown));
assertTrue(rawType.isSubtypeOf(templatizedUnknownUnknown));

assertFalse(templatizedStringNumber.isSubtypeOf(templatizedStringAll));
assertFalse(templatizedStringAll.isSubtypeOf(templatizedStringNumber));

assertTrue(templatizedStringAll.isSubtypeOf(templatizedStringUnknown));
assertTrue(templatizedStringUnknown.isSubtypeOf(templatizedStringAll));
}

/** Returns an unspecialized type with the provided name and two type parameters. */
private ObjectType createCustomTemplatizedType(String rawName) {
FunctionType ctor = // function<T,U>(new:Foo<T,U>)
registry.createConstructorType( registry.createConstructorType(
"Foo", rawName,
null, null,
null, null,
null, null,
ImmutableList.of(registry.createTemplateType("T"), registry.createTemplateType("U")), ImmutableList.of(registry.createTemplateType("T"), registry.createTemplateType("U")),
false); false);
ObjectType baseType = ctor.getInstanceType(); return ctor.getInstanceType(); // Foo<T, U> == Foo

}
JSType templatizedType1 = registry.createTemplatizedType(
baseType, ImmutableList.of(STRING_TYPE, NUMBER_TYPE)); /** Assert that a type can assign to itself. */
JSType templatizedType2 = registry.createTemplatizedType( private void assertTypeCanAssignToItself(JSType type) {
baseType, ImmutableList.of(STRING_TYPE, ALL_TYPE)); assertTrue(type.isSubtypeOf(type));
JSType templatizedType3 = registry.createTemplatizedType(
baseType, ImmutableList.of(STRING_TYPE, UNKNOWN_TYPE));
JSType templatizedType4 = registry.createTemplatizedType(
baseType, ImmutableList.<JSType>of(UNKNOWN_TYPE, UNKNOWN_TYPE));

assertTrue(templatizedType1.isSubtypeOf(baseType));
assertTrue(templatizedType2.isSubtypeOf(baseType));
assertTrue(templatizedType3.isSubtypeOf(baseType));
assertTrue(templatizedType4.isSubtypeOf(baseType));

assertFalse(templatizedType1.isEquivalentTo(baseType));
assertFalse(templatizedType2.isEquivalentTo(baseType));
assertFalse(templatizedType3.isEquivalentTo(baseType));
assertTrue(templatizedType4.isEquivalentTo(baseType));

assertTrue(baseType.isSubtypeOf(templatizedType1));
assertTrue(baseType.isSubtypeOf(templatizedType2));
assertTrue(baseType.isSubtypeOf(templatizedType3));
assertTrue(baseType.isSubtypeOf(templatizedType4));

assertFalse(templatizedType1.isSubtypeOf(templatizedType2));
assertFalse(templatizedType2.isSubtypeOf(templatizedType1));

assertTrue(templatizedType2.isSubtypeOf(templatizedType3));
assertTrue(templatizedType3.isSubtypeOf(templatizedType2));
} }
} }

0 comments on commit b23a7f5

Please sign in to comment.