From 28c7c849b49cf934453fd9b9b9808aeae1cacc64 Mon Sep 17 00:00:00 2001 From: "andrea.bergia" Date: Mon, 7 Oct 2024 10:20:37 +0200 Subject: [PATCH] Fix: make optional chaining expression work I.e. code such as: ```js o = {a: true}; o?.['a'] ``` Note that the spec says that the expression on the right-hand side should _not_ be evaluated if the left-hand side object is null or undefined. --- .../org/mozilla/javascript/CodeGenerator.java | 28 ++++++++++++ .../org/mozilla/javascript/IRFactory.java | 10 ++++- .../java/org/mozilla/javascript/Parser.java | 23 ++++++++++ .../java/org/mozilla/javascript/Token.java | 7 ++- .../javascript/optimizer/BodyCodegen.java | 44 +++++++++++++++++++ .../tests/OptionalChainingOperatorTests.java | 27 ++++++++++++ tests/testsrc/test262.properties | 6 +-- 7 files changed, 138 insertions(+), 7 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java b/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java index 1ece422f8d..453c68e278 100644 --- a/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java +++ b/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java @@ -742,6 +742,34 @@ private void visitExpression(Node node, int contextFlags) { stackChange(-1); break; + case Token.GETELEM_OPTIONAL: + visitExpression(child, 0); + + // If object == null (shallow eq) + addIcode(Icode_DUP); + stackChange(1); + addToken(Token.NULL); + stackChange(1); + addToken(Token.EQ); + stackChange(-1); + int getExpr = iCodeTop; + addGotoOp(Token.IFNE); + stackChange(-1); + + // Put undefined + addStringOp(Token.NAME, "undefined"); + int after = iCodeTop; + addGotoOp(Token.GOTO); + + // Else: get property key and do a GETELEM + resolveForwardGoto(getExpr); + child = child.getNext(); + visitExpression(child, 0); + addToken(Token.GETELEM); + stackChange(-1); + resolveForwardGoto(after); + break; + case Token.POS: case Token.NEG: case Token.NOT: diff --git a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java index 283d6f15ab..774c8520e1 100644 --- a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java +++ b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java @@ -163,6 +163,11 @@ private Node transform(AstNode node) { case Token.GETELEM: return transformElementGet((ElementGet) node); case Token.QUESTION_DOT: + if (node instanceof ElementGet) { + return transformElementGet((ElementGet) node); + } else { + return transformPropertyGet((PropertyGet) node); + } case Token.GETPROP: return transformPropertyGet((PropertyGet) node); case Token.HOOK: @@ -521,7 +526,10 @@ private Node transformElementGet(ElementGet node) { // iff elem is string that can not be number Node target = transform(node.getTarget()); Node element = transform(node.getElement()); - return new Node(Token.GETELEM, target, element); + return new Node( + node.type == Token.QUESTION_DOT ? Token.GETELEM_OPTIONAL : Token.GETELEM, + target, + element); } private Node transformExprStmt(ExpressionStatement node) { diff --git a/rhino/src/main/java/org/mozilla/javascript/Parser.java b/rhino/src/main/java/org/mozilla/javascript/Parser.java index 27ff2e8afb..8c5571f6be 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Parser.java +++ b/rhino/src/main/java/org/mozilla/javascript/Parser.java @@ -3078,6 +3078,29 @@ private AstNode propertyAccess(int tt, AstNode pn) throws IOException { ref = propertyName(-1, memberTypeFlags); break; } + + case Token.LB: + { + if (tt == Token.QUESTION_DOT) { + consumeToken(); + ref = expr(true); + mustMatchToken(Token.RB, "msg.no.bracket.index", true); + + ElementGet result = new ElementGet(); + int pos = pn.getPosition(); + result.setPosition(pos); + result.setLength(getNodeEnd(ref) - pos); + result.setLineno(pn.getLineno()); + result.setTarget(pn); + result.setElement(ref); + result.setType(Token.QUESTION_DOT); + return result; + } else { + reportError("msg.no.name.after.dot"); + return makeErrorNode(); + } + } + default: if (compilerEnv.isReservedKeywordAsIdentifier()) { // allow keywords as property names, e.g. ({if: 1}) diff --git a/rhino/src/main/java/org/mozilla/javascript/Token.java b/rhino/src/main/java/org/mozilla/javascript/Token.java index 5218c13760..481a82ede8 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Token.java +++ b/rhino/src/main/java/org/mozilla/javascript/Token.java @@ -124,10 +124,11 @@ public static enum CommentType { BIGINT = REF_NS_NAME + 1, // ES2020 BigInt GETPROP_OPTIONAL = BIGINT + 1, REF_SPECIAL_OPTIONAL = GETPROP_OPTIONAL + 1, - CALL_OPTIONAL = REF_SPECIAL_OPTIONAL + 1; + CALL_OPTIONAL = REF_SPECIAL_OPTIONAL + 1, + GETELEM_OPTIONAL = CALL_OPTIONAL + 1; // End of interpreter bytecodes - public static final int LAST_BYTECODE_TOKEN = CALL_OPTIONAL, + public static final int LAST_BYTECODE_TOKEN = GETELEM_OPTIONAL, TRY = LAST_BYTECODE_TOKEN + 1, SEMI = TRY + 1, // semicolon LB = SEMI + 1, // left and right brackets @@ -615,6 +616,8 @@ public static String typeToName(int token) { return "REF_SPECIAL_OPTIONAL"; case CALL_OPTIONAL: return "CALL_OPTIONAL"; + case GETELEM_OPTIONAL: + return "GETELEM_OPTIONAL"; case TEMPLATE_LITERAL: return "TEMPLATE_LITERAL"; case TEMPLATE_CHARS: diff --git a/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java b/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java index a18e5a8ca4..eca5dd457e 100644 --- a/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java +++ b/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java @@ -1370,6 +1370,50 @@ private void generateExpression(Node node, Node parent) { } break; + case Token.GETELEM_OPTIONAL: + { + generateExpression(child, node); // object + + int getExpr = cfw.acquireLabel(); + int putUndefined = cfw.acquireLabel(); + int after = cfw.acquireLabel(); + + // if (object == null) goto putUndefined + cfw.add(ByteCode.DUP); + ++maxStack; + cfw.add(ByteCode.IFNULL, putUndefined); + + // if (!Undefined.isUndefined(object)) goto getExpr + cfw.add(ByteCode.DUP); // No need to increase maxStack again! + cfw.addInvoke( + ByteCode.INVOKESTATIC, + "org.mozilla.javascript.Undefined", + "isUndefined", + "(Ljava/lang/Object;)Z"); + cfw.add(ByteCode.IFEQ, getExpr); + + cfw.markLabel(putUndefined); + cfw.add(ByteCode.POP); // Remove object + cfw.add( + ByteCode.GETSTATIC, + "org/mozilla/javascript/Undefined", + "instance", + "Ljava/lang/Object;"); + cfw.add(ByteCode.GOTO, after); + + cfw.markLabel(getExpr); + generateExpression(child.getNext(), node); // id + cfw.addALoad(contextLocal); + cfw.addALoad(variableObjectLocal); + if (node.getIntProp(Node.ISNUMBER_PROP, -1) != -1) { + addDynamicInvoke("PROP:GETINDEX", Signatures.PROP_GET_INDEX); + } else { + addDynamicInvoke("PROP:GETELEMENT", Signatures.PROP_GET_ELEMENT); + } + cfw.markLabel(after); + break; + } + case Token.GET_REF: generateExpression(child, node); // reference cfw.addALoad(contextLocal); diff --git a/rhino/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTests.java b/rhino/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTests.java index 482b64a97c..662dc45bd9 100644 --- a/rhino/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTests.java +++ b/rhino/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTests.java @@ -126,4 +126,31 @@ public void testOptionalChainingOperator() { return null; }); } + + @Test + public void expressionsInOptionalChaining() { + Utils.assertWithAllOptimizationLevelsES6(true, "o = {a: true}; o?.['a']"); + Utils.assertWithAllOptimizationLevelsES6(Undefined.instance, "o = null; o?.['a']"); + Utils.assertWithAllOptimizationLevelsES6(Undefined.instance, "o = undefined; o?.['a']"); + } + + @Test + public void expressionsInOptionalChainingAreNotEvaluatedIfUnnecessary() { + Utils.assertWithAllOptimizationLevelsES6(1, + "c = 0;\n" + + "function f() { ++c; return 0; }\n" + + "o = {}\n" + + "o?.[f()];\n" + + "c\n"); + Utils.assertWithAllOptimizationLevelsES6(0, + "c = 0;\n" + + "function f() { ++c; return 0; }\n" + + "null?.[f()];\n" + + "c\n"); + Utils.assertWithAllOptimizationLevelsES6(0, + "c = 0;\n" + + "function f() { ++c; return 0; }\n" + + "undefined?.[f()];\n" + + "c\n"); + } } diff --git a/tests/testsrc/test262.properties b/tests/testsrc/test262.properties index 2eff3ec3f8..09dc1d143b 100644 --- a/tests/testsrc/test262.properties +++ b/tests/testsrc/test262.properties @@ -5893,14 +5893,13 @@ language/expressions/object 809/1169 (69.2%) yield-non-strict-access.js non-strict yield-non-strict-syntax.js non-strict -language/expressions/optional-chaining 25/38 (65.79%) +language/expressions/optional-chaining 23/38 (60.53%) call-expression.js early-errors-tail-position-null-optchain-template-string.js early-errors-tail-position-null-optchain-template-string-esi.js early-errors-tail-position-optchain-template-string.js early-errors-tail-position-optchain-template-string-esi.js eval-optional-call.js - iteration-statement-for.js iteration-statement-for-await-of.js {unsupported: [async]} iteration-statement-for-in.js iteration-statement-for-of-type-error.js @@ -5913,9 +5912,8 @@ language/expressions/optional-chaining 25/38 (65.79%) optional-chain.js optional-chain-async-optional-chain-square-brackets.js {unsupported: [async]} optional-chain-async-square-brackets.js {unsupported: [async]} - optional-chain-expression-optional-expression.js + optional-chain-expression-optional-expression.js {strict: [-1], non-strict: [-1]} optional-chain-prod-arguments.js - optional-chain-prod-expression.js punctuator-decimal-lookahead.js short-circuiting.js super-property-optional-call.js