[NTI] Infer function signature for an unannotated callback during NTI…

… because at that point we have better type information.

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

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

@@ -2547,12 +2547,16 @@ private void maybeWarnFunctionDeclaration(Node funNode, DeclaredFunctionType fun
       }
     }
 
-    // We only return a non-null result if the arity of declNode matches the
-    // arity we get from declaredTypeAsJSType.
+    /**
+     * Computes a declared type for an unannotated callback using the corresponding formal
+     * type from the callee.
+     * We redo this in NewTypeInference and may find a better type for the callback there,
+     * but we still need to do it here as well, to infer constants inside the callback.
+     */
     private DeclaredFunctionType computeFnDeclaredTypeFromCallee(
-        Node declNode, JSType declaredTypeAsJSType) {
-      checkArgument(declNode.isFunction());
-      checkArgument(declNode.getParent().isCall());
+        Node callback, JSType declaredTypeAsJSType) {
+      checkArgument(callback.isFunction());
+      checkArgument(callback.getParent().isCall());
 
       if (declaredTypeAsJSType == null) {
         return null;
@@ -2567,14 +2571,8 @@ private DeclaredFunctionType computeFnDeclaredTypeFromCallee(
       if (declType == null) {
         return null;
       }
-      int numFormals = declNode.getSecondChild().getChildCount();
-      int optArity = declType.getOptionalArity();
-      boolean hasRestFormals = declType.hasRestFormals();
-      if ((hasRestFormals && numFormals <= optArity + 1)
-          || (!hasRestFormals && numFormals <= optArity)) {
-        return declType;
-      }
-      return null;
+      return NodeUtil.getApproxRequiredArity(callback) <= declType.getMaxArity()
+          ? declType : null;
     }
 
     // Returns null if it can't find a suitable type in the context

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

@@ -59,9 +59,6 @@ static enum VarKind {
     INFERRED
   }
 
-  /**
-   * Used for local variables.
-   */
   static class LocalVarInfo implements Serializable {
     // When we don't know the type of a local variable, this field is null, not ?.
     private final JSType type;

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

@@ -790,8 +790,12 @@ private void analyzeFunction(NTIScope scope) {
     println("=== Analyzing function: ", scope.getReadableName(), " ===");
     currentScope = scope;
     exitEnvs = new ArrayList<>();
+    Node scopeRoot = scope.getRoot();
+    if (NodeUtil.isUnannotatedCallback(scopeRoot)) {
+      computeFnDeclaredTypeForCallback(scope);
+    }
     ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, false);
-    cfa.process(null, scope.getRoot());
+    cfa.process(null, scopeRoot);
     this.cfg = cfa.getCfg();
     println(this.cfg);
     // The size is > 1 when multiple files are compiled
@@ -2230,6 +2234,7 @@ private EnvTypePair analyzeInvocationFwd(
       ImmutableMap<String, JSType> typeMap =
           calcTypeInstantiationFwd(expr, receiver, firstArg, funType, envAfterCallee);
       funType = instantiateCalleeMaybeWithTTL(funType, typeMap);
+      callee.setTypeI(this.commonTypes.fromFunctionType(funType));
       println("Instantiated function type: ", funType);
     }
     // argTypes collects types of actuals for deferred checks.
@@ -2385,14 +2390,16 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) {
       if (!pair.type.isSubtypeOf(reqThisType)) {
         warnings.add(JSError.make(call, INVALID_THIS_TYPE_IN_BIND,
                 errorMsgWithTypeDiff(reqThisType, pair.type)));
+      } else {
+        instantiateGoogBind(call.getFirstChild(), pair.type);
       }
     }
 
     Iterable<Node> parametersIterable =
         bindComponents.parameters == null
         ? ImmutableList.<Node>of()
         : bindComponents.parameters.siblings();
-    // We're passing an arraylist but don't do deferred checks for bind.
+    // We are passing an arraylist but don't do deferred checks for bind.
     env = analyzeInvocationArgumentsFwd(
         call, parametersIterable, boundFunType, new ArrayList<JSType>(), env);
     // For any formal not bound here, add it to the resulting function type.
@@ -2411,24 +2418,26 @@ private EnvTypePair analyzeFunctionBindFwd(Node call, TypeEnv inEnv) {
         builder.addRetType(boundFunType.getReturnType()).buildFunction()));
   }
 
-  private TypeEnv analyzeInvocationArgumentsFwd(Node node, Iterable<Node> args,
+  private TypeEnv analyzeInvocationArgumentsFwd(Node n, Iterable<Node> args,
       FunctionType funType, List<JSType> argTypesForDeferredCheck, TypeEnv inEnv) {
-    checkState(NodeUtil.isCallOrNew(node) || node.isTemplateLit());
+    checkState(NodeUtil.isCallOrNew(n) || n.isTemplateLit());
     TypeEnv env = inEnv;
-    int i = node.isTemplateLit() ? 1 : 0;
+    int i = n.isTemplateLit() ? 1 : 0;
     for (Node arg : args) {
       JSType formalType = funType.getFormalType(i);
       checkState(!formalType.isBottom());
       EnvTypePair pair = analyzeExprFwd(arg, env, formalType);
+      if (NodeUtil.isUnannotatedCallback(arg) && formalType.getFunType() != null) {
+        i++;
+        argTypesForDeferredCheck.add(null); // No deferred check needed.
+        continue;
+      }
       JSType argTypeForDeferredCheck = pair.type;
       // Allow passing undefined for an optional argument.
       if (funType.isOptionalArg(i) && pair.type.equals(UNDEFINED)) {
         argTypeForDeferredCheck = null; // No deferred check needed.
       } else if (!pair.type.isSubtypeOf(formalType)) {
-        String fnName = getReadableCalleeName(node.getFirstChild());
-        JSError error = JSError.make(arg, INVALID_ARGUMENT_TYPE, Integer.toString(i + 1), fnName,
-            errorMsgWithTypeDiff(formalType, pair.type));
-        registerMismatchAndWarn(error, pair.type, formalType);
+        warnAboutInvalidArgument(n, arg, i, formalType, pair.type);
         argTypeForDeferredCheck = null; // No deferred check needed.
       } else {
         registerImplicitUses(arg, pair.type, formalType);
@@ -2440,6 +2449,73 @@ private TypeEnv analyzeInvocationArgumentsFwd(Node node, Iterable<Node> args,
     return env;
   }
 
+  private void instantiateGoogBind(Node n, JSType recvType) {
+    if (NodeUtil.isGoogBind(n)) {
+      JSType t = (JSType) n.getTypeI();
+      if (t.isFunctionType()) {
+        FunctionType ft = t.getFunType();
+        if (ft.isGeneric()) {
+          FunctionType instantiatedFunction =
+              ft.instantiateGenericsFromArgumentTypes(null, ImmutableList.of(UNKNOWN, recvType));
+          n.setTypeI(this.commonTypes.fromFunctionType(instantiatedFunction));
+        }
+      }
+    }
+  }
+
+  private void warnAboutInvalidArgument(
+      Node call, Node arg, int argIndex, JSType formal, JSType actual) {
+    String fnName = getReadableCalleeName(call.getFirstChild());
+    JSError error = JSError.make(
+        arg, INVALID_ARGUMENT_TYPE, Integer.toString(argIndex + 1), fnName,
+        errorMsgWithTypeDiff(formal, actual));
+    registerMismatchAndWarn(error, actual, formal);
+  }
+
+  /**
+   * Given a scope whose root is an unannotated callback, finds a declared type for the callback
+   * using the types in the callback's context.
+   * Similar to GlobalTypeInfoCollector#computeFnDeclaredTypeFromCallee, but not similar enough
+   * to use the same code for both.
+   */
+  private void computeFnDeclaredTypeForCallback(NTIScope scope) {
+    Node callback = scope.getRoot();
+    checkState(NodeUtil.isUnannotatedCallback(callback));
+    Node call = callback.getParent();
+    JSType calleeType = (JSType) call.getFirstChild().getTypeI();
+    if (calleeType == null) {
+      return;
+    }
+    FunctionType calleeFunType = calleeType.getFunType();
+    if (calleeFunType == null) {
+      return;
+    }
+    int argIndex = call.getIndexOfChild(callback) - 1;
+    JSType formalType = calleeFunType.getFormalType(argIndex);
+    if (formalType == null) {
+      return;
+    }
+    FunctionType ft = formalType.getFunType();
+    if (ft == null || ft.isLoose()) {
+      return;
+    }
+    DeclaredFunctionType callbackDft = scope.getDeclaredFunctionType();
+    JSType scopeType = this.commonTypes.fromFunctionType(callbackDft.toFunctionType());
+    if (ft.isUniqueConstructor() || ft.isInterfaceDefinition()) {
+      warnAboutInvalidArgument(call, callback, argIndex, formalType, scopeType);
+      return;
+    }
+    DeclaredFunctionType declaredDft = checkNotNull(ft.toDeclaredFunctionType());
+    // Only grab a type from the callee if the arity of the declared type is compatible with
+    // the arity of the callback.
+    if (ft.acceptsAnyArguments()
+        || callbackDft.getRequiredArity() <= declaredDft.getMaxArity()) {
+      scope.setDeclaredType(declaredDft);
+    } else {
+      warnAboutInvalidArgument(call, callback, argIndex, formalType, scopeType);
+    }
+  }
+
   private EnvTypePair analyzeAssertionCall(
       Node callNode, TypeEnv env, AssertionFunctionSpec assertionFunctionSpec) {
     analyzeExprFwdIgnoreResult(callNode.getFirstChild(), env);
@@ -4938,8 +5014,8 @@ private void runCheck(Map<NTIScope, JSType> summaries, WarningReporter warnings)
           "Running deferred check of function: ", calleeScope.getReadableName(),
           " with FunctionSummary of: ", fnSummary, " and callsite ret: ",
           expectedRetType, " args: ", argTypes);
-      if (this.expectedRetType != null &&
-          !fnSummary.getReturnType().isSubtypeOf(this.expectedRetType)) {
+      if (this.expectedRetType != null
+          && !fnSummary.getReturnType().isSubtypeOf(this.expectedRetType)) {
         warnings.add(JSError.make(
             this.callSite, INVALID_INFERRED_RETURN_TYPE,
             errorMsgWithTypeDiff(

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

@@ -4605,6 +4605,26 @@ public static Node getFunctionParameters(Node fnNode) {
     return fnNode.getSecondChild();
   }
 
+  /**
+   * Counts the parameters of a function that are not marked optional or varargs.
+   * In ES5 functions, that's all parameters, in ES6 it's a prefix of the parameters.
+   * The result is an overapproximation: if a parameter is not marked as optional, it may still
+   * be optional, but it doesn't have a default value, and wasn't marked as optional
+   * during transpilation.
+   */
+  public static int getApproxRequiredArity(Node fun) {
+    checkArgument(fun.isFunction());
+    checkArgument(getBestJSDocInfo(fun) == null, "Expected unannotated function, found: %s", fun);
+    int result = 0;
+    for (Node param : fun.getSecondChild().children()) {
+      if (param.isOptionalArg() || param.isVarArgs()) {
+        break;
+      }
+      result++;
+    }
+    return result;
+  }
+
   static boolean isConstantVar(Node node, @Nullable Scope scope) {
     if (isConstantName(node)) {
       return true;

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

@@ -152,8 +152,7 @@ public int getMaxArity() {
   }
 
   private int getSyntacticArity() {
-    return this.getOptionalArity()
-        + (this.restFormals == null ? 0 : 1);
+    return this.getOptionalArity() + (this.restFormals == null ? 0 : 1);
   }
 
   public boolean hasRestFormals() {

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

@@ -350,11 +350,11 @@ public JSType getRestFormalsType() {
   /**
    * Returns the formal parameter in the given (0-indexed) position,
    * or null if the position is past the end of the parameter list.
-   *
-   * @throws IllegalStateException if this is the top function (rather than returning bottom).
    */
   public JSType getFormalType(int argpos) {
-    checkState(!isTopFunction());
+    if (isTopFunction()) {
+      return this.commonTypes.UNKNOWN;
+    }
     int numReqFormals = requiredFormals.size();
     if (argpos < numReqFormals) {
       return requiredFormals.get(argpos);
@@ -659,7 +659,7 @@ static void whyNotSubtypeOf(FunctionType f1, FunctionType f2,
    * it to represent, for example, a constructor of Foos with whatever
    * arguments.
    */
-  private boolean acceptsAnyArguments() {
+  public boolean acceptsAnyArguments() {
     return this.requiredFormals.isEmpty() && this.optionalFormals.isEmpty()
         && this.restFormals != null && this.restFormals.isUnknown();
   }
@@ -719,8 +719,7 @@ private boolean isSubtypeOfHelper(FunctionType other, boolean isMethodOverrideCh
       }
 
       if (other.restFormals != null) {
-        int thisMaxTotalArity =
-            this.requiredFormals.size() + this.optionalFormals.size();
+        int thisMaxTotalArity = this.requiredFormals.size() + this.optionalFormals.size();
         if (this.restFormals != null) {
           thisMaxTotalArity++;
         }
@@ -967,8 +966,7 @@ static FunctionType meet(FunctionType f1, FunctionType f2) {
       builder.addOptFormal(optFormalType);
     }
     if (f1.restFormals != null || f2.restFormals != null) {
-      JSType restFormalsType =
-          JSType.nullAcceptingJoin(f1.restFormals, f2.restFormals);
+      JSType restFormalsType = JSType.nullAcceptingJoin(f1.restFormals, f2.restFormals);
       if (restFormalsType.isBottom()) {
         return commonTypes.BOTTOM_FUNCTION;
       }

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

@@ -1079,4 +1079,19 @@ public void testPromiseThen() {
         "MyPromise.prototype.then = function(x, y, z) { return any(); };",
         "var x = (new MyPromise).then(null);"));
   }
+
+  public void testInferUnannotatedCallback() {
+    typeCheck(LINE_JOINER.join(
+        "/**",
+        " * @param {!Promise<!string>} a",
+        " * @return {!Promise<!number>}",
+        " */",
+        "function f(a) {",
+        "  var b = a;",
+        "  return b.then(function(x) {",
+        "    return x - 1;",
+        "  });",
+        "}"),
+        NewTypeInference.INVALID_OPERAND_TYPE);
+  }
 }