From 916c9d8bc914d919041c2cd9b5855dc70a61e1ca Mon Sep 17 00:00:00 2001 From: LowR Date: Wed, 6 Oct 2021 01:50:22 +0900 Subject: [PATCH] Allow `BindingPattern`s as `CatchParameter` (#1628) --- boa/src/syntax/ast/node/block/mod.rs | 10 +- boa/src/syntax/ast/node/try_node/mod.rs | 50 +++++--- boa/src/syntax/ast/node/try_node/tests.rs | 32 +++++ .../syntax/parser/statement/try_stm/catch.rs | 92 +++++++++++++-- .../syntax/parser/statement/try_stm/tests.rs | 109 ++++++++++++++++-- 5 files changed, 258 insertions(+), 35 deletions(-) diff --git a/boa/src/syntax/ast/node/block/mod.rs b/boa/src/syntax/ast/node/block/mod.rs index c89b0d0113..71ce68ed2a 100644 --- a/boa/src/syntax/ast/node/block/mod.rs +++ b/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, "{{")?; diff --git a/boa/src/syntax/ast/node/try_node/mod.rs b/boa/src/syntax/ast/node/try_node/mod.rs index 6dac6a95e4..c460c10a63 100644 --- a/boa/src/syntax/ast/node/try_node/mod.rs +++ b/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(); - context.push_environment(DeclarativeEnvironmentRecord::new(Some(env))); - - if let Some(param) = catch.parameter() { - context.create_mutable_binding(param, false, VariableScope::Block)?; - context.initialize_binding(param, err)?; + let env = context.get_current_environment(); + context.push_environment(DeclarativeEnvironmentRecord::new(Some(env))); + + if let Some(param) = catch.parameter() { + match param { + Declaration::Identifier { ident, init } => { + 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 for Node { #[cfg_attr(feature = "deser", derive(Serialize, Deserialize))] #[derive(Clone, Debug, Trace, Finalize, PartialEq)] pub struct Catch { - parameter: Option, + parameter: Option>, block: Block, } impl Catch { /// Creates a new catch block. - pub(in crate::syntax) fn new(parameter: OI, block: B) -> Self + pub(in crate::syntax) fn new(parameter: OD, block: B) -> Self where - OI: Into>, - I: Into, + OD: Into>, + D: Into, B: Into, { 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> { - self.parameter.as_ref().map(Identifier::as_ref) + pub fn parameter(&self) -> Option<&Declaration> { + self.parameter.as_deref() } /// Retrieves the catch execution block. diff --git a/boa/src/syntax/ast/node/try_node/tests.rs b/boa/src/syntax/ast/node/try_node/tests.rs index 87654c81a4..bb50c535b3 100644 --- a/boa/src/syntax/ast/node/try_node/tests.rs +++ b/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#" diff --git a/boa/src/syntax/parser/statement/try_stm/catch.rs b/boa/src/syntax/parser/statement/try_stm/catch.rs index 23bdcaecb0..8295b1ac5e 100644 --- a/boa/src/syntax/parser/statement/try_stm/catch.rs +++ b/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, _>( - catch_param, - Block::new(self.allow_yield, self.allow_await, self.allow_return).parse(cursor)?, - )) + let catch_block = + 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 TokenParser for CatchParameter where R: Read, { - type Output = Identifier; + type Output = node::Declaration; - fn parse(self, cursor: &mut Cursor) -> Result { - // TODO: should accept BindingPattern - BindingIdentifier::new(self.allow_yield, self.allow_await) - .parse(cursor) - .map(Identifier::from) + fn parse(self, cursor: &mut Cursor) -> Result { + let token = cursor.peek(0)?.ok_or(ParseError::AbruptEnd)?; + + match token.kind() { + 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)), + } } } diff --git a/boa/src/syntax/parser/statement/try_stm/tests.rs b/boa/src/syntax/parser/statement/try_stm/tests.rs index ef5464a60d..619fd805b4 100644 --- a/boa/src/syntax/parser/statement/try_stm/tests.rs +++ b/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' }"); +}