Skip to content

Commit 678b655

Browse files
lauraharkercopybara-github
authored andcommitted
Fix Es6RewriteDestructuring bug moving await/yield into an arrow function
Es6RewriteDestructuring wraps nested destructuring assignments in arrow functions, to simplify followup rewriting. This is invalid if destructuring assignment references `await` or `yield`. To address this: This CL looks for await/yield references, and: * always passes the right-hand side of the assignment as an argument to the arrow function, instead of evaluating it inside the arrow function. * throws a compile-time exception if the destructuring pattern references await/yield. I haven't seen this case in practice & it would be trickier to fix. We'd have to extract out sub-expressions from within the destructuring pattern, preserving order of operations. So I think it makes more sense to just explicitly throw an exception for now. PiperOrigin-RevId: 857235746
1 parent 1ca59ab commit 678b655

File tree

4 files changed

+439
-318
lines changed

4 files changed

+439
-318
lines changed

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

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.google.javascript.rhino.Node;
3131
import com.google.javascript.rhino.StaticScope;
3232
import com.google.javascript.rhino.Token;
33+
import com.google.javascript.rhino.jstype.JSTypeNative;
3334
import java.util.ArrayDeque;
3435
import java.util.ArrayList;
3536
import java.util.Deque;
@@ -299,7 +300,7 @@ private Node replacePatternParamWithTempVar(
299300
patternParam.replaceWith(newParam);
300301
Node newDecl = IR.var(patternParam, createTempVarNameNode(tempVarName, paramType));
301302
newDecl.srcrefTreeIfMissing(patternParam);
302-
303+
303304
if (insertSpot == null) {
304305
// insert new declarations only after all inner function declarations in that function body to
305306
// preserve normalization
@@ -770,34 +771,36 @@ private void replaceArrayPattern(
770771
* Transform
771772
* [x, y] = rhs
772773
* into
773-
* {@code (() => {
774-
* let temp0 = rhs;
774+
* {@code ((temp0) => {
775775
* var temp1 = $jscomp.makeIterator(temp0);
776776
* var x = temp0.next().value;
777777
* var y = temp0.next().value;
778778
* return temp0;
779-
* })()}
779+
* })(rhs)}
780780
*
781781
* Transform
782782
* {x: a, y: b} = rhs
783783
* into
784-
* {@code (() => {
785-
* let temp0 = rhs;
784+
* {@code ((temp0) => {
786785
* var temp1 = temp0;
787786
* var a = temp0.x;
788787
* var b = temp0.y;
789788
* return temp0;
790-
* })()}
789+
* })(rhs)}
791790
* </pre>
792791
*/
793792
private void wrapAssignOrDestructuringInCallToArrow(NodeTraversal t, Node assignment) {
794-
String tempVarName = getTempVariableName();
793+
Node lhs = assignment.getFirstChild();
795794
Node rhs = assignment.getLastChild().detach();
795+
// NOTE: we do not presently support await/yield in the LHS of nested destructuring assignments.
796+
// See b/475296868.
797+
validateNoAwaitOrYieldInNestedAssignmentPattern(lhs);
798+
799+
String tempVarName = getTempVariableName();
796800

797801
Node tempVarModel = createTempVarNameNode(tempVarName, type(rhs));
798-
// let temp0 = rhs;
799-
Node newAssignment = IR.let(tempVarModel.cloneNode(), rhs);
800-
NodeUtil.addFeatureToScript(t.getCurrentScript(), Feature.LET_DECLARATIONS, compiler);
802+
// ((temp0) => {...})
803+
Node paramList = IR.paramList(tempVarModel.cloneNode());
801804
// [x, y] = temp0;
802805
Node replacementExpr =
803806
astFactory.createAssign(assignment.removeFirstChild(), tempVarModel.cloneNode());
@@ -806,17 +809,23 @@ private void wrapAssignOrDestructuringInCallToArrow(NodeTraversal t, Node assign
806809
Node returnNode = IR.returnNode(tempVarModel.cloneNode());
807810

808811
// Create a function to hold these assignments:
809-
Node block = IR.block(newAssignment, exprResult, returnNode);
810-
Node arrowFn = astFactory.createZeroArgFunction(/* name= */ "", block, /* returnType= */ null);
812+
Node block = IR.block(exprResult, returnNode);
813+
Node arrowFn =
814+
astFactory.createFunction(
815+
/* name= */ "",
816+
paramList,
817+
block,
818+
type(JSTypeNative.UNKNOWN_TYPE, StandardColors.UNKNOWN));
811819
arrowFn.setIsArrowFunction(true);
812820

813821
// Create a call to the function, and replace the pattern with the call.
814-
Node call = astFactory.createCall(arrowFn, type(rhs));
822+
Node call = astFactory.createCall(arrowFn, type(rhs), rhs);
815823
NodeUtil.addFeatureToScript(t.getCurrentScript(), Feature.ARROW_FUNCTIONS, compiler);
816824
call.srcrefTreeIfMissing(assignment);
817825
call.putBooleanProp(Node.FREE_CALL, true);
818826
assignment.replaceWith(call);
819827
NodeUtil.markNewScopesChanged(call, compiler);
828+
820829
replacePattern(
821830
t,
822831
replacementExpr.getFirstChild(),
@@ -825,6 +834,18 @@ private void wrapAssignOrDestructuringInCallToArrow(NodeTraversal t, Node assign
825834
exprResult);
826835
}
827836

837+
private void validateNoAwaitOrYieldInNestedAssignmentPattern(Node pattern) {
838+
boolean hasAwaitOrYieldInLhs =
839+
NodeUtil.findPreorder(
840+
pattern, (n) -> n.isAwait() || n.isYield(), NodeUtil.MATCH_NOT_FUNCTION)
841+
!= null;
842+
checkState(
843+
!hasAwaitOrYieldInLhs,
844+
"Cannot transpile yet: destructuring assignment referencing await or yield in lhs, in"
845+
+ " nested sub-expression: %s",
846+
pattern);
847+
}
848+
828849
/** for (let [a, b, c] = arr; a < b; a++) */
829850
private void visitDestructuringPatternInVanillaForInnerVars(NodeTraversal t, Node pattern) {
830851
checkArgument(pattern.isDestructuringPattern());
@@ -843,7 +864,7 @@ private void visitDestructuringPatternInVanillaForInnerVars(NodeTraversal t, Nod
843864
insertionPoint.replaceWith(block);
844865
block.addChildToBack(insertionPoint);
845866
}
846-
// Fall through
867+
// Fall through
847868

848869
case VAR:
849870
{

0 commit comments

Comments
 (0)