Allow `BindingPattern`s as `CatchParameter` (#1628)

boa/src/syntax/ast/node/block/mod.rs

@@ -8,7 +8,7 @@ use crate::{
     gc::{Finalize, Trace},
     BoaProfiler, Context, JsResult, JsValue,
 };
-use std::fmt;
+use std::{collections::HashSet, fmt};
 
 #[cfg(feature = "deser")]
 use serde::{Deserialize, Serialize};
@@ -45,6 +45,14 @@ impl Block {
         self.statements.items()
     }
 
+    pub(crate) fn lexically_declared_names(&self) -> HashSet<&str> {
+        self.statements.lexically_declared_names()
+    }
+
+    pub(crate) fn var_declared_named(&self) -> HashSet<&str> {
+        self.statements.var_declared_names()
+    }
+
     /// Implements the display formatting with indentation.
     pub(super) fn display(&self, f: &mut fmt::Formatter<'_>, indentation: usize) -> fmt::Result {
         writeln!(f, "{{")?;

boa/src/syntax/ast/node/try_node/mod.rs

@@ -5,7 +5,7 @@ use crate::{
     },
     exec::Executable,
     gc::{Finalize, Trace},
-    syntax::ast::node::{Block, Identifier, Node},
+    syntax::ast::node::{Block, Declaration, Node},
     BoaProfiler, Context, JsResult, JsValue,
 };
 use std::fmt;
@@ -100,13 +100,33 @@ impl Executable for Try {
         let res = self.block().run(context).map_or_else(
             |err| {
                 if let Some(catch) = self.catch() {
-                    {
+                    let env = context.get_current_environment();
-                        let env = context.get_current_environment();
+                    context.push_environment(DeclarativeEnvironmentRecord::new(Some(env)));
-                        context.push_environment(DeclarativeEnvironmentRecord::new(Some(env)));
+
-
+                    if let Some(param) = catch.parameter() {
-                        if let Some(param) = catch.parameter() {
+                        match param {
-                            context.create_mutable_binding(param, false, VariableScope::Block)?;
+                            Declaration::Identifier { ident, init } => {
-                            context.initialize_binding(param, err)?;
+                                debug_assert!(init.is_none());
+
+                                context.create_mutable_binding(
+                                    ident.as_ref(),
+                                    false,
+                                    VariableScope::Block,
+                                )?;
+                                context.initialize_binding(ident.as_ref(), err)?;
+                            }
+                            Declaration::Pattern(pattern) => {
+                                debug_assert!(pattern.init().is_none());
+
+                                for (ident, value) in pattern.run(Some(err), context)? {
+                                    context.create_mutable_binding(
+                                        ident.as_ref(),
+                                        false,
+                                        VariableScope::Block,
+                                    )?;
+                                    context.initialize_binding(ident.as_ref(), value)?;
+                                }
+                            }
                         }
                     }
 
@@ -147,27 +167,27 @@ impl From<Try> for Node {
 #[cfg_attr(feature = "deser", derive(Serialize, Deserialize))]
 #[derive(Clone, Debug, Trace, Finalize, PartialEq)]
 pub struct Catch {
-    parameter: Option<Identifier>,
+    parameter: Option<Box<Declaration>>,
     block: Block,
 }
 
 impl Catch {
     /// Creates a new catch block.
-    pub(in crate::syntax) fn new<OI, I, B>(parameter: OI, block: B) -> Self
+    pub(in crate::syntax) fn new<OD, D, B>(parameter: OD, block: B) -> Self
     where
-        OI: Into<Option<I>>,
+        OD: Into<Option<D>>,
-        I: Into<Identifier>,
+        D: Into<Declaration>,
         B: Into<Block>,
     {
         Self {
-            parameter: parameter.into().map(I::into),
+            parameter: parameter.into().map(|d| Box::new(d.into())),
             block: block.into(),
         }
     }
 
     /// Gets the parameter of the catch block.
-    pub fn parameter(&self) -> Option<&str> {
+    pub fn parameter(&self) -> Option<&Declaration> {
-        self.parameter.as_ref().map(Identifier::as_ref)
+        self.parameter.as_deref()
     }
 
     /// Retrieves the catch execution block.

boa/src/syntax/ast/node/try_node/tests.rs

@@ -77,6 +77,38 @@ fn catch_binding() {
     assert_eq!(&exec(scenario), "20");
 }
 
+#[test]
+fn catch_binding_pattern_object() {
+    let scenario = r#"
+        let a = 10;
+        try {
+            throw {
+                n: 30,
+            };
+        } catch ({ n }) {
+            a = n;
+        }
+
+        a;
+    "#;
+    assert_eq!(&exec(scenario), "30");
+}
+
+#[test]
+fn catch_binding_pattern_array() {
+    let scenario = r#"
+        let a = 10;
+        try {
+            throw [20, 30];
+        } catch ([, n]) {
+            a = n;
+        }
+
+        a;
+    "#;
+    assert_eq!(&exec(scenario), "30");
+}
+
 #[test]
 fn catch_binding_finally() {
     let scenario = r#"

boa/src/syntax/parser/statement/try_stm/catch.rs

@@ -2,16 +2,20 @@ use crate::{
     syntax::{
         ast::{
             node::{self, Identifier},
-            Keyword, Punctuator,
+            Keyword, Position, Punctuator,
         },
+        lexer::TokenKind,
         parser::{
-            statement::{block::Block, BindingIdentifier},
+            statement::{
+                block::Block, ArrayBindingPattern, BindingIdentifier, ObjectBindingPattern,
+            },
             AllowAwait, AllowReturn, AllowYield, Cursor, ParseError, TokenParser,
         },
     },
     BoaProfiler,
 };
 
+use rustc_hash::FxHashSet;
 use std::io::Read;
 
 /// Catch parsing
@@ -57,17 +61,63 @@ where
         let catch_param = if cursor.next_if(Punctuator::OpenParen)?.is_some() {
             let catch_param =
                 CatchParameter::new(self.allow_yield, self.allow_await).parse(cursor)?;
+
             cursor.expect(Punctuator::CloseParen, "catch in try statement")?;
             Some(catch_param)
         } else {
             None
         };
 
+        let mut set = FxHashSet::default();
+        let idents = match &catch_param {
+            Some(node::Declaration::Identifier { ident, .. }) => vec![ident.as_ref()],
+            Some(node::Declaration::Pattern(p)) => p.idents(),
+            _ => vec![],
+        };
+
+        // It is a Syntax Error if BoundNames of CatchParameter contains any duplicate elements.
+        // https://tc39.es/ecma262/#sec-variablestatements-in-catch-blocks
+        for ident in idents {
+            if !set.insert(ident) {
+                // FIXME: pass correct position once #1295 lands
+                return Err(ParseError::general(
+                    "duplicate identifier",
+                    Position::new(1, 1),
+                ));
+            }
+        }
+
         // Catch block
-        Ok(node::Catch::new::<_, Identifier, _>(
+        let catch_block =
-            catch_param,
+            Block::new(self.allow_yield, self.allow_await, self.allow_return).parse(cursor)?;
-            Block::new(self.allow_yield, self.allow_await, self.allow_return).parse(cursor)?,
+
-        ))
+        // It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the LexicallyDeclaredNames of Block.
+        // It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the VarDeclaredNames of Block.
+        // https://tc39.es/ecma262/#sec-try-statement-static-semantics-early-errors
+
+        // FIXME: `lexically_declared_names` only holds part of LexicallyDeclaredNames of the
+        // Block e.g. function names are *not* included but should be.
+        let lexically_declared_names = catch_block.lexically_declared_names();
+        let var_declared_names = catch_block.var_declared_named();
+
+        for ident in set {
+            // FIXME: pass correct position once #1295 lands
+            if lexically_declared_names.contains(ident) {
+                return Err(ParseError::general(
+                    "identifier redeclared",
+                    Position::new(1, 1),
+                ));
+            }
+            if var_declared_names.contains(ident) {
+                return Err(ParseError::general(
+                    "identifier redeclared",
+                    Position::new(1, 1),
+                ));
+            }
+        }
+
+        let catch_node = node::Catch::new::<_, node::Declaration, _>(catch_param, catch_block);
+        Ok(catch_node)
     }
 }
 
@@ -103,12 +153,30 @@ impl<R> TokenParser<R> for CatchParameter
 where
     R: Read,
 {
-    type Output = Identifier;
+    type Output = node::Declaration;
 
-    fn parse(self, cursor: &mut Cursor<R>) -> Result<Identifier, ParseError> {
+    fn parse(self, cursor: &mut Cursor<R>) -> Result<Self::Output, ParseError> {
-        // TODO: should accept BindingPattern
+        let token = cursor.peek(0)?.ok_or(ParseError::AbruptEnd)?;
-        BindingIdentifier::new(self.allow_yield, self.allow_await)
+
-            .parse(cursor)
+        match token.kind() {
-            .map(Identifier::from)
+            TokenKind::Punctuator(Punctuator::OpenBlock) => {
+                let pat = ObjectBindingPattern::new(true, self.allow_yield, self.allow_await)
+                    .parse(cursor)?;
+
+                Ok(node::Declaration::new_with_object_pattern(pat, None))
+            }
+            TokenKind::Punctuator(Punctuator::OpenBracket) => {
+                let pat = ArrayBindingPattern::new(true, self.allow_yield, self.allow_await)
+                    .parse(cursor)?;
+                Ok(node::Declaration::new_with_array_pattern(pat, None))
+            }
+            TokenKind::Identifier(_) => {
+                let ident = BindingIdentifier::new(self.allow_yield, self.allow_await)
+                    .parse(cursor)
+                    .map(Identifier::from)?;
+                Ok(node::Declaration::new_with_identifier(ident, None))
+            }
+            _ => Err(ParseError::unexpected(token.clone(), None)),
+        }
     }
 }

boa/src/syntax/parser/statement/try_stm/tests.rs

@@ -1,6 +1,9 @@
 use crate::syntax::{
     ast::{
-        node::{Block, Catch, Declaration, DeclarationList, Finally, Identifier, Try},
+        node::{
+            declaration::{BindingPatternTypeArray, BindingPatternTypeObject},
+            Block, Catch, Declaration, DeclarationList, Finally, Try,
+        },
         Const,
     },
     parser::tests::{check_invalid, check_parser},
@@ -10,7 +13,15 @@ use crate::syntax::{
 fn check_inline_with_empty_try_catch() {
     check_parser(
         "try { } catch(e) {}",
-        vec![Try::new(vec![], Some(Catch::new("e", vec![])), None).into()],
+        vec![Try::new(
+            vec![],
+            Some(Catch::new(
+                Declaration::new_with_identifier("e", None),
+                vec![],
+            )),
+            None,
+        )
+        .into()],
     );
 }
 
@@ -27,7 +38,10 @@ fn check_inline_with_var_decl_inside_try() {
                 .into(),
             )
             .into()],
-            Some(Catch::new("e", vec![])),
+            Some(Catch::new(
+                Declaration::new_with_identifier("e", None),
+                vec![],
+            )),
             None,
         )
         .into()],
@@ -48,7 +62,7 @@ fn check_inline_with_var_decl_inside_catch() {
             )
             .into()],
             Some(Catch::new(
-                "e",
+                Declaration::new_with_identifier("e", None),
                 vec![DeclarationList::Var(
                     vec![Declaration::new_with_identifier(
                         "x",
@@ -70,7 +84,10 @@ fn check_inline_with_empty_try_catch_finally() {
         "try {} catch(e) {} finally {}",
         vec![Try::new(
             vec![],
-            Some(Catch::new("e", vec![])),
+            Some(Catch::new(
+                Declaration::new_with_identifier("e", None),
+                vec![],
+            )),
             Some(Finally::from(vec![])),
         )
         .into()],
@@ -111,7 +128,7 @@ fn check_inline_empty_try_paramless_catch() {
         "try {} catch { var x = 1; }",
         vec![Try::new(
             Block::from(vec![]),
-            Some(Catch::new::<_, Identifier, _>(
+            Some(Catch::new::<_, Declaration, _>(
                 None,
                 vec![DeclarationList::Var(
                     vec![Declaration::new_with_identifier(
@@ -128,6 +145,64 @@ fn check_inline_empty_try_paramless_catch() {
     );
 }
 
+#[test]
+fn check_inline_with_binding_pattern_object() {
+    check_parser(
+        "try {} catch ({ a, b: c }) {}",
+        vec![Try::new(
+            Block::from(vec![]),
+            Some(Catch::new::<_, Declaration, _>(
+                Some(Declaration::new_with_object_pattern(
+                    vec![
+                        BindingPatternTypeObject::SingleName {
+                            ident: "a".into(),
+                            property_name: "a".into(),
+                            default_init: None,
+                        },
+                        BindingPatternTypeObject::SingleName {
+                            ident: "c".into(),
+                            property_name: "b".into(),
+                            default_init: None,
+                        },
+                    ],
+                    None,
+                )),
+                vec![],
+            )),
+            None,
+        )
+        .into()],
+    );
+}
+
+#[test]
+fn check_inline_with_binding_pattern_array() {
+    check_parser(
+        "try {} catch ([a, b]) {}",
+        vec![Try::new(
+            Block::from(vec![]),
+            Some(Catch::new::<_, Declaration, _>(
+                Some(Declaration::new_with_array_pattern(
+                    vec![
+                        BindingPatternTypeArray::SingleName {
+                            ident: "a".into(),
+                            default_init: None,
+                        },
+                        BindingPatternTypeArray::SingleName {
+                            ident: "b".into(),
+                            default_init: None,
+                        },
+                    ],
+                    None,
+                )),
+                vec![],
+            )),
+            None,
+        )
+        .into()],
+    );
+}
+
 #[test]
 fn check_inline_invalid_catch() {
     check_invalid("try {} catch");
@@ -144,6 +219,26 @@ fn check_inline_invalid_catch_parameter() {
 }
 
 #[test]
-fn check_invalide_try_no_catch_finally() {
+fn check_invalid_try_no_catch_finally() {
     check_invalid("try {} let a = 10;");
 }
+
+#[test]
+fn check_invalid_catch_with_empty_paren() {
+    check_invalid("try {} catch() {}");
+}
+
+#[test]
+fn check_invalid_catch_with_duplicate_params() {
+    check_invalid("try {} catch({ a, b: a }) {}");
+}
+
+#[test]
+fn check_invalid_catch_with_lexical_redeclaration() {
+    check_invalid("try {} catch(e) { let e = 'oh' }");
+}
+
+#[test]
+fn check_invalid_catch_with_var_redeclaration() {
+    check_invalid("try {} catch(e) { var e = 'oh' }");
+}