[NTI] If possible, use the receiver type when calculating the type in…

…stantiation.

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

src/com/google/javascript/jscomp/NewTypeInference.java

@@ -1805,7 +1805,9 @@ private EnvTypePair analyzeCallNewFwd(
     FunctionType origFunType = funType; // save for later
     if (funType.isGeneric()) {
       Map<String, JSType> typeMap = calcTypeInstantiationFwd(
-          expr, expr.getChildAtIndex(1), funType, inEnv);
+          expr,
+          callee.isGetProp() ? callee.getFirstChild() : null,
+          expr.getChildAtIndex(1), funType, inEnv);
       funType = funType.instantiateGenerics(typeMap);
       println("Instantiated function type: " + funType);
     }
@@ -1903,34 +1905,13 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) {
     // can completely calculate the type instantiation at the .bind call site.
     // We don't support splitting the instantiation between call sites.
     //
-    // Also, we don't use the THIS argument when calculating the instantiation
-    // of a .bind call.
-    // See the following snippet:
-    // /**
-    //  * @constructor
-    //  * @template T
-    //  * @param {T} x
-    //  */
-    // function Foo(x) {}
-    // /**
-    //  * @template T
-    //  * @param {T} x
-    //  */
-    // Foo.prototype.f = function(x) {};
-    // Foo.prototype.f.bind(new Foo(123), 'asdf');
-    //
-    // Here, the receiver type of f is Foo<T>, but the T is the class's T,
-    // not the T of f's template declaration.
-    // Otoh, if f had an @this annotation that contained T, T would refer to
-    // f's T. There is no way of knowing what's the scope of the type variables
-    // in the receiver of the function type, that's why we don't use it here.
+    Node receiver = bindComponents.thisValue;
     if (boundFunType.isGeneric()) {
       Map<String, JSType> typeMap = calcTypeInstantiationFwd(
-          call, bindComponents.parameters, boundFunType, env);
+          call, receiver, bindComponents.parameters, boundFunType, env);
       boundFunType = boundFunType.instantiateGenerics(typeMap);
     }
     FunctionTypeBuilder builder = new FunctionTypeBuilder();
-    Node receiver = bindComponents.thisValue;
     if (receiver != null) {// receiver is null for goog.partial
       JSType reqThisType = boundFunType.getThisType();
       if (reqThisType == null || boundFunType.isSomeConstructorOrInterface()) {
@@ -2244,14 +2225,14 @@ private void checkInvalidTypename(Node typeString) {
   }
 
   private Map<String, JSType> calcTypeInstantiationFwd(
-      Node callNode, Node firstArg, FunctionType funType, TypeEnv typeEnv) {
-    return calcTypeInstantiation(callNode, firstArg, funType, typeEnv, true);
+      Node callNode, Node receiver, Node firstArg, FunctionType funType, TypeEnv typeEnv) {
+    return calcTypeInstantiation(callNode, receiver, firstArg, funType, typeEnv, true);
   }
 
   private Map<String, JSType> calcTypeInstantiationBwd(
       Node callNode, FunctionType funType, TypeEnv typeEnv) {
     return calcTypeInstantiation(
-        callNode, callNode.getChildAtIndex(1), funType, typeEnv, false);
+        callNode, null, callNode.getChildAtIndex(1), funType, typeEnv, false);
   }
 
   /*
@@ -2263,30 +2244,53 @@ private Map<String, JSType> calcTypeInstantiationBwd(
    * 2) It's hard to give good error messages in cases like: id('str') - 5
    *    We want an invalid-operand-type, not a not-unique-instantiation.
    *
+   *
    * We don't take the arg evaluation order into account during instantiation.
+   *
+   *
+   * When calculating the instantiation, when do we use the receiver type?
+   * See the following snippet:
+   * /**
+   *  * @constructor
+   *  * @template T
+   *  * @param {T} x
+   *  * /
+   * function Foo(x) {}
+   * /**
+   *  * @template T
+   *  * @param {T} x
+   *  * /
+   * Foo.prototype.f = function(x) {};
+   * Foo.prototype.f.bind(new Foo(123), 'asdf');
+   *
+   * Here, the receiver type of f is Foo<T>, but the T is the class's T,
+   * not the T of f's template declaration.
+   * OTOH, if f had a @this annotation that contained T, T would refer to
+   * f's T. There is no way of knowing what's the scope of the type variables
+   * in the receiver of the function type.
+   * But when THIS comes from the class, it is always a singleton object. So,
+   * we use a heuristic: if THIS is not a singleton obj, we know it comes from
+   * @this, and we use it for the instantiation.
    */
   private ImmutableMap<String, JSType> calcTypeInstantiation(Node callNode,
-      Node firstArg, FunctionType funType, TypeEnv typeEnv, boolean isFwd) {
+      Node receiver, Node firstArg, FunctionType funType, TypeEnv typeEnv, boolean isFwd) {
+    Preconditions.checkState(receiver == null || isFwd);
     List<String> typeParameters = funType.getTypeParameters();
     Multimap<String, JSType> typeMultimap = LinkedHashMultimap.create();
+    JSType funRecvType = funType.getThisType();
+    if (receiver != null && funRecvType != null && !funRecvType.isSingletonObj()) {
+      EnvTypePair pair = analyzeExprFwd(receiver, typeEnv);
+      unifyWithSubtypeWarnIfFail(funRecvType, pair.type, typeParameters,
+          typeMultimap, receiver, isFwd);
+      typeEnv = pair.env;
+    }
     Node arg = firstArg;
     int i = 0;
     while (arg != null) {
       EnvTypePair pair =
           isFwd ? analyzeExprFwd(arg, typeEnv) : analyzeExprBwd(arg, typeEnv);
-      JSType formalType = funType.getFormalType(i);
-      JSType passedArgType = pair.type;
-      if (!formalType.unifyWithSubtype(passedArgType, typeParameters, typeMultimap)) {
-        // Unification may fail b/c of types irrelevant to generics, eg,
-        // number vs string.
-        // In this case, don't warn here; we'll show invalid-arg-type later.
-        JSType formalAfterInstantiation = formalType.substituteGenericsWithUnknown();
-        if (!formalType.equals(formalAfterInstantiation)
-            && passedArgType.isSubtypeOf(formalAfterInstantiation)) {
-          warnings.add(JSError.make(arg, FAILED_TO_UNIFY,
-                  formalType.toString(), passedArgType.toString()));
-        }
-      }
+      unifyWithSubtypeWarnIfFail(funType.getFormalType(i), pair.type,
+          typeParameters, typeMultimap, arg, isFwd);
       arg = arg.getNext();
       typeEnv = pair.env;
       i++;
@@ -2311,6 +2315,22 @@ private ImmutableMap<String, JSType> calcTypeInstantiation(Node callNode,
     return builder.build();
   }
 
+  private void unifyWithSubtypeWarnIfFail(JSType genericType, JSType concreteType,
+      List<String> typeParameters, Multimap<String, JSType> typeMultimap,
+      Node toWarnOn, boolean isFwd) {
+    if (!genericType.unifyWithSubtype(concreteType, typeParameters, typeMultimap) && isFwd) {
+      // Unification may fail b/c of types irrelevant to generics, eg,
+      // number vs string.
+      // In this case, don't warn here; we'll show invalid-arg-type later.
+      JSType afterInstantiation = genericType.substituteGenericsWithUnknown();
+      if (!genericType.equals(afterInstantiation)
+          && concreteType.isSubtypeOf(afterInstantiation)) {
+        warnings.add(JSError.make(toWarnOn, FAILED_TO_UNIFY,
+                genericType.toString(), concreteType.toString()));
+      }
+    }
+  }
+
   private EnvTypePair analyzeNonStrictComparisonFwd(
       Node expr, TypeEnv inEnv, JSType specializedType) {
     int tokenType = expr.getType();

src/com/google/javascript/jscomp/newtypes/JSType.java

@@ -996,11 +996,12 @@ public JSType withFunction(FunctionType ft, NominalType fnNominal) {
     return fromObjectType(ot.withFunction(ft, fnNominal));
   }
 
+  public boolean isSingletonObj() {
+    return getMask() == NON_SCALAR_MASK && getObjs().size() == 1;
+  }
+
   public ObjectType getObjTypeIfSingletonObj() {
-    if (getMask() != NON_SCALAR_MASK || getObjs().size() > 1) {
-      return null;
-    }
-    return Iterables.getOnlyElement(getObjs());
+    return isSingletonObj() ? Iterables.getOnlyElement(getObjs()) : null;
   }
 
   public FunctionType getFunTypeIfSingletonObj() {
@@ -1019,10 +1020,8 @@ public FunctionType getFunType() {
   }
 
   public NominalType getNominalTypeIfSingletonObj() {
-    if (getMask() != NON_SCALAR_MASK || getObjs().size() > 1) {
-      return null;
-    }
-    return Iterables.getOnlyElement(getObjs()).getNominalType();
+    return isSingletonObj()
+        ? Iterables.getOnlyElement(getObjs()).getNominalType() : null;
   }
 
   public boolean isInterfaceDefinition() {

test/com/google/javascript/jscomp/NewTypeInferenceES5OrLowerTest.java

@@ -14280,4 +14280,72 @@ public void testInstantiateToTheSuperType() {
         "var foo = f(123, E.A);"),
         NewTypeInference.MISTYPED_ASSIGN_RHS);
   }
+
+  public void testUseThisForTypeInstantiation() {
+    typeCheck(Joiner.on('\n').join(
+        "/** @constructor */",
+        "function Foo() {}",
+        "/** @constructor */",
+        "function Bar() {}",
+        "/**",
+        " * @template T",
+        " * @this {T}",
+        " * @param {T} x",
+        " */",
+        "Foo.prototype.f = function(x) {};",
+        "(new Foo).f(new Bar);"),
+        NewTypeInference.NOT_UNIQUE_INSTANTIATION);
+
+    typeCheck(Joiner.on('\n').join(
+        "/**",
+        " * @template T",
+        " * @this {T|number}",
+        " */",
+        "function f() {};",
+        "/** @return {*} */",
+        "function g() { return null; }",
+        "f.bind(g());"),
+        NewTypeInference.FAILED_TO_UNIFY);
+
+    typeCheck(Joiner.on('\n').join(
+        "/** @constructor */",
+        "function Foo() {}",
+        "/** @constructor */",
+        "function Bar() {}",
+        "/**",
+        " * @template T",
+        " * @this {T}",
+        " * @param {T} x",
+        " */",
+        "function f(x) {}",
+        "f.bind(new Foo, new Bar);"),
+        NewTypeInference.NOT_UNIQUE_INSTANTIATION);
+
+    // We miss the incompatibility between MyArray<number> and  MyArray<string>.
+    // We don't catch it because our heuristic for using the receiver type to
+    // calculate the instantiation is not enough here.
+    typeCheck(Joiner.on('\n').join(
+        "/**",
+        " * @interface",
+        " * @template T",
+        " */",
+        "function MyArrayLike() {}",
+        "/** @type {number} */",
+        "MyArrayLike.length;",
+        "/**",
+        " * @constructor",
+        " * @implements {MyArrayLike<T>}",
+        " * @param {T} x",
+        " * @template T",
+        " */",
+        "function MyArray(x) {}",
+        "MyArray.prototype.length = 123;",
+        "/**",
+        " * @this {!MyArrayLike<T>}",
+        " * @param {!MyArrayLike<T>} x",
+        " * @template T",
+        " */",
+        "MyArray.prototype.m = function(x) {};",
+        "(new MyArray(123)).m(new MyArray('asdf'));"));
+  }
 }