Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Transform default values correctly in destructuring #1708

Merged
merged 2 commits into from
Oct 23, 2024
+117 −233
Merged

FIX: Transform default values correctly in destructuring #1708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ffcfe4
FIX: Transform default values correctly in destructuring
0xe Oct 22, 2024
cfe9c80
FIX: update test262.properties
0xe Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions rhino/src/main/java/org/mozilla/javascript/IRFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,10 @@ private Node transformFunction(FunctionNode fn) {
if (dfns != null) {
for (var i : dfns) {
Node a = i[0];
AstNode b = (AstNode) i[1];
a.replaceChild(b, transform(b));
if (i[1] instanceof AstNode) {
AstNode b = (AstNode) i[1];
a.replaceChild(b, transform(b));
}
}
}

Expand Down
1 change: 1 addition & 0 deletions rhino/src/main/java/org/mozilla/javascript/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ public void removeChild(Node child) {
}

public void replaceChild(Node child, Node newChild) {
if (child == newChild) return;
newChild.next = child.next;
if (child == first) {
first = newChild;
Expand Down
90 changes: 29 additions & 61 deletions rhino/src/main/java/org/mozilla/javascript/Parser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ private void parseFunctionParams(FunctionNode fnNode) throws IOException {
// Would prefer not to call createDestructuringAssignment until codegen,
// but the symbol definitions have to happen now, before body is parsed.
Map<String, Node> destructuring = null;
Map<String, Node> destructuringDefault = null;
Map<String, AstNode> destructuringDefault = null;

Set<String> paramNames = new HashSet<>();
do {
Expand Down Expand Up @@ -878,7 +878,7 @@ private void parseFunctionParams(FunctionNode fnNode) throws IOException {
Node destructuringNode = new Node(Token.COMMA);
// Add assignment helper for each destructuring parameter
for (Map.Entry<String, Node> param : destructuring.entrySet()) {
Node defaultValue = null;
AstNode defaultValue = null;
if (destructuringDefault != null) {
defaultValue = destructuringDefault.get(param.getKey());
}
Expand Down Expand Up @@ -1024,7 +1024,7 @@ private AstNode arrowFunction(AstNode params) throws IOException {
// Would prefer not to call createDestructuringAssignment until codegen,
// but the symbol definitions have to happen now, before body is parsed.
Map<String, Node> destructuring = new HashMap<>();
Map<String, Node> destructuringDefault = new HashMap<>();
Map<String, AstNode> destructuringDefault = new HashMap<>();
Set<String> paramNames = new HashSet<>();

PerFunctionVariables savedVars = new PerFunctionVariables(fnNode);
Expand All @@ -1047,7 +1047,7 @@ private AstNode arrowFunction(AstNode params) throws IOException {
Node destructuringNode = new Node(Token.COMMA);
// Add assignment helper for each destructuring parameter
for (Map.Entry<String, Node> param : destructuring.entrySet()) {
Node defaultValue = null;
AstNode defaultValue = null;
if (destructuringDefault != null) {
defaultValue = destructuringDefault.get(param.getKey());
}
Expand Down Expand Up @@ -1087,7 +1087,7 @@ private void arrowFunctionParams(
FunctionNode fnNode,
AstNode params,
Map<String, Node> destructuring,
Map<String, Node> destructuringDefault,
Map<String, AstNode> destructuringDefault,
Set<String> paramNames)
throws IOException {
if (params instanceof ArrayLiteral || params instanceof ObjectLiteral) {
Expand Down Expand Up @@ -4194,7 +4194,7 @@ PerFunctionVariables createPerFunctionVariables(FunctionNode fnNode) {
* @return expression that performs a series of assignments to the variables defined in left
*/
Node createDestructuringAssignment(
int type, Node left, Node right, Node defaultValue, Transformer transformer) {
int type, Node left, Node right, AstNode defaultValue, Transformer transformer) {
String tempName = currentScriptOrFn.getNextTempName();
Node result =
destructuringAssignmentHelper(
Expand All @@ -4208,7 +4208,7 @@ Node createDestructuringAssignment(int type, Node left, Node right, Transformer
return createDestructuringAssignment(type, left, right, null, transformer);
}

Node createDestructuringAssignment(int type, Node left, Node right, Node defaultValue) {
Node createDestructuringAssignment(int type, Node left, Node right, AstNode defaultValue) {
return createDestructuringAssignment(type, left, right, defaultValue, null);
}

Expand All @@ -4217,7 +4217,7 @@ Node destructuringAssignmentHelper(
Node left,
Node right,
String tempName,
Node defaultValue,
AstNode defaultValue,
Transformer transformer) {
Scope result = createScopeNode(Token.LETEXPR, left.getLineno());
result.addChildToFront(new Node(Token.LET, createName(Token.NAME, tempName, right)));
Expand Down Expand Up @@ -4276,7 +4276,7 @@ boolean destructuringArray(
String tempName,
Node parent,
List<String> destructuringNames,
Node defaultValue, /* defaultValue to use in function param decls */
AstNode defaultValue, /* defaultValue to use in function param decls */
Transformer transformer) {
boolean empty = true;
int setOp = variableType == Token.CONST ? Token.SETCONST : Token.SETNAME;
Expand Down Expand Up @@ -4347,14 +4347,7 @@ private void processDestructuringDefaults(
// : $1[0])
// : x

if ((n.getRight() instanceof FunctionNode
|| n.getRight() instanceof UpdateExpression
|| n.getRight() instanceof ParenthesizedExpression)
&& transformer != null) {
right = transformer.transform(n.getRight());
} else {
right = n.getRight();
}
right = (transformer != null) ? transformer.transform(n.getRight()) : n.getRight();

Node cond_inner =
new Node(
Expand All @@ -4363,21 +4356,18 @@ private void processDestructuringDefaults(
right,
rightElem);

// if right is a function/update expression, it should be processed later
// store it in the node to be processed
if ((right instanceof FunctionNode
|| right instanceof UpdateExpression
|| right instanceof ParenthesizedExpression)
&& transformer == null) {
currentScriptOrFn.putDestructuringRvalues(cond_inner, right);
}

Node cond =
new Node(
Token.HOOK,
new Node(Token.SHEQ, createName("undefined"), createName(name)),
cond_inner,
left);

// store it to be transformed later
if (transformer == null) {
currentScriptOrFn.putDestructuringRvalues(cond_inner, right);
}

parent.addChildToBack(new Node(setOp, createName(Token.BINDNAME, name, null), cond));
if (variableType != -1) {
defineSymbol(variableType, name, true);
Expand Down Expand Up @@ -4406,43 +4396,17 @@ static Object getPropKey(Node id) {
}

private void setupDefaultValues(
String tempName, Node parent, Node defaultValue, int setOp, Transformer transformer) {
String tempName,
Node parent,
AstNode defaultValue,
int setOp,
Transformer transformer) {
if (defaultValue != null) {
// if there's defaultValue it can be substituted for tempName if that's undefined
// i.e. $1 = ($1 == undefined) ? defaultValue : $1
Node defaultRvalue = new Node(defaultValue.getType());

if (defaultValue instanceof ArrayLiteral) {
for (AstNode child : ((ArrayLiteral) defaultValue).getElements())
defaultRvalue.addChildToBack(child);
} else if (defaultValue instanceof ObjectLiteral) {
// TODO: check if "Symbol.iterator" is defined
// Node error_call = new Node(Token.NEW, createName("Error"));
// error_call.addChildToBack(Node.newString("value is not
// iterable"));
//
// Node check_iterator = new Node(
// Token.HOOK,
// new Node(Token.SHEQ,
// new Node(Token.GETPROP,
// defaultValue,
// createName("Symbol.iterator")),
// createName("undefined")),
// error_call,
// new Node(Token.TRUE));
// parent.addChildToBack(check_iterator);

List<ObjectProperty> elems = ((ObjectLiteral) defaultValue).getElements();
Object[] props = new Object[elems.size()];
int i = 0;
for (ObjectProperty child : elems) {
Object key = getPropKey(child.getLeft());
Node right = child.getRight();
props[i++] = key;
defaultRvalue.addChildToBack(right);
}
defaultRvalue.putProp(Node.OBJECT_IDS_PROP, props);
}

Node defaultRvalue =
transformer != null ? transformer.transform(defaultValue) : defaultValue;

Node cond_default =
new Node(
Expand All @@ -4451,6 +4415,10 @@ private void setupDefaultValues(
defaultRvalue,
createName(tempName));

if (transformer == null) {
currentScriptOrFn.putDestructuringRvalues(cond_default, defaultRvalue);
}

Node set_default =
new Node(setOp, createName(Token.BINDNAME, tempName, null), cond_default);
parent.addChildToBack(set_default);
Expand All @@ -4463,7 +4431,7 @@ boolean destructuringObject(
String tempName,
Node parent,
List<String> destructuringNames,
Node defaultValue, /* defaultValue to use in function param decls */
AstNode defaultValue, /* defaultValue to use in function param decls */
Transformer transformer) {
boolean empty = true;
int setOp = variableType == Token.CONST ? Token.SETCONST : Token.SETNAME;
Expand Down
74 changes: 70 additions & 4 deletions tests/src/test/java/org/mozilla/javascript/tests/DefaultParametersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,82 @@ public void functionDefaultArgsObjectArrow() throws Exception {
}

@Test
@Ignore("defaults-not-supported-in-let-destructuring")
@Ignore("destructuring-not-supported-in-for-let-expressions")
public void letExprDestructuring() throws Exception {
// JavaScript
final String script =
"function a() {}; (function() { "
"var a = 12; (function() { "
+ " for (let {x = a} = {}; ; ) { "
+ " return x; "
+ " }"
+ " })()";
Utils.assertWithAllOptimizationLevelsES6(12, script);
}

@Test
public void normObjectLiteralDestructuringFunCall() throws Exception {
// JavaScript
final String script = "function a() { return 2;}; let {x = a()} = {x: 12}; x";

final String script2 = "function a() { return 2;}; let {x = 12} = {x: a()}; x";
Utils.assertWithAllOptimizationLevelsES6(12, script);
Utils.assertWithAllOptimizationLevelsES6(2, script2);
}

@Test
public void normDefaultParametersObjectDestructuringFunCall() throws Exception {
// JavaScript
final String script =
"function a() { return 12;}; function b({x = a()} = {x: 1}) { return x }; b()";
final String script2 =
"function a() { return 12;}; function b({x = a()} = {}) { return x }; b()";
final String script3 =
"var a = { p1: { p2: 121}}; function b({x = a.p1.p2} = {}) { return x }; b()";
final String script4 =
"function a() { return 12;}; function b({x = 1} = {x: a()}) { return x }; b()\n";

Utils.assertWithAllOptimizationLevelsES6(1, script);
Utils.assertWithAllOptimizationLevelsES6(12, script2);
Utils.assertWithAllOptimizationLevelsES6(121, script3);
Utils.assertWithAllOptimizationLevelsES6(12, script4);
}

@Test
public void normDefaultParametersArrayDestructuringFunCall() throws Exception {
// JavaScript
final String script =
"function a() { return 12;}; function b([x = a()] = [1]) { return x }; b()";
final String script2 =
"function a() { return 12;}; function b([x = a()] = []) { return x }; b()";
final String script3 =
"var a = { p1: { p2: 121}}; function b([x = a.p1.p2] = []) { return x }; b()";
final String script4 =
"function a() { return 12;}; function b([x = 1] = [a()]) { return x }; b()\n";

Utils.assertWithAllOptimizationLevelsES6(1, script);
Utils.assertWithAllOptimizationLevelsES6(12, script2);
Utils.assertWithAllOptimizationLevelsES6(121, script3);
Utils.assertWithAllOptimizationLevelsES6(12, script4);
}

@Test
public void normDefaultParametersFunCall() throws Exception {
// JavaScript
final String script = "function a() { return 12;}; function b(x = a()) { return x }; b()";
Utils.assertWithAllOptimizationLevelsES6(12, script);
}

@Test
@Ignore("destructuring-not-supported-in-for-let-expressions")
public void letExprDestructuringFunCall() throws Exception {
// JavaScript
final String script =
"function a() { return 4; }; (function() { "
+ " for (let {x = a()} = {}; ; ) { "
+ " return 3; "
+ " return x; "
+ " }"
+ " })()";
Utils.assertWithAllOptimizationLevelsES6(3, script);
Utils.assertWithAllOptimizationLevelsES6(4, script);
}

@Test
Expand Down
Loading
Loading