Skip to content

Commit

Permalink
FIX: Transform default values correctly in destructuring (#1708)
Browse files Browse the repository at this point in the history 
FIX: Transform default values correctly in destructuring

This addresses some of the issues in #1641
  • Loading branch information
@0xe
0xe authored Oct 23, 2024
1 parent ca9e763 commit e8b23f5
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 233 deletions.
6 changes: 4 additions & 2 deletions rhino/src/main/java/org/mozilla/javascript/IRFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,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
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,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
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 @@ -4192,7 +4192,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 @@ -4206,7 +4206,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 @@ -4215,7 +4215,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 @@ -4274,7 +4274,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 @@ -4345,14 +4345,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 @@ -4361,21 +4354,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 @@ -4404,43 +4394,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 @@ -4449,6 +4413,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 @@ -4461,7 +4429,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
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

0 comments on commit e8b23f5

Please sign in to comment.