Allow `BindingPattern` in function parameters (#1666)

<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel neccesary.
--->

This Pull Request fixes/closes #1601

It changes the following:

- implements `FormalParameter` using `syntax::ast::node::declaration::Declaration` as 
```
pub struct FormalParameter {

    declaration: Declaration,
    is_rest_param: bool,

}
```
- changes `fn test_formatting(source: &'static str)` in `ast::node` to setup tests based on the new struct definition
- changes in various tests using `FormalParameter`
- changes in files using `FormalParameter`

What it does not change:
- if the function uses `FormalParameter` but does not seem affected due to the changed definition it is not altered while keeping all the required features i.e. if it wasn't affected but ended up giving undesired (not error) behaviour it has been changed


Co-authored-by: Aman Kumar <57605821+am-a-man@users.noreply.github.com>

Cargo.lock

@@ -490,9 +490,9 @@ dependencies = [
 
 [[package]]
 name = "half"
-version = "1.8.1"
+version = "1.8.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3bd566f08830dd1fa75b2a5f1d8def8336de7f08b2692a9a71efbe7188aea36f"
+checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7"
 
 [[package]]
 name = "hashbrown"
@@ -616,9 +616,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
 
 [[package]]
 name = "libc"
-version = "0.2.104"
+version = "0.2.106"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7b2f96d100e1cf1929e7719b7edb3b90ab5298072638fccd77be9ce942ecdfce"
+checksum = "a60553f9a9e039a333b4e9b20573b9e9b9c0bb3a11e201ccc48ef4283456d673"
 
 [[package]]
 name = "libgit2-sys"
@@ -827,9 +827,9 @@ checksum = "28988d872ab76095a6e6ac88d99b54fd267702734fd7ffe610ca27f533ddb95a"
 
 [[package]]
 name = "openssl-sys"
-version = "0.9.67"
+version = "0.9.68"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "69df2d8dfc6ce3aaf44b40dec6f487d5a886516cf6879c49e98e0710f310a058"
+checksum = "1c571f25d3f66dd427e417cebf73dbe2361d6125cf6e3a70d143fdf97c9f5150"
 dependencies = [
  "autocfg",
  "cc",
@@ -880,9 +880,9 @@ dependencies = [
 
 [[package]]
 name = "pkg-config"
-version = "0.3.20"
+version = "0.3.22"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7c9b1041b4387893b91ee6746cddfc28516aff326a3519fb2adf820932c5e6cb"
+checksum = "12295df4f294471248581bc09bef3c38a5e46f1e36d6a37353621a0c6c357e1f"
 
 [[package]]
 name = "plotters"
@@ -914,9 +914,9 @@ dependencies = [
 
 [[package]]
 name = "ppv-lite86"
-version = "0.2.14"
+version = "0.2.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c3ca011bd0129ff4ae15cd04c4eef202cadf6c51c21e47aba319b4e0501db741"
+checksum = "ed0cfbc8191465bed66e1718596ee0b0b35d5ee1f41c5df2189d0fe8bde535ba"
 
 [[package]]
 name = "proc-macro-error"
@@ -944,9 +944,9 @@ dependencies = [
 
 [[package]]
 name = "proc-macro2"
-version = "1.0.30"
+version = "1.0.32"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "edc3358ebc67bc8b7fa0c007f945b0b18226f78437d61bec735a9eb96b61ee70"
+checksum = "ba508cc11742c0dc5c1659771673afbab7a0efab23aa17e854cbab0837ed0b43"
 dependencies = [
  "unicode-xid",
 ]
@@ -1265,9 +1265,9 @@ dependencies = [
 
 [[package]]
 name = "syn"
-version = "1.0.80"
+version = "1.0.81"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d010a1623fbd906d51d650a9916aaefc05ffa0e4053ff7fe601167f3e715d194"
+checksum = "f2afee18b8beb5a596ecb4a2dce128c719b4ba399d34126b9e4396e3f9860966"
 dependencies = [
  "proc-macro2",
  "quote",

boa/src/builtins/function/arguments.rs

@@ -169,72 +169,74 @@ impl Arguments {
         // 18. Set index to numberOfParameters - 1.
         // 19. Repeat, while index ≥ 0,
         // a. Let name be parameterNames[index].
-        for (index, parameter_name) in formals.iter().map(|fp| fp.name()).enumerate().rev() {
-            // b. If name is not an element of mappedNames, then
-            if !mapped_names.contains(parameter_name) {
-                // i. Add name as an element of the list mappedNames.
-                mapped_names.insert(parameter_name);
-                // ii. If index < len, then
-                if index < len {
-                    // 1. Let g be MakeArgGetter(name, env).
-                    // https://tc39.es/ecma262/#sec-makearggetter
-                    let g = {
-                        // 2. Let getter be ! CreateBuiltinFunction(getterClosure, 0, "", « »).
-                        // 3. NOTE: getter is never directly accessible to ECMAScript code.
-                        // 4. Return getter.
-                        FunctionBuilder::closure_with_captures(
-                            context,
-                            // 1. Let getterClosure be a new Abstract Closure with no parameters that captures
-                            // name and env and performs the following steps when called:
-                            |_, _, captures, context| {
-                                captures.0.get_binding_value(&captures.1, false, context)
-                            },
-                            (env.clone(), parameter_name.to_owned()),
-                        )
-                        .length(0)
-                        .name("")
-                        .build()
-                    };
 
-                    // 2. Let p be MakeArgSetter(name, env).
-                    // https://tc39.es/ecma262/#sec-makeargsetter
-                    let p = {
-                        // 2. Let setter be ! CreateBuiltinFunction(setterClosure, 1, "", « »).
-                        // 3. NOTE: setter is never directly accessible to ECMAScript code.
-                        // 4. Return setter.
-                        FunctionBuilder::closure_with_captures(
+        for (index, parameter_name_vec) in formals.iter().map(|fp| fp.names()).enumerate().rev() {
+            for parameter_name in parameter_name_vec.iter().cloned() {
+                // b. If name is not an element of mappedNames, then
+                if !mapped_names.contains(parameter_name) {
+                    // i. Add name as an element of the list mappedNames.
+                    mapped_names.insert(parameter_name);
+                    // ii. If index < len, then
+                    if index < len {
+                        // 1. Let g be MakeArgGetter(name, env).
+                        // https://tc39.es/ecma262/#sec-makearggetter
+                        let g = {
+                            // 2. Let getter be ! CreateBuiltinFunction(getterClosure, 0, "", « »).
+                            // 3. NOTE: getter is never directly accessible to ECMAScript code.
+                            // 4. Return getter.
+                            FunctionBuilder::closure_with_captures(
+                                context,
+                                // 1. Let getterClosure be a new Abstract Closure with no parameters that captures
+                                // name and env and performs the following steps when called:
+                                |_, _, captures, context| {
+                                    captures.0.get_binding_value(&captures.1, false, context)
+                                },
+                                (env.clone(), parameter_name.to_owned()),
+                            )
+                            .length(0)
+                            .name("")
+                            .build()
+                        };
+                        // 2. Let p be MakeArgSetter(name, env).
+                        // https://tc39.es/ecma262/#sec-makeargsetter
+                        let p = {
+                            // 2. Let setter be ! CreateBuiltinFunction(setterClosure, 1, "", « »).
+                            // 3. NOTE: setter is never directly accessible to ECMAScript code.
+                            // 4. Return setter.
+                            FunctionBuilder::closure_with_captures(
+                                context,
+                                // 1. Let setterClosure be a new Abstract Closure with parameters (value) that captures
+                                // name and env and performs the following steps when called:
+                                |_, args, captures, context| {
+                                    let value = args.get(0).cloned().unwrap_or_default();
+                                    // a. Return env.SetMutableBinding(name, value, false).
+                                    captures
+                                        .0
+                                        .set_mutable_binding(&captures.1, value, false, context)
+                                        .map(|_| JsValue::Undefined)
+                                    // Ok(JsValue::Undefined)
+                                },
+                                (env.clone(), parameter_name.to_owned()),
+                            )
+                            .length(1)
+                            .name("")
+                            .build()
+                        };
+
+                        // 3. Perform map.[[DefineOwnProperty]](! ToString(𝔽(index)), PropertyDescriptor {
+                        // [[Set]]: p, [[Get]]: g, [[Enumerable]]: false, [[Configurable]]: true }).
+                        map.__define_own_property__(
+                            index.into(),
+                            PropertyDescriptor::builder()
+                                .set(p)
+                                .get(g)
+                                .enumerable(false)
+                                .configurable(true)
+                                .build(),
                             context,
-                            // 1. Let setterClosure be a new Abstract Closure with parameters (value) that captures
-                            // name and env and performs the following steps when called:
-                            |_, args, captures, context| {
-                                let value = args.get(0).cloned().unwrap_or_default();
-                                // a. Return env.SetMutableBinding(name, value, false).
-                                captures
-                                    .0
-                                    .set_mutable_binding(&captures.1, value, false, context)
-                                    .map(|_| JsValue::Undefined)
-                                // Ok(JsValue::Undefined)
-                            },
-                            (env.clone(), parameter_name.to_owned()),
                         )
-                        .length(1)
-                        .name("")
-                        .build()
-                    };
-
-                    // 3. Perform map.[[DefineOwnProperty]](! ToString(𝔽(index)), PropertyDescriptor {
-                    // [[Set]]: p, [[Get]]: g, [[Enumerable]]: false, [[Configurable]]: true }).
-                    map.__define_own_property__(
-                        index.into(),
-                        PropertyDescriptor::builder()
-                            .set(p)
-                            .get(g)
-                            .enumerable(false)
-                            .configurable(true)
-                            .build(),
-                        context,
-                    )
-                    .expect("[[DefineOwnProperty]] must not fail per the spec");
+                        .expect("[[DefineOwnProperty]] must not fail per the spec");
+                    }
                 }
             }
         }

boa/src/builtins/function/mod.rs

@@ -30,6 +30,7 @@ use crate::{
     object::{internal_methods::get_prototype_from_constructor, NativeObject, ObjectData},
     property::Attribute,
     property::PropertyDescriptor,
+    syntax::ast::node::declaration::Declaration,
     syntax::ast::node::{FormalParameter, RcStatementList},
     BoaProfiler, Context, JsResult, JsValue,
 };
@@ -220,16 +221,22 @@ impl Function {
         Array::add_to_array_object(&array, args_list.get(index..).unwrap_or_default(), context)
             .unwrap();
 
-        // Create binding
-        local_env
-            // Function parameters can share names in JavaScript...
-            .create_mutable_binding(param.name(), false, true, context)
-            .expect("Failed to create binding for rest param");
-
-        // Set Binding to value
-        local_env
-            .initialize_binding(param.name(), array, context)
-            .expect("Failed to initialize rest param");
+        let binding_params = param.run(Some(array), context).unwrap_or_default();
+        for binding_items in binding_params.iter() {
+            // Create binding
+            local_env
+                .create_mutable_binding(binding_items.0.as_ref(), false, true, context)
+                .expect("Failed to create binding");
+
+            // Set binding to value
+            local_env
+                .initialize_binding(
+                    binding_items.0.as_ref(),
+                    JsValue::new(binding_items.1.clone()),
+                    context,
+                )
+                .expect("Failed to intialize binding");
+        }
     }
 
     // Adds an argument to the environment
@@ -239,15 +246,22 @@ impl Function {
         local_env: &Environment,
         context: &mut Context,
     ) {
-        // Create binding
-        local_env
-            .create_mutable_binding(param.name(), false, true, context)
-            .expect("Failed to create binding");
-
-        // Set Binding to value
-        local_env
-            .initialize_binding(param.name(), value, context)
-            .expect("Failed to intialize binding");
+        let binding_params = param.run(Some(value), context).unwrap_or_default();
+        for binding_items in binding_params.iter() {
+            // Create binding
+            local_env
+                .create_mutable_binding(binding_items.0.as_ref(), false, true, context)
+                .expect("Failed to create binding");
+
+            // Set binding to value
+            local_env
+                .initialize_binding(
+                    binding_items.0.as_ref(),
+                    JsValue::new(binding_items.1.clone()),
+                    context,
+                )
+                .expect("Failed to intialize binding");
+        }
     }
 
     /// Returns true if the function object is a constructor.
@@ -520,11 +534,19 @@ impl BuiltInFunctionObject {
                 Some(name),
             ) => Ok(format!("function {}() {{\n  [native Code]\n}}", &name).into()),
             (Function::Ordinary { body, params, .. }, Some(name)) => {
-                let arguments: String = params
-                    .iter()
-                    .map(|param| param.name())
-                    .collect::<Vec<&str>>()
-                    .join(", ");
+                let arguments: String = {
+                    let mut argument_list: Vec<Cow<'_, str>> = Vec::new();
+                    for params_item in params.iter() {
+                        let argument_item = match &params_item.declaration() {
+                            Declaration::Identifier { ident, .. } => Cow::Borrowed(ident.as_ref()),
+                            Declaration::Pattern(pattern) => {
+                                Cow::Owned(format!("{{{}}}", pattern.idents().join(",")))
+                            }
+                        };
+                        argument_list.push(argument_item);
+                    }
+                    argument_list.join(",")
+                };
 
                 let statement_list = &*body;
                 // This is a kluge. The implementaion in browser seems to suggest that

boa/src/object/internal_methods/function.rs

@@ -171,10 +171,15 @@ pub(super) fn call_construct(
                     for param in params.iter() {
                         has_parameter_expressions =
                             has_parameter_expressions || param.init().is_some();
-                        arguments_in_parameter_names =
-                            arguments_in_parameter_names || param.name() == "arguments";
+
+                        for param_name in param.names() {
+                            arguments_in_parameter_names =
+                                arguments_in_parameter_names || param_name == "arguments";
+                        }
+
                         is_simple_parameter_list = is_simple_parameter_list
                             && !param.is_rest_param()
+                            && param.is_identifier()
                             && param.init().is_none()
                     }
 

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

@@ -32,8 +32,8 @@ pub use self::{
     declaration::{
         async_generator_decl::AsyncGeneratorDecl, async_generator_expr::AsyncGeneratorExpr,
         generator_decl::GeneratorDecl, generator_expr::GeneratorExpr, ArrowFunctionDecl,
-        AsyncFunctionDecl, AsyncFunctionExpr, Declaration, DeclarationList, FunctionDecl,
-        FunctionExpr,
+        AsyncFunctionDecl, AsyncFunctionExpr, Declaration, DeclarationList, DeclarationPattern,
+        FunctionDecl, FunctionExpr,
     },
     field::{GetConstField, GetField},
     identifier::Identifier,
@@ -430,38 +430,71 @@ where
 #[cfg_attr(feature = "deser", derive(Serialize, Deserialize))]
 #[derive(Clone, Debug, PartialEq, Trace, Finalize)]
 pub struct FormalParameter {
-    name: Box<str>,
-    init: Option<Node>,
+    declaration: Declaration,
     is_rest_param: bool,
 }
 
 impl FormalParameter {
     /// Creates a new formal parameter.
-    pub(in crate::syntax) fn new<N>(name: N, init: Option<Node>, is_rest_param: bool) -> Self
+    pub(in crate::syntax) fn new<D>(declaration: D, is_rest_param: bool) -> Self
     where
-        N: Into<Box<str>>,
+        D: Into<Declaration>,
     {
         Self {
-            name: name.into(),
-            init,
+            declaration: declaration.into(),
             is_rest_param,
         }
     }
 
     /// Gets the name of the formal parameter.
-    pub fn name(&self) -> &str {
-        &self.name
+    pub fn names(&self) -> Vec<&str> {
+        match &self.declaration {
+            Declaration::Identifier { ident, .. } => vec![ident.as_ref()],
+            Declaration::Pattern(pattern) => match pattern {
+                DeclarationPattern::Object(object_pattern) => object_pattern.idents(),
+
+                DeclarationPattern::Array(array_pattern) => array_pattern.idents(),
+            },
+        }
+    }
+
+    /// Get the declaration of the formal parameter
+    pub fn declaration(&self) -> &Declaration {
+        &self.declaration
     }
 
     /// Gets the initialization node of the formal parameter, if any.
     pub fn init(&self) -> Option<&Node> {
-        self.init.as_ref()
+        self.declaration.init()
     }
 
     /// Gets wether the parameter is a rest parameter.
     pub fn is_rest_param(&self) -> bool {
         self.is_rest_param
     }
+
+    pub fn run(
+        &self,
+        init: Option<JsValue>,
+        context: &mut Context,
+    ) -> JsResult<Vec<(Box<str>, JsValue)>> {
+        match &self.declaration {
+            Declaration::Identifier { ident, .. } => Ok(vec![(
+                ident.as_ref().to_string().into_boxed_str(),
+                init.unwrap(),
+            )]),
+
+            Declaration::Pattern(pattern) => match &pattern {
+                DeclarationPattern::Object(object_pattern) => object_pattern.run(init, context),
+
+                DeclarationPattern::Array(array_pattern) => array_pattern.run(init, context),
+            },
+        }
+    }
+
+    pub fn is_identifier(&self) -> bool {
+        matches!(&self.declaration, Declaration::Identifier { .. })
+    }
 }
 
 impl Display for FormalParameter {
@@ -469,10 +502,7 @@ impl Display for FormalParameter {
         if self.is_rest_param {
             write!(f, "...")?;
         }
-        write!(f, "{}", self.name)?;
-        if let Some(n) = self.init.as_ref() {
-            write!(f, " = {}", n)?;
-        }
+        write!(f, "{}", self.declaration)?;
         Ok(())
     }
 }

boa/src/syntax/parser/expression/assignment/arrow_function.rs

@@ -11,7 +11,10 @@ use super::AssignmentExpression;
 use crate::{
     syntax::{
         ast::{
-            node::{ArrowFunctionDecl, FormalParameter, Node, Return, StatementList},
+            node::{
+                declaration::Declaration, ArrowFunctionDecl, FormalParameter, Node, Return,
+                StatementList,
+            },
             Punctuator,
         },
         lexer::{Error as LexError, Position, TokenKind},
@@ -91,7 +94,10 @@ where
                 .context("arrow function")?;
             (
                 FormalParameterList {
-                    parameters: Box::new([FormalParameter::new(param, None, false)]),
+                    parameters: Box::new([FormalParameter::new(
+                        Declaration::new_with_identifier(param, None),
+                        false,
+                    )]),
                     is_simple: true,
                     has_duplicates: false,
                 },
@@ -127,14 +133,16 @@ where
         {
             let lexically_declared_names = body.lexically_declared_names();
             for param in params.parameters.as_ref() {
-                if lexically_declared_names.contains(param.name()) {
-                    return Err(ParseError::lex(LexError::Syntax(
-                        format!("Redeclaration of formal parameter `{}`", param.name()).into(),
-                        match cursor.peek(0)? {
-                            Some(token) => token.span().end(),
-                            None => Position::new(1, 1),
-                        },
-                    )));
+                for param_name in param.names() {
+                    if lexically_declared_names.contains(param_name) {
+                        return Err(ParseError::lex(LexError::Syntax(
+                            format!("Redeclaration of formal parameter `{}`", param_name).into(),
+                            match cursor.peek(0)? {
+                                Some(token) => token.span().end(),
+                                None => Position::new(1, 1),
+                            },
+                        )));
+                    }
                 }
             }
         }

boa/src/syntax/parser/expression/primary/async_function_expression/mod.rs

@@ -115,14 +115,16 @@ where
         {
             let lexically_declared_names = body.lexically_declared_names();
             for param in params.parameters.as_ref() {
-                if lexically_declared_names.contains(param.name()) {
-                    return Err(ParseError::lex(LexError::Syntax(
-                        format!("Redeclaration of formal parameter `{}`", param.name()).into(),
-                        match cursor.peek(0)? {
-                            Some(token) => token.span().end(),
-                            None => Position::new(1, 1),
-                        },
-                    )));
+                for param_name in param.names() {
+                    if lexically_declared_names.contains(param_name) {
+                        return Err(ParseError::lex(LexError::Syntax(
+                            format!("Redeclaration of formal parameter `{}`", param_name).into(),
+                            match cursor.peek(0)? {
+                                Some(token) => token.span().end(),
+                                None => Position::new(1, 1),
+                            },
+                        )));
+                    }
                 }
             }
         }

boa/src/syntax/parser/expression/primary/async_generator_expression/mod.rs

@@ -111,14 +111,16 @@ where
         {
             let lexically_declared_names = body.lexically_declared_names();
             for param in params.parameters.as_ref() {
-                if lexically_declared_names.contains(param.name()) {
-                    return Err(ParseError::lex(LexError::Syntax(
-                        format!("Redeclaration of formal parameter `{}`", param.name()).into(),
-                        match cursor.peek(0)? {
-                            Some(token) => token.span().end(),
-                            None => Position::new(1, 1),
-                        },
-                    )));
+                for param_name in param.names() {
+                    if lexically_declared_names.contains(param_name) {
+                        return Err(ParseError::lex(LexError::Syntax(
+                            format!("Redeclaration of formal parameter `{}`", param_name).into(),
+                            match cursor.peek(0)? {
+                                Some(token) => token.span().end(),
+                                None => Position::new(1, 1),
+                            },
+                        )));
+                    }
                 }
             }
         }

boa/src/syntax/parser/expression/primary/function_expression/mod.rs

@@ -110,14 +110,16 @@ where
         {
             let lexically_declared_names = body.lexically_declared_names();
             for param in params.parameters.as_ref() {
-                if lexically_declared_names.contains(param.name()) {
-                    return Err(ParseError::lex(LexError::Syntax(
-                        format!("Redeclaration of formal parameter `{}`", param.name()).into(),
-                        match cursor.peek(0)? {
-                            Some(token) => token.span().end(),
-                            None => Position::new(1, 1),
-                        },
-                    )));
+                for param_name in param.names() {
+                    if lexically_declared_names.contains(param_name) {
+                        return Err(ParseError::lex(LexError::Syntax(
+                            format!("Redeclaration of formal parameter `{}`", param_name).into(),
+                            match cursor.peek(0)? {
+                                Some(token) => token.span().end(),
+                                None => Position::new(1, 1),
+                            },
+                        )));
+                    }
                 }
             }
         }

boa/src/syntax/parser/expression/primary/generator_expression/mod.rs

@@ -111,14 +111,16 @@ where
         {
             let lexically_declared_names = body.lexically_declared_names();
             for param in params.parameters.as_ref() {
-                if lexically_declared_names.contains(param.name()) {
-                    return Err(ParseError::lex(LexError::Syntax(
-                        format!("Redeclaration of formal parameter `{}`", param.name()).into(),
-                        match cursor.peek(0)? {
-                            Some(token) => token.span().end(),
-                            None => Position::new(1, 1),
-                        },
-                    )));
+                for param_name in param.names() {
+                    if lexically_declared_names.contains(param_name) {
+                        return Err(ParseError::lex(LexError::Syntax(
+                            format!("Redeclaration of formal parameter `{}`", param_name).into(),
+                            match cursor.peek(0)? {
+                                Some(token) => token.span().end(),
+                                None => Position::new(1, 1),
+                            },
+                        )));
+                    }
                 }
             }
         }

boa/src/syntax/parser/expression/primary/object_initializer/mod.rs

@@ -245,15 +245,17 @@ where
                 {
                     let lexically_declared_names = body.lexically_declared_names();
                     for param in params.parameters.as_ref() {
-                        if lexically_declared_names.contains(param.name()) {
-                            return Err(ParseError::lex(LexError::Syntax(
-                                format!("Redeclaration of formal parameter `{}`", param.name())
-                                    .into(),
-                                match cursor.peek(0)? {
-                                    Some(token) => token.span().end(),
-                                    None => Position::new(1, 1),
-                                },
-                            )));
+                        for param_name in param.names() {
+                            if lexically_declared_names.contains(param_name) {
+                                return Err(ParseError::lex(LexError::Syntax(
+                                    format!("Redeclaration of formal parameter `{}`", param_name)
+                                        .into(),
+                                    match cursor.peek(0)? {
+                                        Some(token) => token.span().end(),
+                                        None => Position::new(1, 1),
+                                    },
+                                )));
+                            }
                         }
                     }
                 }
@@ -307,15 +309,17 @@ where
                 {
                     let lexically_declared_names = body.lexically_declared_names();
                     for param in params.parameters.as_ref() {
-                        if lexically_declared_names.contains(param.name()) {
-                            return Err(ParseError::lex(LexError::Syntax(
-                                format!("Redeclaration of formal parameter `{}`", param.name())
-                                    .into(),
-                                match cursor.peek(0)? {
-                                    Some(token) => token.span().end(),
-                                    None => Position::new(1, 1),
-                                },
-                            )));
+                        for param_name in param.names() {
+                            if lexically_declared_names.contains(param_name) {
+                                return Err(ParseError::lex(LexError::Syntax(
+                                    format!("Redeclaration of formal parameter `{}`", param_name)
+                                        .into(),
+                                    match cursor.peek(0)? {
+                                        Some(token) => token.span().end(),
+                                        None => Position::new(1, 1),
+                                    },
+                                )));
+                            }
                         }
                     }
                 }
@@ -373,14 +377,17 @@ where
             {
                 let lexically_declared_names = body.lexically_declared_names();
                 for param in params.parameters.as_ref() {
-                    if lexically_declared_names.contains(param.name()) {
-                        return Err(ParseError::lex(LexError::Syntax(
-                            format!("Redeclaration of formal parameter `{}`", param.name()).into(),
-                            match cursor.peek(0)? {
-                                Some(token) => token.span().end(),
-                                None => Position::new(1, 1),
-                            },
-                        )));
+                    for param_name in param.names() {
+                        if lexically_declared_names.contains(param_name) {
+                            return Err(ParseError::lex(LexError::Syntax(
+                                format!("Redeclaration of formal parameter `{}`", param_name)
+                                    .into(),
+                                match cursor.peek(0)? {
+                                    Some(token) => token.span().end(),
+                                    None => Position::new(1, 1),
+                                },
+                            )));
+                        }
                     }
                 }
             }

boa/src/syntax/parser/expression/primary/object_initializer/tests.rs

@@ -73,7 +73,10 @@ fn check_object_short_function_arguments() {
             "b",
             FunctionExpr::new(
                 None,
-                vec![FormalParameter::new("test", None, false)],
+                vec![FormalParameter::new(
+                    Declaration::new_with_identifier("test", None),
+                    false,
+                )],
                 vec![],
             ),
         ),
@@ -133,7 +136,10 @@ fn check_object_setter() {
             "b",
             FunctionExpr::new(
                 None,
-                vec![FormalParameter::new("test", None, false)],
+                vec![FormalParameter::new(
+                    Declaration::new_with_identifier("test", None),
+                    false,
+                )],
                 vec![],
             ),
         ),

boa/src/syntax/parser/function/mod.rs

@@ -12,11 +12,13 @@ mod tests;
 
 use crate::{
     syntax::{
-        ast::{node, Punctuator},
+        ast::{node, node::declaration::Declaration, Punctuator},
         lexer::{Error as LexError, InputElement, TokenKind},
         parser::{
             expression::Initializer,
-            statement::{BindingIdentifier, StatementList},
+            statement::{
+                ArrayBindingPattern, BindingIdentifier, ObjectBindingPattern, StatementList,
+            },
             AllowAwait, AllowYield, Cursor, ParseError, TokenParser,
         },
     },
@@ -97,14 +99,25 @@ where
                 _ => FormalParameter::new(self.allow_yield, self.allow_await).parse(cursor)?,
             };
 
-            if next_param.is_rest_param() || next_param.init().is_some() {
+            if next_param.is_rest_param() && next_param.init().is_some() {
+                return Err(ParseError::lex(LexError::Syntax(
+                    "Rest parameter may not have a default initializer".into(),
+                    start_position,
+                )));
+            }
+
+            if next_param.is_rest_param()
+                || next_param.init().is_some()
+                || !next_param.is_identifier()
+            {
                 is_simple = false;
             }
-            if parameter_names.contains(next_param.name()) {
-                has_duplicates = true;
+            for param_name in next_param.names() {
+                if parameter_names.contains(param_name) {
+                    has_duplicates = true;
+                }
+                parameter_names.insert(Box::from(param_name));
             }
-            parameter_names.insert(Box::from(next_param.name()));
-
             params.push(next_param);
 
             if cursor.peek(0)?.ok_or(ParseError::AbruptEnd)?.kind()
@@ -188,10 +201,59 @@ where
         let _timer = BoaProfiler::global().start_event("BindingRestElement", "Parsing");
         cursor.expect(Punctuator::Spread, "rest parameter")?;
 
-        let param = BindingIdentifier::new(self.allow_yield, self.allow_await).parse(cursor)?;
-        // TODO: BindingPattern
+        if let Some(t) = cursor.peek(0)? {
+            let declaration = match *t.kind() {
+                TokenKind::Punctuator(Punctuator::OpenBlock) => {
+                    let param = ObjectBindingPattern::new(true, self.allow_yield, self.allow_await)
+                        .parse(cursor)?;
+
+                    let init = cursor
+                        .peek(0)?
+                        .cloned()
+                        .filter(|t| {
+                            // Check that this is an initializer before attempting parse.
+                            *t.kind() == TokenKind::Punctuator(Punctuator::Assign)
+                        })
+                        .map(|_| {
+                            Initializer::new(true, self.allow_yield, self.allow_await).parse(cursor)
+                        })
+                        .transpose()?;
+                    Declaration::new_with_object_pattern(param, init)
+                }
+
+                TokenKind::Punctuator(Punctuator::OpenBracket) => {
+                    Declaration::new_with_array_pattern(
+                        ArrayBindingPattern::new(true, self.allow_yield, self.allow_await)
+                            .parse(cursor)?,
+                        None,
+                    )
+                }
 
-        Ok(Self::Output::new(param, None, true))
+                _ => {
+                    let params =
+                        BindingIdentifier::new(self.allow_yield, self.allow_await).parse(cursor)?;
+                    let init = cursor
+                        .peek(0)?
+                        .cloned()
+                        .filter(|t| {
+                            // Check that this is an initializer before attempting parse.
+                            *t.kind() == TokenKind::Punctuator(Punctuator::Assign)
+                        })
+                        .map(|_| {
+                            Initializer::new(true, self.allow_yield, self.allow_await).parse(cursor)
+                        })
+                        .transpose()?;
+
+                    Declaration::new_with_identifier(params, init)
+                }
+            };
+            Ok(Self::Output::new(declaration, true))
+        } else {
+            Ok(Self::Output::new(
+                Declaration::new_with_identifier("", None),
+                true,
+            ))
+        }
     }
 }
 
@@ -232,22 +294,60 @@ where
     fn parse(self, cursor: &mut Cursor<R>) -> Result<Self::Output, ParseError> {
         let _timer = BoaProfiler::global().start_event("FormalParameter", "Parsing");
 
-        // TODO: BindingPattern
+        if let Some(t) = cursor.peek(0)? {
+            let declaration = match *t.kind() {
+                TokenKind::Punctuator(Punctuator::OpenBlock) => {
+                    let param = ObjectBindingPattern::new(true, self.allow_yield, self.allow_await)
+                        .parse(cursor)?;
+
+                    let init = cursor
+                        .peek(0)?
+                        .cloned()
+                        .filter(|t| {
+                            // Check that this is an initializer before attempting parse.
+                            *t.kind() == TokenKind::Punctuator(Punctuator::Assign)
+                        })
+                        .map(|_| {
+                            Initializer::new(true, self.allow_yield, self.allow_await).parse(cursor)
+                        })
+                        .transpose()?;
+
+                    Declaration::new_with_object_pattern(param, init)
+                }
 
-        let param = BindingIdentifier::new(self.allow_yield, self.allow_await).parse(cursor)?;
+                TokenKind::Punctuator(Punctuator::OpenBracket) => {
+                    Declaration::new_with_array_pattern(
+                        ArrayBindingPattern::new(true, self.allow_yield, self.allow_await)
+                            .parse(cursor)?,
+                        None,
+                    )
+                }
 
-        let init = if let Some(t) = cursor.peek(0)? {
-            // Check that this is an initilizer before attempting parse.
-            if *t.kind() == TokenKind::Punctuator(Punctuator::Assign) {
-                Some(Initializer::new(true, self.allow_yield, self.allow_await).parse(cursor)?)
-            } else {
-                None
-            }
+                _ => {
+                    let params =
+                        BindingIdentifier::new(self.allow_yield, self.allow_await).parse(cursor)?;
+                    let init = cursor
+                        .peek(0)?
+                        .cloned()
+                        .filter(|t| {
+                            // Check that this is an initializer before attempting parse.
+                            *t.kind() == TokenKind::Punctuator(Punctuator::Assign)
+                        })
+                        .map(|_| {
+                            Initializer::new(true, self.allow_yield, self.allow_await).parse(cursor)
+                        })
+                        .transpose()?;
+
+                    Declaration::new_with_identifier(params, init)
+                }
+            };
+            Ok(Self::Output::new(declaration, false))
         } else {
-            None
-        };
-
-        Ok(Self::Output::new(param, init, false))
+            Ok(Self::Output::new(
+                Declaration::new_with_identifier("", None),
+                false,
+            ))
+        }
     }
 }
 

boa/src/syntax/parser/function/tests.rs

@@ -14,7 +14,10 @@ fn check_basic() {
         "function foo(a) { return a; }",
         vec![FunctionDecl::new(
             Box::from("foo"),
-            vec![FormalParameter::new("a", None, false)],
+            vec![FormalParameter::new(
+                Declaration::new_with_identifier("a", None),
+                false,
+            )],
             vec![Return::new(Identifier::from("a"), None).into()],
         )
         .into()],
@@ -29,8 +32,8 @@ fn check_duplicates_strict_off() {
         vec![FunctionDecl::new(
             Box::from("foo"),
             vec![
-                FormalParameter::new("a", None, false),
-                FormalParameter::new("a", None, false),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
             ],
             vec![Return::new(Identifier::from("a"), None).into()],
         )
@@ -55,7 +58,10 @@ fn check_basic_semicolon_insertion() {
         "function foo(a) { return a }",
         vec![FunctionDecl::new(
             Box::from("foo"),
-            vec![FormalParameter::new("a", None, false)],
+            vec![FormalParameter::new(
+                Declaration::new_with_identifier("a", None),
+                false,
+            )],
             vec![Return::new(Identifier::from("a"), None).into()],
         )
         .into()],
@@ -69,7 +75,10 @@ fn check_empty_return() {
         "function foo(a) { return; }",
         vec![FunctionDecl::new(
             Box::from("foo"),
-            vec![FormalParameter::new("a", None, false)],
+            vec![FormalParameter::new(
+                Declaration::new_with_identifier("a", None),
+                false,
+            )],
             vec![Return::new::<Node, Option<Node>, Option<_>>(None, None).into()],
         )
         .into()],
@@ -83,7 +92,10 @@ fn check_empty_return_semicolon_insertion() {
         "function foo(a) { return }",
         vec![FunctionDecl::new(
             Box::from("foo"),
-            vec![FormalParameter::new("a", None, false)],
+            vec![FormalParameter::new(
+                Declaration::new_with_identifier("a", None),
+                false,
+            )],
             vec![Return::new::<Node, Option<Node>, Option<_>>(None, None).into()],
         )
         .into()],
@@ -98,8 +110,8 @@ fn check_rest_operator() {
         vec![FunctionDecl::new(
             Box::from("foo"),
             vec![
-                FormalParameter::new("a", None, false),
-                FormalParameter::new("b", None, true),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("b", None), true),
             ],
             vec![],
         )
@@ -112,7 +124,14 @@ fn check_rest_operator() {
 fn check_arrow_only_rest() {
     check_parser(
         "(...a) => {}",
-        vec![ArrowFunctionDecl::new(vec![FormalParameter::new("a", None, true)], vec![]).into()],
+        vec![ArrowFunctionDecl::new(
+            vec![FormalParameter::new(
+                Declaration::new_with_identifier("a", None),
+                true,
+            )],
+            vec![],
+        )
+        .into()],
     );
 }
 
@@ -123,9 +142,9 @@ fn check_arrow_rest() {
         "(a, b, ...c) => {}",
         vec![ArrowFunctionDecl::new(
             vec![
-                FormalParameter::new("a", None, false),
-                FormalParameter::new("b", None, false),
-                FormalParameter::new("c", None, true),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("b", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("c", None), true),
             ],
             vec![],
         )
@@ -140,8 +159,8 @@ fn check_arrow() {
         "(a, b) => { return a + b; }",
         vec![ArrowFunctionDecl::new(
             vec![
-                FormalParameter::new("a", None, false),
-                FormalParameter::new("b", None, false),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("b", None), false),
             ],
             vec![Return::new(
                 BinOp::new(NumOp::Add, Identifier::from("a"), Identifier::from("b")),
@@ -160,8 +179,8 @@ fn check_arrow_semicolon_insertion() {
         "(a, b) => { return a + b }",
         vec![ArrowFunctionDecl::new(
             vec![
-                FormalParameter::new("a", None, false),
-                FormalParameter::new("b", None, false),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("b", None), false),
             ],
             vec![Return::new(
                 BinOp::new(NumOp::Add, Identifier::from("a"), Identifier::from("b")),
@@ -180,8 +199,8 @@ fn check_arrow_epty_return() {
         "(a, b) => { return; }",
         vec![ArrowFunctionDecl::new(
             vec![
-                FormalParameter::new("a", None, false),
-                FormalParameter::new("b", None, false),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("b", None), false),
             ],
             vec![Return::new::<Node, Option<_>, Option<_>>(None, None).into()],
         )
@@ -196,8 +215,8 @@ fn check_arrow_empty_return_semicolon_insertion() {
         "(a, b) => { return }",
         vec![ArrowFunctionDecl::new(
             vec![
-                FormalParameter::new("a", None, false),
-                FormalParameter::new("b", None, false),
+                FormalParameter::new(Declaration::new_with_identifier("a", None), false),
+                FormalParameter::new(Declaration::new_with_identifier("b", None), false),
             ],
             vec![Return::new::<Node, Option<_>, Option<_>>(None, None).into()],
         )
@@ -214,7 +233,10 @@ fn check_arrow_assignment() {
                 Identifier::from("foo"),
                 Some(
                     ArrowFunctionDecl::new(
-                        vec![FormalParameter::new("a", None, false)],
+                        vec![FormalParameter::new(
+                            Declaration::new_with_identifier("a", None),
+                            false,
+                        )],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),
                             None,
@@ -239,7 +261,10 @@ fn check_arrow_assignment_nobrackets() {
                 Identifier::from("foo"),
                 Some(
                     ArrowFunctionDecl::new(
-                        vec![FormalParameter::new("a", None, false)],
+                        vec![FormalParameter::new(
+                            Declaration::new_with_identifier("a", None),
+                            false,
+                        )],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),
                             None,
@@ -264,7 +289,10 @@ fn check_arrow_assignment_noparenthesis() {
                 Identifier::from("foo"),
                 Some(
                     ArrowFunctionDecl::new(
-                        vec![FormalParameter::new("a", None, false)],
+                        vec![FormalParameter::new(
+                            Declaration::new_with_identifier("a", None),
+                            false,
+                        )],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),
                             None,
@@ -289,7 +317,10 @@ fn check_arrow_assignment_noparenthesis_nobrackets() {
                 Identifier::from("foo"),
                 Some(
                     ArrowFunctionDecl::new(
-                        vec![FormalParameter::new("a", None, false)],
+                        vec![FormalParameter::new(
+                            Declaration::new_with_identifier("a", None),
+                            false,
+                        )],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),
                             None,
@@ -315,8 +346,14 @@ fn check_arrow_assignment_2arg() {
                 Some(
                     ArrowFunctionDecl::new(
                         vec![
-                            FormalParameter::new("a", None, false),
-                            FormalParameter::new("b", None, false),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("a", None),
+                                false,
+                            ),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("b", None),
+                                false,
+                            ),
                         ],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),
@@ -343,8 +380,14 @@ fn check_arrow_assignment_2arg_nobrackets() {
                 Some(
                     ArrowFunctionDecl::new(
                         vec![
-                            FormalParameter::new("a", None, false),
-                            FormalParameter::new("b", None, false),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("a", None),
+                                false,
+                            ),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("b", None),
+                                false,
+                            ),
                         ],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),
@@ -371,9 +414,18 @@ fn check_arrow_assignment_3arg() {
                 Some(
                     ArrowFunctionDecl::new(
                         vec![
-                            FormalParameter::new("a", None, false),
-                            FormalParameter::new("b", None, false),
-                            FormalParameter::new("c", None, false),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("a", None),
+                                false,
+                            ),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("b", None),
+                                false,
+                            ),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("c", None),
+                                false,
+                            ),
                         ],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),
@@ -400,9 +452,18 @@ fn check_arrow_assignment_3arg_nobrackets() {
                 Some(
                     ArrowFunctionDecl::new(
                         vec![
-                            FormalParameter::new("a", None, false),
-                            FormalParameter::new("b", None, false),
-                            FormalParameter::new("c", None, false),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("a", None),
+                                false,
+                            ),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("b", None),
+                                false,
+                            ),
+                            FormalParameter::new(
+                                Declaration::new_with_identifier("c", None),
+                                false,
+                            ),
                         ],
                         vec![Return::new::<Node, Option<_>, Option<_>>(
                             Some(Identifier::from("a").into()),

boa/src/syntax/parser/statement/declaration/hoistable/mod.rs

@@ -192,14 +192,16 @@ fn parse_callable_declaration<R: Read, C: CallableDeclaration>(
     {
         let lexically_declared_names = body.lexically_declared_names();
         for param in params.parameters.as_ref() {
-            if lexically_declared_names.contains(param.name()) {
-                return Err(ParseError::lex(LexError::Syntax(
-                    format!("Redeclaration of formal parameter `{}`", param.name()).into(),
-                    match cursor.peek(0)? {
-                        Some(token) => token.span().end(),
-                        None => Position::new(1, 1),
-                    },
-                )));
+            for param_name in param.names() {
+                if lexically_declared_names.contains(param_name) {
+                    return Err(ParseError::lex(LexError::Syntax(
+                        format!("Redeclaration of formal parameter `{}`", param_name).into(),
+                        match cursor.peek(0)? {
+                            Some(token) => token.span().end(),
+                            None => Position::new(1, 1),
+                        },
+                    )));
+                }
             }
         }
     }

boa/src/syntax/parser/tests.rs

@@ -339,7 +339,10 @@ fn spread_in_arrow_function() {
         s,
         vec![
             ArrowFunctionDecl::new::<Box<[FormalParameter]>, StatementList>(
-                Box::new([FormalParameter::new("b", None, true)]),
+                Box::new([FormalParameter::new(
+                    Declaration::new_with_identifier("b", None),
+                    true,
+                )]),
                 vec![Identifier::from("b").into()].into(),
             )
             .into(),