Skip to content

Commit

Permalink
Move TemplateTypeMap creation to use atomic mutations of existing m…
Browse files Browse the repository at this point in the history 
…aps rather than exposing a constructor.

This pattern allows us to better describe the intent of each map creation. That information can be leveraged in an upcoming CL to simplify and improve the correctness of creating maps. The updated design also allows us to enforce some invariants of maps that were being ignored prior.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=263956152
  • Loading branch information
@nreid260 @lauraharker
nreid260 authored and lauraharker committed Aug 19, 2019
1 parent 105b62d commit 236f9ed
Show file tree
Hide file tree
Showing 13 changed files with 495 additions and 208 deletions.
35 changes: 11 additions & 24 deletions src/com/google/javascript/jscomp/AstFactory.java
View file
Expand Up @@ -803,12 +803,7 @@ Node createJSCompMakeIteratorCall(Node iterable, Scope scope) {
// function(Iterable<T>): Iterator<T>
// with
// function(Iterable<number>): Iterator<number>
TemplateTypeMap typeMap =
registry.createTemplateTypeMap(
makeIteratorType.getTemplateTypeMap().getTemplateKeys(),
ImmutableList.of(iterableType));
TemplateTypeReplacer replacer = TemplateTypeReplacer.forPartialReplacement(registry, typeMap);
makeIteratorName.setJSType(makeIteratorType.visit(replacer));
makeIteratorName.setJSType(replaceTemplate(makeIteratorType, ImmutableList.of(iterableType)));
}
return createCall(makeIteratorName, iterable);
}
Expand All @@ -832,12 +827,7 @@ Node createJscompArrayFromIteratorCall(Node iterator, Scope scope) {
// function(Iterator<T>): Array<T>
// with
// function(Iterator<number>): Array<number>
TemplateTypeMap typeMap =
registry.createTemplateTypeMap(
makeIteratorType.getTemplateTypeMap().getTemplateKeys(),
ImmutableList.of(iterableType));
TemplateTypeReplacer replacer = TemplateTypeReplacer.forPartialReplacement(registry, typeMap);
makeIteratorName.setJSType(makeIteratorType.visit(replacer));
makeIteratorName.setJSType(replaceTemplate(makeIteratorType, ImmutableList.of(iterableType)));
}
return createCall(makeIteratorName, iterator);
}
Expand Down Expand Up @@ -872,21 +862,17 @@ Node createJSCompMakeAsyncIteratorCall(Node iterable, Scope scope) {
// function(AsyncIterable<T>): AsyncIterator<T>
// with
// function(AsyncIterable<number>): AsyncIterator<number>
TemplateTypeMap typeMap =
registry.createTemplateTypeMap(
makeAsyncIteratorType.getTemplateTypeMap().getTemplateKeys(),
ImmutableList.of(asyncIterableType));
TemplateTypeReplacer replacer = TemplateTypeReplacer.forPartialReplacement(registry, typeMap);
makeIteratorAsyncName.setJSType(makeAsyncIteratorType.visit(replacer));
makeIteratorAsyncName.setJSType(
replaceTemplate(makeAsyncIteratorType, ImmutableList.of(asyncIterableType)));
}
return createCall(makeIteratorAsyncName, iterable);
}

private JSType replaceTemplate(JSType templatedType, JSType... templateTypes) {
private JSType replaceTemplate(JSType templatedType, ImmutableList<JSType> templateTypes) {
TemplateTypeMap typeMap =
registry.createTemplateTypeMap(
templatedType.getTemplateTypeMap().getTemplateKeys(),
ImmutableList.copyOf(templateTypes));
registry
.getEmptyTemplateTypeMap()
.copyWithExtension(templatedType.getTemplateTypeMap().getTemplateKeys(), templateTypes);
TemplateTypeReplacer replacer = TemplateTypeReplacer.forPartialReplacement(registry, typeMap);
return templatedType.visit(replacer);
}
Expand Down Expand Up @@ -915,7 +901,7 @@ Node createAsyncGeneratorWrapperReference(JSType originalFunctionType, Scope sco
// AsyncGeneratorWrapper<T>
// with
// AsyncGeneratorWrapper<number>
ctor.setJSType(replaceTemplate(ctor.getJSType(), yieldedType));
ctor.setJSType(replaceTemplate(ctor.getJSType(), ImmutableList.of(yieldedType)));
}

return ctor;
Expand All @@ -937,7 +923,8 @@ Node createEmptyAsyncGeneratorWrapperArgument(JSType asyncGeneratorWrapperType)
// Not injecting libraries?
generatorType =
registry.createFunctionType(
replaceTemplate(getNativeType(JSTypeNative.GENERATOR_TYPE), unknownType));
replaceTemplate(
getNativeType(JSTypeNative.GENERATOR_TYPE), ImmutableList.of(unknownType)));
} else {
// Generator<$jscomp.AsyncGeneratorWrapper$ActionRecord<number>>
JSType innerFunctionReturnType =
Expand Down
8 changes: 5 additions & 3 deletions src/com/google/javascript/jscomp/FunctionTypeBuilder.java
View file
Expand Up @@ -975,10 +975,12 @@ FunctionType buildAndRegister() {
}

private void maybeSetBaseType(FunctionType fnType) {
if (fnType.hasInstanceType() && baseType != null) {
fnType.setPrototypeBasedOn(baseType);
fnType.extendTemplateTypeMapBasedOn(baseType);
if (!fnType.hasInstanceType() || baseType == null) {
return;
}

fnType.setPrototypeBasedOn(baseType);
fnType.getInstanceType().prependTemplateTypeMap(baseType.getTemplateTypeMap());
}

/**
Expand Down
52 changes: 15 additions & 37 deletions src/com/google/javascript/rhino/jstype/FunctionType.java
View file
Expand Up @@ -493,15 +493,6 @@ final void setPrototypeBasedOn(ObjectType baseType, Node propertyNode) {
setPrototype(baseType, propertyNode);
}

/**
* Extends the TemplateTypeMap of the function's this type, based on the specified type.
*
* @param type
*/
public final void extendTemplateTypeMapBasedOn(ObjectType type) {
typeOfThis.extendTemplateTypeMap(type.getTemplateTypeMap());
}

/**
* Sets the prototype.
*
Expand Down Expand Up @@ -620,15 +611,12 @@ public final ImmutableList<ObjectType> getOwnImplementedInterfaces() {
}

public final void setImplementedInterfaces(List<ObjectType> implementedInterfaces) {
if (isConstructor()) {
// Records this type for each implemented interface.
for (ObjectType type : implementedInterfaces) {
registry.registerTypeImplementingInterface(this, type);
typeOfThis.extendTemplateTypeMap(type.getTemplateTypeMap());
}
this.implementedInterfaces = ImmutableList.copyOf(implementedInterfaces);
} else {
throw new UnsupportedOperationException("An interface cannot implement other inferfaces");
checkState(isConstructor());

this.implementedInterfaces = ImmutableList.copyOf(implementedInterfaces);
for (ObjectType type : implementedInterfaces) {
registry.registerTypeImplementingInterface(this, type);
typeOfThis.prependTemplateTypeMap(type.getTemplateTypeMap());
}
}

Expand All @@ -643,13 +631,11 @@ public final int getExtendedInterfacesCount() {
}

public final void setExtendedInterfaces(List<ObjectType> extendedInterfaces) {
if (isInterface()) {
this.extendedInterfaces = ImmutableList.copyOf(extendedInterfaces);
for (ObjectType extendedInterface : this.extendedInterfaces) {
typeOfThis.extendTemplateTypeMap(extendedInterface.getTemplateTypeMap());
}
} else {
throw new UnsupportedOperationException();
checkState(isInterface());

this.extendedInterfaces = ImmutableList.copyOf(extendedInterfaces);
for (ObjectType extendedInterface : extendedInterfaces) {
typeOfThis.prependTemplateTypeMap(extendedInterface.getTemplateTypeMap());
}
}

Expand Down Expand Up @@ -1655,22 +1641,14 @@ public Builder withTypeOfThis(JSType typeOfThis) {

/** Set the template name. */
public Builder withTemplateKeys(ImmutableList<TemplateType> templateKeys) {
this.templateTypeMap = registry.createTemplateTypeMap(templateKeys, null);
this.templateTypeMap =
registry.getEmptyTemplateTypeMap().copyWithExtension(templateKeys, ImmutableList.of());
return this;
}

/** Set the template name. */
public Builder withTemplateKeys(TemplateType... templateKeys) {
this.templateTypeMap =
registry.createTemplateTypeMap(ImmutableList.copyOf(templateKeys), null);
return this;
}

Builder withExtendedTemplate(TemplateType key, JSType value) {
this.templateTypeMap =
templateTypeMap.extend(
registry.createTemplateTypeMap(ImmutableList.of(key), ImmutableList.of(value)));
return this;
return withTemplateKeys(ImmutableList.copyOf(templateKeys));
}

Builder withTemplateTypeMap(TemplateTypeMap templateTypeMap) {
Expand Down Expand Up @@ -1790,7 +1768,7 @@ public FunctionType build() {
if (hasConstructorOnlyKeys) {
ft.setInstanceType(
new InstanceObjectType(
registry, ft, isNative, templateTypeMap.remove(constructorOnlyKeys)));
registry, ft, isNative, templateTypeMap.copyWithoutKeys(constructorOnlyKeys)));
}
return ft;
}
Expand Down
12 changes: 6 additions & 6 deletions src/com/google/javascript/rhino/jstype/JSType.java
View file
Expand Up @@ -103,8 +103,8 @@ static final boolean areIdentical(JSType a, JSType b) {
JSType(JSTypeRegistry registry, TemplateTypeMap templateTypeMap) {
this.registry = registry;

this.templateTypeMap = templateTypeMap == null ?
registry.createTemplateTypeMap(null, null) : templateTypeMap;
this.templateTypeMap =
(templateTypeMap == null) ? registry.getEmptyTemplateTypeMap() : templateTypeMap;
}

/**
Expand Down Expand Up @@ -543,11 +543,11 @@ public final ImmutableSet<JSType> getTypeParameters() {
}

/**
* Extends the template type map associated with this type, merging in the
* keys and values of the specified map.
* Prepends the template type map associated with this type, merging in the keys and values of the
* specified map.
*/
public void extendTemplateTypeMap(TemplateTypeMap otherMap) {
templateTypeMap = templateTypeMap.extend(otherMap);
public void prependTemplateTypeMap(TemplateTypeMap otherMap) {
templateTypeMap = otherMap.copyWithExtension(templateTypeMap);
}

/**
Expand Down
48 changes: 16 additions & 32 deletions src/com/google/javascript/rhino/jstype/JSTypeRegistry.java
View file
Expand Up @@ -234,9 +234,9 @@ public JSTypeRegistry(ErrorReporter reporter) {
public JSTypeRegistry(ErrorReporter reporter, Set<String> forwardDeclaredTypes) {
this.reporter = reporter;
this.forwardDeclaredTypes = forwardDeclaredTypes;
this.emptyTemplateTypeMap = new TemplateTypeMap(
this, ImmutableList.<TemplateType>of(), ImmutableList.<JSType>of());
nativeTypes = new JSType[JSTypeNative.values().length];
this.emptyTemplateTypeMap = TemplateTypeMap.createEmpty(this);
this.nativeTypes = new JSType[JSTypeNative.values().length];

resetForTypeCheck();
}

Expand Down Expand Up @@ -372,7 +372,8 @@ private void initializeBuiltInTypes() {
FunctionType iObjectFunctionType =
nativeInterface("IObject", iObjectIndexTemplateKey, iObjectElementTemplateKey);
registerNativeType(JSTypeNative.I_OBJECT_FUNCTION_TYPE, iObjectFunctionType);
registerNativeType(JSTypeNative.I_OBJECT_TYPE, iObjectFunctionType.getInstanceType());
ObjectType iObjectType = iObjectFunctionType.getInstanceType();
registerNativeType(JSTypeNative.I_OBJECT_TYPE, iObjectType);

// Object
FunctionType objectFunctionType =
Expand Down Expand Up @@ -428,8 +429,9 @@ private void initializeBuiltInTypes() {
registerNativeType(JSTypeNative.I_ITERABLE_RESULT_TYPE, iiterableResultType);

// IArrayLike.
// TODO(lharker): Should the native Array implement IArrayLike?
FunctionType iArrayLikeFunctionType = nativeRecord("IArrayLike", iArrayLikeTemplate);
iArrayLikeFunctionType.setExtendedInterfaces(
ImmutableList.of(createTemplatizedType(iObjectType, numberType, iArrayLikeTemplate)));
registerNativeType(JSTypeNative.I_ARRAY_LIKE_FUNCTION_TYPE, iArrayLikeFunctionType);
ObjectType iArrayLikeType = iArrayLikeFunctionType.getInstanceType();
registerNativeType(JSTypeNative.I_ARRAY_LIKE_TYPE, iArrayLikeType);
Expand All @@ -439,16 +441,13 @@ private void initializeBuiltInTypes() {
nativeConstructorBuilder("Array")
.withParamsNode(createParametersWithVarArgs(allType))
.withReturnsOwnInstanceType()
// TODO(nickreid): Could this be achieved by having `Array` implement `IObject`?
.withTemplateTypeMap(
new TemplateTypeMap(
this,
ImmutableList.of(iObjectElementTemplateKey, arrayElementTemplateKey),
ImmutableList.<JSType>of(arrayElementTemplateKey)))
.withTemplateKeys(arrayElementTemplateKey)
.build();
arrayFunctionType.getPrototype(); // Force initialization
arrayFunctionType.setImplementedInterfaces(
ImmutableList.of(createTemplatizedType(iterableType, arrayElementTemplateKey)));
ImmutableList.of(
createTemplatizedType(iArrayLikeType, arrayElementTemplateKey),
createTemplatizedType(iterableType, arrayElementTemplateKey)));
registerNativeType(JSTypeNative.ARRAY_FUNCTION_TYPE, arrayFunctionType);

ObjectType arrayType = arrayFunctionType.getInstanceType();
Expand Down Expand Up @@ -523,7 +522,6 @@ private void initializeBuiltInTypes() {
nativeConstructorBuilder("Promise")
.withParamsNode(IR.paramList(promiseParameter))
.withTemplateKeys(promiseTemplateKey)
.withExtendedTemplate(iThenableTemplateKey, promiseTemplateKey)
.build();
promiseFunctionType.setImplementedInterfaces(
ImmutableList.of(createTemplatizedType(ithenableType, promiseTemplateKey)));
Expand Down Expand Up @@ -1740,7 +1738,7 @@ public FunctionType createConstructorType(
Node source,
Node parameters,
JSType returnType,
ImmutableList<TemplateType> templateKeys,
@Nullable ImmutableList<TemplateType> templateKeys,
boolean isAbstract) {
checkArgument(source == null || source.isFunction() || source.isClass());
return FunctionType.builder(this)
Expand All @@ -1749,7 +1747,7 @@ public FunctionType createConstructorType(
.withSourceNode(source)
.withParamsNode(parameters)
.withReturnType(returnType)
.withTemplateKeys(templateKeys)
.withTemplateKeys((templateKeys == null) ? ImmutableList.of() : templateKeys)
.withIsAbstract(isAbstract)
.build();
}
Expand All @@ -1770,7 +1768,7 @@ public FunctionType createInterfaceType(
.withName(name)
.withSourceNode(source)
.withEmptyParams()
.withTemplateKeys(templateKeys)
.withTemplateKeys((templateKeys == null) ? ImmutableList.of() : templateKeys)
.build();
if (struct) {
fn.setStruct();
Expand All @@ -1793,22 +1791,8 @@ public TemplateType createTemplateTypeWithTransformation(
return new TemplateType(this, name, expr);
}

/**
* Creates a template type map from the specified list of template keys and
* template value types.
*/
public TemplateTypeMap createTemplateTypeMap(
ImmutableList<TemplateType> templateKeys,
ImmutableList<JSType> templateValues) {
if (templateKeys == null) {
templateKeys = ImmutableList.of();
}
if (templateValues == null) {
templateValues = ImmutableList.of();
}
return (templateKeys.isEmpty() && templateValues.isEmpty())
? emptyTemplateTypeMap
: new TemplateTypeMap(this, templateKeys, templateValues);
public TemplateTypeMap getEmptyTemplateTypeMap() {
return this.emptyTemplateTypeMap;
}

public ObjectType instantiateGenericsWithUnknown(ObjectType obj) {
Expand Down

0 comments on commit 236f9ed

Please sign in to comment.