From 653c7b4c8c59503e4822475e428e0de48875aef8 Mon Sep 17 00:00:00 2001 From: dimvar Date: Thu, 22 Jun 2017 10:14:25 -0700 Subject: [PATCH] [NTI] Split NominalType#instantiateGenerics to two methods: substitute and instantiate. Also, emphasize the difference between instantiateGenerics and substituteGenerics in javadocs. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=159841473 --- .../jscomp/newtypes/FunctionType.java | 5 +- .../javascript/jscomp/newtypes/JSType.java | 23 +++- .../newtypes/JSTypeCreatorFromJSDoc.java | 6 +- .../javascript/jscomp/newtypes/JSTypes.java | 14 ++- .../jscomp/newtypes/NominalType.java | 114 ++++++++++-------- .../jscomp/newtypes/ObjectType.java | 14 +-- 6 files changed, 104 insertions(+), 72 deletions(-) diff --git a/src/com/google/javascript/jscomp/newtypes/FunctionType.java b/src/com/google/javascript/jscomp/newtypes/FunctionType.java index d3b66af23f7..422fc705665 100644 --- a/src/com/google/javascript/jscomp/newtypes/FunctionType.java +++ b/src/com/google/javascript/jscomp/newtypes/FunctionType.java @@ -443,7 +443,7 @@ public JSType getInstanceTypeOfCtor() { NominalType nominal = getNominalTypeIfSingletonObj(this.nominalType); return nominal == null ? null - : nominal.instantiateGenerics(this.commonTypes.MAP_TO_UNKNOWN).getInstanceAsJSType(); + : nominal.substituteGenerics(this.commonTypes.MAP_TO_UNKNOWN).getInstanceAsJSType(); } /** @@ -1226,8 +1226,7 @@ private static JSType substGenericsInNomType(JSType nt, Map type if (typeMap.isEmpty()) { return nt; } - return JSType.fromObjectType(ObjectType.fromNominalType( - tmp.instantiateGenerics(typeMap))); + return JSType.fromObjectType(ObjectType.fromNominalType(tmp.substituteGenerics(typeMap))); } /** diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index 59174f423f6..fb70d39a3c3 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -649,9 +649,21 @@ public static JSType join(JSType lhs, JSType rhs) { } /** - * Creates a new JSType by deeply substituting the type variables in this type with concrete - * replacements from the given map. Note that this method works for any type, including function - * literals (unlike {@link #instantiateGenerics}, which only works on singleton objects). + * Creates a new JSType by deeply substituting the *free* type variables in this type with + * concrete replacements from the given map. + * This is also what all other substituteGenerics methods in this package do. + * + * Do not confuse with the instantiateGenerics methods, which take an *uninstantiated* + * generic type (a nominal type or a generic function), and substitute the *bound* type variables + * with the concrete types from the map. + + * For example, when you define a class Foo with T, an + * uninstantiated Foo object has type: {@code ForAll T.Foo}. + * When you define a generic function f with U, that takes a {@code Foo} as an argument and + * returns a U, f's type is {@code ForAll U.Foo => U}. + * When you call f with some argument, say Foo of string, you instantiate U in f's type. + * The part of f's type without the ForAll contains U as a free type variable. + * So you call substituteGenerics in order to convert the {@code Foo} to a {@code Foo} */ public final JSType substituteGenerics(Map concreteTypes) { if (isTop() @@ -680,9 +692,8 @@ public final JSType substituteGenerics(Map concreteTypes) { } /** - * Creates a new type by filling in the given concrete types into the type parameters of this - * generic singleton object type. The number of arguments to the method must match the number of - * declared template parameters on this type. + * Assume that this JSType wraps an uninstantiated nominal type, and instantiate it using + * the given types. */ final JSType instantiateGenerics(JSType... types) { checkState(this.isSingletonObj()); diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java b/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java index 9cb648721d8..db862ffed31 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java @@ -855,7 +855,7 @@ private DeclaredFunctionType getFunTypeFromTypicalFunctionJsdoc( return builder.buildDeclaration(); } else if (jsdoc.isConstructor()) { handleConstructorAnnotation(functionName, funNode, constructorType, - parentClass, implementedIntfs, registry, builder); + parentClass, implementedIntfs, builder); } else if (jsdoc.isInterface()) { handleInterfaceAnnotation(jsdoc, functionName, funNode, constructorType, implementedIntfs, typeParameters, registry, builder); @@ -1010,7 +1010,7 @@ private NominalType getMaybeHigherOrderParentClass(Node docNode, String function private void handleConstructorAnnotation( String functionName, Node funNode, RawNominalType constructorType, NominalType parentClass, ImmutableSet implementedIntfs, - DeclaredTypeRegistry registry, FunctionTypeBuilder builder) { + FunctionTypeBuilder builder) { String className = constructorType.toString(); NominalType builtinObject = this.commonTypes.getObjectType(); if (parentClass == null && !functionName.equals("Object")) { @@ -1049,7 +1049,7 @@ private void handleInterfaceAnnotation( } builder.addNominalType(constructorType.getInstanceAsJSType()); } - + // /** @param {...?} var_args */ function f(var_args) { ... } // var_args shouldn't be used in the body of f public static boolean isRestArg(JSDocInfo funJsdoc, String formalParamName) { diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypes.java b/src/com/google/javascript/jscomp/newtypes/JSTypes.java index bacd55fda27..68e13bad0f5 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypes.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypes.java @@ -275,6 +275,11 @@ public int hashCode() { public boolean equals(Object o) { return o == this; } + + @Override + public String toString() { + return "MAP_TO_UNKNOWN"; + } } this.MAP_TO_UNKNOWN = new MapToUnknown(); @@ -323,8 +328,6 @@ public JSType getIterableInstance(JSType t) { ? this.UNKNOWN : this.iterable.getInstanceAsJSType().instantiateGenerics(t); } - - public NominalType getObjectType() { return this.builtinObject == null ? null : this.builtinObject.getAsNominalType(); @@ -371,13 +374,12 @@ public JSType getArrayInstance(JSType t) { return this.UNKNOWN; } ImmutableList typeParams = arrayType.getTypeParameters(); - JSType result = arrayType.getInstanceAsJSType(); // typeParams can be != 1 in old externs files :-S if (typeParams.size() == 1) { - String typeParam = Iterables.getOnlyElement(typeParams); - result = result.substituteGenerics(ImmutableMap.of(typeParam, t)); + return JSType.fromObjectType(ObjectType.fromNominalType( + this.arrayType.getAsNominalType().instantiateGenerics(ImmutableList.of(t)))); } - return result; + return arrayType.getInstanceAsJSType(); } public JSType getArgumentsArrayType(JSType t) { diff --git a/src/com/google/javascript/jscomp/newtypes/NominalType.java b/src/com/google/javascript/jscomp/newtypes/NominalType.java index de0990593ec..777e28324ae 100644 --- a/src/com/google/javascript/jscomp/newtypes/NominalType.java +++ b/src/com/google/javascript/jscomp/newtypes/NominalType.java @@ -204,14 +204,39 @@ public FunctionType getConstructorFunction() { return this.rawType.getConstructorFunction().instantiateGenerics(this.typeMap); } - NominalType instantiateGenerics(List types) { - ImmutableList typeParams = this.rawType.getTypeParameters(); - checkState(types.size() == typeParams.size()); - Map typeMap = new LinkedHashMap<>(); - for (int i = 0; i < typeParams.size(); i++) { - typeMap.put(typeParams.get(i), types.get(i)); + /** + * Substitute the free type variables in this type using the provided type map. + * The most common case when this happens is when a nominal type has already been "instantiated" + * to type variables, and now we want to substitute concrete types for these type variables. + * For example, in the program below, Array's T is instantiated to U in the type of f, + * and when we call f, we substitute boolean for U. + */ + // Written as line comment to enable use of jsdoc + // /** + // * @template U + // * @param {!Array} x + // */ + // function f(x) { return x[0]; } + // f([true, false]); + NominalType substituteGenerics(Map newTypeMap) { + if (!isGeneric()) { + return this.rawType.getAsNominalType(); } - return instantiateGenerics(typeMap); + // NOTE(dimvar): in rare cases, because of the way we represent types, we may end up calling + // substituteGenerics on a nominal type that has an empty type map, which is counter-intuitive. + // Might be worth it at some point to identify all those cases and make sure that types are + // instantiated to identity, rather than having an empty type map. Not super important though. + if (this.typeMap.isEmpty()) { + return instantiateGenerics(newTypeMap); + } + if (newTypeMap.isEmpty()) { + return this; + } + ImmutableMap.Builder builder = ImmutableMap.builder(); + for (String oldKey : this.typeMap.keySet()) { + builder.put(oldKey, this.typeMap.get(oldKey).substituteGenerics(newTypeMap)); + } + return new NominalType(builder.build(), this.rawType); } NominalType instantiateGenerics(Map newTypeMap) { @@ -221,45 +246,40 @@ NominalType instantiateGenerics(Map newTypeMap) { if (!this.rawType.isGeneric()) { return this.rawType.getAsNominalType(); } + Preconditions.checkState(this.typeMap.isEmpty(), + "Expected empty typemap, found: %s", this.typeMap); ImmutableMap.Builder builder = ImmutableMap.builder(); ImmutableMap resultMap; - if (!typeMap.isEmpty()) { - // This branch is entered when a generic type appears "instantiated" - // in some other type, and now we're actually instantiating it to concrete - // types rather than to type variables, eg, here we instantiate Array's T to U, - // and when we call f, we instantiate U to boolean. - // /** - // * @template U - // * @param {!Array} x - // */ - // function f(x) { return x[0]; } - // f([true, false]); - for (String oldKey : typeMap.keySet()) { - builder.put(oldKey, typeMap.get(oldKey).substituteGenerics(newTypeMap)); - } - resultMap = builder.build(); - } else { - ImmutableList typeParams = this.rawType.getTypeParameters(); - for (String newKey : typeParams) { - if (newTypeMap.containsKey(newKey)) { - builder.put(newKey, newTypeMap.get(newKey)); - } - } - resultMap = builder.build(); - if (resultMap.isEmpty()) { - return this; - } - // This works around a bug in FunctionType, because we can't know where - // FunctionType#receiverType is coming from. - // If the condition is true, receiverType comes from a method declaration, - // and we should not create a new type here. - if (resultMap.size() < typeParams.size()) { - return this; + ImmutableList typeParams = getTypeParameters(); + for (String newKey : typeParams) { + if (newTypeMap.containsKey(newKey)) { + builder.put(newKey, newTypeMap.get(newKey)); } } + resultMap = builder.build(); + if (resultMap.isEmpty()) { + return this; + } + // This works around a bug in FunctionType, because we can't know where + // FunctionType#receiverType is coming from. + // If the condition is true, receiverType comes from a method declaration, + // and we should not create a new type here. + if (resultMap.size() < typeParams.size()) { + return this; + } return new NominalType(resultMap, this.rawType); } + NominalType instantiateGenerics(List types) { + ImmutableList typeParams = this.rawType.getTypeParameters(); + checkState(types.size() == typeParams.size()); + Map typeMap = new LinkedHashMap<>(); + for (int i = 0; i < typeParams.size(); i++) { + typeMap.put(typeParams.get(i), types.get(i)); + } + return instantiateGenerics(typeMap); + } + NominalType instantiateGenericsWithUnknown() { NominalType thisWithoutTypemap = this.rawType.getAsNominalType(); return thisWithoutTypemap.instantiateGenerics(getCommonTypes().MAP_TO_UNKNOWN); @@ -332,7 +352,7 @@ public NominalType getInstantiatedSuperclass() { if (this.rawType.getSuperClass() == null) { return null; } - return this.rawType.getSuperClass().instantiateGenerics(typeMap); + return this.rawType.getSuperClass().substituteGenerics(typeMap); } public JSType getPrototypePropertyOfCtor() { @@ -346,7 +366,7 @@ public ImmutableSet getInstantiatedInterfaces() { checkState(this.rawType.isFrozen()); ImmutableSet.Builder result = ImmutableSet.builder(); for (NominalType interf : this.rawType.getInterfaces()) { - result.add(interf.instantiateGenerics(typeMap)); + result.add(interf.substituteGenerics(typeMap)); } return result.build(); } @@ -357,7 +377,7 @@ private ImmutableSet getInstantiatedIObjectInterfaces() { ImmutableSet.Builder result = ImmutableSet.builder(); for (NominalType interf : this.rawType.getInterfaces()) { if (interf.inheritsFromIObjectReflexive()) { - result.add(interf.instantiateGenerics(typeMap)); + result.add(interf.substituteGenerics(typeMap)); } } return result.build(); @@ -458,14 +478,14 @@ boolean isNominalSubtypeOf(NominalType other) { if (other.isInterface()) { // If thisRaw is not frozen, thisRaw.interfaces may be null. for (NominalType i : thisRaw.getInterfaces()) { - if (i.instantiateGenerics(this.typeMap).isNominalSubtypeOf(other)) { + if (i.substituteGenerics(this.typeMap).isNominalSubtypeOf(other)) { return true; } } } // Note that other can still be an interface here (implemented by a superclass) return isClass() && thisRaw.getSuperClass() != null - && thisRaw.getSuperClass().instantiateGenerics(this.typeMap).isNominalSubtypeOf(other); + && thisRaw.getSuperClass().substituteGenerics(this.typeMap).isNominalSubtypeOf(other); } boolean isIObjectSubtypeOf(NominalType other) { @@ -654,7 +674,7 @@ private NominalType findMatchingAncestorWith(NominalType other) { } if (other.isInterface()) { for (NominalType i : thisRaw.getInterfaces()) { - NominalType nt = i.instantiateGenerics(this.typeMap).findMatchingAncestorWith(other); + NominalType nt = i.substituteGenerics(this.typeMap).findMatchingAncestorWith(other); if (nt != null) { return nt; } @@ -662,8 +682,8 @@ private NominalType findMatchingAncestorWith(NominalType other) { } // Note that other can still be an interface here (implemented by a superclass) if (isClass() && thisRaw.getSuperClass() != null) { - return thisRaw.getSuperClass().instantiateGenerics(this.typeMap) - .findMatchingAncestorWith(other); + return thisRaw.getSuperClass().substituteGenerics(this.typeMap) + .findMatchingAncestorWith(other); } return null; } diff --git a/src/com/google/javascript/jscomp/newtypes/ObjectType.java b/src/com/google/javascript/jscomp/newtypes/ObjectType.java index 73baf17be98..1480122eba5 100644 --- a/src/com/google/javascript/jscomp/newtypes/ObjectType.java +++ b/src/com/google/javascript/jscomp/newtypes/ObjectType.java @@ -1212,8 +1212,8 @@ private static boolean areRelatedNominalTypes(NominalType c1, NominalType c2) { * * TODO(dimvar): handle greatest lower bound of interface types. * If we do that, we need to normalize the output, otherwise it could - * contain two object types that are in a subtype relation, e.g., see - * {@link NewTypeInferenceTest#testDifficultObjectSpecialization}. + * contain two object types that are in a subtype relation, e.g., + * see NewTypeInferenceTest#testDifficultObjectSpecialization. */ static ImmutableSet meetSetsHelper( boolean specializeObjs1, Set objs1, Set objs2) { @@ -1535,21 +1535,21 @@ private boolean unifyPropsWithSubtype(ObjectType other, * M, then it gets deeply replaced with the corresponding concrete type * in the returned ObjectType. */ - ObjectType substituteGenerics(Map concreteTypes) { - if (isTopObject() || concreteTypes.isEmpty()) { + ObjectType substituteGenerics(Map typeMap) { + if (isTopObject() || typeMap.isEmpty()) { return this; } PersistentMap newProps = PersistentMap.create(); for (Map.Entry propsEntry : this.props.entrySet()) { String pname = propsEntry.getKey(); Property newProp = - propsEntry.getValue().substituteGenerics(concreteTypes); + propsEntry.getValue().substituteGenerics(typeMap); newProps = newProps.with(pname, newProp); } - FunctionType newFn = fn == null ? null : fn.substituteGenerics(concreteTypes); + FunctionType newFn = fn == null ? null : fn.substituteGenerics(typeMap); return makeObjectType( this.commonTypes, - this.nominalType.instantiateGenerics(concreteTypes), + this.nominalType.substituteGenerics(typeMap), newProps, newFn, this.ns,