[Merged by Bors] - Allow BindingPattern in function parameters

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");
+                    }
                 }
             }
         }
@@ -267,6 +269,7 @@ impl Arguments {
         .expect("DefinePropertyOrThrow must not fail per the spec");
 
         // 22. Return obj.
+
         obj
     }
 }

boa/src/builtins/function/mod.rs

@@ -16,6 +16,7 @@ use std::{
     borrow::Cow,
     fmt,
     ops::{Deref, DerefMut},
+    option::Option,
 };
 
 use dyn_clone::DynClone;
@@ -30,6 +31,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 +222,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 +247,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 +535,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().iter() {
+                            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/object/jsobject.rs

@@ -109,7 +109,6 @@ impl JsObject {
     pub fn equals(lhs: &Self, rhs: &Self) -> bool {
         std::ptr::eq(lhs.as_ref(), rhs.as_ref())
     }
-
     /// Converts an object to a primitive.
     ///
     /// Diverges from the spec to prevent a stack overflow when the object is recursive.

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

@@ -31,8 +31,8 @@ pub use self::{
     conditional::{ConditionalOp, If},
     declaration::{
         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,
@@ -419,49 +419,79 @@ 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 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         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(())
     }
 }
@@ -693,6 +723,7 @@ unsafe impl Trace for PropertyName {
 /// are using different indents in their source files. This fixes
 /// any strings which may have been changed in a different indent
 /// level.
+
 #[cfg(test)]
 fn test_formatting(source: &'static str) {
     // Remove preceding newline.