[NTI] Split NominalType#instantiateGenerics to two methods: substitut…

…e and instantiate.

Also, emphasize the difference between instantiateGenerics and substituteGenerics in javadocs.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=159841473

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<String, JSType> type
     if (typeMap.isEmpty()) {
       return nt;
     }
-    return JSType.fromObjectType(ObjectType.fromNominalType(
-        tmp.instantiateGenerics(typeMap)));
+    return JSType.fromObjectType(ObjectType.fromNominalType(tmp.substituteGenerics(typeMap)));
   }
 
   /**

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<T>}.
+   * When you define a generic function f with U, that takes a {@code Foo<U>} as an argument and
+   * returns a U, f's type is {@code ForAll U.Foo<U> => 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<U>} to a {@code Foo<string>}
    */
   public final JSType substituteGenerics(Map<String, JSType> concreteTypes) {
     if (isTop()
@@ -680,9 +692,8 @@ public final JSType substituteGenerics(Map<String, JSType> 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());

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<NominalType> 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) {

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<String> 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) {

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<JSType> types) {
-    ImmutableList<String> typeParams = this.rawType.getTypeParameters();
-    checkState(types.size() == typeParams.size());
-    Map<String, JSType> 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<U>} x
+  //  */
+  // function f(x) { return x[0]; }
+  // f([true, false]);
+  NominalType substituteGenerics(Map<String, JSType> 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<String, JSType> 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<String, JSType> newTypeMap) {
@@ -221,45 +246,40 @@ NominalType instantiateGenerics(Map<String, JSType> newTypeMap) {
     if (!this.rawType.isGeneric()) {
       return this.rawType.getAsNominalType();
     }
+    Preconditions.checkState(this.typeMap.isEmpty(),
+        "Expected empty typemap, found: %s", this.typeMap);
     ImmutableMap.Builder<String, JSType> builder = ImmutableMap.builder();
     ImmutableMap<String, JSType> 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<U>} 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<String> 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<String> 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<JSType> types) {
+    ImmutableList<String> typeParams = this.rawType.getTypeParameters();
+    checkState(types.size() == typeParams.size());
+    Map<String, JSType> 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<NominalType> getInstantiatedInterfaces() {
     checkState(this.rawType.isFrozen());
     ImmutableSet.Builder<NominalType> 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<NominalType> getInstantiatedIObjectInterfaces() {
     ImmutableSet.Builder<NominalType> 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,16 +674,16 @@ 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;
         }
       }
     }
     // 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;
   }

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<ObjectType> meetSetsHelper(
       boolean specializeObjs1, Set<ObjectType> objs1, Set<ObjectType> 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<String, JSType> concreteTypes) {
-    if (isTopObject() || concreteTypes.isEmpty()) {
+  ObjectType substituteGenerics(Map<String, JSType> typeMap) {
+    if (isTopObject() || typeMap.isEmpty()) {
       return this;
     }
     PersistentMap<String, Property> newProps = PersistentMap.create();
     for (Map.Entry<String, Property> 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,