From 96ff5942cb7c28624c632c0104cbf48a5916fdeb Mon Sep 17 00:00:00 2001 From: yitingwang Date: Tue, 13 Jun 2017 11:25:39 -0700 Subject: [PATCH] Changed Es6RewriteDestructuring to handle an assignment to an object pattern that is not wrapped in an expr result. Transformed the destructuring assignment into an arrow function that performs the original destructuring assignment and returns the rhs of the assignment as the result. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=158872413 --- .../jscomp/Es6RewriteDestructuring.java | 95 +++++++++---------- .../jscomp/Es6RewriteDestructuringTest.java | 51 +++++++++- .../javascript/jscomp/MultiPassTest.java | 47 +++++++++ .../runtime_tests/mixed_destructuring_test.js | 18 ++++ 4 files changed, 160 insertions(+), 51 deletions(-) diff --git a/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java b/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java index c82486a8078..9fcbca1236a 100644 --- a/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java +++ b/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java @@ -75,10 +75,8 @@ public void visit(NodeTraversal t, Node n, Node parent) { } switch (n.getToken()) { case ARRAY_PATTERN: - visitArrayPattern(t, n, parent); - break; case OBJECT_PATTERN: - visitObjectPattern(t, n, parent); + visitPattern(t, n, parent); break; default: break; @@ -218,35 +216,54 @@ private void visitForOf(Node node) { } } - private void visitObjectPattern(NodeTraversal t, Node objectPattern, Node parent) { - Node rhs; - Node nodeToDetach; + private void visitPattern(NodeTraversal t, Node pattern, Node parent) { if (NodeUtil.isNameDeclaration(parent) && !NodeUtil.isEnhancedFor(parent.getParent())) { - rhs = objectPattern.getNext(); - nodeToDetach = parent; - } else if (parent.isAssign() && parent.getParent().isExprResult()) { - rhs = parent.getLastChild(); - nodeToDetach = parent.getParent(); + replacePattern(t, pattern, pattern.getNext(), parent, parent); + } else if (parent.isAssign()) { + if (parent.getParent().isExprResult()) { + replacePattern(t, pattern, pattern.getNext(), parent, parent.getParent()); + } else { + wrapAssignmentInCallToArrow(t, parent); + } } else if (parent.isRest() || parent.isStringKey() || parent.isArrayPattern() || parent.isDefaultValue()) { - // Nested object pattern; do nothing. We will visit it after rewriting the parent. - return; + // Nested pattern; do nothing. We will visit it after rewriting the parent. } else if (NodeUtil.isEnhancedFor(parent) || NodeUtil.isEnhancedFor(parent.getParent())) { - visitDestructuringPatternInEnhancedFor(objectPattern); - return; + visitDestructuringPatternInEnhancedFor(pattern); } else if (parent.isCatch()) { - visitDestructuringPatternInCatch(objectPattern); - return; + visitDestructuringPatternInCatch(pattern); } else { - throw new IllegalStateException("Unexpected OBJECT_PATTERN parent: " + parent); + if (pattern.isArrayPattern()) { + throw new IllegalStateException("Unexpected ARRAY_PATTERN parent: " + parent); + } else { + throw new IllegalStateException("Unexpected OBJECT_PATTERN parent: " + parent); + } + } + } + + private void replacePattern( + NodeTraversal t, Node pattern, Node rhs, Node parent, Node nodeToDetach) { + switch (pattern.getToken()) { + case ARRAY_PATTERN: + replaceArrayPattern(t, pattern, rhs, parent, nodeToDetach); + break; + case OBJECT_PATTERN: + replaceObjectPattern(t, pattern, rhs, parent, nodeToDetach); + break; + default: + throw new IllegalStateException("Unexpected pattern: " + parent.getFirstChild()); } + } - // Convert 'var {a: b, c: d} = rhs' to: - // /** @const */ var temp = rhs; - // var b = temp.a; - // var d = temp.c; + /** + * Convert 'var {a: b, c: d} = rhs' to: + * + * @const var temp = rhs; var b = temp.a; var d = temp.c; + */ + private void replaceObjectPattern( + NodeTraversal t, Node objectPattern, Node rhs, Node parent, Node nodeToDetach) { String tempVarName = DESTRUCTURING_TEMP_VAR + (destructuringVarCounter++); Node tempDecl = IR.var(IR.name(tempVarName), rhs.detach()) .useSourceInfoIfMissingFromForTree(objectPattern); @@ -331,30 +348,6 @@ private void visitObjectPattern(NodeTraversal t, Node objectPattern, Node parent t.reportCodeChange(); } - private void visitArrayPattern(NodeTraversal t, Node arrayPattern, Node parent) { - if (NodeUtil.isNameDeclaration(parent) && !NodeUtil.isEnhancedFor(parent.getParent())) { - replaceArrayPattern(t, arrayPattern, arrayPattern.getNext(), parent, parent); - } else if (parent.isAssign()) { - if (parent.getParent().isExprResult()) { - replaceArrayPattern(t, arrayPattern, arrayPattern.getNext(), parent, parent.getParent()); - } else { - wrapAssignmentInCallToArrow(t, parent); - } - } else if (parent.isArrayPattern() - || parent.isRest() - || parent.isDefaultValue() - || parent.isStringKey()) { - // This is a nested array pattern. Don't do anything now; we'll visit it - // after visiting the parent. - } else if (NodeUtil.isEnhancedFor(parent) || NodeUtil.isEnhancedFor(parent.getParent())) { - visitDestructuringPatternInEnhancedFor(arrayPattern); - } else if (parent.isCatch()) { - visitDestructuringPatternInCatch(arrayPattern); - } else { - throw new IllegalStateException("Unexpected ARRAY_PATTERN parent: " + parent); - } - } - /** * Convert 'var [x, y] = rhs' to: var temp = $jscomp.makeIterator(rhs); var x = temp.next().value; * var y = temp.next().value; @@ -443,9 +436,11 @@ private void replaceArrayPattern( } /** - * Convert the assignment '[x, y] = rhs' (used as an expression and not an expr result) to: (() => - * { var temp = $jscomp.makeIterator(rhs); var x = temp.next().value; var y = temp.next().value; - * return temp; }) + * Convert the assignment '[x, y] = rhs' that is used as an expression and not an expr result to: + * (() => let temp0 = rhs; var temp1 = $jscomp.makeIterator(temp0); var x = temp0.next().value; + * var y = temp0.next().value; return temp0; }) And the assignment '{x: a, y: b} = rhs' used as an + * expression and not an expr result to: (() => let temp0 = rhs; var temp1 = temp0; var a = + * temp0.x; var b = temp0.y; return temp0; }) */ private void wrapAssignmentInCallToArrow(NodeTraversal t, Node assignment) { String tempVarName = DESTRUCTURING_TEMP_VAR + (destructuringVarCounter++); @@ -460,7 +455,7 @@ private void wrapAssignmentInCallToArrow(NodeTraversal t, Node assignment) { call.putBooleanProp(Node.FREE_CALL, true); assignment.getParent().replaceChild(assignment, call); NodeUtil.markNewScopesChanged(call, compiler); - replaceArrayPattern( + replacePattern( t, replacementExpr.getFirstChild(), replacementExpr.getLastChild(), diff --git a/test/com/google/javascript/jscomp/Es6RewriteDestructuringTest.java b/test/com/google/javascript/jscomp/Es6RewriteDestructuringTest.java index 42fe0604c75..88fbdf8c04f 100644 --- a/test/com/google/javascript/jscomp/Es6RewriteDestructuringTest.java +++ b/test/com/google/javascript/jscomp/Es6RewriteDestructuringTest.java @@ -638,7 +638,7 @@ public void testTypeCheck_inlineAnnotations() { TYPE_MISMATCH_WARNING); } - public void testDestructuringNotInExprResult() { + public void testDestructuringArrayNotInExprResult() { test( LINE_JOINER.join("var x, a, b;", "x = ([a,b] = [1,2])"), LINE_JOINER.join( @@ -713,6 +713,55 @@ public void testDestructuringNotInExprResult() { "}")); } + public void testDestructuringObjectNotInExprResult() { + test( + "var x = ({a: b, c: d} = foo());", + LINE_JOINER.join( + "var x = (()=>{", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " b = $jscomp$destructuring$var1.a;", + " d = $jscomp$destructuring$var1.c;", + " return $jscomp$destructuring$var0;", + "})();")); + + test( + "var x = ({a: b, c: d} = foo());", + LINE_JOINER.join( + "var x = (()=>{", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " b = $jscomp$destructuring$var1.a;", + " d = $jscomp$destructuring$var1.c;", + " return $jscomp$destructuring$var0;", + "})();")); + + test( + "var x; var y = ({a: x} = foo());", + LINE_JOINER.join( + "var x;", + "var y = (()=>{", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " x = $jscomp$destructuring$var1.a;", + " return $jscomp$destructuring$var0;", + "})();")); + + test( + "var x; var y = (() => {return {a,b} = foo();})();", + LINE_JOINER.join( + "var x;", + "var y = (()=>{", + " return (()=>{", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " a = $jscomp$destructuring$var1.a;", + " b = $jscomp$destructuring$var1.b;", + " return $jscomp$destructuring$var0;", + " })();", + "})();")); + } + public void testNestedDestructuring() { test( "var [[x]] = [[1]];", diff --git a/test/com/google/javascript/jscomp/MultiPassTest.java b/test/com/google/javascript/jscomp/MultiPassTest.java index 9803830a179..8389d99c6cd 100644 --- a/test/com/google/javascript/jscomp/MultiPassTest.java +++ b/test/com/google/javascript/jscomp/MultiPassTest.java @@ -259,6 +259,53 @@ public void testDestructuringAndArrowFunction() { " } ()){", "console.log(x);", "}")); + + test( + "var x = ({a: b, c: d} = foo());", + LINE_JOINER.join( + "var x = function () {", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " b = $jscomp$destructuring$var1.a;", + " d = $jscomp$destructuring$var1.c;", + " return $jscomp$destructuring$var0;", + "} ();")); + + test( + "var x = ({a: b, c: d} = foo());", + LINE_JOINER.join( + "var x = function () {", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " b = $jscomp$destructuring$var1.a;", + " d = $jscomp$destructuring$var1.c;", + " return $jscomp$destructuring$var0;", + "} ();")); + + test( + "var x; var y = ({a: x} = foo());", + LINE_JOINER.join( + "var x;", + "var y = function () {", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " x = $jscomp$destructuring$var1.a;", + " return $jscomp$destructuring$var0;", + "} ();")); + + test( + "var x; var y = (() => {return {a,b} = foo();})();", + LINE_JOINER.join( + "var x;", + "var y = function () {", + " return function () {", + " let $jscomp$destructuring$var0 = foo();", + " var $jscomp$destructuring$var1 = $jscomp$destructuring$var0;", + " a = $jscomp$destructuring$var1.a;", + " b = $jscomp$destructuring$var1.b;", + " return $jscomp$destructuring$var0;", + " } ();", + "} ();")); } private void addCollapseObjectLiterals() { diff --git a/test/com/google/javascript/jscomp/runtime_tests/mixed_destructuring_test.js b/test/com/google/javascript/jscomp/runtime_tests/mixed_destructuring_test.js index 95a211bdeda..a037166a872 100644 --- a/test/com/google/javascript/jscomp/runtime_tests/mixed_destructuring_test.js +++ b/test/com/google/javascript/jscomp/runtime_tests/mixed_destructuring_test.js @@ -83,3 +83,21 @@ function testDestructuringArray() { assertEquals(z[0], 0); assertEquals(z[1], 1); } + +function testDestructuringObject() { + var x, y, z; + x = ({a: y, b: z} = {a: 1, b: 2}); + assertEquals(y, 1); + assertEquals(z, 2); + assertEquals(x.a, 1); + assertEquals(x.b, 2); + + let id = 0; + let getNextId = () => (id++); + let a, b, c; + a = ({b, c} = {b: getNextId(), c: getNextId()}); + assertEquals(b, 0); + assertEquals(c, 1); + assertEquals(a.b, 0); + assertEquals(a.c, 1); +}