Have PeepholeRemoveDeadCode not hoist block scoped variables.

Changed tryMergeBlock in NodeUtil to take a boolean on whether it should always merge or not.

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

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

@@ -1398,7 +1398,7 @@ void updateModuleBody(Node moduleBody) {
         moduleBody.isModuleBody() && moduleBody.getParent().getBooleanProp(Node.GOOG_MODULE),
         moduleBody);
     moduleBody.setToken(Token.BLOCK);
-    NodeUtil.tryMergeBlock(moduleBody);
+    NodeUtil.tryMergeBlock(moduleBody, true);
 
     updateEndModule();
     popScript();

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

@@ -2826,12 +2826,13 @@ static void maybeAddFinally(Node tryNode) {
    * Merge a block with its parent block.
    * @return Whether the block was removed.
    */
-  public static boolean tryMergeBlock(Node block) {
+  public static boolean tryMergeBlock(Node block, boolean alwaysMerge) {
     checkState(block.isNormalBlock());
     Node parent = block.getParent();
+    boolean canMerge = alwaysMerge || canMergeBlock(block);
     // Try to remove the block if its parent is a block/script or if its
     // parent is label and it has exactly one child.
-    if (isStatementBlock(parent)) {
+    if (isStatementBlock(parent) && canMerge) {
       Node previous = block;
       while (block.hasChildren()) {
         Node child = block.removeFirstChild();
@@ -2845,6 +2846,34 @@ public static boolean tryMergeBlock(Node block) {
     }
   }
 
+  /**
+   * A check inside a block to see if there are const, let, class, or function declarations
+   * to be safe and not hoist them into the upper block.
+   * @return Whether the block can be removed
+   */
+  public static boolean canMergeBlock(Node block) {
+    for (Node c = block.getFirstChild(); c != null; c = c.getNext()) {
+      switch (c.getToken()) {
+        case LABEL:
+          if (canMergeBlock(c)){
+            continue;
+          } else {
+            return false;
+          }
+
+        case CONST:
+        case LET:
+        case CLASS:
+        case FUNCTION:
+          return false;
+
+        default:
+          continue;
+      }
+    }
+    return true;
+  }
+
   /**
    * @param node A node
    * @return Whether the call is a NEW or CALL node.

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

@@ -593,7 +593,7 @@ Node tryOptimizeBlock(Node n) {
     }
 
     // Try to remove the block.
-    if (NodeUtil.tryMergeBlock(n)) {
+    if (NodeUtil.tryMergeBlock(n, false)) {
       reportCodeChange();
       return null;
     }
@@ -602,7 +602,8 @@ Node tryOptimizeBlock(Node n) {
   }
 
   /**
-   * Some nodes unremovable node don't have side-effects.
+   * Some nodes that are unremovable don't have side effects so they aren't caught by
+   * mayHaveSideEffects
    */
   private static boolean isUnremovableNode(Node n) {
     return (n.isNormalBlock() && n.isSyntheticBlock()) || n.isScript();

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

@@ -249,7 +249,7 @@ private void visitLabel(NodeTraversal t, Node node, Node parent) {
         node.removeChild(newChild);
         parent.replaceChild(node, newChild);
         if (newChild.isNormalBlock()) {
-          NodeUtil.tryMergeBlock(newChild);
+          NodeUtil.tryMergeBlock(newChild, false);
         }
         if (markChanges) {
           t.reportCodeChange();

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

@@ -191,7 +191,7 @@ public void hotSwapScript(Node root, Node originalRoot) {
         expressionWithScopeCall.replaceWith(scopeClosureBlock);
         NodeUtil.markFunctionsDeleted(expressionWithScopeCall, compiler);
         compiler.reportChangeToEnclosingScope(scopeClosureBlock);
-        NodeUtil.tryMergeBlock(scopeClosureBlock);
+        NodeUtil.tryMergeBlock(scopeClosureBlock, false);
       }
     }
   }

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

@@ -259,7 +259,7 @@ private void moveCallbackContentToTopLevel(Node defineParent, Node script,
         script.addChildBefore(callbackBlock, before);
       }
       script.addChildToBack(callbackBlock);
-      NodeUtil.tryMergeBlock(callbackBlock);
+      NodeUtil.tryMergeBlock(callbackBlock, false);
     }
   }
 

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

@@ -48,7 +48,7 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
     }
     Node moduleBody = scriptRoot.getFirstChild();
     moduleBody.setToken(Token.BLOCK);
-    NodeUtil.tryMergeBlock(moduleBody);
+    NodeUtil.tryMergeBlock(moduleBody, true);
     compiler.reportChangeToEnclosingScope(scriptRoot);
 
     // As per ClosureBundler:

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

@@ -1141,43 +1141,72 @@ public void testReplaceDeclarationName() {
     replaceDeclChild("const x =1, y = 2, z = 3, w = 4;", 1, "const x = 1; {} const z = 3, w = 4;");
   }
 
-  public void testMergeBlock1() {
+  public void testTryMergeBlock1() {
     // Test removing the initializer.
     Node actual = parse("{{a();b();}}");
 
     Node parentBlock = actual.getFirstChild();
     Node childBlock = parentBlock.getFirstChild();
 
-    assertTrue(NodeUtil.tryMergeBlock(childBlock));
+    assertTrue(NodeUtil.tryMergeBlock(childBlock, false));
     String expected = "{a();b();}";
     String difference = parse(expected).checkTreeEquals(actual);
     assertNull("Nodes do not match:\n" + difference, difference);
   }
 
-  public void testMergeBlock2() {
+  public void testTryMergeBlock2() {
     // Test removing the initializer.
     Node actual = parse("foo:{a();}");
 
     Node parentLabel = actual.getFirstChild();
     Node childBlock = parentLabel.getLastChild();
 
-    assertFalse(NodeUtil.tryMergeBlock(childBlock));
+    assertFalse(NodeUtil.tryMergeBlock(childBlock, false));
   }
 
-  public void testMergeBlock3() {
+  public void testTryMergeBlock3() {
     // Test removing the initializer.
     String code = "foo:{a();boo()}";
     Node actual = parse("foo:{a();boo()}");
 
     Node parentLabel = actual.getFirstChild();
     Node childBlock = parentLabel.getLastChild();
 
-    assertFalse(NodeUtil.tryMergeBlock(childBlock));
+    assertFalse(NodeUtil.tryMergeBlock(childBlock, false));
     String expected = code;
     String difference = parse(expected).checkTreeEquals(actual);
     assertNull("Nodes do not match:\n" + difference, difference);
   }
 
+  public void testTryMergeBlock4() {
+    Node actual = parse("{const module$exports$Foo=class{}}");
+    String expected = "const module$exports$Foo=class{}";
+
+    Node block = actual.getFirstChild();
+
+    assertTrue(NodeUtil.tryMergeBlock(block, true));
+    String difference = parse(expected).checkTreeEquals(actual);
+    assertNull("Nodes do not match:\n" + difference, difference);
+  }
+
+  public void testCanMergeBlock1() {
+    Node actual = parse("{a(); let x;}");
+
+    Node block = actual.getFirstChild();
+
+    assertFalse(NodeUtil.canMergeBlock(block));
+  }
+
+  public void testCanMergeBlock2() {
+    Node actual = parse("{a(); f(); var x; {const y = 2;}}");
+
+    Node parentBlock = actual.getFirstChild();
+    Node childBlock = parentBlock.getLastChild();
+
+    assertTrue(NodeUtil.canMergeBlock(parentBlock));
+    assertFalse(NodeUtil.canMergeBlock(childBlock));
+  }
+
   public void testGetSourceName() {
     Node n = new Node(Token.BLOCK);
     Node parent = new Node(Token.BLOCK, n);

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

@@ -88,6 +88,7 @@ public void testFoldBlock() {
 
     fold("{'hi'}", "");
     fold("{x==3}", "");
+    fold("{`hello ${foo}`}", "");
     fold("{ (function(){x++}) }", "");
     foldSame("function f(){return;}");
     fold("function f(){return 3;}", "function f(){return 3}");
@@ -99,15 +100,31 @@ public void testFoldBlock() {
     fold("while(x()){x()}", "while(x())x()");
     fold("for(x=0;x<100;x++){x}", "for(x=0;x<100;x++);");
     fold("for(x in y){x}", "for(x in y);");
+    fold("for (x of y) {x}", "for(x of y);");
+
+    // Block with declarations
+    foldSame("{let x}");
+    foldSame("function f() {let x}");
+    foldSame("{const x = 1}");
+    foldSame("{x = 2; y = 4; let z;}");
+    fold("{'hi'; let x;}", "{let x}");
+    fold("{x = 4; {let y}}", "x = 4; {let y}");
+    foldSame("{function f() {} } {function f() {}}");
+    foldSame("{class C {}} {class C {}}");
+    foldSame("{label: let x}");
+    fold("{label: var x}", "label: var x");
+    foldSame("{label: var x; let y;}");
   }
 
   /** Try to remove spurious blocks with multiple children */
   public void testFoldBlocksWithManyChildren() {
     fold("function f() { if (false) {} }", "function f(){}");
     fold("function f() { { if (false) {} if (true) {} {} } }",
          "function f(){}");
-    test("{var x; var y; var z; function f() { { var a; { var b; } } } }",
-         "var x;var y;var z;function f(){var a;var b}");
+    fold("{var x; var y; var z; function f() { { var a; { var b; } } } }",
+         "{var x;var y;var z;function f(){var a;var b} }");
+    fold("{var x; var y; var z; { { var a; { var b; } } } }",
+        "var x;var y;var z; var a;var b");
   }
 
   public void testIf() {
@@ -187,6 +204,13 @@ public void testVarLifting() {
     // More var lifting tests in PeepholeIntegrationTests
   }
 
+  public void testLetConstLifting(){
+    fold("if(true) {const x = 1}", "{const x = 1}");
+    fold("if(false) {const x = 1}", "");
+    fold("if(true) {let x}", "{let x}");
+    fold("if(false) {let x}", "");
+  }
+
   public void testFoldUselessWhile() {
     fold("while(false) { foo() }", "");
 
@@ -210,6 +234,8 @@ public void testFoldUselessFor() {
     fold("for(;true;) foo() ", "for(;;) foo() ");
     foldSame("for(;;) foo()");
     fold("for(;false;) { var a = 0; }", "var a");
+    fold("for(;false;) { const a = 0; }", "");
+    fold("for(;false;) { let a = 0; }", "");
 
     // Make sure it plays nice with minimizing
     fold("for(;false;) { foo(); continue }", "");
@@ -277,6 +303,7 @@ public void testRemoveUselessOps2() {
     // Uncalled function expressions are removed
     fold("(function () {});", "");
     fold("(function f() {});", "");
+    fold("(function* f() {})", "");
     // ... including any code they contain.
     fold("(function () {foo();});", "");
 
@@ -537,6 +564,18 @@ public void testOptimizeSwitchWithLabellessBreak() {
             "  case 21: throw 'x';",
             "  default : console.log('good');",
             "}"));
+
+    test(
+        LINE_JOINER.join(
+            "let x = 1;",
+            "switch('x') {",
+            "  case 'x': let x = 2; break;",
+            "}"
+        ),
+        LINE_JOINER.join(
+            "let x = 1;",
+            "{let x = 2}"
+        ));
   }
 
   public void testOptimizeSwitchWithLabelledBreak() {
@@ -574,20 +613,23 @@ public void testOptimizeSwitchWithReturn() {
             "}"),
         "function f() { return 1; }");
 
-    // TODO(moz): This is to show that the current optimization might break if the input has block
-    // scoped declarations. We will need to fix the logic for removing blocks to account for this.
     test(
         LINE_JOINER.join(
             "function f() {",
             "  let x = 1;",
             "  switch('x') {",
-            "    case 'x': { let x = 2; return 3; }",
+            "    case 'x': { let x = 2; } return 3;",
             "    case 'y': return 4;",
             "  }",
             "}"),
-        "function f() { let x = 1; let x = 2; return 3; }");
+        LINE_JOINER.join(
+            "function f() {",
+            "  let x = 1;",
+            "  { let x = 2; } return 3; ",
+            "}"));
   }
 
+
   public void testOptimizeSwitchWithThrow() {
     test(
         LINE_JOINER.join(

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

@@ -45,7 +45,7 @@ public void testRenameInFunction() {
               "}" +
             "}" +
           "}",
-          "function x(){function goo(){a:{ a(); break a; }}}");
+          "function x(){{function goo(){a:{ a(); break a; }}}}");
 
     test("function x() { " +
           "Foo:{ " +