[NTI] The name of a function expression shouldn't leak to the enclosi…

…ng scope.

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

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

@@ -1181,6 +1181,11 @@ private void visitFunctionEarly(Node fn) {
 
     private String createFunctionInternalName(Node fn, Node nameNode) {
       String internalName = null;
+      // The name of a function expression isn't visible in the scope where the function expression
+      // appears, so don't use it as the internalName.
+      if (nameNode == fn.getFirstChild() && NodeUtil.isFunctionExpression(fn)) {
+        nameNode = null;
+      }
       if (nameNode == null || !nameNode.isName()
           || nameNode.getParent().isAssign()) {
         // Anonymous functions, qualified names, and stray assignments

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

@@ -788,7 +788,6 @@ private void analyzeFunction(NTIScope scope) {
     // The size is > 1 when multiple files are compiled
     // Preconditions.checkState(cfg.getEntry().getOutEdges().size() == 1);
     NTIWorkset workset = NTIWorkset.create(this.cfg);
-    /* println("Workset: ", workset); */
     this.typeEnvFromDeclaredTypes = getTypeEnvFromDeclaredTypes();
     if (scope.isFunction() && scope.hasUndeclaredFormalsOrOuters()) {
       // Ideally, we would like to only set the in-edges of the implicit return
@@ -2164,7 +2163,7 @@ private EnvTypePair analyzeInvocationFwd(
       return analyzeInvocationArgsFwdWhenError(expr, envAfterCallee);
     }
 
-    FunctionType origFunType = funType; // save for later
+    FunctionType originalFunType = funType; // save for later
     if (funType.isGeneric()) {
       Node receiver = callee.isGetProp() ? callee.getFirstChild() : null;
       Node firstArg = expr.getSecondChild();
@@ -2187,10 +2186,9 @@ private EnvTypePair analyzeInvocationFwd(
         // exactly using their summaries, and don't need deferred checks
         if (this.currentScope.isLocalFunDef(calleeName)) {
           tmpEnv = collectTypesForEscapedVarsFwd(callee, tmpEnv);
-        } else if (!origFunType.isGeneric()) {
+        } else if (!originalFunType.isGeneric()) {
           JSType expectedRetType = requiredType;
-          println("Updating deferred check with ret: ", expectedRetType,
-              " and args: ", argTypes);
+          println("Updating deferred check with ret: ", expectedRetType, " and args: ", argTypes);
           DeferredCheck dc;
           if (isConstructorCall(expr)) {
             dc = new DeferredCheck(expr, null,
@@ -4212,10 +4210,8 @@ private void createDeferredCheckBwd(Node expr, JSType requiredType) {
         // No deferred check if the return type is declared
         expectedRetType = null;
       }
-      println("Putting deferred check of function: ", calleeName,
-          " with ret: ", expectedRetType);
-      DeferredCheck dc = new DeferredCheck(
-          expr, expectedRetType, currentScope, s);
+      println("Putting deferred check of function: ", calleeName, " with ret: ", expectedRetType);
+      DeferredCheck dc = new DeferredCheck(expr, expectedRetType, currentScope, s);
       deferredChecks.put(expr, dc);
     }
   }
@@ -4840,8 +4836,7 @@ private class DeferredCheck {
     final Node callSite;
     final NTIScope callerScope;
     final NTIScope calleeScope;
-    // Null types means that they were declared
-    // (and should have been checked during inference)
+    // Null types means that they were declared (and should have been checked during inference)
     JSType expectedRetType;
     List<JSType> argTypes;
 
@@ -4866,8 +4861,7 @@ void updateArgTypes(List<JSType> argTypes) {
       this.argTypes = argTypes;
     }
 
-    private void runCheck(
-        Map<NTIScope, JSType> summaries, WarningReporter warnings) {
+    private void runCheck(Map<NTIScope, JSType> summaries, WarningReporter warnings) {
       FunctionType fnSummary = summaries.get(this.calleeScope).getFunType();
       println(
           "Running deferred check of function: ", calleeScope.getReadableName(),
@@ -4883,7 +4877,7 @@ private void runCheck(
       int i = 0;
       Iterable<Node> args = NodeUtil.getInvocationArgsAsIterable(callSite);
       // this.argTypes can be null if in the fwd direction the analysis of the
-      // call return prematurely, eg, because of a WRONG_ARGUMENT_COUNT.
+      // call returned prematurely, e.g., because of a WRONG_ARGUMENT_COUNT.
       if (this.argTypes == null) {
         return;
       }

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

@@ -21952,4 +21952,18 @@ public void testGetDeclaredTypeForUnannotatedFunctionFromCast() {
         ");"),
         NewTypeInference.INEXISTENT_PROPERTY);
   }
+
+  public void testDontRegisterFunctionExpressions() {
+    // The first line is a function expression, not a function statement, so it doesn't define
+    // a global variable f. Therefore the typechecker assumes the second line refers to a global
+    // 'f' which is defined externally. So there's no warning for passing the wrong number of args.
+    typeCheck(LINE_JOINER.join(
+        "(function f() {})();",
+        "f(234);"));
+
+    // The deferred check for the call should refer to the second f; don't crash.
+    typeCheck(LINE_JOINER.join(
+        "(function f() {})();",
+        "(function f(x) { f(234); })(123);"));
+  }
 }