diff --git a/src/com/google/javascript/jscomp/FlowSensitiveInlineVariables.java b/src/com/google/javascript/jscomp/FlowSensitiveInlineVariables.java index 6c5dd4b5afc..af5488aa110 100644 --- a/src/com/google/javascript/jscomp/FlowSensitiveInlineVariables.java +++ b/src/com/google/javascript/jscomp/FlowSensitiveInlineVariables.java @@ -258,7 +258,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { // Make sure that the name node is purely a read. if ((NodeUtil.isAssignmentOp(parent) && parent.getFirstChild() == n) - || parent.isVar() + || NodeUtil.isNameDeclaration(parent) || parent.isInc() || parent.isDec() || parent.isParamList() diff --git a/src/com/google/javascript/jscomp/MaybeReachingVariableUse.java b/src/com/google/javascript/jscomp/MaybeReachingVariableUse.java index 463420d0d17..e3f1d370bed 100644 --- a/src/com/google/javascript/jscomp/MaybeReachingVariableUse.java +++ b/src/com/google/javascript/jscomp/MaybeReachingVariableUse.java @@ -207,14 +207,20 @@ private void computeMayUse( return; case FOR_IN: + case FOR_OF: // for(x in y) {...} Node lhs = n.getFirstChild(); Node rhs = lhs.getNext(); - if (lhs.isVar()) { + if (NodeUtil.isNameDeclaration(lhs)) { lhs = lhs.getLastChild(); // for(var x in y) {...} + if (lhs.isDestructuringLhs()) { + lhs = lhs.getFirstChild(); // for (let [x] of obj) {...} + } } if (lhs.isName() && !conditional) { removeFromUseIfLocal(lhs.getString(), output); + } else if (lhs.isDestructuringPattern()) { + computeMayUse(lhs, cfgNode, output, true); } computeMayUse(rhs, cfgNode, output, conditional); return; @@ -232,7 +238,8 @@ private void computeMayUse( return; case VAR: - // TODO(b/73123594): this should also handle LET/CONST + case LET: + case CONST: Node varName = n.getFirstChild(); checkState(n.hasChildren(), "AST should be normalized", n); diff --git a/src/com/google/javascript/jscomp/MustBeReachingVariableDef.java b/src/com/google/javascript/jscomp/MustBeReachingVariableDef.java index 388459f4839..05bb0fc0503 100644 --- a/src/com/google/javascript/jscomp/MustBeReachingVariableDef.java +++ b/src/com/google/javascript/jscomp/MustBeReachingVariableDef.java @@ -258,13 +258,15 @@ private void computeMustDef( return; case FOR_IN: + case FOR_OF: // for(x in y) {...} Node lhs = n.getFirstChild(); Node rhs = lhs.getNext(); - if (lhs.isVar()) { + if (NodeUtil.isNameDeclaration(lhs)) { lhs = lhs.getLastChild(); // for(var x in y) {...} } if (lhs.isName()) { + // TODO(lharker): This doesn't seem right - given for (x in y), the value set to x isn't y addToDefIfLocal(lhs.getString(), cfgNode, rhs, output); } return; diff --git a/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java b/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java index 475719590d7..99011aa3f53 100644 --- a/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java +++ b/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java @@ -68,6 +68,19 @@ public void testSimpleVar() { inline("var x = 1; x = x + 1", "var x; x = 1 + 1"); } + public void testSimpleLet() { + inline("let x = 1; print(x)", "let x; print(1)"); + inline("let x = 1; x", "let x; 1"); + inline("let x = 1; let a = x", "let x; let a = 1"); + inline("let x = 1; x = x + 1", "let x; x = 1 + 1"); + } + + public void testSimpleConst() { + inline("const x = 1; print(x)", "const x = undefined; print(1)"); + inline("const x = 1; x", "const x = undefined; 1"); + inline("const x = 1; const a = x", "const x = undefined; const a = 1"); + } + public void testSimpleForIn() { inline("var a,b,x = a in b; x", "var a,b,x; a in b"); @@ -477,7 +490,15 @@ public void testForIn() { noInline("var x; var y = {}; for(x in y){}"); noInline("var x; var y = {}; var z; for(x in z = y){print(z)}"); noInline("var x; var y = {}; var z; for(x in y){print(z)}"); + } + public void testForInDestructuring() { + noInline("var x = 1, y = [], z; for ({z = x} in y) {}"); + noInline("var x = 1, y = [], z; for ([z = x] in y) {}"); + noInline("var x = 1, y = [], z; print(x); for ({z = x} in y) {}"); + noInline("var x = 1, y = [], z; print(x); for ([z = x] in y) {}"); + noInline("var x = 1, y = [], z; print(x); for (let {z = x} in y) {}"); + noInline("var x = 1, y = [], z; print(x); for (const {z = x} in y) {}"); } public void testNotOkToSkipCheckPathBetweenNodes() { @@ -568,7 +589,7 @@ public void testIssue794b() { "return x;"); } - public void testVarAssinInsideHookIssue965() { + public void testVarAssignInsideHookIssue965() { noInline("var i = 0; return 1 ? (i = 5) : 0, i;"); noInline("var i = 0; return (1 ? (i = 5) : 0) ? i : 0;"); noInline("var i = 0; return (1 ? (i = 5) : 0) || i;"); @@ -706,6 +727,30 @@ public void testBlockScoping() { "return(a)")); } + public void testBlockScoping_shouldntInline() { + // TODO(b/73123594): fix this test. It adds a reference to a block-scoped let outside its block. + inline( + lines( + "var JSCompiler_inline_result;", + "{", + " let a = 1;", + " if (3 < 4) {", + " a = 2;", + " }", + " JSCompiler_inline_result = a;", + "}", + "alert(JSCompiler_inline_result);"), + lines( + "var JSCompiler_inline_result;", + "{", + " let a = 1;", + " if (3 < 4) {", + " a = 2;", + " }", + "}", + "alert(a);")); // a is not defined here! This is wrong. + } + public void testInlineInGenerators() { test( lines( @@ -729,6 +774,17 @@ public void testNoInlineForOf() { noInline("var x = 1; var n = {}; for(x of n) {}"); } + public void testForOfDestructuring() { + noInline("var x = 1, y = [], z; for ({z = x} of y) {}"); + noInline("var x = 1, y = [], z; for ([z = x] of y) {}"); + + noInline("var x = 1, y = [], z; print(x); for ({z = x} of y) {}"); + noInline("var x = 1, y = [], z; print(x); for ([z = x] of y) {}"); + + noInline("var x = 1, y = [], z; print(x); for (let [z = x] of y) {}"); + noInline("var x = 1, y = [], z; print(x); for (const [z = x] of y) {}"); + } + public void testTemplateStrings() { inline("var name = 'Foo'; `Hello ${name}`", "var name; `Hello ${'Foo'}`"); diff --git a/test/com/google/javascript/jscomp/MaybeReachingVariableUseTest.java b/test/com/google/javascript/jscomp/MaybeReachingVariableUseTest.java index d7451f8fc5d..c87268894bb 100644 --- a/test/com/google/javascript/jscomp/MaybeReachingVariableUseTest.java +++ b/test/com/google/javascript/jscomp/MaybeReachingVariableUseTest.java @@ -110,6 +110,20 @@ public void testForIn() { assertNotMatch("D: var x = [], foo; U: for (x in foo) { }"); assertNotMatch("D: var x = [], foo; for (x in foo) { U:x }"); assertMatch("var x = [], foo; D: for (x in foo) { U:x }"); + assertMatch("var foo; D: for (let x in foo) { U:x }"); + assertMatch("var foo; D: for (const x in foo) { U:x }"); + assertMatch("D: var x = 1, foo; U: x; U: for (let [z = x] in foo) {}"); + assertMatch("D: var x = 1, foo; U: x; for (let [x] in foo) {}"); + } + + public void testForOf() { + assertNotMatch("D: var x = [], foo; U: for (x of foo) { }"); + assertNotMatch("D: var x = [], foo; for (x of foo) { U:x }"); + assertMatch("var x = [], foo; D: for (x of foo) { U:x }"); + assertMatch("var foo; D: for (let x of foo) { U:x }"); + assertMatch("var foo; D: for (const x of foo) { U:x }"); + assertMatch("D: var x = 1, foo; U: x; U: for (let [z = x] of foo) {}"); + assertMatch("D: var x = 1, foo; U: x; for (let [x] of foo) {}"); } public void testTryCatch() {