Slight tweak in how scope variables are handled when a function and o…

…ne of its parameters have the same name. This will allow us to simplify MakeDeclaredNamesUnique.

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

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

@@ -131,16 +131,17 @@ void populate() {
           final Node fnNameNode = n.getFirstChild();
           final Node args = fnNameNode.getNext();
 
-          // Bleed the function name into the scope, if it hasn't
-          // been declared in the outer scope.
+          // Args: Declare function variables
+          checkState(args.isParamList());
+          declareLHS(scope, args);
+
+          // Bleed the function name into the scope, if it hasn't been declared in the outer scope
+          // and the name isn't already in the scope via the param list.
           String fnName = fnNameNode.getString();
           if (!fnName.isEmpty() && NodeUtil.isFunctionExpression(n)) {
             declareVar(scope, fnNameNode);
           }
 
-          // Args: Declare function variables
-          checkState(args.isParamList());
-          declareLHS(scope, args);
           // Since we create a separate scope for body, stop scanning here
           return;
         }
@@ -321,7 +322,7 @@ private void declareVar(Scope s, Node n) {
 
       CompilerInput input = compiler.getInput(inputId);
       if (v != null
-          || isShadowingDisallowed(name, s)
+          || !isShadowingAllowed(name, s)
           || ((s.isFunctionScope()
               || s.isFunctionBlockScope()) && name.equals(ARGUMENTS))) {
         redeclarationHandler.onRedeclaration(s, name, n, input);
@@ -332,12 +333,12 @@ private void declareVar(Scope s, Node n) {
 
     // Function body declarations are not allowed to shadow
     // function parameters.
-    private static boolean isShadowingDisallowed(String name, Scope s) {
+    private static boolean isShadowingAllowed(String name, Scope s) {
       if (s.isFunctionBlockScope()) {
         Var maybeParam = s.getParent().getOwnSlot(name);
-        return maybeParam != null && maybeParam.isParam();
+        return maybeParam == null || !maybeParam.isParam();
       }
-      return false;
+      return true;
     }
   }
 

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

@@ -209,9 +209,8 @@ public void testParamAndVarShadowFunctionName() {
     assertThat(Iterables.transform(functionScope.getVarIterable(), Var::getName))
         .containsExactly("g");
 
-    // TODO(tbreisacher): "var g" doesn't declare a new var, so the body scope should have no
-    // variables in it.
-    assertThat(Iterables.transform(bodyScope.getVarIterable(), Var::getName)).containsExactly("g");
+    // "var g" doesn't declare a new var, so there is no 'g' variable in the function body scope.
+    assertThat(bodyScope.getVarIterable()).isEmpty();
   }
 
   public void testVarRedeclaration1_inES6Module() {
@@ -1007,9 +1006,9 @@ public void testFunctionNameMatchesParamName1() {
     assertFalse(fScope.isDeclared("f", false));
     assertTrue(fScope.isDeclared("foo", false));
 
-    // The Es6SyntacticScopeCreator considers the function name to be the declaration of 'foo',
-    // even though a reference to 'foo' inside the function would refer to the parameter foo.
-    assertNode(fScope.getVar("foo").getNode().getParent()).hasType(Token.FUNCTION);
+    // The parameter 'foo', not the function name, is the declaration of the variable 'foo' in this
+    // scope.
+    assertNode(fScope.getVar("foo").getNode().getParent()).hasType(Token.PARAM_LIST);
   }
 
   public void testFunctionNameMatchesParamName2() {
@@ -1024,9 +1023,9 @@ public void testFunctionNameMatchesParamName2() {
     assertFalse(fScope.isDeclared("f", false));
     assertTrue(fScope.isDeclared("foo", false));
 
-    // The Es6SyntacticScopeCreator considers the function name to be the declaration of 'foo',
-    // even though a reference to 'foo' inside the function would refer to the parameter foo.
-    assertNode(fScope.getVar("foo").getNode().getParent()).hasType(Token.FUNCTION);
+    // The parameter 'foo', not the function name, is the declaration of the variable 'foo' in this
+    // scope.
+    assertNode(fScope.getVar("foo").getNode().getParent()).hasType(Token.PARAM_LIST);
   }
 
   public void testClassName() {

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

@@ -237,8 +237,8 @@ public void testBleedingRecursiveFunctions2() {
             "}"),
         LINE_JOINER.join(
             "function d() {",
-            "  var e = function b(a) { return a ? 1 : b(1); };",
-            "  var f = function a(c) { return c ? 2 : a(2); };",
+            "  var e = function a(b) { return b ? 1 : a(1); };",
+            "  var f = function c(a) { return a ? 2 : c(2); };",
             "}"));
   }
 
@@ -252,9 +252,9 @@ public void testBleedingRecursiveFunctions3() {
             "}"),
         LINE_JOINER.join(
             "function f() {",
-            "  var g = function c(a) { return a ? 1 : c(1); };",
-            "  var d = function a(b) { return b ? 2 : a(2); };",
-            "  var h = function b(e) { return e ? d : b(2); };",
+            "  var g = function a(c) { return c ? 1 : a(1); };",
+            "  var d = function b(a) { return a ? 2 : b(2); };",
+            "  var h = function e(b) { return b ? d : e(2); };",
             "}"));
   }