diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index 1f428ef7a01..2ebb1ba7a43 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -49,6 +49,7 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumSet; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -2774,6 +2775,31 @@ public static void removeChild(Node parent, Node node) { // need something for the condition. Others need to be replaced // or the structure removed. parent.replaceChild(node, IR.empty()); + } else if (parent.isObjectPattern()) { + // Remove the name from the object pattern + + // TODO (simranarora) remove the pattern and declaration when nothing is left and if there is + // an object literal on the right hand side, wrap it in an expression result + // E.g. var { } = { a:foo(), b:bar()} --> ({a:foo(), b:bar()}) + + parent.removeChild(node); + } else if (parent.isArrayPattern()) { + // Leave an empty slot for the array pattern node that gets removed. + // E.g. var [a, b] = [1, 2] becomes var [a, ,] = [1, 2] + + // TODO (simranarora) remove the pattern and declaration when nothing is left and just keep + // the initializer expression. + // E.g. var [ , ,] = [1, 2]; --> [1, 2]; + + parent.replaceChild(node, IR.empty()); + } else if (parent.isDestructuringLhs()) { + // Destructuring is empty so we should remove the node + parent.removeChild(node); + if (parent.getParent().hasChildren()) { + // removing the destructuring could leave an empty variable declaration node, so we would + // want to remove it from the AST + removeChild(parent.getParent(), parent); + } } else { throw new IllegalStateException("Invalid attempt to remove node: " + node + " of " + parent); } @@ -3236,6 +3262,39 @@ static Node getObjectLitKeyNode(Node key) { throw new IllegalStateException("Unexpected node type: " + key); } + /** + * Determine whether the destructuring object pattern is nested + * + * @param n object pattern node + */ + static boolean isNestedObjectPattern(Node n) { + checkState(n.isObjectPattern()); + Map destructuringMap = NodeUtil.getDestructuringMap(n.getParent()); + for (Node value : destructuringMap.values()) { + if (value.isObjectLit() + || value.isArrayLit() + || (value.isName() && value.getFirstChild() == null)) { + return true; + } + } + return false; + } + + /** + * Determine whether the destructuring array pattern is nested + * + * @param n array pattern node + */ + static boolean isNestedArrayPattern(Node n) { + checkState(n.isArrayPattern()); + for (Node key = n.getFirstChild(); key != null; key = key.getNext()) { + if (key.hasChildren()) { + return true; + } + } + return false; + } + /** * Determines whether a node represents an object literal get or set key * (e.g. key1 in {get key1() {}, set key2(a){}). @@ -5189,6 +5248,48 @@ public void visit(NodeTraversal t, Node n, Node parent) {} t.traverseAtScope(scope); } + /** + * Matches together a destructuring assignment with the value assigned + * + * @param n a destructuring node where the first child is the pattern array or object and the + * second child is an optional array or object literal + * @return map from the variable to value assigned (value assigned can be null!) + */ + static Map getDestructuringMap(Node n) { + checkState(n.isDestructuringLhs()); + Map destructurePairs = new HashMap<>(); + + boolean objLitRhs = n.getSecondChild().isObjectLit(); + boolean arrLitRhs = n.getSecondChild().isArrayLit(); + if (objLitRhs) { + Node literal = n.getSecondChild(); + for (Node value : literal.children()) { + destructurePairs.put(value.getString(), value.getFirstChild()); + } + } else if (arrLitRhs) { + Node pattern = n.getFirstChild(); + Node literal = n.getSecondChild(); + Node value = literal.getFirstChild(); + for (Node key : pattern.children()) { + destructurePairs.put(key.getString(), value); + if (value != null) { + value = value.getNext(); + } + } + } else if (n.getSecondChild() != null) { + Node pattern = n.getFirstChild(); + for (Node key : pattern.children()) { + destructurePairs.put(key.getString(), n.getSecondChild()); + } + } else { + Node pattern = n.getFirstChild(); + for (Node key : pattern.children()) { + destructurePairs.put(key.getString(), null); + } + } + return destructurePairs; + } + /** Returns true if the node is a property of an object literal. */ public static boolean isObjLitProperty(Node node) { return node.isStringKey() diff --git a/src/com/google/javascript/jscomp/RemoveUnusedVars.java b/src/com/google/javascript/jscomp/RemoveUnusedVars.java index 9917ac09c48..f7dfa9a3a0b 100644 --- a/src/com/google/javascript/jscomp/RemoveUnusedVars.java +++ b/src/com/google/javascript/jscomp/RemoveUnusedVars.java @@ -310,6 +310,39 @@ private void traverseNode(Node n, Node parent, Scope scope) { } return; + case ARRAY_PATTERN: + // VAR or LET or CONST + // DESTRUCTURING_LHS + // ARRAY_PATTERN + // NAME + + // back off if there are nested array patterns + if (n.getParent().isDestructuringLhs()) { + if (NodeUtil.isNestedArrayPattern(n)) { + break; + } else { + return; + } + } + break; + + case OBJECT_PATTERN: + // VAR or LET or CONST + // DESTRUCTURING_LHS + // OBJECT_PATTERN + // STRING + // NAME + + // back off if there are nested object patterns + if (n.getParent().isDestructuringLhs()) { + if (NodeUtil.isNestedObjectPattern(n)) { + break; + } else { + return; + } + } + break; + case NAME: var = scope.getVar(n.getString()); if (NodeUtil.isNameDeclaration(parent)) { @@ -775,7 +808,6 @@ private Definition getFunctionDefinition(Node function) { } } - /** * Look at all the property assigns to all variables. * These may or may not count as references. For example, @@ -893,10 +925,14 @@ private void removeUnreferencedVars() { Node nameNode = var.nameNode; Node toRemove = nameNode.getParent(); Node parent = toRemove.getParent(); + Node grandParent = toRemove.getGrandparent(); checkState( NodeUtil.isNameDeclaration(toRemove) || toRemove.isFunction() || (toRemove.isParamList() && parent.isFunction()) + || NodeUtil.isDestructuringDeclaration(grandParent) // Array Pattern + || (parent.isObjectPattern() + && NodeUtil.isDestructuringDeclaration(grandParent.getParent())) // Object Pattern || toRemove.isClass(), "We should only declare Vars and functions and function args and classes"); @@ -913,6 +949,12 @@ private void removeUnreferencedVars() { // Don't remove bleeding functions. } else if (parent.isForIn()) { // foreach iterations have 3 children. Leave them alone. + } else if (parent.isDestructuringPattern()) { + compiler.reportChangeToEnclosingScope(toRemove); + NodeUtil.removeChild(parent, toRemove); + } else if (parent.isDestructuringLhs()) { + compiler.reportChangeToEnclosingScope(nameNode); + NodeUtil.removeChild(toRemove, nameNode); } else if (NodeUtil.isNameDeclaration(toRemove) && nameNode.hasChildren() && NodeUtil.mayHaveSideEffects(nameNode.getFirstChild(), compiler)) { diff --git a/test/com/google/javascript/jscomp/NodeUtilTest.java b/test/com/google/javascript/jscomp/NodeUtilTest.java index f326a0673a9..c58c8fdcbda 100644 --- a/test/com/google/javascript/jscomp/NodeUtilTest.java +++ b/test/com/google/javascript/jscomp/NodeUtilTest.java @@ -1059,6 +1059,41 @@ public void testRemoveLabelChild2() { } } + public void testRemovePatternChild() { + // remove variable declaration from object pattern + Node actual = parse("var {a, b, c} = {a:1, b:2, c:3}"); + Node varNode = actual.getFirstChild(); + Node destructure = varNode.getFirstChild(); + Node pattern = destructure.getFirstChild(); + Node a = pattern.getFirstChild(); + Node b = a.getNext(); + Node c = b.getNext(); + + NodeUtil.removeChild(pattern, a); + String expected = "var {b, c} = {a:1, b:2, c:3};"; + String difference = parse(expected).checkTreeEquals(actual); + assertNull("Nodes do not match:\n" + difference, difference); + + // Remove all entries in object pattern + NodeUtil.removeChild(pattern, b); + NodeUtil.removeChild(pattern, c); + expected = "var { } = {a:1, b:2, c:3};"; + difference = parse(expected).checkTreeEquals(actual); + assertNull("Nodes do not match:\n" + difference, difference); + + // remove variable declaration from array pattern + actual = parse("var [a, b] = [1, 2]"); + varNode = actual.getFirstChild(); + destructure = varNode.getFirstChild(); + pattern = destructure.getFirstChild(); + a = pattern.getFirstChild(); + + NodeUtil.removeChild(pattern, a); + expected = "var [ , b] = [1, 2];"; + difference = parse(expected).checkTreeEquals(actual); + assertNull("Nodes do not match:\n" + difference, difference); + } + public void testRemoveForChild() { // Test removing the initializer. Node actual = parse("for(var a=0;a<0;a++)foo()"); @@ -2052,6 +2087,25 @@ public void testIsNotLValue() { assertNotLValueNamedX(x); } + public void testIsNestedObjectPattern() { + Node root = parse("var {a, b} = {a:1, b:2}"); + Node destructuring = root.getFirstFirstChild(); + Node objPattern = destructuring.getFirstChild(); + assertFalse(NodeUtil.isNestedObjectPattern(objPattern)); + + root = parse("var {a, b:{c}} = {a:{}, b:{c:5}};"); + destructuring = root.getFirstFirstChild(); + objPattern = destructuring.getFirstChild(); + assertTrue(NodeUtil.isNestedObjectPattern(objPattern)); + } + + public void testIsNestedArrayPattern() { + Node root = parse("var [a, b] = [1, 2]"); + Node destructuring = root.getFirstFirstChild(); + Node arrayPattern = destructuring.getFirstChild(); + assertFalse(NodeUtil.isNestedArrayPattern(arrayPattern)); + } + public void testLhsByDestructuring1() { Node root = parse("var [a, b] = obj;"); Node destructLhs = root.getFirstFirstChild(); @@ -2641,6 +2695,53 @@ public void testIsVarArgs() { assertFalse(NodeUtil.isVarArgsFunction(getNode("() => arguments"))); } + public void testDestructuringMap1() { + String code = "var {a, b, c} = {a: 1, b: 2, c: 3};"; + Node ast = parse(code); + Node varNode = ast.getFirstChild(); + Node destructuringNode = varNode.getFirstChild(); + Map destructuringMap = NodeUtil.getDestructuringMap(destructuringNode); + assertEquals(3, destructuringMap.size()); + } + + public void testDestructuringMap2() { + String code = "var {a} = {a:1, b:2};"; + Node ast = parse(code); + Node varNode = ast.getFirstChild(); + Node destructuringNode = varNode.getFirstChild(); + Map destructuringMap = NodeUtil.getDestructuringMap(destructuringNode); + assertEquals(2, destructuringMap.size()); + } + + public void testDestructuringMap3() { + String code = "var [a, b] = [1, 2];"; + Node ast = parse(code); + Node varNode = ast.getFirstChild(); + Node destructuringNode = varNode.getFirstChild(); + Map destructuringMap = NodeUtil.getDestructuringMap(destructuringNode); + assertEquals(2, destructuringMap.size()); + } + + public void testDestructuringMap4() { + // Destructuring pattern and object literal are in different orders + String code = "var {a, b} = { b:2, a:1 }"; + Node ast = parse(code); + Node varNode = ast.getFirstChild(); + Node destructuringNode = varNode.getFirstChild(); + Map destructuringMap = NodeUtil.getDestructuringMap(destructuringNode); + assertEquals(1, (int) destructuringMap.get("a").getDouble()); + } + + public void testDestructuringMap5() { + String code = "var {a, b} = foo();"; + Node ast = parse(code); + Node varNode = ast.getFirstChild(); + Node destructuringNode = varNode.getFirstChild(); + Map destructuringMap = NodeUtil.getDestructuringMap(destructuringNode); + assertTrue(destructuringMap.get("a").isCall()); + assertTrue(destructuringMap.get("b").isCall()); + } + private boolean executedOnceTestCase(String code) { Node ast = parse(code); Node nameNode = getNameNode(ast, "x"); diff --git a/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java b/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java index c496c99795c..5330fde2745 100644 --- a/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java +++ b/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java @@ -1108,7 +1108,7 @@ public void testTemplateStrings() { ); } - public void testDestructuring() { + public void testDestructuringArrayPattern() { testSame( LINE_JOINER.join( "var a; var b", @@ -1123,8 +1123,45 @@ public void testDestructuring() { "[a] = [1]" )); - testSame("var [a, b] = [1, 2]"); + testSame("var [a, b] = [1, 2]; f(a); f(b);"); + } + + public void testDestructuringObjectPattern() { + /*testSame(LINE_JOINER.join("var a; var b;", "({a, b} = {a:1, b:2})")); + + test( + LINE_JOINER.join("var a; var b;", "({a} = {a:1})"), + LINE_JOINER.join("var a; ", "({a} = {a:1})")); + testSame("var {a, b} = {a:1, b:2}; f(a); f(b);"); + + testSame("var {a} = {p:{}, q:{}}; a.q = 4;");*/ + + // Nested Destructuring + testSame("const someObject = {a:{ b:1 }}; var {a: {b}} = someObject; someObject.a.b;"); } + public void testRemoveUnusedVarsDeclaredInDestructuring() { + // Array destructuring + test("var [a, b] = [1, 2]; f(a);", "var [a, ,] = [1, 2]; f(a);"); + + test("var [a, b] = [1, 2];", "var [,,] = [1, 2];"); + + // Object pattern destructuring + test("var {a, b} = {a:1, b:2}; f(a);", "var {a, } = {a:1, b:2}; f(a);"); + + test("var {a, b} = {a:1, b:2};", "var { } = {a:1, b:2};"); + + // Nested pattern - pass backs off for now + // TODO (simranarora) Implement support for nested destructures + testSame("var {a, b:{c}} = {a:1, b:{c:5}}; f(a);"); + + // Value may have side effects + test("var {a, b} = {a:foo(), b:bar()};", "var { } = {a:foo(), b:bar()};"); + + test("var {a, b} = foo();", "var {} = foo();"); + + // Same as above case without the destructuring declaration + test("var a, b = foo();", "foo();"); + } }