From 8903e9e28e361959b9df041559ec165e8281e49f Mon Sep 17 00:00:00 2001 From: "andrea.bergia" Date: Wed, 3 Dec 2025 11:27:30 +0100 Subject: [PATCH] Implement support for destructuring in `catch` Extend the parser and IR to support things like `catch ({message})` or `catch ([a, b])`. Note that, while Rhino supports a non-standard extension for conditions in catch clauses, i.e. `catch (a if b)`, I have opted to _not_ support it if we have a destructuring expression. --- .../org/mozilla/javascript/IRFactory.java | 53 ++++++- .../java/org/mozilla/javascript/Parser.java | 50 +++--- .../mozilla/javascript/ast/CatchClause.java | 17 ++- .../javascript/resources/Messages.properties | 3 + .../tests/CatchDestructuringTest.java | 142 ++++++++++++++++++ .../mozilla/javascript/tests/ParserTest.java | 3 +- tests/testsrc/test262.properties | 51 +------ 7 files changed, 237 insertions(+), 82 deletions(-) create mode 100644 rhino/src/test/java/org/mozilla/javascript/tests/CatchDestructuringTest.java diff --git a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java index 516cc04459..de497e4a9c 100644 --- a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java +++ b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java @@ -1210,22 +1210,61 @@ private Node transformTry(TryStatement node) { Node catchBlocks = new Block(); for (CatchClause cc : node.getCatchClauses()) { - Name varName = cc.getVarName(); + AstNode varName = cc.getVarName(); Node catchCond = null; Node varNameNode = null; + Scope catchBody = cc.getBody(); if (varName != null) { - varNameNode = parser.createName(varName.getIdentifier()); + if (varName instanceof Name) { + // Simple identifier + varNameNode = parser.createName(((Name) varName).getIdentifier()); - AstNode ccc = cc.getCatchCondition(); - if (ccc != null) { - catchCond = transform(ccc); - } else { + AstNode ccc = cc.getCatchCondition(); + if (ccc != null) { + catchCond = transform(ccc); + } else { + catchCond = new EmptyExpression(); + } + } else if (varName instanceof org.mozilla.javascript.ast.ArrayLiteral + || varName instanceof org.mozilla.javascript.ast.ObjectLiteral) { + // Destructuring pattern. We basically replace: + // catch ( {message} ) { body } + // into: + // catch ( $tempname ) { let {message} = $tempname; body } + + // The exception will be stored in the temp name + String tempVarName = parser.currentScriptOrFn.getNextTempName(); + varNameNode = parser.createName(tempVarName); + + // The let statement will be used to do the destructuring + VariableDeclaration letStatement = new VariableDeclaration(); + letStatement.setType(Token.LET); + + VariableInitializer letVar = new VariableInitializer(); + letStatement.addVariable(letVar); + + // LHS: the destructuring declaration + letVar.setTarget(varName); + + // RHS: the temp name (which we need to wrap in a new name node) + Name tempVarNameNode = new Name(); + tempVarNameNode.setIdentifier(tempVarName); + letVar.setInitializer(tempVarNameNode); + + // Prepend the destructuring "let" to the catch body + catchBody.addChildToFront(letStatement); + + // Our non-standard condition is not supported for destructuring (we throw an + // error at parse time), so here we can simply force it to an empty expression catchCond = new EmptyExpression(); + } else { + throw new IllegalArgumentException( + "Unexpected catch parameter type: " + varName.getClass().getName()); } } - Node body = transform(cc.getBody()); + Node body = transform(catchBody); catchBlocks.addChildToBack( createCatch(varNameNode, catchCond, body, cc.getLineno(), cc.getColumn())); diff --git a/rhino/src/main/java/org/mozilla/javascript/Parser.java b/rhino/src/main/java/org/mozilla/javascript/Parser.java index 6807ec081f..486c072df6 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Parser.java +++ b/rhino/src/main/java/org/mozilla/javascript/Parser.java @@ -1821,7 +1821,7 @@ private TryStatement tryStatement() throws IOException { guardPos = -1, catchLine = lineNumber(), catchColumn = columnNumber(); - Name varName = null; + AstNode varName = null; AstNode catchCond = null; switch (peekToken()) { @@ -1829,27 +1829,41 @@ private TryStatement tryStatement() throws IOException { { matchToken(Token.LP, true); lp = ts.tokenBeg; - if (!matchToken(Token.UNDEFINED, true)) { - mustMatchToken(Token.NAME, "msg.bad.catchcond", true); - } - varName = createNameNode(); - Comment jsdocNodeForName = getAndResetJsDoc(); - if (jsdocNodeForName != null) { - varName.setJsDocNode(jsdocNodeForName); - } - String varNameString = varName.getIdentifier(); - if ("undefined".equals(varNameString)) { - hasUndefinedBeenRedefined = true; - } - if (inUseStrictDirective) { - if ("eval".equals(varNameString) - || "arguments".equals(varNameString)) { - reportError("msg.bad.id.strict", varNameString); + int tt = peekToken(); + if (tt == Token.LB || tt == Token.LC) { + // Destructuring pattern + if (compilerEnv.getLanguageVersion() >= Context.VERSION_ES6) { + varName = destructuringPrimaryExpr(); + markDestructuring(varName); + } else { + reportError("msg.catch.destructuring.requires.es6"); + } + } else { + // Simple identifier + if (!matchToken(Token.UNDEFINED, true)) { + mustMatchToken(Token.NAME, "msg.bad.catchcond", true); + } + + varName = createNameNode(); + Comment jsdocNodeForName = getAndResetJsDoc(); + if (jsdocNodeForName != null) { + varName.setJsDocNode(jsdocNodeForName); + } + String varNameString = ((Name) varName).getIdentifier(); + if ("undefined".equals(varNameString)) { + hasUndefinedBeenRedefined = true; + } + if (inUseStrictDirective) { + if ("eval".equals(varNameString) + || "arguments".equals(varNameString)) { + reportError("msg.bad.id.strict", varNameString); + } } } - if (matchToken(Token.IF, true)) { + // Non-standard extension: we support "catch (e if cond) + if (varName instanceof Name && matchToken(Token.IF, true)) { guardPos = ts.tokenBeg; catchCond = expr(false); } else { diff --git a/rhino/src/main/java/org/mozilla/javascript/ast/CatchClause.java b/rhino/src/main/java/org/mozilla/javascript/ast/CatchClause.java index 0ab97f7a49..e7983efd2f 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ast/CatchClause.java +++ b/rhino/src/main/java/org/mozilla/javascript/ast/CatchClause.java @@ -12,11 +12,13 @@ * Node representing a catch-clause of a try-statement. Node type is {@link Token#CATCH}. * * 
CatchClause :
- *        catch ( Identifier [if Expression] ) Block
+ * catch ( Identifier [if Expression] ) Block + * catch ( ObjectPattern ) Block + * catch ( ArrayPattern ) Block */ public class CatchClause extends AstNode { - private Name varName; + private AstNode varName; private AstNode catchCondition; private Scope body; private int ifPosition = -1; @@ -40,20 +42,23 @@ public CatchClause(int pos, int len) { /** * Returns catch variable node * - * @return catch variable + * @return catch variable (can be Name, ArrayLiteral, or ObjectLiteral) */ - public Name getVarName() { + public AstNode getVarName() { return varName; } /** * Sets catch variable node, and sets its parent to this node. * - * @param varName catch variable + * @param varName catch variable (can be Name, ArrayLiteral, or ObjectLiteral) */ - public void setVarName(Name varName) { + public void setVarName(AstNode varName) { this.varName = varName; if (varName != null) { + assert varName instanceof Name + || varName instanceof ArrayLiteral + || varName instanceof ObjectLiteral; varName.setParent(this); } } diff --git a/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties b/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties index 9884dd37f0..0c485d9dd1 100644 --- a/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties +++ b/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties @@ -492,6 +492,9 @@ msg.bad.catchcond =\ msg.catch.unreachable =\ any catch clauses following an unqualified catch are unreachable +msg.catch.destructuring.requires.es6 =\ + Destructuring in catch blocks requires ES6 or later + msg.no.brace.try =\ missing '{' before try block diff --git a/rhino/src/test/java/org/mozilla/javascript/tests/CatchDestructuringTest.java b/rhino/src/test/java/org/mozilla/javascript/tests/CatchDestructuringTest.java new file mode 100644 index 0000000000..e27332cd66 --- /dev/null +++ b/rhino/src/test/java/org/mozilla/javascript/tests/CatchDestructuringTest.java @@ -0,0 +1,142 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.javascript.tests; + +import org.junit.jupiter.api.Test; +import org.mozilla.javascript.testutils.Utils; + +class CatchDestructuringTest { + @Test + void cannotUseObjectDestructuringInEs5() { + Utils.assertEvaluatorException_1_8( + "Destructuring in catch blocks requires ES6 or later (test#3)", + "try {\n" + + " throw new Error('test');\n" + + "} catch ({message}) {\n" + + " message;\n" + + "}\n"); + } + + @Test + void canUseObjectDestructuring() { + Utils.assertWithAllModes_ES6( + "hey", + "try {\n" + + " throw new Error('hey');\n" + + "} catch ({message}) {\n" + + " message;\n" + + "}\n"); + } + + @Test + void canUseObjectDestructuringWithRenaming() { + Utils.assertWithAllModes_ES6( + "hey", + "try {\n" + + " throw new Error('hey');\n" + + "} catch ({message: m}) {\n" + + " m;\n" + + "}\n"); + } + + @Test + void canUseNestedObjectDestructuring() { + Utils.assertWithAllModes_ES6( + "nested", + "try {\n" + + " throw {error: {code: 'nested'}};\n" + + "} catch ({error: {code}}) {\n" + + " code;\n" + + "}\n"); + } + + @Test + void canUseDefaultValuesInDestructuring() { + Utils.assertWithAllModes_ES6( + "default", + "try {\n" + + " throw {};\n" + + "} catch ({message = 'default'}) {\n" + + " message;\n" + + "}\n"); + } + + @Test + void cannotUseObjectDestructuringAndConditions() { + Utils.assertEvaluatorExceptionES6( + "invalid catch block condition (test#3)", + "try {\n" + + " throw new Error('hey');\n" + + "} catch ( {e} if e.message ) {\n" + + " e.message;\n" + + "}\n"); + } + + @Test + void cannotUseArrayDestructuringInEs5() { + Utils.assertEvaluatorException_1_8( + "Destructuring in catch blocks requires ES6 or later (test#3)", + "try {\n" + + " throw ['h', 'e', 'y']\n" + + "} catch ([h, e, y]) {\n" + + " h + e + y;\n" + + "}\n"); + } + + @Test + void canUseArrayDestructuring() { + Utils.assertWithAllModes_ES6( + "hey", + "try {\n" + + " throw ['h', 'e', 'y']\n" + + "} catch ([h, e, y]) {\n" + + " h + e + y;\n" + + "}\n"); + } + + @Test + void canSkipArrayElements() { + Utils.assertWithAllModes_ES6( + "c", + "try {\n" + + " throw ['a', 'b', 'c'];\n" + + "} catch ([, , third]) {\n" + + " third;\n" + + "}\n"); + } + + @Test + void canUseNestedArrayDestructuring() { + Utils.assertWithAllModes_ES6( + "b", + "try {\n" + + " throw [['a', 'b'], ['c', 'd']];\n" + + "} catch ([[x, y], [z, w]]) {\n" + + " y;\n" + + "}\n"); + } + + @Test + void canCombineObjectAndArrayDestructuring() { + Utils.assertWithAllModes_ES6( + "value", + "try {\n" + + " throw {data: ['value', 'other']};\n" + + "} catch ({data: [first]}) {\n" + + " first;\n" + + "}\n"); + } + + @Test + void canCombineArrayAndObjectDestructuring() { + Utils.assertWithAllModes_ES6( + "test", + "try {\n" + + " throw [{message: 'test'}];\n" + + "} catch ([{message}]) {\n" + + " message;\n" + + "}\n"); + } +} diff --git a/tests/src/test/java/org/mozilla/javascript/tests/ParserTest.java b/tests/src/test/java/org/mozilla/javascript/tests/ParserTest.java index 11a86b6efc..006f58f4fb 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/ParserTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/ParserTest.java @@ -9,6 +9,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import java.io.IOException; import java.util.List; @@ -555,7 +556,7 @@ public void linenoTry() { List catchBlocks = tryStmt.getCatchClauses(); CatchClause catchClause = catchBlocks.get(0); Scope catchVarBlock = catchClause.getBody(); - Name catchVar = catchClause.getVarName(); + Name catchVar = assertInstanceOf(Name.class, catchClause.getVarName()); AstNode finallyBlock = tryStmt.getFinallyBlock(); AstNode finallyStmt = (AstNode) finallyBlock.getFirstChild(); diff --git a/tests/testsrc/test262.properties b/tests/testsrc/test262.properties index c367b35bbd..1679e8b5cf 100644 --- a/tests/testsrc/test262.properties +++ b/tests/testsrc/test262.properties @@ -6786,48 +6786,23 @@ language/statements/switch 60/111 (54.05%) language/statements/throw 0/14 (0.0%) -language/statements/try 113/201 (56.22%) +language/statements/try 64/201 (31.84%) dstr/ary-init-iter-close.js dstr/ary-init-iter-get-err.js dstr/ary-init-iter-get-err-array-prototype.js - dstr/ary-init-iter-no-close.js - dstr/ary-name-iter-val.js - dstr/ary-ptrn-elem-ary-elem-init.js - dstr/ary-ptrn-elem-ary-elem-iter.js dstr/ary-ptrn-elem-ary-elision-init.js - dstr/ary-ptrn-elem-ary-elision-iter.js - dstr/ary-ptrn-elem-ary-empty-init.js - dstr/ary-ptrn-elem-ary-empty-iter.js dstr/ary-ptrn-elem-ary-rest-init.js dstr/ary-ptrn-elem-ary-rest-iter.js - dstr/ary-ptrn-elem-ary-val-null.js - dstr/ary-ptrn-elem-id-init-exhausted.js dstr/ary-ptrn-elem-id-init-fn-name-arrow.js dstr/ary-ptrn-elem-id-init-fn-name-class.js dstr/ary-ptrn-elem-id-init-fn-name-cover.js dstr/ary-ptrn-elem-id-init-fn-name-fn.js dstr/ary-ptrn-elem-id-init-fn-name-gen.js - dstr/ary-ptrn-elem-id-init-hole.js - dstr/ary-ptrn-elem-id-init-skipped.js - dstr/ary-ptrn-elem-id-init-throws.js - dstr/ary-ptrn-elem-id-init-undef.js - dstr/ary-ptrn-elem-id-init-unresolvable.js - dstr/ary-ptrn-elem-id-iter-complete.js - dstr/ary-ptrn-elem-id-iter-done.js dstr/ary-ptrn-elem-id-iter-step-err.js - dstr/ary-ptrn-elem-id-iter-val.js dstr/ary-ptrn-elem-id-iter-val-array-prototype.js dstr/ary-ptrn-elem-id-iter-val-err.js - dstr/ary-ptrn-elem-obj-id.js - dstr/ary-ptrn-elem-obj-id-init.js - dstr/ary-ptrn-elem-obj-prop-id.js - dstr/ary-ptrn-elem-obj-prop-id-init.js - dstr/ary-ptrn-elem-obj-val-null.js - dstr/ary-ptrn-elem-obj-val-undef.js dstr/ary-ptrn-elision.js - dstr/ary-ptrn-elision-exhausted.js dstr/ary-ptrn-elision-step-err.js - dstr/ary-ptrn-empty.js dstr/ary-ptrn-rest-ary-elem.js dstr/ary-ptrn-rest-ary-elision.js dstr/ary-ptrn-rest-ary-empty.js @@ -6843,34 +6818,12 @@ language/statements/try 113/201 (56.22%) dstr/ary-ptrn-rest-obj-prop-id.js dstr/obj-init-null.js dstr/obj-init-undefined.js - dstr/obj-ptrn-empty.js - dstr/obj-ptrn-id-get-value-err.js dstr/obj-ptrn-id-init-fn-name-arrow.js dstr/obj-ptrn-id-init-fn-name-class.js dstr/obj-ptrn-id-init-fn-name-cover.js dstr/obj-ptrn-id-init-fn-name-fn.js dstr/obj-ptrn-id-init-fn-name-gen.js - dstr/obj-ptrn-id-init-skipped.js - dstr/obj-ptrn-id-init-throws.js - dstr/obj-ptrn-id-init-unresolvable.js - dstr/obj-ptrn-id-trailing-comma.js - dstr/obj-ptrn-list-err.js - dstr/obj-ptrn-prop-ary.js - dstr/obj-ptrn-prop-ary-init.js - dstr/obj-ptrn-prop-ary-trailing-comma.js - dstr/obj-ptrn-prop-ary-value-null.js dstr/obj-ptrn-prop-eval-err.js - dstr/obj-ptrn-prop-id.js - dstr/obj-ptrn-prop-id-get-value-err.js - dstr/obj-ptrn-prop-id-init.js - dstr/obj-ptrn-prop-id-init-skipped.js - dstr/obj-ptrn-prop-id-init-throws.js - dstr/obj-ptrn-prop-id-init-unresolvable.js - dstr/obj-ptrn-prop-id-trailing-comma.js - dstr/obj-ptrn-prop-obj.js - dstr/obj-ptrn-prop-obj-init.js - dstr/obj-ptrn-prop-obj-value-null.js - dstr/obj-ptrn-prop-obj-value-undef.js dstr/obj-ptrn-rest-getter.js {unsupported: [object-rest]} dstr/obj-ptrn-rest-skip-non-enumerable.js {unsupported: [object-rest]} dstr/obj-ptrn-rest-val-obj.js {unsupported: [object-rest]} @@ -6894,8 +6847,6 @@ language/statements/try 113/201 (56.22%) early-catch-function.js early-catch-lex.js scope-catch-block-lex-open.js - scope-catch-param-lex-open.js - scope-catch-param-var-none.js non-strict static-init-await-binding-valid.js tco-catch.js {unsupported: [tail-call-optimization]} tco-catch-finally.js {unsupported: [tail-call-optimization]}