Allow Value.set_field to thow (#1329)

boa/src/builtins/array/mod.rs

@@ -155,12 +155,12 @@ impl Array {
 
         if !length.is_number() {
             array.set_property(0, DataDescriptor::new(length, Attribute::all()));
-            array.set_field("length", 1, context)?;
+            array.set_field("length", 1, true, context)?;
         } else {
             if length.is_double() {
                 return context.throw_range_error("Invalid array length");
             }
-            array.set_field("length", length.to_u32(context).unwrap(), context)?;
+            array.set_field("length", length.to_u32(context).unwrap(), true, context)?;
         }
 
         Ok(array)
@@ -325,6 +325,7 @@ impl Array {
         array_ptr.set_field(
             "length",
             Value::from(orig_length.wrapping_add(add_values.len())),
+            false,
             context,
         )?;
 
@@ -374,7 +375,7 @@ impl Array {
         }
 
         // set length
-        array.set_field("length", args.len(), context)?;
+        array.set_field("length", args.len(), true, context)?;
 
         Ok(array)
     }
@@ -454,7 +455,7 @@ impl Array {
         let pop_index = curr_length.wrapping_sub(1);
         let pop_value: Value = this.get_field(pop_index.to_string(), context)?;
         this.remove_property(pop_index);
-        this.set_field("length", Value::from(pop_index), context)?;
+        this.set_field("length", Value::from(pop_index), true, context)?;
         Ok(pop_value)
     }
 
@@ -616,7 +617,7 @@ impl Array {
         let len = this.get_field("length", context)?.to_length(context)?;
 
         if len == 0 {
-            this.set_field("length", 0, context)?;
+            this.set_field("length", 0, true, context)?;
             return Ok(Value::undefined());
         }
 
@@ -636,7 +637,7 @@ impl Array {
 
         let final_index = len.wrapping_sub(1);
         this.remove_property(final_index);
-        this.set_field("length", Value::from(final_index), context)?;
+        this.set_field("length", Value::from(final_index), true, context)?;
 
         Ok(first)
     }
@@ -682,7 +683,7 @@ impl Array {
         }
 
         let temp = len.wrapping_add(arg_c);
-        this.set_field("length", Value::from(temp), context)?;
+        this.set_field("length", Value::from(temp), true, context)?;
         Ok(Value::from(temp))
     }
 
@@ -1011,7 +1012,7 @@ impl Array {
             &Value::undefined(),
             &Value::undefined(),
         )?;
-        new_array.set_field("length", len.to_length(context)?, context)?;
+        new_array.set_field("length", len.to_length(context)?, false, context)?;
 
         Ok(new_array)
     }
@@ -1058,7 +1059,7 @@ impl Array {
             &mapper_function,
             &this_arg,
         )?;
-        new_array.set_field("length", len.to_length(context)?, context)?;
+        new_array.set_field("length", len.to_length(context)?, false, context)?;
 
         // 6. Return A
         Ok(new_array)
@@ -1271,7 +1272,7 @@ impl Array {
             );
             new_array_len = new_array_len.saturating_add(1);
         }
-        new_array.set_field("length", Value::from(new_array_len), context)?;
+        new_array.set_field("length", Value::from(new_array_len), true, context)?;
         Ok(new_array)
     }
 

boa/src/builtins/error/eval.rs

@@ -76,7 +76,7 @@ impl EvalError {
         let this = Value::from(obj);
         if let Some(message) = args.get(0) {
             if !message.is_undefined() {
-                this.set_field("message", message.to_string(context)?, context)?;
+                this.set_field("message", message.to_string(context)?, false, context)?;
             }
         }
 

boa/src/builtins/error/mod.rs

@@ -92,7 +92,7 @@ impl Error {
         let this = Value::from(obj);
         if let Some(message) = args.get(0) {
             if !message.is_undefined() {
-                this.set_field("message", message.to_string(context)?, context)?;
+                this.set_field("message", message.to_string(context)?, false, context)?;
             }
         }
 

boa/src/builtins/error/range.rs

@@ -73,7 +73,7 @@ impl RangeError {
         let this = Value::from(obj);
         if let Some(message) = args.get(0) {
             if !message.is_undefined() {
-                this.set_field("message", message.to_string(context)?, context)?;
+                this.set_field("message", message.to_string(context)?, false, context)?;
             }
         }
 

boa/src/builtins/error/reference.rs

@@ -72,7 +72,7 @@ impl ReferenceError {
         let this = Value::from(obj);
         if let Some(message) = args.get(0) {
             if !message.is_undefined() {
-                this.set_field("message", message.to_string(context)?, context)?;
+                this.set_field("message", message.to_string(context)?, false, context)?;
             }
         }
 

boa/src/builtins/error/syntax.rs

@@ -75,7 +75,7 @@ impl SyntaxError {
         let this = Value::from(obj);
         if let Some(message) = args.get(0) {
             if !message.is_undefined() {
-                this.set_field("message", message.to_string(context)?, context)?;
+                this.set_field("message", message.to_string(context)?, false, context)?;
             }
         }
 

boa/src/builtins/error/type.rs

@@ -78,7 +78,7 @@ impl TypeError {
         let this = Value::from(obj);
         if let Some(message) = args.get(0) {
             if !message.is_undefined() {
-                this.set_field("message", message.to_string(context)?, context)?;
+                this.set_field("message", message.to_string(context)?, false, context)?;
             }
         }
 

boa/src/builtins/error/uri.rs

@@ -74,7 +74,7 @@ impl UriError {
         let this = Value::from(obj);
         if let Some(message) = args.get(0) {
             if !message.is_undefined() {
-                this.set_field("message", message.to_string(context)?, context)?;
+                this.set_field("message", message.to_string(context)?, false, context)?;
             }
         }
 

boa/src/builtins/json/mod.rs

@@ -80,7 +80,7 @@ impl Json {
                 match args.get(1) {
                     Some(reviver) if reviver.is_function() => {
                         let mut holder = Value::object(Object::default());
-                        holder.set_field("", j, context)?;
+                        holder.set_field("", j, true, context)?;
                         Self::walk(reviver, context, &mut holder, &PropertyKey::from(""))
                     }
                     _ => Ok(j),
@@ -111,7 +111,7 @@ impl Json {
                 let v = Self::walk(reviver, context, &mut value.clone(), &key);
                 match v {
                     Ok(v) if !v.is_undefined() => {
-                        value.set_field(key, v, context)?;
+                        value.set_field(key, v, false, context)?;
                     }
                     Ok(_) => {
                         value.remove_property(key);

boa/src/builtins/reflect/mod.rs

@@ -308,7 +308,7 @@ impl Reflect {
 
         let keys = target.own_property_keys();
         for (i, k) in keys.iter().enumerate() {
-            result.set_field(i, k, context)?;
+            result.set_field(i, k, true, context)?;
         }
 
         Ok(result)

boa/src/builtins/regexp/mod.rs

@@ -536,7 +536,7 @@ impl RegExp {
             return context
                 .throw_type_error("RegExp.prototype.exec method called on incompatible value");
         };
-        this.set_field("lastIndex", Value::from(last_index), context)?;
+        this.set_field("lastIndex", Value::from(last_index), true, context)?;
         result
     }
 
@@ -616,7 +616,7 @@ impl RegExp {
             return context.throw_type_error("exec method called on incompatible value");
         };
 
-        this.set_field("lastIndex", Value::from(last_index), context)?;
+        this.set_field("lastIndex", Value::from(last_index), true, context)?;
         result
     }
 
@@ -742,7 +742,7 @@ impl RegExp {
 
         let length = matches.len();
         let result = Value::from(matches);
-        result.set_field("length", Value::from(length), context)?;
+        result.set_field("length", Value::from(length), false, context)?;
         result.set_data(ObjectData::Array);
 
         Ok(result)
@@ -780,7 +780,7 @@ impl RegExp {
         // 5. If SameValue(previousLastIndex, +0𝔽) is false, then
         if previous_last_index != 0 {
             // a. Perform ? Set(rx, "lastIndex", +0𝔽, true).
-            this.set_field("lastIndex", 0, context)?;
+            this.set_field("lastIndex", 0, true, context)?;
         }
 
         // 6. Let result be ? RegExpExec(rx, S).
@@ -792,7 +792,7 @@ impl RegExp {
         // 8. If SameValue(currentLastIndex, previousLastIndex) is false, then
         if current_last_index != previous_last_index {
             // a. Perform ? Set(rx, "lastIndex", previousLastIndex, true).
-            this.set_field("lastIndex", previous_last_index, context)?;
+            this.set_field("lastIndex", previous_last_index, true, context)?;
         }
 
         // 9. If result is null, return -1𝔽.

boa/src/context.rs

@@ -495,10 +495,10 @@ impl Context {
         let val = Value::from(new_func);
 
         // Set constructor field to the newly created Value (function object)
-        proto.set_field("constructor", val.clone(), self)?;
+        proto.set_field("constructor", val.clone(), false, self)?;
 
-        val.set_field(PROTOTYPE, proto, self)?;
-        val.set_field("length", Value::from(params_len), self)?;
+        val.set_field(PROTOTYPE, proto, false, self)?;
+        val.set_field("length", Value::from(params_len), false, self)?;
 
         Ok(val)
     }
@@ -557,11 +557,14 @@ impl Context {
             Node::GetConstField(ref get_const_field_node) => Ok(get_const_field_node
                 .obj()
                 .run(self)?
-                .set_field(get_const_field_node.field(), value, self)?),
+                .set_field(get_const_field_node.field(), value, false, self)?),
             Node::GetField(ref get_field) => {
                 let field = get_field.field().run(self)?;
                 let key = field.to_property_key(self)?;
-                Ok(get_field.obj().run(self)?.set_field(key, value, self)?)
+                Ok(get_field
+                    .obj()
+                    .run(self)?
+                    .set_field(key, value, false, self)?)
             }
             _ => self.throw_type_error(format!("invalid assignment to {}", node)),
         }

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

@@ -95,7 +95,7 @@ impl Executable for FunctionDecl {
         )?;
 
         // Set the name and assign it in the current environment
-        val.set_field("name", self.name(), context)?;
+        val.set_field("name", self.name(), false, context)?;
 
         if context.has_binding(self.name()) {
             context.set_mutable_binding(self.name(), val, true)?;

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

@@ -106,7 +106,7 @@ impl Executable for FunctionExpr {
         )?;
 
         if let Some(name) = self.name() {
-            val.set_field("name", Value::from(name), context)?;
+            val.set_field("name", Value::from(name), false, context)?;
         }
 
         Ok(val)

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

@@ -72,13 +72,13 @@ impl Executable for Assign {
             }
             Node::GetConstField(ref get_const_field) => {
                 let val_obj = get_const_field.obj().run(context)?;
-                val_obj.set_field(get_const_field.field(), val.clone(), context)?;
+                val_obj.set_field(get_const_field.field(), val.clone(), false, context)?;
             }
             Node::GetField(ref get_field) => {
                 let object = get_field.obj().run(context)?;
                 let field = get_field.field().run(context)?;
                 let key = field.to_property_key(context)?;
-                object.set_field(key, val.clone(), context)?;
+                object.set_field(key, val.clone(), false, context)?;
             }
             _ => (),
         }

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

@@ -216,7 +216,7 @@ impl Executable for BinOp {
                     let v_r_a = get_const_field.obj().run(context)?;
                     let v_a = v_r_a.get_field(get_const_field.field(), context)?;
                     let value = Self::run_assign(op, v_a, self.rhs(), context)?;
-                    v_r_a.set_field(get_const_field.field(), value.clone(), context)?;
+                    v_r_a.set_field(get_const_field.field(), value.clone(), false, context)?;
                     Ok(value)
                 }
                 _ => Ok(Value::undefined()),

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

@@ -97,17 +97,17 @@ impl Executable for TaggedTemplate {
         let raw_array = Array::new_array(context);
 
         for (i, raw) in self.raws.iter().enumerate() {
-            raw_array.set_field(i, Value::from(raw), context)?;
+            raw_array.set_field(i, Value::from(raw), false, context)?;
         }
 
         for (i, cooked) in self.cookeds.iter().enumerate() {
             if let Some(cooked) = cooked {
-                template_object.set_field(i, Value::from(cooked), context)?;
+                template_object.set_field(i, Value::from(cooked), false, context)?;
             } else {
-                template_object.set_field(i, Value::undefined(), context)?;
+                template_object.set_field(i, Value::undefined(), false, context)?;
             }
         }
-        template_object.set_field("raw", raw_array, context)?;
+        template_object.set_field("raw", raw_array, false, context)?;
 
         let (this, func) = match *self.tag {
             Node::GetConstField(ref get_const_field) => {

boa/src/value/mod.rs

@@ -472,18 +472,47 @@ impl Value {
     }
 
     /// Set the field in the value
+    ///
+    /// Similar to `7.3.4 Set ( O, P, V, Throw )`, but returns the value instead of a boolean.
+    ///
+    /// More information:
+    ///  - [ECMAScript][spec]
+    ///
+    /// [spec]: https://tc39.es/ecma262/#sec-set-o-p-v-throw
     #[inline]
-    pub fn set_field<K, V>(&self, key: K, value: V, context: &mut Context) -> Result<Value>
+    pub fn set_field<K, V>(
+        &self,
+        key: K,
+        value: V,
+        throw: bool,
+        context: &mut Context,
+    ) -> Result<Value>
     where
         K: Into<PropertyKey>,
         V: Into<Value>,
     {
+        // 1. Assert: Type(O) is Object.
+        // TODO: Currently the value may not be an object.
+        //       In that case this function does nothing.
+        // 2. Assert: IsPropertyKey(P) is true.
+        // 3. Assert: Type(Throw) is Boolean.
+
         let key = key.into();
         let value = value.into();
         let _timer = BoaProfiler::global().start_event("Value::set_field", "value");
         if let Self::Object(ref obj) = *self {
-            obj.clone()
+            // 4. Let success be ? O.[[Set]](P, V, O).
+            let success = obj
+                .clone()
                 .set(key, value.clone(), obj.clone().into(), context)?;
+
+            // 5. If success is false and Throw is true, throw a TypeError exception.
+            // 6. Return success.
+            if !success && throw {
+                return Err(context.construct_type_error("Cannot assign value to property"));
+            } else {
+                return Ok(value);
+            }
         }
         Ok(value)
     }

boa/src/value/tests.rs

@@ -34,7 +34,7 @@ fn get_set_field() {
     let obj = Value::new_object(&context);
     // Create string and convert it to a Value
     let s = Value::from("bar");
-    obj.set_field("foo", s, &mut context).unwrap();
+    obj.set_field("foo", s, false, &mut context).unwrap();
     assert_eq!(
         obj.get_field("foo", &mut context)
             .unwrap()