Add default parameter support (#1273)

* Add default parameter support

* Update second environment

* Use map instead of if let, remove empty line

* Inline call and construct methods

Co-authored-by: Iban Eguia <razican@protonmail.ch>

* Handle cases where arguments object is not needed

* Reword arguments object comment

Co-authored-by: Iban Eguia <razican@protonmail.ch>

boa/src/builtins/reflect/mod.rs

@@ -125,7 +125,7 @@ impl Reflect {
         };
 
         let args = args_list.create_list_from_array_like(&[], context)?;
-        target.construct(&args, new_target, context)
+        target.construct(&args, &new_target, context)
     }
 
     /// Defines a property on an object.

boa/src/object/gcobject.rs

@@ -105,121 +105,44 @@ impl GcObject {
         std::ptr::eq(lhs.as_ref(), rhs.as_ref())
     }
 
-    /// Call this object.
+    /// Internal implementation of [`call`](#method.call) and [`construct`](#method.construct).
     ///
     /// # Panics
     ///
     /// Panics if the object is currently mutably borrowed.
-    // <https://tc39.es/ecma262/#sec-prepareforordinarycall>
-    // <https://tc39.es/ecma262/#sec-ecmascript-function-objects-call-thisargument-argumentslist>
-    #[track_caller]
-    pub fn call(&self, this: &Value, args: &[Value], context: &mut Context) -> Result<Value> {
-        let this_function_object = self.clone();
-        let f_body = if let Some(function) = self.borrow().as_function() {
-            if function.is_callable() {
-                match function {
-                    Function::BuiltIn(BuiltInFunction(function), flags) => {
-                        if flags.is_constructable() {
-                            FunctionBody::BuiltInConstructor(*function)
-                        } else {
-                            FunctionBody::BuiltInFunction(*function)
-                        }
-                    }
-                    Function::Ordinary {
-                        body,
-                        params,
-                        environment,
-                        flags,
-                    } => {
-                        // Create a new Function environment whose parent is set to the scope of the function declaration (self.environment)
-                        // <https://tc39.es/ecma262/#sec-prepareforordinarycall>
-                        let local_env = FunctionEnvironmentRecord::new(
-                            this_function_object,
-                            if flags.is_lexical_this_mode() {
-                                None
-                            } else {
-                                Some(this.clone())
-                            },
-                            Some(environment.clone()),
-                            // Arrow functions do not have a this binding https://tc39.es/ecma262/#sec-function-environment-records
-                            if flags.is_lexical_this_mode() {
-                                BindingStatus::Lexical
-                            } else {
-                                BindingStatus::Uninitialized
-                            },
-                            Value::undefined(),
-                        );
-
-                        // Add argument bindings to the function environment
-                        for (i, param) in params.iter().enumerate() {
-                            // Rest Parameters
-                            if param.is_rest_param() {
-                                function.add_rest_param(param, i, args, context, &local_env);
-                                break;
-                            }
-
-                            let value = args.get(i).cloned().unwrap_or_else(Value::undefined);
-                            function
-                                .add_arguments_to_environment(param, value, &local_env, context);
-                        }
-
-                        // Add arguments object
-                        let arguments_obj = create_unmapped_arguments_object(args);
-                        local_env.borrow_mut().create_mutable_binding(
-                            "arguments".to_string(),
-                            false,
-                            true,
-                            context,
-                        )?;
-                        local_env.borrow_mut().initialize_binding(
-                            "arguments",
-                            arguments_obj,
-                            context,
-                        )?;
-
-                        context.push_environment(local_env);
-
-                        FunctionBody::Ordinary(body.clone())
-                    }
-                }
-            } else {
-                return context.throw_type_error("function object is not callable");
-            }
-        } else {
-            return context.throw_type_error("not a function");
-        };
-
-        match f_body {
-            FunctionBody::BuiltInFunction(func) => func(this, args, context),
-            FunctionBody::BuiltInConstructor(func) => func(&Value::undefined(), args, context),
-            FunctionBody::Ordinary(body) => {
-                let result = body.run(context);
-                context.pop_environment();
-
-                result
-            }
-        }
-    }
-
-    /// Construct an instance of this object with the specified arguments.
-    ///
-    /// # Panics
     ///
-    /// Panics if the object is currently mutably borrowed.
+    /// <https://tc39.es/ecma262/#sec-prepareforordinarycall>
-    // <https://tc39.es/ecma262/#sec-ecmascript-function-objects-construct-argumentslist-newtarget>
+    /// <https://tc39.es/ecma262/#sec-ordinarycallbindthis>
+    /// <https://tc39.es/ecma262/#sec-runtime-semantics-evaluatebody>
+    /// <https://tc39.es/ecma262/#sec-ordinarycallevaluatebody>
     #[track_caller]
-    pub fn construct(
+    fn call_construct(
         &self,
+        this_target: &Value,
         args: &[Value],
-        new_target: Value,
         context: &mut Context,
+        construct: bool,
     ) -> Result<Value> {
         let this_function_object = self.clone();
+        let mut has_parameter_expressions = false;
+
         let body = if let Some(function) = self.borrow().as_function() {
-            if function.is_constructable() {
+            if construct && !function.is_constructable() {
+                let name = self
+                    .get(&"name".into(), self.clone().into(), context)?
+                    .display()
+                    .to_string();
+                return context.throw_type_error(format!("{} is not a constructor", name));
+            } else if !construct && !function.is_callable() {
+                return context.throw_type_error("function object is not callable");
+            } else {
                 match function {
-                    Function::BuiltIn(BuiltInFunction(function), _) => {
+                    Function::BuiltIn(BuiltInFunction(function), flags) => {
+                        if flags.is_constructable() || construct {
                             FunctionBody::BuiltInConstructor(*function)
+                        } else {
+                            FunctionBody::BuiltInFunction(*function)
+                        }
                     }
                     Function::Ordinary {
                         body,
@@ -227,13 +150,14 @@ impl GcObject {
                         environment,
                         flags,
                     } => {
+                        let this = if construct {
                             // If the prototype of the constructor is not an object, then use the default object
                             // prototype as prototype for the new object
                             // see <https://tc39.es/ecma262/#sec-ordinarycreatefromconstructor>
                             // see <https://tc39.es/ecma262/#sec-getprototypefromconstructor>
-                        let proto = new_target.as_object().unwrap().get(
+                            let proto = this_target.as_object().unwrap().get(
                                 &PROTOTYPE.into(),
-                            new_target.clone(),
+                                this_target.clone(),
                                 context,
                             )?;
                             let proto = if proto.is_object() {
@@ -245,13 +169,20 @@ impl GcObject {
                                     .prototype()
                                     .into()
                             };
-                        let this = Value::from(Object::create(proto));
+                            Value::from(Object::create(proto))
+                        } else {
+                            this_target.clone()
+                        };
 
-                        // Create a new Function environment who's parent is set to the scope of the function declaration (self.environment)
+                        // Create a new Function environment whose parent is set to the scope of the function declaration (self.environment)
                         // <https://tc39.es/ecma262/#sec-prepareforordinarycall>
                         let local_env = FunctionEnvironmentRecord::new(
-                            this_function_object,
+                            this_function_object.clone(),
-                            Some(this),
+                            if construct || !flags.is_lexical_this_mode() {
+                                Some(this.clone())
+                            } else {
+                                None
+                            },
                             Some(environment.clone()),
                             // Arrow functions do not have a this binding https://tc39.es/ecma262/#sec-function-environment-records
                             if flags.is_lexical_this_mode() {
@@ -259,22 +190,30 @@ impl GcObject {
                             } else {
                                 BindingStatus::Uninitialized
                             },
-                            new_target.clone(),
+                            Value::undefined(),
                         );
 
-                        // Add argument bindings to the function environment
+                        let mut arguments_in_parameter_names = false;
-                        for (i, param) in params.iter().enumerate() {
-                            // Rest Parameters
-                            if param.is_rest_param() {
-                                function.add_rest_param(param, i, args, context, &local_env);
-                                break;
-                            }
 
-                            let value = args.get(i).cloned().unwrap_or_else(Value::undefined);
+                        for param in params.iter() {
-                            function
+                            has_parameter_expressions =
-                                .add_arguments_to_environment(param, value, &local_env, context);
+                                has_parameter_expressions || param.init().is_some();
+                            arguments_in_parameter_names =
+                                arguments_in_parameter_names || param.name() == "arguments";
                         }
 
+                        // An arguments object is added when all of the following conditions are met
+                        // - If not in an arrow function (10.2.11.16)
+                        // - If the parameter list does not contain `arguments` (10.2.11.17)
+                        // - If there are default parameters or if lexical names and function names do not contain `arguments` (10.2.11.18)
+                        //
+                        // https://tc39.es/ecma262/#sec-functiondeclarationinstantiation
+                        if !flags.is_lexical_this_mode()
+                            && !arguments_in_parameter_names
+                            && (has_parameter_expressions
+                                || (!body.lexically_declared_names().contains("arguments")
+                                    && !body.function_declared_names().contains("arguments")))
+                        {
                             // Add arguments object
                             let arguments_obj = create_unmapped_arguments_object(args);
                             local_env.borrow_mut().create_mutable_binding(
@@ -288,35 +227,118 @@ impl GcObject {
                                 arguments_obj,
                                 context,
                             )?;
-                        context.push_environment(local_env);
+                        }
+                        // push the environment first so that it will be used by default parameters
+                        context.push_environment(local_env.clone());
 
-                        FunctionBody::Ordinary(body.clone())
+                        // Add argument bindings to the function environment
+                        for (i, param) in params.iter().enumerate() {
+                            // Rest Parameters
+                            if param.is_rest_param() {
+                                function.add_rest_param(param, i, args, context, &local_env);
+                                break;
                             }
+
+                            let value = match args.get(i).cloned() {
+                                None | Some(Value::Undefined) => param
+                                    .init()
+                                    .map(|init| init.run(context).ok())
+                                    .flatten()
+                                    .unwrap_or_default(),
+                                Some(value) => value,
+                            };
+
+                            function
+                                .add_arguments_to_environment(param, value, &local_env, context);
                         }
+
+                        if has_parameter_expressions {
+                            // Create a second environment when default parameter expressions are used
+                            // This prevents variables declared in the function body from being
+                            // used in default parameter initializers.
+                            // https://tc39.es/ecma262/#sec-functiondeclarationinstantiation
+                            let second_env = FunctionEnvironmentRecord::new(
+                                this_function_object,
+                                if construct || !flags.is_lexical_this_mode() {
+                                    Some(this)
                                 } else {
-                let name = self
+                                    None
-                    .get(&"name".into(), self.clone().into(), context)?
+                                },
-                    .display()
+                                Some(local_env),
-                    .to_string();
+                                // Arrow functions do not have a this binding https://tc39.es/ecma262/#sec-function-environment-records
-                return context.throw_type_error(format!("{} is not a constructor", name));
+                                if flags.is_lexical_this_mode() {
+                                    BindingStatus::Lexical
+                                } else {
+                                    BindingStatus::Uninitialized
+                                },
+                                Value::undefined(),
+                            );
+                            context.push_environment(second_env);
+                        }
+
+                        FunctionBody::Ordinary(body.clone())
+                    }
+                }
             }
         } else {
             return context.throw_type_error("not a function");
         };
 
         match body {
-            FunctionBody::BuiltInConstructor(function) => function(&new_target, args, context),
+            FunctionBody::BuiltInConstructor(function) if construct => {
+                function(&this_target, args, context)
+            }
+            FunctionBody::BuiltInConstructor(function) => {
+                function(&Value::undefined(), args, context)
+            }
+            FunctionBody::BuiltInFunction(function) => function(this_target, args, context),
             FunctionBody::Ordinary(body) => {
-                let _ = body.run(context);
+                let result = body.run(context);
+                let this = context.get_this_binding();
 
-                // local_env gets dropped here, its no longer needed
+                if has_parameter_expressions {
-                let result = context.get_this_binding();
+                    context.pop_environment();
+                }
                 context.pop_environment();
+
+                if construct {
+                    this
+                } else {
                     result
                 }
-            FunctionBody::BuiltInFunction(_) => unreachable!("Cannot have a function in construct"),
             }
         }
+    }
+
+    /// Call this object.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the object is currently mutably borrowed.
+    // <https://tc39.es/ecma262/#sec-prepareforordinarycall>
+    // <https://tc39.es/ecma262/#sec-ecmascript-function-objects-call-thisargument-argumentslist>
+    #[track_caller]
+    #[inline]
+    pub fn call(&self, this: &Value, args: &[Value], context: &mut Context) -> Result<Value> {
+        self.call_construct(this, args, context, false)
+    }
+
+    /// Construct an instance of this object with the specified arguments.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the object is currently mutably borrowed.
+    // <https://tc39.es/ecma262/#sec-ecmascript-function-objects-construct-argumentslist-newtarget>
+    #[track_caller]
+    #[inline]
+    pub fn construct(
+        &self,
+        args: &[Value],
+        new_target: &Value,
+        context: &mut Context,
+    ) -> Result<Value> {
+        self.call_construct(new_target, args, context, true)
+    }
 
     /// Converts an object to a primitive.
     ///

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

@@ -69,7 +69,7 @@ impl Executable for New {
         }
 
         match func_object {
-            Value::Object(ref object) => object.construct(&v_args, object.clone().into(), context),
+            Value::Object(ref object) => object.construct(&v_args, &object.clone().into(), context),
             _ => context
                 .throw_type_error(format!("{} is not a constructor", self.expr().to_string(),)),
         }

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

@@ -72,6 +72,16 @@ impl StatementList {
         set
     }
 
+    pub fn function_declared_names(&self) -> HashSet<&str> {
+        let mut set = HashSet::new();
+        for stmt in self.items() {
+            if let Node::FunctionDecl(decl) = stmt {
+                set.insert(decl.name());
+            }
+        }
+        set
+    }
+
     pub fn var_declared_names(&self) -> HashSet<&str> {
         let mut set = HashSet::new();
         for stmt in self.items() {