Fix Reference.isAssignedOnceInLifetime which was incorrect for a few …

…cases when the ES6 scope creator is used. Inching towards #949 being complete!

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

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

@@ -533,11 +533,12 @@ boolean isAssignedOnceInLifetime() {
         return false;
       }
 
-      // Make sure this assignment is not in a loop.
+      // Make sure this assignment is not in a loop or an enclosing function.
       for (BasicBlock block = ref.getBasicBlock();
            block != null; block = block.getParent()) {
         if (block.isFunction) {
-          if (ref.getSymbol().getScope() != ref.scope) {
+          if (ref.getSymbol().getScope().getClosestHoistScope()
+              != ref.scope.getClosestHoistScope()) {
             return false;
           }
           break;

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

@@ -136,8 +136,7 @@ public void testDefaultValue() {
   }
 
   public void testVarInBlock() {
-    // TODO(tbreisacher): x should be const.
-    testNotConsts("function f() { if (true) { var x = function() {}; x(); } }", "x");
+    testConsts("function f() { if (true) { var x = function() {}; x(); } }", "x");
   }
 
   private void testConsts(String js, String... constants) {

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

@@ -25,19 +25,102 @@
 
 public final class ReferenceCollectingCallbackTest extends CompilerTestCase {
   private Behavior behavior;
+  private boolean es6ScopeCreator;
 
   @Override
   public void setUp() {
     setLanguage(ECMASCRIPT_NEXT, ECMASCRIPT_NEXT);
     behavior = null;
+    es6ScopeCreator = true;
   }
 
   @Override
   protected CompilerPass getProcessor(final Compiler compiler) {
+    ScopeCreator scopeCreator =
+        es6ScopeCreator
+            ? new Es6SyntacticScopeCreator(compiler)
+            : SyntacticScopeCreator.makeUntyped(compiler);
     return new ReferenceCollectingCallback(
         compiler,
         this.behavior,
-        new Es6SyntacticScopeCreator(compiler));
+        scopeCreator);
+  }
+
+  public void testVarInBlock_oldScopeCreator() {
+    es6ScopeCreator = false;
+    behavior = new Behavior() {
+      @Override
+      public void afterExitScope(NodeTraversal t, ReferenceMap rm) {
+        if (t.getScope().isFunctionScope()) {
+          ReferenceCollection y = rm.getReferences(t.getScope().getVar("y"));
+          assertThat(y.isAssignedOnceInLifetime()).isTrue();
+          assertThat(y.isWellDefined()).isTrue();
+        }
+      }
+    };
+    testSame(
+        LINE_JOINER.join(
+            "function f(x) {",
+            "  if (true) {",
+            "    var y = x;",
+            "    y;",
+            "    y;",
+            "  }",
+            "}"));
+  }
+
+  public void testVarInBlock() {
+    behavior = new Behavior() {
+      @Override
+      public void afterExitScope(NodeTraversal t, ReferenceMap rm) {
+        if (t.getScope().isBlockScope() && t.getScope().getParent().isFunctionBlockScope()) {
+          ReferenceCollection y = rm.getReferences(t.getScope().getVar("y"));
+          assertThat(y.isAssignedOnceInLifetime()).isTrue();
+          assertThat(y.isWellDefined()).isTrue();
+        }
+      }
+    };
+    testSame(
+        LINE_JOINER.join(
+            "function f(x) {",
+            "  if (true) {",
+            "    var y = x;",
+            "    y;",
+            "    y;",
+            "  }",
+            "}"));
+  }
+
+  public void testVarInLoopNotAssignedOnlyOnceInLifetime() {
+    behavior = new Behavior() {
+      @Override
+      public void afterExitScope(NodeTraversal t, ReferenceMap rm)  {
+        if (t.getScope().isBlockScope()) {
+          ReferenceCollection x = rm.getReferences(t.getScope().getVar("x"));
+          assertThat(x.isAssignedOnceInLifetime()).isFalse();
+        }
+      }
+    };
+    testSame("while (true) { var x = 0; }");
+    testSame("while (true) { let x = 0; }");
+  }
+
+  /**
+   * Although there is only one assignment to x in the code, it's in a function which could be
+   * called multiple times, so {@code isAssignedOnceInLifetime()} returns false.
+   */
+  public void testVarInFunctionNotAssignedOnlyOnceInLifetime() {
+    behavior = new Behavior() {
+      @Override
+      public void afterExitScope(NodeTraversal t, ReferenceMap rm)  {
+        if (t.getScope().isGlobal()) {
+          ReferenceCollection x = rm.getReferences(t.getScope().getVar("x"));
+          assertThat(x.isAssignedOnceInLifetime()).isFalse();
+        }
+      }
+    };
+    testSame("var x; function f() { x = 0; }");
+    testSame("let x; function f() { x = 0; }");
   }
 
   public void testParameterAssignedOnlyOnceInLifetime() {