Skip to content

Commit 88a0b70

Browse files
0xe0xe
committed
Fix issues with destructuring
- Implement support for reading from [Symbol.iterator] - Implement support for nested destructuring
1 parent 93057c9 commit 88a0b70

File tree

9 files changed

+481
-100
lines changed

9 files changed

+481
-100
lines changed

‎rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,14 @@ private void visitExpression(Node node, int contextFlags) {
806806
}
807807
break;
808808

809+
case Token.TO_ITERABLE_ARRAY:
810+
visitExpression(child, 0);
811+
addToken(type);
812+
// count for iterator finalization
813+
int elemCount = node.getIntProp(Node.DESTRUCTURING_ARRAY_LENGTH, 0);
814+
addUint8(Math.min(elemCount, 255));
815+
break;
816+
809817
case Token.GET_REF:
810818
case Token.DEL_REF:
811819
visitExpression(child, 0);

‎rhino/src/main/java/org/mozilla/javascript/DestructuringIterator.java‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
package org.mozilla.javascript;
6+
7+
import java.util.ArrayList;
8+
import java.util.List;
9+
10+
/**
11+
* A wrapper around an iterator for use in array destructuring. This allows destructuring to consume
12+
* iterator values one-by-one on demand, rather draining values to an array in one shot.
13+
*/
14+
public class DestructuringIterator extends IdScriptableObject {
15+
private static final long serialVersionUID = 1L;
16+
17+
private final Context cx;
18+
private final Scriptable scope;
19+
private final Callable nextFunc;
20+
private final Scriptable iteratorThis;
21+
private final Callable returnFunc;
22+
private final List<Object> cachedValues;
23+
private boolean done;
24+
private boolean closed;
25+
private int elementsNeeded;
26+
private boolean finalized;
27+
28+
public DestructuringIterator(Context cx, Scriptable scope, Object iteratorObj) {
29+
this.cx = cx;
30+
this.scope = scope;
31+
this.cachedValues = new ArrayList<>();
32+
this.done = false;
33+
this.closed = false;
34+
this.elementsNeeded = Integer.MAX_VALUE;
35+
this.finalized = false;
36+
37+
// Get the "next" function
38+
ScriptRuntime.LookupResult nextCall =
39+
ScriptRuntime.getPropAndThis(iteratorObj, ES6Iterator.NEXT_METHOD, cx, scope);
40+
this.nextFunc = nextCall.getCallable();
41+
this.iteratorThis = nextCall.getThis();
42+
43+
// Get the "return" function (may be null/undefined)
44+
Object retObj =
45+
ScriptRuntime.getObjectPropNoWarn(
46+
iteratorObj, ES6Iterator.RETURN_PROPERTY, cx, scope);
47+
if ((retObj != null) && !Undefined.isUndefined(retObj)) {
48+
if (!(retObj instanceof Callable)) {
49+
throw ScriptRuntime.notFunctionError(
50+
iteratorObj, retObj, ES6Iterator.RETURN_PROPERTY);
51+
}
52+
this.returnFunc = (Callable) retObj;
53+
} else {
54+
this.returnFunc = null;
55+
}
56+
}
57+
58+
@Override
59+
public String getClassName() {
60+
return "DestructuringIterator";
61+
}
62+
63+
/** Set the number of elements needed for destructuring. */
64+
public void setElementsNeeded(int count) {
65+
this.elementsNeeded = count;
66+
}
67+
68+
@Override
69+
public Object get(int index, Scriptable start) {
70+
// Fetch values up to the requested index
71+
while (cachedValues.size() <= index && !done) {
72+
fetchNext();
73+
}
74+
75+
// Auto-finalize if we've fetched all needed elements
76+
if (!finalized && cachedValues.size() >= elementsNeeded) {
77+
finalizeIfNeeded();
78+
}
79+
80+
if (index < cachedValues.size()) {
81+
return cachedValues.get(index);
82+
}
83+
84+
// Index beyond what iterator produced - return undefined
85+
return Undefined.instance;
86+
}
87+
88+
/** Fetch the next value from the iterator. */
89+
private void fetchNext() {
90+
if (done) {
91+
return;
92+
}
93+
94+
try {
95+
Object result = nextFunc.call(cx, scope, iteratorThis, ScriptRuntime.emptyArgs);
96+
// Check the "done" property
97+
Object doneVal =
98+
ScriptableObject.getProperty(
99+
ScriptableObject.ensureScriptable(result), ES6Iterator.DONE_PROPERTY);
100+
if (doneVal == Scriptable.NOT_FOUND) {
101+
doneVal = Undefined.instance;
102+
}
103+
104+
if (ScriptRuntime.toBoolean(doneVal)) {
105+
done = true;
106+
return;
107+
}
108+
109+
// Get the value
110+
Object value =
111+
ScriptRuntime.getObjectPropNoWarn(
112+
result, ES6Iterator.VALUE_PROPERTY, cx, scope);
113+
cachedValues.add(value);
114+
} catch (Exception e) {
115+
// Error during iteration - close the iterator
116+
closeIterator();
117+
throw e;
118+
}
119+
}
120+
121+
/** Close the iterator by calling its return() method. */
122+
public void closeIterator() {
123+
if (closed || returnFunc == null) {
124+
return;
125+
}
126+
127+
closed = true;
128+
try {
129+
returnFunc.call(cx, scope, iteratorThis, ScriptRuntime.emptyArgs);
130+
} catch (Exception e) {
131+
// TODO: ignore errors / should we log a warning?
132+
}
133+
}
134+
135+
/** Finalize destructuring. Close iterator if not exhausted. */
136+
private void finalizeIfNeeded() {
137+
if (finalized) {
138+
return;
139+
}
140+
finalized = true;
141+
142+
// If we're not exhausted after getting what we need, close the iterator
143+
if (!done) {
144+
closeIterator();
145+
}
146+
}
147+
}

‎rhino/src/main/java/org/mozilla/javascript/Interpreter.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,7 @@ private abstract static class InstructionClass {
15811581
instructionObjs[base + Token.REF_NS_MEMBER] = new DoRefNsMember();
15821582
instructionObjs[base + Token.REF_NAME] = new DoRefName();
15831583
instructionObjs[base + Token.REF_NS_NAME] = new DoRefNsName();
1584+
instructionObjs[base + Token.TO_ITERABLE_ARRAY] = new DoToIterableArray();
15841585
instructionObjs[base + Icode_SCOPE_LOAD] = new DoScopeLoad();
15851586
instructionObjs[base + Icode_SCOPE_SAVE] = new DoScopeSave();
15861587
instructionObjs[base + Icode_SPREAD] = new DoSpread();
@@ -4217,6 +4218,26 @@ NewState execute(Context cx, CallFrame frame, InterpreterState state, int op) {
42174218
}
42184219
}
42194220

4221+
private static class DoToIterableArray extends InstructionClass {
4222+
@Override
4223+
NewState execute(Context cx, CallFrame frame, InterpreterState state, int op) {
4224+
final Object obj = frame.stack[state.stackTop];
4225+
// wrap the iterator
4226+
DestructuringIterator wrapper =
4227+
(DestructuringIterator)
4228+
ScriptRuntime.wrapDestructuringIterator(obj, cx, frame.scope);
4229+
// element count from bytecode
4230+
int elementsNeeded = frame.idata.itsICode[frame.pc] & 0xFF;
4231+
if (elementsNeeded == 0) {
4232+
elementsNeeded = Integer.MAX_VALUE; // Unknown, fetch as needed
4233+
}
4234+
wrapper.setElementsNeeded(elementsNeeded);
4235+
frame.stack[state.stackTop] = wrapper;
4236+
frame.pc++; // Skip the element count byte
4237+
return null;
4238+
}
4239+
}
4240+
42204241
private static class DoScopeLoad extends InstructionClass {
42214242
@Override
42224243
NewState execute(Context cx, CallFrame frame, InterpreterState state, int op) {

‎rhino/src/main/java/org/mozilla/javascript/Parser.java‎

Lines changed: 64 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4446,22 +4446,38 @@ PerFunctionVariables createPerFunctionVariables(FunctionNode fnNode) {
44464446
* @return expression that performs a series of assignments to the variables defined in left
44474447
*/
44484448
Node createDestructuringAssignment(
4449-
int type, Node left, Node right, AstNode defaultValue, Transformer transformer) {
4449+
int type,
4450+
Node left,
4451+
Node right,
4452+
AstNode defaultValue,
4453+
Transformer transformer,
4454+
boolean isFunctionParameter) {
44504455
String tempName = currentScriptOrFn.getNextTempName();
44514456
Node result =
44524457
destructuringAssignmentHelper(
4453-
type, left, right, tempName, defaultValue, transformer);
4458+
type,
4459+
left,
4460+
right,
4461+
tempName,
4462+
defaultValue,
4463+
transformer,
4464+
isFunctionParameter);
44544465
Node comma = result.getLastChild();
44554466
comma.addChildToBack(createName(tempName));
44564467
return result;
44574468
}
44584469

4470+
Node createDestructuringAssignment(
4471+
int type, Node left, Node right, AstNode defaultValue, Transformer transformer) {
4472+
return createDestructuringAssignment(type, left, right, defaultValue, transformer, true);
4473+
}
4474+
44594475
Node createDestructuringAssignment(int type, Node left, Node right, Transformer transformer) {
4460-
return createDestructuringAssignment(type, left, right, null, transformer);
4476+
return createDestructuringAssignment(type, left, right, null, transformer, false);
44614477
}
44624478

44634479
Node createDestructuringAssignment(int type, Node left, Node right, AstNode defaultValue) {
4464-
return createDestructuringAssignment(type, left, right, defaultValue, null);
4480+
return createDestructuringAssignment(type, left, right, defaultValue, null, true);
44654481
}
44664482

44674483
Node destructuringAssignmentHelper(
@@ -4470,7 +4486,8 @@ Node destructuringAssignmentHelper(
44704486
Node right,
44714487
String tempName,
44724488
AstNode defaultValue,
4473-
Transformer transformer) {
4489+
Transformer transformer,
4490+
boolean isFunctionParameter) {
44744491
Scope result = createScopeNode(Token.LETEXPR, left.getLineno(), left.getColumn());
44754492
result.addChildToFront(new Node(Token.LET, createName(Token.NAME, tempName, right)));
44764493
try {
@@ -4492,7 +4509,8 @@ Node destructuringAssignmentHelper(
44924509
comma,
44934510
destructuringNames,
44944511
defaultValue,
4495-
transformer);
4512+
transformer,
4513+
isFunctionParameter);
44964514
} else if (left instanceof ObjectLiteral) {
44974515
empty =
44984516
destructuringObject(
@@ -4502,7 +4520,8 @@ Node destructuringAssignmentHelper(
45024520
comma,
45034521
destructuringNames,
45044522
defaultValue,
4505-
transformer);
4523+
transformer,
4524+
isFunctionParameter);
45064525
} else if (left.getType() == Token.GETPROP || left.getType() == Token.GETELEM) {
45074526
switch (variableType) {
45084527
case Token.CONST:
@@ -4529,11 +4548,13 @@ boolean destructuringArray(
45294548
Node parent,
45304549
List<String> destructuringNames,
45314550
AstNode defaultValue, /* defaultValue to use in function param decls */
4532-
Transformer transformer) {
4551+
Transformer transformer,
4552+
boolean isFunctionParameter) {
45334553
boolean empty = true;
45344554
int setOp = variableType == Token.CONST ? Token.SETCONST : Token.SETNAME;
45354555
int index = 0;
45364556
boolean defaultValuesSetup = false;
4557+
boolean iteratorConverted = false;
45374558
for (AstNode n : array.getElements()) {
45384559
if (n.getType() == Token.EMPTY) {
45394560
index++;
@@ -4546,6 +4567,27 @@ boolean destructuringArray(
45464567
defaultValuesSetup = true;
45474568
}
45484569

4570+
// make iterable array after default value is applied
4571+
if (isFunctionParameter && !iteratorConverted) {
4572+
// we need to know how many elements we actually need
4573+
int elementsNeeded = 0;
4574+
for (AstNode elem : array.getElements()) {
4575+
if (elem.getType() != Token.EMPTY) {
4576+
elementsNeeded++;
4577+
}
4578+
}
4579+
Node toArrayOp = new Node(Token.TO_ITERABLE_ARRAY, createName(tempName));
4580+
toArrayOp.putIntProp(Node.DESTRUCTURING_ARRAY_LENGTH, elementsNeeded);
4581+
Node reassign =
4582+
new Node(
4583+
Token.SETNAME,
4584+
createName(Token.BINDNAME, tempName, null),
4585+
toArrayOp);
4586+
parent.addChildToBack(reassign);
4587+
iteratorConverted = true;
4588+
empty = false;
4589+
}
4590+
45494591
if (n.getType() == Token.NAME) {
45504592
/* [x] = [1] */
45514593
String name = n.getString();
@@ -4564,7 +4606,8 @@ boolean destructuringArray(
45644606
(Assignment) n,
45654607
rightElem,
45664608
setOp,
4567-
transformer);
4609+
transformer,
4610+
isFunctionParameter);
45684611
} else {
45694612
parent.addChildToBack(
45704613
destructuringAssignmentHelper(
@@ -4573,7 +4616,8 @@ boolean destructuringArray(
45734616
rightElem,
45744617
currentScriptOrFn.getNextTempName(),
45754618
null,
4576-
transformer));
4619+
transformer,
4620+
isFunctionParameter));
45774621
}
45784622
index++;
45794623
empty = false;
@@ -4588,7 +4632,8 @@ private void processDestructuringDefaults(
45884632
Assignment n,
45894633
Node rightElem,
45904634
int setOp,
4591-
Transformer transformer) {
4635+
Transformer transformer,
4636+
boolean isFunctionParameter) {
45924637
Node left = n.getLeft();
45934638
Node right = null;
45944639
if (left.getType() == Token.NAME) {
@@ -4657,7 +4702,8 @@ private void processDestructuringDefaults(
46574702
cond_default,
46584703
currentScriptOrFn.getNextTempName(),
46594704
null,
4660-
transformer));
4705+
transformer,
4706+
isFunctionParameter));
46614707
} else {
46624708
reportError("msg.bad.assign.left");
46634709
}
@@ -4722,7 +4768,8 @@ boolean destructuringObject(
47224768
Node parent,
47234769
List<String> destructuringNames,
47244770
AstNode defaultValue, /* defaultValue to use in function param decls */
4725-
Transformer transformer) {
4771+
Transformer transformer,
4772+
boolean isFunctionParameter) {
47264773
boolean empty = true;
47274774
int setOp = variableType == Token.CONST ? Token.SETCONST : Token.SETNAME;
47284775
boolean defaultValuesSetup = false;
@@ -4781,7 +4828,8 @@ boolean destructuringObject(
47814828
(Assignment) value,
47824829
rightElem,
47834830
setOp,
4784-
transformer);
4831+
transformer,
4832+
isFunctionParameter);
47854833
} else {
47864834
parent.addChildToBack(
47874835
destructuringAssignmentHelper(
@@ -4790,7 +4838,8 @@ boolean destructuringObject(
47904838
rightElem,
47914839
currentScriptOrFn.getNextTempName(),
47924840
null,
4793-
transformer));
4841+
transformer,
4842+
isFunctionParameter));
47944843
}
47954844
empty = false;
47964845
}

0 commit comments

Comments
 (0)