Refactor PropertyDescriptor

- Add DataDescriptor
- Add AccessorDescriptor
- Remove PropertyDescriptor::data_descriptor()
- Make AccessorDescriptor accessors GcObjects
- Make Object::get_own_property return a Option<PropertyDescriptor>
- Remove PropertyDescriptor::is_none()
- Remove PropertyDescriptor::empty()
- Removed PropertyDescriptor::value()
- Added spec complaint ToPropertyDescriptor (to_property_descriptor)
- Remove From<&PropertyDescriptor> for Value

boa/src/builtins/array/array_iterator.rs

@@ -1,7 +1,7 @@
 use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object, Array, Value},
     object::ObjectData,
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result,
 };
 use gc::{Finalize, Trace};
@@ -131,8 +131,7 @@ impl ArrayIterator {
             .set_prototype_instance(iterator_prototype);
 
         let to_string_tag = ctx.well_known_symbols().to_string_tag_symbol();
-        let to_string_tag_property =
-            Property::data_descriptor(Value::string("Array Iterator"), Attribute::CONFIGURABLE);
+        let to_string_tag_property = DataDescriptor::new("Array Iterator", Attribute::CONFIGURABLE);
         array_iterator.set_property(to_string_tag, to_string_tag_property);
         array_iterator
     }

boa/src/builtins/array/mod.rs

@@ -17,7 +17,7 @@ use crate::{
     builtins::array::array_iterator::{ArrayIterationKind, ArrayIterator},
     builtins::BuiltIn,
     object::{ConstructorBuilder, FunctionBuilder, ObjectData, PROTOTYPE},
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     value::{same_value_zero, Value},
     BoaProfiler, Context, Result,
 };
@@ -129,8 +129,8 @@ impl Array {
         }
 
         // finally create length property
-        let length = Property::data_descriptor(
-            length.into(),
+        let length = DataDescriptor::new(
+            length,
             Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
         );
 
@@ -171,8 +171,8 @@ impl Array {
         }
 
         // Create length
-        let length = Property::data_descriptor(
-            array_contents.len().into(),
+        let length = DataDescriptor::new(
+            array_contents.len(),
             Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
         );
         array_obj_ptr.set_property("length".to_string(), length);

boa/src/builtins/function/mod.rs

@@ -15,7 +15,7 @@ use crate::{
     builtins::{Array, BuiltIn},
     environment::lexical_environment::Environment,
     object::{ConstructorBuilder, FunctionBuilder, Object, ObjectData, PROTOTYPE},
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     syntax::ast::node::{FormalParameter, RcStatementList},
     BoaProfiler, Context, Result, Value,
 };
@@ -174,16 +174,16 @@ pub fn create_unmapped_arguments_object(arguments_list: &[Value]) -> Value {
     let len = arguments_list.len();
     let mut obj = Object::default();
     // Set length
-    let length = Property::data_descriptor(
-        len.into(),
+    let length = DataDescriptor::new(
+        len,
         Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
     );
     // Define length as a property
-    obj.define_own_property("length", length);
+    obj.define_own_property("length", length.into());
     let mut index: usize = 0;
     while index < len {
         let val = arguments_list.get(index).expect("Could not get argument");
-        let prop = Property::data_descriptor(
+        let prop = DataDescriptor::new(
             val.clone(),
             Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE,
         );
@@ -222,14 +222,14 @@ pub fn make_constructor_fn(
     let mut constructor =
         Object::function(function, global.get_field("Function").get_field(PROTOTYPE));
 
-    let length = Property::data_descriptor(
-        length.into(),
+    let length = DataDescriptor::new(
+        length,
         Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
     );
     constructor.insert("length", length);
 
-    let name = Property::data_descriptor(
-        name.into(),
+    let name = DataDescriptor::new(
+        name,
         Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
     );
     constructor.insert("name", name);

boa/src/builtins/iterable/mod.rs

@@ -2,7 +2,7 @@ use crate::{
     builtins::string::string_iterator::StringIterator,
     builtins::ArrayIterator,
     object::{GcObject, ObjectInitializer},
-    property::Property,
+    property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result, Value,
 };
 
@@ -47,8 +47,9 @@ impl IteratorPrototypes {
 /// Generates an object supporting the IteratorResult interface.
 pub fn create_iter_result_object(ctx: &mut Context, value: Value, done: bool) -> Value {
     let object = Value::new_object(Some(ctx.global_object()));
-    let value_property = Property::default().value(value);
-    let done_property = Property::default().value(Value::boolean(done));
+    // TODO: Fix attributes of value and done
+    let value_property = DataDescriptor::new(value, Attribute::all());
+    let done_property = DataDescriptor::new(done, Attribute::all());
     object.set_property("value", value_property);
     object.set_property("done", done_property);
     object

boa/src/builtins/json/mod.rs

@@ -16,7 +16,7 @@
 use crate::{
     builtins::BuiltIn,
     object::ObjectInitializer,
-    property::{Attribute, Property, PropertyKey},
+    property::{Attribute, DataDescriptor, PropertyKey},
     BoaProfiler, Context, Result, Value,
 };
 use serde_json::{self, Value as JSONValue};
@@ -160,11 +160,14 @@ impl Json {
                         let this_arg = object.clone();
                         object_to_return.set_property(
                             key.to_owned(),
-                            Property::default().value(ctx.call(
-                                replacer,
-                                &this_arg,
-                                &[Value::from(key.clone()), val.clone()],
-                            )?),
+                            DataDescriptor::new(
+                                ctx.call(
+                                    replacer,
+                                    &this_arg,
+                                    &[Value::from(key.clone()), val.clone()],
+                                )?,
+                                Attribute::all(),
+                            ),
                         );
                     }
                     Ok(Value::from(object_to_return.to_json(ctx)?.to_string()))

boa/src/builtins/map/mod.rs

@@ -3,7 +3,7 @@
 use crate::{
     builtins::BuiltIn,
     object::{ConstructorBuilder, ObjectData, PROTOTYPE},
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result, Value,
 };
 use ordered_map::OrderedMap;
@@ -100,8 +100,8 @@ impl Map {
 
     /// Helper function to set the size property.
     fn set_size(this: &Value, size: usize) {
-        let size = Property::data_descriptor(
-            size.into(),
+        let size = DataDescriptor::new(
+            size,
             Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
         );
 

boa/src/builtins/mod.rs

@@ -43,7 +43,7 @@ pub(crate) use self::{
     undefined::Undefined,
 };
 use crate::{
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     Context, Value,
 };
 
@@ -96,7 +96,7 @@ pub fn init(context: &mut Context) {
 
     for init in &globals {
         let (name, value, attribute) = init(context);
-        let property = Property::data_descriptor(value, attribute);
+        let property = DataDescriptor::new(value, attribute);
         global_object.borrow_mut().insert(name, property);
     }
 }

boa/src/builtins/object/mod.rs

@@ -16,7 +16,7 @@
 use crate::{
     builtins::BuiltIn,
     object::{ConstructorBuilder, Object as BuiltinObject, ObjectData},
-    property::{Attribute, Property},
+    property::Attribute,
     value::{same_value, Value},
     BoaProfiler, Context, Result,
 };
@@ -134,13 +134,18 @@ impl Object {
     }
 
     /// Define a property in an object
-    pub fn define_property(_: &Value, args: &[Value], ctx: &mut Context) -> Result<Value> {
+    pub fn define_property(_: &Value, args: &[Value], context: &mut Context) -> Result<Value> {
         let obj = args.get(0).expect("Cannot get object");
         let prop = args
             .get(1)
             .expect("Cannot get object")
-            .to_property_key(ctx)?;
-        let desc = Property::from(args.get(2).expect("Cannot get object"));
+            .to_property_key(context)?;
+
+        let desc = if let Value::Object(ref object) = args.get(2).cloned().unwrap_or_default() {
+            object.to_property_descriptor(context)?
+        } else {
+            return context.throw_type_error("Property description must be an object");
+        };
         obj.set_property(prop, desc);
         Ok(Value::undefined())
     }
@@ -246,12 +251,10 @@ impl Object {
         };
 
         let key = key.to_property_key(ctx)?;
-        let own_property = this
-            .to_object(ctx)
-            .map(|obj| obj.borrow().get_own_property(&key));
+        let own_property = this.to_object(ctx)?.borrow().get_own_property(&key);
 
         Ok(own_property.map_or(Value::from(false), |own_prop| {
-            Value::from(own_prop.enumerable_or(false))
+            Value::from(own_prop.enumerable())
         }))
     }
 }

boa/src/builtins/regexp/mod.rs

@@ -13,7 +13,7 @@ use crate::{
     builtins::BuiltIn,
     gc::{empty_trace, Finalize, Trace},
     object::{ConstructorBuilder, ObjectData},
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     value::{RcString, Value},
     BoaProfiler, Context, Result,
 };
@@ -370,9 +370,9 @@ impl RegExp {
                     let result = Value::from(result);
                     result.set_property(
                         "index",
-                        Property::default().value(Value::from(m.total().start)),
+                        DataDescriptor::new(m.total().start, Attribute::all()),
                     );
-                    result.set_property("input", Property::default().value(Value::from(arg_str)));
+                    result.set_property("input", DataDescriptor::new(arg_str, Attribute::all()));
                     result
                 } else {
                     if regex.use_last_index {
@@ -473,11 +473,11 @@ impl RegExp {
 
                 match_val.set_property(
                     "index",
-                    Property::default().value(Value::from(mat.total().start)),
+                    DataDescriptor::new(mat.total().start, Attribute::all()),
                 );
                 match_val.set_property(
                     "input",
-                    Property::default().value(Value::from(arg_str.clone())),
+                    DataDescriptor::new(arg_str.clone(), Attribute::all()),
                 );
                 matches.push(match_val);
 

boa/src/builtins/string/string_iterator.rs

@@ -2,7 +2,7 @@ use crate::builtins::string::code_point_at;
 use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object},
     object::ObjectData,
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     BoaProfiler, Context, Result, Value,
 };
 use gc::{Finalize, Trace};
@@ -82,7 +82,7 @@ impl StringIterator {
 
         let to_string_tag = ctx.well_known_symbols().to_string_tag_symbol();
         let to_string_tag_property =
-            Property::data_descriptor(Value::string("String Iterator"), Attribute::CONFIGURABLE);
+            DataDescriptor::new("String Iterator", Attribute::CONFIGURABLE);
         array_iterator.set_property(to_string_tag, to_string_tag_property);
         array_iterator
     }

boa/src/context.rs

@@ -10,7 +10,7 @@ use crate::{
     class::{Class, ClassBuilder},
     exec::Interpreter,
     object::{GcObject, Object, ObjectData, PROTOTYPE},
-    property::{Property, PropertyKey},
+    property::{DataDescriptor, PropertyKey},
     realm::Realm,
     syntax::{
         ast::{
@@ -608,7 +608,7 @@ impl Context {
         T::init(&mut class_builder)?;
 
         let class = class_builder.build();
-        let property = Property::data_descriptor(class.into(), T::ATTRIBUTE);
+        let property = DataDescriptor::new(class, T::ATTRIBUTE);
         self.global_object()
             .as_object_mut()
             .unwrap()

boa/src/environment/global_environment_record.rs

@@ -14,7 +14,7 @@ use crate::{
         lexical_environment::{Environment, EnvironmentType},
         object_environment_record::ObjectEnvironmentRecord,
     },
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     Value,
 };
 use gc::{Finalize, Trace};
@@ -71,16 +71,16 @@ impl GlobalEnvironmentRecord {
         let global_object = &mut self.object_record.bindings;
         let existing_prop = global_object.get_property(name);
         if let Some(prop) = existing_prop {
-            if prop.value.is_none() || prop.configurable_or(false) {
+            if prop.value.is_none() || prop.configurable() {
                 let mut property =
-                    Property::data_descriptor(value, Attribute::WRITABLE | Attribute::ENUMERABLE);
+                    DataDescriptor::new(value, Attribute::WRITABLE | Attribute::ENUMERABLE);
                 property.set_configurable(deletion);
 
                 global_object.update_property(name, property);
             }
         } else {
             let mut property =
-                Property::data_descriptor(value, Attribute::WRITABLE | Attribute::ENUMERABLE);
+                DataDescriptor::new(value, Attribute::WRITABLE | Attribute::ENUMERABLE);
             property.set_configurable(deletion);
 
             global_object.update_property(name, property);

boa/src/environment/object_environment_record.rs

@@ -11,7 +11,7 @@ use crate::{
         environment_record_trait::EnvironmentRecordTrait,
         lexical_environment::{Environment, EnvironmentType},
     },
-    property::{Attribute, Property},
+    property::{Attribute, DataDescriptor},
     Value,
 };
 use gc::{Finalize, Trace};
@@ -39,7 +39,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord {
         // TODO: could save time here and not bother generating a new undefined object,
         // only for it to be replace with the real value later. We could just add the name to a Vector instead
         let bindings = &mut self.bindings;
-        let mut prop = Property::data_descriptor(
+        let mut prop = DataDescriptor::new(
             Value::undefined(),
             Attribute::WRITABLE | Attribute::ENUMERABLE,
         );
@@ -63,7 +63,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord {
     fn set_mutable_binding(&mut self, name: &str, value: Value, strict: bool) {
         debug_assert!(value.is_object() || value.is_function());
 
-        let mut property = Property::data_descriptor(value, Attribute::ENUMERABLE);
+        let mut property = DataDescriptor::new(value, Attribute::ENUMERABLE);
         property.set_configurable(strict);
         self.bindings.update_property(name, property);
     }