From 5ffcfe48c4001053cc9e3fb7baaada68ab2d3e1f Mon Sep 17 00:00:00 2001 From: "satish.srinivasan" Date: Wed, 23 Oct 2024 00:01:34 +0530 Subject: [PATCH] FIX: Transform default values correctly in destructuring --- .../org/mozilla/javascript/IRFactory.java | 6 +- .../java/org/mozilla/javascript/Node.java | 1 + .../java/org/mozilla/javascript/Parser.java | 90 ++++++------------- .../tests/DefaultParametersTest.java | 74 ++++++++++++++- 4 files changed, 104 insertions(+), 67 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java index 283d6f15ab..f018736d86 100644 --- a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java +++ b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java @@ -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)); + } } } diff --git a/rhino/src/main/java/org/mozilla/javascript/Node.java b/rhino/src/main/java/org/mozilla/javascript/Node.java index 25e57a58e6..fa07326d15 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Node.java +++ b/rhino/src/main/java/org/mozilla/javascript/Node.java @@ -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; diff --git a/rhino/src/main/java/org/mozilla/javascript/Parser.java b/rhino/src/main/java/org/mozilla/javascript/Parser.java index 27ff2e8afb..fce55a743c 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Parser.java +++ b/rhino/src/main/java/org/mozilla/javascript/Parser.java @@ -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 destructuring = null; - Map destructuringDefault = null; + Map destructuringDefault = null; Set paramNames = new HashSet<>(); do { @@ -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 param : destructuring.entrySet()) { - Node defaultValue = null; + AstNode defaultValue = null; if (destructuringDefault != null) { defaultValue = destructuringDefault.get(param.getKey()); } @@ -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 destructuring = new HashMap<>(); - Map destructuringDefault = new HashMap<>(); + Map destructuringDefault = new HashMap<>(); Set paramNames = new HashSet<>(); PerFunctionVariables savedVars = new PerFunctionVariables(fnNode); @@ -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 param : destructuring.entrySet()) { - Node defaultValue = null; + AstNode defaultValue = null; if (destructuringDefault != null) { defaultValue = destructuringDefault.get(param.getKey()); } @@ -1087,7 +1087,7 @@ private void arrowFunctionParams( FunctionNode fnNode, AstNode params, Map destructuring, - Map destructuringDefault, + Map destructuringDefault, Set paramNames) throws IOException { if (params instanceof ArrayLiteral || params instanceof ObjectLiteral) { @@ -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( @@ -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); } @@ -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))); @@ -4276,7 +4276,7 @@ boolean destructuringArray( String tempName, Node parent, List 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; @@ -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( @@ -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); @@ -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 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( @@ -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); @@ -4463,7 +4431,7 @@ boolean destructuringObject( String tempName, Node parent, List 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; diff --git a/tests/src/test/java/org/mozilla/javascript/tests/DefaultParametersTest.java b/tests/src/test/java/org/mozilla/javascript/tests/DefaultParametersTest.java index 633e6b42aa..6d6c2eef4e 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/DefaultParametersTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/DefaultParametersTest.java @@ -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