diff --git a/src/com/google/javascript/jscomp/JsIterables.java b/src/com/google/javascript/jscomp/JsIterables.java index f7e2e4eb559..f9749ba374e 100644 --- a/src/com/google/javascript/jscomp/JsIterables.java +++ b/src/com/google/javascript/jscomp/JsIterables.java @@ -135,7 +135,7 @@ static final MaybeBoxedIterableOrAsyncIterable maybeBoxIterableOrAsyncIterable( // Note: we don't just use JSType.autobox() here because that removes null and undefined. // We want to keep null and undefined around because they should cause a mismatch. if (type.isUnionType()) { - for (JSType alt : type.toMaybeUnionType().getAlternatesWithoutStructuralTyping()) { + for (JSType alt : type.toMaybeUnionType().getAlternates()) { alt = alt.isBoxableScalar() ? alt.autoboxesTo() : alt; boolean isIterable = alt.isSubtypeOf(typeRegistry.getNativeType(ITERABLE_TYPE)); boolean isAsyncIterable = alt.isSubtypeOf(typeRegistry.getNativeType(ASYNC_ITERABLE_TYPE)); diff --git a/src/com/google/javascript/jscomp/Promises.java b/src/com/google/javascript/jscomp/Promises.java index fb6fbd5b561..3c0ff441a02 100644 --- a/src/com/google/javascript/jscomp/Promises.java +++ b/src/com/google/javascript/jscomp/Promises.java @@ -74,7 +74,7 @@ static final JSType getResolvedType(JSTypeRegistry registry, JSType type) { if (type.isUnionType()) { UnionTypeBuilder unionTypeBuilder = UnionTypeBuilder.create(registry); - for (JSType alternate : type.toMaybeUnionType().getAlternatesWithoutStructuralTyping()) { + for (JSType alternate : type.toMaybeUnionType().getAlternates()) { unionTypeBuilder.addAlternate(getResolvedType(registry, alternate)); } return unionTypeBuilder.build(); diff --git a/src/com/google/javascript/jscomp/TypeValidator.java b/src/com/google/javascript/jscomp/TypeValidator.java index 7c470e1786f..ba4154a18c2 100644 --- a/src/com/google/javascript/jscomp/TypeValidator.java +++ b/src/com/google/javascript/jscomp/TypeValidator.java @@ -283,7 +283,7 @@ boolean expectAutoboxesToIterable(Node n, JSType type, String msg) { // Note: we don't just use JSType.autobox() here because that removes null and undefined. // We want to keep null and undefined around. if (type.isUnionType()) { - for (JSType alt : type.toMaybeUnionType().getAlternatesWithoutStructuralTyping()) { + for (JSType alt : type.toMaybeUnionType().getAlternates()) { alt = alt.isBoxableScalar() ? alt.autoboxesTo() : alt; if (!alt.isSubtypeOf(getNativeType(ITERABLE_TYPE))) { mismatch(n, msg, type, ITERABLE_TYPE); diff --git a/src/com/google/javascript/rhino/jstype/JSType.java b/src/com/google/javascript/rhino/jstype/JSType.java index e4fee49a23f..1264df16cbc 100644 --- a/src/com/google/javascript/rhino/jstype/JSType.java +++ b/src/com/google/javascript/rhino/jstype/JSType.java @@ -1448,9 +1448,7 @@ public TypePair getTypesUnderShallowInequality(JSType that) { } public Iterable getUnionMembers() { - return isUnionType() - ? this.toMaybeUnionType().getAlternatesWithoutStructuralTyping() - : null; + return isUnionType() ? this.toMaybeUnionType().getAlternates() : null; } /** @@ -1558,10 +1556,9 @@ static boolean isSubtypeHelper(JSType thisType, JSType thatType, if (thatType.isUnionType()) { UnionType union = thatType.toMaybeUnionType(); // use an indexed for-loop to avoid allocations - ImmutableList alternatesWithoutStucturalTyping = - union.getAlternatesWithoutStructuralTyping(); - for (int i = 0; i < alternatesWithoutStucturalTyping.size(); i++) { - JSType element = alternatesWithoutStucturalTyping.get(i); + ImmutableList alternates = union.getAlternates(); + for (int i = 0; i < alternates.size(); i++) { + JSType element = alternates.get(i); if (thisType.isSubtype(element, implicitImplCache, subtypingMode)) { return true; } diff --git a/src/com/google/javascript/rhino/jstype/UnionType.java b/src/com/google/javascript/rhino/jstype/UnionType.java index 15f9c506726..3bc2f71011d 100644 --- a/src/com/google/javascript/rhino/jstype/UnionType.java +++ b/src/com/google/javascript/rhino/jstype/UnionType.java @@ -71,37 +71,22 @@ public class UnionType extends JSType { // NOTE: to avoid allocating iterators, all the loops below iterate over alternates by index // instead of using the for-each loop idiom. - // We currently keep two separate lists because `DisambiguateProperties` needs to keep track - // of exactly how types are conflated. If we forget that a nominal type and its strucutral - // subtype were members of the same union, we over-conflate for some reason, and knee-cap that + // We currently avoid collapsing structural subtypes because `DisambiguateProperties` needs to + // keep track of exactly how types are conflated. If we forget that a nominal type and its + // strucutral subtype were members of the same union, we over-conflate and knee-cap that // optimization. - // TODO(nickreid): Find a less complex/expensize way to prevent this conflation. It's a high cost - // for a pretty uncommon case. - private ImmutableList alternatesRetainingStructuralSubtypes; - private ImmutableList alternatesCollapsingStructuralSubtypes; + // TODO(nickreid): Find a less complex way to prevent this conflation. + private ImmutableList alternates; /** * Creates a union type. * - * @param alternatesRetainingStructuralSubtypes the alternates of the union without structural - * typing subtype + * @param alternates the alternates of the union without structural typing subtype */ - UnionType(JSTypeRegistry registry, ImmutableList alternatesRetainingStructuralSubtypes) { + UnionType(JSTypeRegistry registry, ImmutableList alternates) { super(registry); - // TODO(nickreid): This assignment is load bearing and should be cleaned-up. It, and the loop - // below, duplicate `rebuildAlternates()`. However, if that method were called eagerly it would - // break some assumptions of `JSTypeRegisty` by using a builder with a default configuration to - // do the rebuild. The registry just seems to trust this will never happen. The design of - // `UnionType(Builder)` should be changed to ensure that rebuild uses a builder with the same - // configuration. - this.alternatesRetainingStructuralSubtypes = alternatesRetainingStructuralSubtypes; - - UnionTypeBuilder builder = UnionTypeBuilder.createForCollapsingStructuralSubtypes(registry); - for (JSType alternate : alternatesRetainingStructuralSubtypes) { - builder.addAlternate(alternate); - } - this.alternatesCollapsingStructuralSubtypes = builder.getAlternates(); + this.alternates = alternates; } /** @@ -110,41 +95,19 @@ public class UnionType extends JSType { * @return The alternate types of this union type. The returned set is immutable. */ public ImmutableList getAlternates() { - if (anyMatch(JSType::isUnionType, alternatesRetainingStructuralSubtypes)) { - rebuildAlternates(); - } - return alternatesCollapsingStructuralSubtypes; - } - - /** - * Gets the alternate types of this union type, including structural interfaces and implicit - * implementations as distinct alternatesCollapsingStructuralSubtypes. - * - * @return The alternate types of this union type. The returned set is immutable. - */ - public ImmutableList getAlternatesWithoutStructuralTyping() { - if (anyMatch(JSType::isUnionType, alternatesRetainingStructuralSubtypes)) { + if (anyMatch(JSType::isUnionType, alternates)) { rebuildAlternates(); } - return alternatesRetainingStructuralSubtypes; + return alternates; } /** - * Use UnionTypeBuilder to rebuild the list of alternatesCollapsingStructuralSubtypes and hashcode - * of the current UnionType. + * Use UnionTypeBuilder to rebuild the list of alternates and hashcode of the current UnionType. */ private void rebuildAlternates() { - UnionTypeBuilder nonCollapsingBuilder = UnionTypeBuilder.create(registry); - UnionTypeBuilder collapsingBuilder = - UnionTypeBuilder.createForCollapsingStructuralSubtypes(registry); - - for (JSType alternate : alternatesRetainingStructuralSubtypes) { - nonCollapsingBuilder.addAlternate(alternate); - collapsingBuilder.addAlternate(alternate); - } - - alternatesRetainingStructuralSubtypes = nonCollapsingBuilder.getAlternates(); - alternatesCollapsingStructuralSubtypes = collapsingBuilder.getAlternates(); + UnionTypeBuilder builder = UnionTypeBuilder.create(registry); + builder.addAlternates(alternates); + alternates = builder.getAlternates(); } /** @@ -156,7 +119,7 @@ private void rebuildAlternates() { @Override public boolean matchesNumberContext() { // TODO(user): Reverse this logic to make it correct instead of generous. - return anyMatch(JSType::matchesNumberContext, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::matchesNumberContext, alternates); } /** @@ -173,7 +136,7 @@ public boolean matchesNumberContext() { @Override public boolean matchesStringContext() { // TODO(user): Reverse this logic to make it correct instead of generous. - return anyMatch(JSType::matchesStringContext, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::matchesStringContext, alternates); } /** @@ -184,7 +147,7 @@ public boolean matchesStringContext() { @Override public boolean matchesSymbolContext() { // TODO(user): Reverse this logic to make it correct instead of generous. - return anyMatch(JSType::matchesSymbolContext, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::matchesSymbolContext, alternates); } /** @@ -206,14 +169,14 @@ public boolean matchesSymbolContext() { @Override public boolean matchesObjectContext() { // TODO(user): Reverse this logic to make it correct instead of generous. - return anyMatch(JSType::matchesObjectContext, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::matchesObjectContext, alternates); } @Override protected JSType findPropertyTypeWithoutConsideringTemplateTypes(String propertyName) { JSType propertyType = null; - for (JSType alternate : getAlternates()) { + for (JSType alternate : alternates) { // Filter out the null/undefined type. if (alternate.isNullType() || alternate.isVoidType()) { continue; @@ -236,14 +199,14 @@ protected JSType findPropertyTypeWithoutConsideringTemplateTypes(String property @Override public boolean canBeCalled() { - return allMatch(JSType::canBeCalled, alternatesRetainingStructuralSubtypes); + return allMatch(JSType::canBeCalled, alternates); } @Override public JSType autobox() { UnionTypeBuilder restricted = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType t = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType t = alternates.get(i); restricted.addAlternate(t.autobox()); } return restricted.build(); @@ -252,8 +215,8 @@ public JSType autobox() { @Override public JSType restrictByNotNullOrUndefined() { UnionTypeBuilder restricted = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType t = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType t = alternates.get(i); restricted.addAlternate(t.restrictByNotNullOrUndefined()); } return restricted.build(); @@ -262,7 +225,7 @@ public JSType restrictByNotNullOrUndefined() { @Override public JSType restrictByNotUndefined() { UnionTypeBuilder restricted = UnionTypeBuilder.create(registry); - for (JSType t : alternatesRetainingStructuralSubtypes) { + for (JSType t : alternates) { restricted.addAlternate(t.restrictByNotUndefined()); } return restricted.build(); @@ -271,8 +234,8 @@ public JSType restrictByNotUndefined() { @Override public TernaryValue testForEquality(JSType that) { TernaryValue result = null; - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType t = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType t = alternates.get(i); TernaryValue test = t.testForEquality(that); if (result == null) { result = test; @@ -293,7 +256,7 @@ public TernaryValue testForEquality(JSType that) { */ @Override public boolean isNullable() { - return anyMatch(JSType::isNullable, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::isNullable, alternates); } /** @@ -301,35 +264,35 @@ public boolean isNullable() { */ @Override public boolean isVoidable() { - return anyMatch(JSType::isVoidable, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::isVoidable, alternates); } /** Tests whether this type explicitly allows undefined (as opposed to ? or *). */ @Override public boolean isExplicitlyVoidable() { - return anyMatch(JSType::isExplicitlyVoidable, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::isExplicitlyVoidable, alternates); } @Override public boolean isUnknownType() { - return anyMatch(JSType::isUnknownType, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::isUnknownType, alternates); } @Override public boolean isStruct() { - return anyMatch(JSType::isStruct, getAlternates()); + return anyMatch(JSType::isStruct, alternates); } @Override public boolean isDict() { - return anyMatch(JSType::isDict, getAlternates()); + return anyMatch(JSType::isDict, alternates); } @Override public JSType getLeastSupertype(JSType that) { if (!that.isUnknownType() && !that.isUnionType()) { - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType alternate = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType alternate = alternates.get(i); if (!alternate.isUnknownType() && that.isSubtypeOf(alternate)) { return this; } @@ -341,18 +304,17 @@ public JSType getLeastSupertype(JSType that) { JSType meet(JSType that) { UnionTypeBuilder builder = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType alternate = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType alternate = alternates.get(i); if (alternate.isSubtypeOf(that)) { builder.addAlternate(alternate); } } if (that.isUnionType()) { - List thoseAlternatesWithoutStucturalTyping = - that.toMaybeUnionType().alternatesRetainingStructuralSubtypes; - for (int i = 0; i < thoseAlternatesWithoutStucturalTyping.size(); i++) { - JSType otherAlternate = thoseAlternatesWithoutStucturalTyping.get(i); + List thoseAlternates = that.toMaybeUnionType().getAlternates(); + for (int i = 0; i < thoseAlternates.size(); i++) { + JSType otherAlternate = thoseAlternates.get(i); if (otherAlternate.isSubtypeOf(this)) { builder.addAlternate(otherAlternate); } @@ -372,13 +334,11 @@ JSType meet(JSType that) { /** * Two union types are equal if, after flattening nested union types, they have the same number of - * alternatesCollapsingStructuralSubtypes and all alternatesCollapsingStructuralSubtypes are - * equal. + * alternates and all alternates are equal. */ boolean checkUnionEquivalenceHelper(UnionType that, EquivalenceMethod eqMethod, EqCache eqCache) { - List thatAlternates = that.getAlternatesWithoutStructuralTyping(); - if (eqMethod == EquivalenceMethod.IDENTITY - && getAlternatesWithoutStructuralTyping().size() != thatAlternates.size()) { + List thatAlternates = that.getAlternates(); + if (eqMethod == EquivalenceMethod.IDENTITY && alternates.size() != thatAlternates.size()) { return false; } for (int i = 0; i < thatAlternates.size(); i++) { @@ -390,11 +350,9 @@ && getAlternatesWithoutStructuralTyping().size() != thatAlternates.size()) { return true; } - private boolean hasAlternate(JSType type, EquivalenceMethod eqMethod, - EqCache eqCache) { - List alternatesRetainingStructuralSubtypes = getAlternatesWithoutStructuralTyping(); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType alternate = alternatesRetainingStructuralSubtypes.get(i); + private boolean hasAlternate(JSType type, EquivalenceMethod eqMethod, EqCache eqCache) { + for (int i = 0; i < alternates.size(); i++) { + JSType alternate = alternates.get(i); if (alternate.checkEquivalenceHelper(type, eqMethod, eqCache)) { return true; } @@ -406,8 +364,8 @@ private boolean hasAlternate(JSType type, EquivalenceMethod eqMethod, public HasPropertyKind getPropertyKind(String pname, boolean autobox) { boolean found = false; boolean always = true; - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType alternate = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType alternate = alternates.get(i); if (alternate.isNullType() || alternate.isVoidType()) { continue; } @@ -434,12 +392,12 @@ public HasPropertyKind getPropertyKind(String pname, boolean autobox) { @Override final int recursionUnsafeHashCode() { - int hashCode = alternatesRetainingStructuralSubtypes.size(); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { + int hashCode = alternates.size(); + for (int i = 0; i < alternates.size(); i++) { // To be determinisitic this aggregation must be order-independent. Using a commutative // operatator (multiplication) allows us to achieve that without sorting. Multiplication also // has some nice properties about reducing collisions compared to addition or xor. - hashCode *= alternatesRetainingStructuralSubtypes.get(i).hashCode(); + hashCode *= alternates.get(i).hashCode(); } return hashCode; } @@ -451,7 +409,7 @@ public UnionType toMaybeUnionType() { @Override public boolean isObject() { - return allMatch(JSType::isObject, alternatesRetainingStructuralSubtypes); + return allMatch(JSType::isObject, alternates); } /** @@ -463,7 +421,7 @@ public boolean isObject() { * @return {@code true} if the alternate is in the union */ public boolean contains(JSType type) { - return anyMatch(type::isEquivalentTo, alternatesRetainingStructuralSubtypes); + return anyMatch(type::isEquivalentTo, alternates); } /** @@ -482,8 +440,8 @@ public boolean contains(JSType type) { */ public JSType getRestrictedUnion(JSType type) { UnionTypeBuilder restricted = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType t = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType t = alternates.get(i); // Keep all unknown/unresolved types. if (t.isUnknownType() || t.isNoResolvedType() || !t.isSubtypeOf(type)) { restricted.addAlternate(t); @@ -498,7 +456,7 @@ StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) { // Sort types in character value order in order to get consistent results. // This is important for deterministic behavior for testing. SortedSet sortedTypeNames = new TreeSet<>(); - for (JSType jsType : alternatesRetainingStructuralSubtypes) { + for (JSType jsType : alternates) { sortedTypeNames.add(jsType.appendTo(new StringBuilder(), forAnnotations).toString()); } Joiner.on('|').appendTo(sb, sortedTypeNames); @@ -521,8 +479,8 @@ protected boolean isSubtype(JSType that, if (that.isAllType()) { return true; } - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType element = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType element = alternates.get(i); if (subtypingMode == SubtypingMode.IGNORE_NULL_UNDEFINED && (element.isNullType() || element.isVoidType())) { continue; @@ -538,8 +496,8 @@ protected boolean isSubtype(JSType that, public JSType getRestrictedTypeGivenToBooleanOutcome(boolean outcome) { // gather elements after restriction UnionTypeBuilder restricted = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType element = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType element = alternates.get(i); restricted.addAlternate( element.getRestrictedTypeGivenToBooleanOutcome(outcome)); } @@ -549,8 +507,8 @@ public JSType getRestrictedTypeGivenToBooleanOutcome(boolean outcome) { @Override public BooleanLiteralSet getPossibleToBooleanOutcomes() { BooleanLiteralSet literals = BooleanLiteralSet.EMPTY; - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType element = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType element = alternates.get(i); literals = literals.union(element.getPossibleToBooleanOutcomes()); if (literals == BooleanLiteralSet.BOTH) { break; @@ -563,8 +521,8 @@ public BooleanLiteralSet getPossibleToBooleanOutcomes() { public TypePair getTypesUnderEquality(JSType that) { UnionTypeBuilder thisRestricted = UnionTypeBuilder.create(registry); UnionTypeBuilder thatRestricted = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType element = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType element = alternates.get(i); TypePair p = element.getTypesUnderEquality(that); if (p.typeA != null) { thisRestricted.addAlternate(p.typeA); @@ -582,8 +540,8 @@ public TypePair getTypesUnderEquality(JSType that) { public TypePair getTypesUnderInequality(JSType that) { UnionTypeBuilder thisRestricted = UnionTypeBuilder.create(registry); UnionTypeBuilder thatRestricted = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType element = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType element = alternates.get(i); TypePair p = element.getTypesUnderInequality(that); if (p.typeA != null) { thisRestricted.addAlternate(p.typeA); @@ -601,8 +559,8 @@ public TypePair getTypesUnderInequality(JSType that) { public TypePair getTypesUnderShallowInequality(JSType that) { UnionTypeBuilder thisRestricted = UnionTypeBuilder.create(registry); UnionTypeBuilder thatRestricted = UnionTypeBuilder.create(registry); - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType element = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType element = alternates.get(i); TypePair p = element.getTypesUnderShallowInequality(that); if (p.typeA != null) { thisRestricted.addAlternate(p.typeA); @@ -629,8 +587,8 @@ public T visit(Visitor visitor) { JSType resolveInternal(ErrorReporter reporter) { setResolvedTypeInternal(this); // for circularly defined types. - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType alternate = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType alternate = alternates.get(i); alternate.resolve(reporter); } // Ensure the union is in a normalized state. @@ -641,7 +599,7 @@ JSType resolveInternal(ErrorReporter reporter) { @Override public String toDebugHashCodeString() { List hashCodes = new ArrayList<>(); - for (JSType a : alternatesRetainingStructuralSubtypes) { + for (JSType a : alternates) { hashCodes.add(a.toDebugHashCodeString()); } return "{(" + Joiner.on(",").join(hashCodes) + ")}"; @@ -649,8 +607,8 @@ public String toDebugHashCodeString() { @Override public boolean setValidator(Predicate validator) { - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType a = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType a = alternates.get(i); a.setValidator(validator); } return true; @@ -660,8 +618,8 @@ public boolean setValidator(Predicate validator) { public JSType collapseUnion() { JSType currentValue = null; ObjectType currentCommonSuper = null; - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType a = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType a = alternates.get(i); if (a.isUnknownType()) { return getNativeType(JSTypeNative.UNKNOWN_TYPE); } @@ -690,15 +648,15 @@ public JSType collapseUnion() { @Override public void matchConstraint(JSType constraint) { - for (int i = 0; i < alternatesRetainingStructuralSubtypes.size(); i++) { - JSType alternate = alternatesRetainingStructuralSubtypes.get(i); + for (int i = 0; i < alternates.size(); i++) { + JSType alternate = alternates.get(i); alternate.matchConstraint(constraint); } } @Override public boolean hasAnyTemplateTypesInternal() { - return anyMatch(JSType::hasAnyTemplateTypes, alternatesRetainingStructuralSubtypes); + return anyMatch(JSType::hasAnyTemplateTypes, alternates); } /** diff --git a/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java b/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java index 776847fb360..6041f6ba413 100644 --- a/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java +++ b/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java @@ -80,7 +80,6 @@ public final class UnionTypeBuilder implements Serializable { private static final int PROPERTY_CHECKING_MAX_UNION_SIZE = 3000; private final JSTypeRegistry registry; - private final boolean structuralSubtypesAreCollapsed; private final int maxUnionSize; private final List alternates = new ArrayList<>(); @@ -115,24 +114,17 @@ public final class UnionTypeBuilder implements Serializable { private JSType result = null; public static UnionTypeBuilder create(JSTypeRegistry registry) { - return new UnionTypeBuilder(registry, DEFAULT_MAX_UNION_SIZE, false); + return new UnionTypeBuilder(registry, DEFAULT_MAX_UNION_SIZE); } // This is only supposed to be used within `JSTypeRegistry`. static UnionTypeBuilder createForPropertyChecking(JSTypeRegistry registry) { - return new UnionTypeBuilder(registry, PROPERTY_CHECKING_MAX_UNION_SIZE, false); + return new UnionTypeBuilder(registry, PROPERTY_CHECKING_MAX_UNION_SIZE); } - // This is only supposed to be used within `UnionType`. - static UnionTypeBuilder createForCollapsingStructuralSubtypes(JSTypeRegistry registry) { - return new UnionTypeBuilder(registry, DEFAULT_MAX_UNION_SIZE, true); - } - - private UnionTypeBuilder( - JSTypeRegistry registry, int maxUnionSize, boolean structuralSubtypesAreCollapsed) { + private UnionTypeBuilder(JSTypeRegistry registry, int maxUnionSize) { this.registry = registry; this.maxUnionSize = maxUnionSize; - this.structuralSubtypesAreCollapsed = structuralSubtypesAreCollapsed; } ImmutableList getAlternates() { @@ -155,17 +147,7 @@ int getAlternatesCount() { } private boolean isSubtype(JSType rightType, JSType leftType) { - // if thisType or thatType is an unresolved templatized type, - // then there is no structural interface matching - boolean thisUnresolved = rightType.isTemplatizedType() - && !rightType.toMaybeTemplatizedType().isResolved(); - boolean thatUnresolved = leftType.isTemplatizedType() - && !leftType.toMaybeTemplatizedType().isResolved(); - if (structuralSubtypesAreCollapsed && !thisUnresolved && !thatUnresolved) { - return rightType.isSubtypeOf(leftType); - } else { - return rightType.isSubtypeWithoutStructuralTyping(leftType); - } + return rightType.isSubtypeWithoutStructuralTyping(leftType); } public UnionTypeBuilder addAlternates(Collection c) { @@ -175,13 +157,20 @@ public UnionTypeBuilder addAlternates(Collection c) { return this; } + public UnionTypeBuilder addAlternates(List list) { + for (int i = 0; i < list.size(); i++) { + addAlternate(list.get(i)); + } + return this; + } + /** * Adds an alternate to the union type under construction. * *

Returns this for easy chaining. * *

TODO(nickreid): This method shouldn't eagerly collapse types. Doing so risks holding onto a - * nested union if an alternate resolves so a union after being added. We should do use some + * nested union if an alternate resolves to a union after being added. We should do use some * technique to determine if the builder has become dirty and caching the result of `build()` * until then. */ @@ -198,18 +187,11 @@ public UnionTypeBuilder addAlternate(JSType alternate) { boolean isAlternateUnknown = alternate instanceof UnknownType; isNativeUnknownType = isNativeUnknownType || isAlternateUnknown; if (isAlternateUnknown) { - areAllUnknownsChecked = areAllUnknownsChecked && - alternate.isCheckedUnknownType(); + areAllUnknownsChecked = areAllUnknownsChecked && alternate.isCheckedUnknownType(); } if (!isAllType && !isNativeUnknownType) { if (alternate.isUnionType()) { - UnionType union = alternate.toMaybeUnionType(); - List alternatesWithoutStructuralTyping = - union.getAlternatesWithoutStructuralTyping(); - for (int i = 0; i < alternatesWithoutStructuralTyping.size(); i++) { - JSType unionAlt = alternatesWithoutStructuralTyping.get(i); - addAlternate(unionAlt); - } + addAlternates(alternate.toMaybeUnionType().getAlternates()); } else { if (alternates.size() > maxUnionSize) { return this; @@ -241,23 +223,23 @@ public UnionTypeBuilder addAlternate(JSType alternate) { // Unknown and NoResolved types may just be names that haven't // been resolved yet. So keep these in the union, and just use // equality checking for simple de-duping. - if (alternate.isUnknownType() || - current.isUnknownType() || - alternate.isNoResolvedType() || - current.isNoResolvedType() || - alternate.hasAnyTemplateTypes() || - current.hasAnyTemplateTypes()) { - if (alternate.isEquivalentTo(current, structuralSubtypesAreCollapsed)) { + if (alternate.isUnknownType() + || current.isUnknownType() + || alternate.isNoResolvedType() + || current.isNoResolvedType() + || alternate.hasAnyTemplateTypes() + || current.hasAnyTemplateTypes()) { + if (alternate.isEquivalentTo(current, false)) { // Alternate is unnecessary. return this; } } else { - // Because "Foo" and "Foo." are roughly equivalent + // Because "Foo" and "Foo" are roughly equivalent // templatized types, special care is needed when building the // union. For example: - // Object is consider a subtype of Object. - // but we want to leave "Object" not "Object." when + // Object is consider a subtype of Object + // but we want to leave "Object" not "Object" when // building the subtype. // @@ -265,7 +247,7 @@ public UnionTypeBuilder addAlternate(JSType alternate) { // Cases: // 1) alternate:Array and current:Object ==> Object // 2) alternate:Array and current:Array ==> Array - // 3) alternate:Object. and + // 3) alternate:Object and // current:Array ==> Array|Object // 4) alternate:Object and current:Array ==> Object // 5) alternate:Array and current:Array ==> Array