diff --git a/src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java b/src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java index 34e818a07ca..aad31f9a0b1 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java +++ b/src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java @@ -81,42 +81,20 @@ public void visit(NodeTraversal t, Node n, Node parent) { if (!n.hasChildren() || !NodeUtil.isBlockScopedDeclaration(n.getFirstChild())) { return; } - - Scope scope = t.getScope(); - Node nameNode = n.getFirstChild(); // NOTE: This pass depends on for-of being transpiled away before it runs. checkState(parent == null || !parent.isForOf(), parent); - // NOTE: This pass depends on classes being transpiled away before it runs. - checkState(!n.isClass(), n); - if (!n.isFunction() - && !nameNode.hasChildren() - && (parent == null || !parent.isForIn()) - && !n.isCatch() - && inLoop(n)) { - Node undefined = createUndefinedNode().srcref(nameNode); - nameNode.addChildToFront(undefined); - compiler.reportChangeToEnclosingScope(undefined); - } - String oldName = nameNode.getString(); if (n.isLet() || n.isConst()) { letConsts.add(n); } - Scope hoistScope = scope.getClosestHoistScope(); - if (scope != hoistScope) { - String newName = oldName; - if (hoistScope.hasSlot(oldName) || undeclaredNames.contains(oldName)) { - do { - newName = oldName + "$" + compiler.getUniqueNameIdSupplier().get(); - } while (hoistScope.hasSlot(newName)); - nameNode.setString(newName); - compiler.reportChangeToEnclosingScope(nameNode); - Node scopeRoot = scope.getRootNode(); - renameTable.put(scopeRoot, oldName, newName); + if (NodeUtil.isNameDeclaration(n)) { + for (Node nameNode : n.children()) { + visitBlockScopedName(t, n, nameNode); } - Var oldVar = scope.getVar(oldName); - scope.undeclare(oldVar); - hoistScope.declare(newName, nameNode, oldVar.input); + } else { + // NOTE: This pass depends on class declarations having been transpiled away + checkState(n.isFunction() || n.isCatch(), "Unexpected declaration node: %s", n); + visitBlockScopedName(t, n, n.getFirstChild()); } } @@ -152,6 +130,44 @@ private boolean getShouldAddTypesOnNewAstNodes() { return compiler.hasTypeCheckingRun(); } + /** + * Renames block-scoped declarations that shadow a variable in an outer scope + * + *

Also normalizes declarations with no initializer in a loop to be initialized to undefined. + */ + private void visitBlockScopedName(NodeTraversal t, Node decl, Node nameNode) { + Scope scope = t.getScope(); + Node parent = decl.getParent(); + // Normalize "let x;" to "let x = undefined;" if in a loop, since we later convert x + // to be $jscomp$loop$0.x and want to reset the property to undefined every loop iteration. + if ((decl.isLet() || decl.isConst()) + && !nameNode.hasChildren() + && (parent == null || !parent.isForIn()) + && inLoop(decl)) { + Node undefined = createUndefinedNode().srcref(nameNode); + nameNode.addChildToFront(undefined); + compiler.reportChangeToEnclosingScope(undefined); + } + + String oldName = nameNode.getString(); + Scope hoistScope = scope.getClosestHoistScope(); + if (scope != hoistScope) { + String newName = oldName; + if (hoistScope.hasSlot(oldName) || undeclaredNames.contains(oldName)) { + do { + newName = oldName + "$" + compiler.getUniqueNameIdSupplier().get(); + } while (hoistScope.hasSlot(newName)); + nameNode.setString(newName); + compiler.reportChangeToEnclosingScope(nameNode); + Node scopeRoot = scope.getRootNode(); + renameTable.put(scopeRoot, oldName, newName); + } + Var oldVar = scope.getVar(oldName); + scope.undeclare(oldVar); + hoistScope.declare(newName, nameNode, oldVar.input); + } + } + /** * Whether n is inside a loop. If n is inside a function which is inside a loop, we do not * consider it to be inside a loop. @@ -184,6 +200,7 @@ private static void maybeAddConstJSDoc(Node srcDeclaration, Node srcParent, Node Node destDeclaration) { if (srcDeclaration.isConst() // Don't add @const for the left side of a for/in. If we do we get warnings from the NTI. + // TODO(lharker): Check if this condition is still necessary, since NTI is deleted && !(srcParent.isForIn() && srcDeclaration == srcParent.getFirstChild())) { extractInlineJSDoc(srcDeclaration, srcName, destDeclaration); JSDocInfoBuilder builder = JSDocInfoBuilder.maybeCopyFrom(destDeclaration.getJSDocInfo()); diff --git a/test/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclarationTest.java b/test/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclarationTest.java index 8298daf951b..712a401f896 100644 --- a/test/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclarationTest.java +++ b/test/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclarationTest.java @@ -156,6 +156,29 @@ public void testLetShadowing() { "}")); } + public void testLetShadowingWithMultivariateDeclaration() { + test( + lines( + "{", // preserve newline + " let x, y;", + "}", + "{", + " let x, y;", + " alert(y);", + "}"), + lines( + "{", // preserve newline + " var x, y;", + "}", + "{", + " var x$0, y$1;", + " alert(y$1);", + "}")); + test( + "var x, y; for (let x, y;;) {}", + "var x, y; for (var x$0 = undefined, y$1 = undefined;;) {}"); + } + public void testNonUniqueLet() { test( lines( @@ -818,15 +841,15 @@ public void testLoopClosureCommaInBody() { lines( "/** @const */ var arr = [];", "var j = 0;", - "var $jscomp$loop$1 = {};", + "var $jscomp$loop$2 = {};", "var i = 0;", - "for (; i < 10; $jscomp$loop$1 = {i$0: $jscomp$loop$1.i$0,", - " j: $jscomp$loop$1.j}, i++) {", - " $jscomp$loop$1.i$0 = undefined;", - " $jscomp$loop$1.j = 0;", - " arr.push((function($jscomp$loop$1) {", - " return function() { return $jscomp$loop$1.i$0 + $jscomp$loop$1.j; };", - " })($jscomp$loop$1));", + "for (; i < 10; $jscomp$loop$2 = {i$0: $jscomp$loop$2.i$0,", + " j$1: $jscomp$loop$2.j$1}, i++) {", + " $jscomp$loop$2.i$0 = undefined;", + " $jscomp$loop$2.j$1 = 0;", + " arr.push((function($jscomp$loop$2) {", + " return function() { return $jscomp$loop$2.i$0 + $jscomp$loop$2.j$1; };", + " })($jscomp$loop$2));", "}")); } @@ -1242,47 +1265,51 @@ public void testFunctionsInLoop() { // https://github.com/google/closure-compiler/issues/1557 public void testNormalizeDeclarations() { - test(lines( - "while(true) {", - " let x, y;", - " let f = function() {", - " x = 1;", - " y = 2;", - " }", - "}"), + test( lines( - "var $jscomp$loop$0 = {};", - "while(true) {", - " $jscomp$loop$0.x = undefined;", - " var f = function($jscomp$loop$0) {", - " return function() {", - " $jscomp$loop$0.x = 1;", - " $jscomp$loop$0.y = 2;", - " }", - " }($jscomp$loop$0);", - " $jscomp$loop$0 = {x: $jscomp$loop$0.x, y: $jscomp$loop$0.y};", - "}")); + "while(true) {", + " let x, y;", + " let f = function() {", + " x = 1;", + " y = 2;", + " }", + "}"), + lines( + "var $jscomp$loop$0 = {};", + "while(true) {", + " $jscomp$loop$0.x = undefined;", + " $jscomp$loop$0.y = undefined;", + " var f = function($jscomp$loop$0) {", + " return function() {", + " $jscomp$loop$0.x = 1;", + " $jscomp$loop$0.y = 2;", + " }", + " }($jscomp$loop$0);", + " $jscomp$loop$0 = {x: $jscomp$loop$0.x, y: $jscomp$loop$0.y};", + "}")); - test(lines( - "while(true) {", - " let x, y;", - " let f = function() {", - " y = 2;", - " x = 1;", - " }", - "}"), + test( lines( - "var $jscomp$loop$0 = {};", - "while(true) {", - " $jscomp$loop$0.x = undefined;", - " var f = function($jscomp$loop$0) {", - " return function() {", - " $jscomp$loop$0.y = 2;", - " $jscomp$loop$0.x = 1;", - " }", - " }($jscomp$loop$0);", - " $jscomp$loop$0 = {y: $jscomp$loop$0.y, x: $jscomp$loop$0.x};", - "}")); + "while(true) {", + " let x, y;", + " let f = function() {", + " y = 2;", + " x = 1;", + " }", + "}"), + lines( + "var $jscomp$loop$0 = {};", + "while(true) {", + " $jscomp$loop$0.x = undefined;", + " $jscomp$loop$0.y = undefined;", + " var f = function($jscomp$loop$0) {", + " return function() {", + " $jscomp$loop$0.y = 2;", + " $jscomp$loop$0.x = 1;", + " }", + " }($jscomp$loop$0);", + " $jscomp$loop$0 = {y: $jscomp$loop$0.y, x: $jscomp$loop$0.x};", + "}")); } public void testTypeAnnotationsOnLetConst() { diff --git a/test/com/google/javascript/jscomp/runtime_tests/let_const_rename_test.js b/test/com/google/javascript/jscomp/runtime_tests/let_const_rename_test.js index 6fa9763f0d3..23ddc3abbcd 100644 --- a/test/com/google/javascript/jscomp/runtime_tests/let_const_rename_test.js +++ b/test/com/google/javascript/jscomp/runtime_tests/let_const_rename_test.js @@ -386,3 +386,33 @@ function testNestedContinueDoesNotBreakClosures() { // i skipped because it was after h and in the same word assertEquals('bcdefgjkl', letters); } + +function testLetWithSameName_multivariateDeclaration() { + // See https://github.com/google/closure-compiler/issues/2969 + { + let x = 1, y = 2; + assertEquals(1, x); + assertEquals(2, y); + } + { + let x = 3, y = 4; + assertEquals(3, x); + assertEquals(4, y); + } +} + +function testLetInLoopClosure_multivariateDeclaration() { + for (let i = 0; i < 3; i++) { + let x, y; + // Verify that x and y are always undefined here, even though they are + // reassigned later in the loop. + assertUndefined(x); + assertUndefined(y); + function f() { + x = y = 3; + } + f(); + assertEquals(3, x); + assertEquals(3, y); + } +}