Skip to content

Commit

Permalink
Convert UnionTypeBuilder into UnionType.Builder, a nested class o…
Browse files Browse the repository at this point in the history 
…f `UnionType`.

This structure better reflects the entangled nature of the two classes, as well as facilitates an upcoming fix for cyclic union resolution.

Some refactoring of affected code is included to simplify the move.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=259079043
  • Loading branch information
@nreid260 @lauraharker
nreid260 authored and lauraharker committed Jul 22, 2019
1 parent 7a91459 commit cfc0fab
Show file tree
Hide file tree
Showing 10 changed files with 515 additions and 508 deletions.
46 changes: 27 additions & 19 deletions src/com/google/javascript/jscomp/Es6ForOfConverter.java
View file
Expand Up @@ -29,7 +29,7 @@
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.UnionTypeBuilder;
import com.google.javascript.rhino.jstype.UnionType;

/**
* Converts ES6 "for of" loops to ES5.
Expand All @@ -45,6 +45,7 @@ public final class Es6ForOfConverter implements NodeTraversal.Callback, HotSwapC
private final JSType unknownType;
private final JSType stringType;
private final JSType booleanType;
private final JSType makeIteratorTypeArg;
private final DefaultNameGenerator namer;

private static final String ITER_BASE = "$jscomp$iter$";
Expand All @@ -59,7 +60,8 @@ public Es6ForOfConverter(AbstractCompiler compiler) {
this.unknownType = createType(addTypes, registry, JSTypeNative.UNKNOWN_TYPE);
this.stringType = createType(addTypes, registry, JSTypeNative.STRING_TYPE);
this.booleanType = createType(addTypes, registry, JSTypeNative.BOOLEAN_TYPE);
namer = new DefaultNameGenerator();
this.makeIteratorTypeArg = createMakeIteratorTypeArg();
this.namer = new DefaultNameGenerator();
}

@Override
Expand Down Expand Up @@ -141,25 +143,9 @@ private void visitForOf(Node node, Node parent) {

Node call = Es6ToEs3Util.makeIterator(compiler, iterable);
if (addTypes) {
// Create the function type for $jscomp.makeIterator.
// Build "@param {string|!Iterable<T>|!Iterator<T>|!Arguments<T>}"
UnionTypeBuilder paramBuilder =
UnionTypeBuilder.create(registry)
.addAlternate(registry.getNativeType(JSTypeNative.STRING_TYPE))
.addAlternate(registry.getNativeType(JSTypeNative.ITERATOR_TYPE))
.addAlternate(registry.getNativeType(JSTypeNative.ITERABLE_TYPE));
JSType argumentsType = registry.getGlobalType("Arguments");
// If the user didn't provide externs for Arguments, let TypeCheck take care of issuing a
// warning.
if (argumentsType != null) {
paramBuilder.addAlternate(argumentsType);
}
FunctionType makeIteratorType =
registry.createFunctionType(iteratorType, paramBuilder.build());

// Put types on the $jscomp.makeIterator getprop
Node getProp = call.getFirstChild();
getProp.setJSType(makeIteratorType);
getProp.setJSType(registry.createFunctionType(iteratorType, makeIteratorTypeArg));
// typing $jscomp as unknown since the $jscomp polyfill may not be injected before
// typechecking. (See https://github.com/google/closure-compiler/issues/2908)
getProp.getFirstChild().setJSType(registry.getNativeType(JSTypeNative.UNKNOWN_TYPE));
Expand Down Expand Up @@ -224,4 +210,26 @@ private Node withStringType(Node n) {
private Node withBooleanType(Node n) {
return withType(n, booleanType);
}

/**
* Create the function type for $jscomp.makeIterator.
*
* <p>Build "{string|!Iterable<T>|!Iterator<T>|!Arguments<T>}"
*/
private JSType createMakeIteratorTypeArg() {
UnionType.Builder builder =
UnionType.builder(registry)
.addAlternate(registry.getNativeType(JSTypeNative.STRING_TYPE))
.addAlternate(registry.getNativeType(JSTypeNative.ITERATOR_TYPE))
.addAlternate(registry.getNativeType(JSTypeNative.ITERABLE_TYPE));

// If the user didn't provide externs for Arguments, let TypeCheck take care of issuing a
// warning.
JSType argumentsType = registry.getGlobalType("Arguments");
if (argumentsType != null) {
builder.addAlternate(argumentsType);
}

return builder.build();
}
}
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/Promises.java
View file
Expand Up @@ -24,7 +24,7 @@
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;
import com.google.javascript.rhino.jstype.UnionType;

/**
* Models different Javascript Promise-related operations
Expand Down Expand Up @@ -73,7 +73,7 @@ static final JSType getResolvedType(JSTypeRegistry registry, JSType type) {
}

if (type.isUnionType()) {
UnionTypeBuilder unionTypeBuilder = UnionTypeBuilder.create(registry);
UnionType.Builder unionTypeBuilder = UnionType.builder(registry);
for (JSType alternate : type.toMaybeUnionType().getAlternates()) {
unionTypeBuilder.addAlternate(getResolvedType(registry, alternate));
}
Expand Down
5 changes: 3 additions & 2 deletions src/com/google/javascript/rhino/jstype/JSType.java
View file
Expand Up @@ -1273,8 +1273,9 @@ static JSType filterNoResolvedType(JSType type) {
}

if (needsFiltering) {
UnionTypeBuilder builder = UnionTypeBuilder.create(type.registry);
builder.addAlternate(type.getNativeType(JSTypeNative.NO_RESOLVED_TYPE));
UnionType.Builder builder =
UnionType.builder(type.registry)
.addAlternate(type.getNativeType(JSTypeNative.NO_RESOLVED_TYPE));
for (int i = 0; i < alternatesList.size(); i++) {
JSType alt = alternatesList.get(i);
if (!alt.isNoResolvedType()) {
Expand Down
26 changes: 10 additions & 16 deletions src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
View file
Expand Up @@ -954,7 +954,7 @@ public JSType getGreatestSubtypeWithProperty(JSType type, String propertyName) {
if (typesIndexedByProperty.containsKey(propertyName)) {
Collection<JSType> typesWithProp = typesIndexedByProperty.get(propertyName);
JSType built =
UnionTypeBuilder.createForPropertyChecking(this).addAlternates(typesWithProp).build();
UnionType.builderForPropertyChecking(this).addAlternates(typesWithProp).build();
greatestSubtypeByProperty.put(propertyName, built);
return built.getGreatestSubtype(type);
}
Expand Down Expand Up @@ -1490,25 +1490,21 @@ public JSType createOptionalNullableType(JSType type) {
* Creates a union type whose variants are the arguments.
*/
public JSType createUnionType(JSType... variants) {
UnionTypeBuilder builder = UnionTypeBuilder.create(this);
for (JSType type : variants) {
builder.addAlternate(type);
}
return builder.build();
return createUnionType(ImmutableList.copyOf(variants));
}

public JSType createUnionType(List<? extends JSType> variants) {
return createUnionType(variants.toArray(new JSType[0]));
return UnionType.builder(this).addAlternates(variants).build();
}

/**
* Creates a union type whose variants are the built-in types specified
* by the arguments.
*/
public JSType createUnionType(JSTypeNative... variants) {
UnionTypeBuilder builder = UnionTypeBuilder.create(this);
for (JSTypeNative typeId : variants) {
builder.addAlternate(getNativeType(typeId));
UnionType.Builder builder = UnionType.builder(this);
for (JSTypeNative type : variants) {
builder.addAlternate(getNativeType(type));
}
return builder.build();
}
Expand Down Expand Up @@ -1990,13 +1986,11 @@ private JSType createFromTypeNodesInternal(
return getNativeType(ALL_TYPE);

case PIPE: // Union type
UnionTypeBuilder builder = UnionTypeBuilder.create(this);
for (Node child = n.getFirstChild(); child != null;
child = child.getNext()) {
builder.addAlternate(
createFromTypeNodesInternal(child, sourceName, scope, recordUnresolvedTypes));
ImmutableList.Builder<JSType> builder = ImmutableList.builder();
for (Node child = n.getFirstChild(); child != null; child = child.getNext()) {
builder.add(createFromTypeNodesInternal(child, sourceName, scope, recordUnresolvedTypes));
}
return builder.build();
return createUnionType(builder.build());

case EMPTY: // When the return value of a function is not specified
return getNativeType(UNKNOWN_TYPE);
Expand Down
6 changes: 1 addition & 5 deletions src/com/google/javascript/rhino/jstype/ModificationVisitor.java
View file
Expand Up @@ -251,11 +251,7 @@ public JSType caseUnionType(UnionType type) {
}

if (changed) {
UnionTypeBuilder builder = UnionTypeBuilder.create(registry);
for (JSType alternate : results) {
builder.addAlternate(alternate);
}
return builder.build(); // maybe not a union
return registry.createUnionType(results); // maybe not a union
}

return type;
Expand Down
11 changes: 6 additions & 5 deletions src/com/google/javascript/rhino/jstype/RecordType.java
View file
Expand Up @@ -170,12 +170,13 @@ JSType getGreatestSubtypeHelper(JSType that) {
// 2) Take the intersection of all of these unions.
for (String propName : getOwnPropertyNames()) {
JSType propType = getPropertyType(propName);
UnionTypeBuilder builder = UnionTypeBuilder.create(registry);
for (ObjectType alt :
registry.getEachReferenceTypeWithProperty(propName)) {
UnionType.Builder builder = UnionType.builder(registry);
for (ObjectType alt : registry.getEachReferenceTypeWithProperty(propName)) {
JSType altPropType = alt.getPropertyType(propName);
if (altPropType != null && !alt.isEquivalentTo(this)
&& alt.isSubtypeOf(that) && altPropType.isSubtypeOf(propType)) {
if (altPropType != null
&& !alt.isEquivalentTo(this)
&& alt.isSubtypeOf(that)
&& altPropType.isSubtypeOf(propType)) {
builder.addAlternate(alt);
}
}
Expand Down

0 comments on commit cfc0fab

Please sign in to comment.