Refactor PropertyDescriptor to follow the spec more closely

boa/src/builtins/array/array_iterator.rs

@@ -2,7 +2,7 @@ use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object, Array, Value},
     gc::{Finalize, Trace},
     object::{GcObject, ObjectData},
-    property::{Attribute, DataDescriptor},
+    property::PropertyDescriptor,
     symbol::WellKnownSymbols,
     BoaProfiler, Context, Result,
 };
@@ -128,10 +128,11 @@ impl ArrayIterator {
         array_iterator.set_prototype_instance(iterator_prototype);
 
         let to_string_tag = WellKnownSymbols::to_string_tag();
-        let to_string_tag_property = DataDescriptor::new(
-            "Array Iterator",
-            Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE,
-        );
+        let to_string_tag_property = PropertyDescriptor::builder()
+            .value("Array Iterator")
+            .writable(false)
+            .enumerable(false)
+            .configurable(true);
         array_iterator.insert(to_string_tag, to_string_tag_property);
         array_iterator
     }

boa/src/builtins/array/mod.rs

@@ -18,7 +18,7 @@ use crate::{
     builtins::BuiltIn,
     builtins::Number,
     object::{ConstructorBuilder, FunctionBuilder, GcObject, ObjectData, PROTOTYPE},
-    property::{Attribute, DataDescriptor},
+    property::{Attribute, PropertyDescriptor},
     symbol::WellKnownSymbols,
     value::{IntegerOrInfinity, Value},
     BoaProfiler, Context, JsString, Result,
@@ -229,11 +229,16 @@ impl Array {
         array.borrow_mut().data = ObjectData::Array;
 
         // 6. Perform ! OrdinaryDefineOwnProperty(A, "length", PropertyDescriptor { [[Value]]: 𝔽(length), [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: false }).
-        let length = DataDescriptor::new(
-            length as f64,
-            Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
+
+        array.ordinary_define_own_property(
+            "length".into(),
+            PropertyDescriptor::builder()
+                .value(length as f64)
+                .writable(true)
+                .enumerable(false)
+                .configurable(false)
+                .build(),
         );
-        array.ordinary_define_own_property("length".into(), length.into());
 
         Ok(array)
     }
@@ -246,11 +251,15 @@ impl Array {
             .as_object()
             .expect("'array' should be an object")
             .set_prototype_instance(context.standard_objects().array_object().prototype().into());
-        let length = DataDescriptor::new(
-            Value::from(0),
-            Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
+        array.set_property(
+            "length",
+            PropertyDescriptor::builder()
+                .value(0)
+                .writable(true)
+                .enumerable(false)
+                .configurable(false)
+                .build(),
         );
-        array.set_property("length", length);
         array
     }
 
@@ -272,14 +281,25 @@ impl Array {
         }
 
         // Create length
-        let length = DataDescriptor::new(
-            array_contents.len(),
-            Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
+        array_obj_ptr.set_property(
+            "length".to_string(),
+            PropertyDescriptor::builder()
+                .value(array_contents.len())
+                .writable(true)
+                .enumerable(false)
+                .configurable(false)
+                .build(),
         );
-        array_obj_ptr.set_property("length".to_string(), length);
 
         for (n, value) in array_contents.iter().enumerate() {
-            array_obj_ptr.set_property(n, DataDescriptor::new(value, Attribute::all()));
+            array_obj_ptr.set_property(
+                n,
+                PropertyDescriptor::builder()
+                    .value(value)
+                    .configurable(true)
+                    .enumerable(true)
+                    .writable(true),
+            );
         }
         Ok(array_obj_ptr)
     }
@@ -393,7 +413,14 @@ impl Array {
 
         for (n, value) in add_values.iter().enumerate() {
             let new_index = orig_length.wrapping_add(n);
-            array_ptr.set_property(new_index, DataDescriptor::new(value, Attribute::all()));
+            array_ptr.set_property(
+                new_index,
+                PropertyDescriptor::builder()
+                    .value(value)
+                    .configurable(true)
+                    .enumerable(true)
+                    .writable(true),
+            );
         }
 
         array_ptr.set_field(
@@ -493,7 +520,7 @@ impl Array {
         // 5. For each element E of items, do
         for item in [Value::from(obj)].iter().chain(args.iter()) {
             // a. Let spreadable be ? IsConcatSpreadable(E).
-            let spreadable = Self::is_concat_spreadable(&item, context)?;
+            let spreadable = Self::is_concat_spreadable(item, context)?;
             // b. If spreadable is true, then
             if spreadable {
                 // item is guaranteed to be an object since is_concat_spreadable checks it,
@@ -1040,7 +1067,12 @@ impl Array {
                 // iii. Perform ? CreateDataPropertyOrThrow(A, Pk, mappedValue).
                 arr.ordinary_define_own_property(
                     k.into(),
-                    DataDescriptor::new(value, Attribute::all()).into(),
+                    PropertyDescriptor::builder()
+                        .value(value)
+                        .configurable(true)
+                        .enumerable(true)
+                        .writable(true)
+                        .into(),
                 );
             }
             // d. Set k to k + 1.
@@ -1445,8 +1477,14 @@ impl Array {
                     }
 
                     // 2. Perform CreateDataPropertyOrThrow(target, targetIndex, element)
-                    target
-                        .set_property(target_index, DataDescriptor::new(element, Attribute::all()));
+                    target.set_property(
+                        target_index,
+                        PropertyDescriptor::builder()
+                            .value(element)
+                            .configurable(true)
+                            .enumerable(true)
+                            .writable(true),
+                    );
 
                     // 3. Set targetIndex to targetIndex + 1
                     target_index = target_index.saturating_add(1);
@@ -1480,7 +1518,14 @@ impl Array {
         let fin = Self::get_relative_end(context, args.get(2), len)?;
 
         for i in start..fin {
-            this.set_property(i, DataDescriptor::new(value.clone(), Attribute::all()));
+            this.set_property(
+                i,
+                PropertyDescriptor::builder()
+                    .value(value.clone())
+                    .configurable(true)
+                    .enumerable(true)
+                    .writable(true),
+            );
         }
 
         Ok(this.clone())
@@ -1562,7 +1607,11 @@ impl Array {
         for i in from..from.saturating_add(span) {
             new_array.set_property(
                 new_array_len,
-                DataDescriptor::new(this.get_field(i, context)?, Attribute::all()),
+                PropertyDescriptor::builder()
+                    .value(this.get_field(i, context)?)
+                    .configurable(true)
+                    .enumerable(true)
+                    .writable(true),
             );
             new_array_len = new_array_len.saturating_add(1);
         }

boa/src/builtins/array/tests.rs

@@ -1540,5 +1540,5 @@ fn array_length_is_not_enumerable() {
 
     let array = Array::new_array(&context);
     let desc = array.get_property("length").unwrap();
-    assert!(!desc.enumerable());
+    assert!(!desc.expect_enumerable());
 }

boa/src/builtins/date/tests.rs

@@ -63,9 +63,8 @@ fn date_this_time_value() {
     let message_property = &error
         .get_property("message")
         .expect("Expected 'message' property")
-        .as_data_descriptor()
-        .unwrap()
-        .value();
+        .expect_value()
+        .clone();
 
     assert_eq!(Value::string("\'this\' is not a Date"), *message_property);
 }

boa/src/builtins/function/mod.rs

@@ -17,7 +17,7 @@ use crate::{
     environment::lexical_environment::Environment,
     gc::{custom_trace, empty_trace, Finalize, Trace},
     object::{ConstructorBuilder, FunctionBuilder, GcObject, Object, ObjectData},
-    property::{Attribute, DataDescriptor},
+    property::{Attribute, PropertyDescriptor},
     syntax::ast::node::{FormalParameter, RcStatementList},
     BoaProfiler, Context, Result, Value,
 };
@@ -181,19 +181,21 @@ pub fn create_unmapped_arguments_object(arguments_list: &[Value]) -> Value {
     let len = arguments_list.len();
     let obj = GcObject::new(Object::default());
     // Set length
-    let length = DataDescriptor::new(
-        len,
-        Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE,
-    );
+    let length = PropertyDescriptor::builder()
+        .value(len)
+        .writable(true)
+        .enumerable(false)
+        .configurable(true);
     // Define length as a property
     obj.ordinary_define_own_property("length".into(), length.into());
     let mut index: usize = 0;
     while index < len {
         let val = arguments_list.get(index).expect("Could not get argument");
-        let prop = DataDescriptor::new(
-            val.clone(),
-            Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE,
-        );
+        let prop = PropertyDescriptor::builder()
+            .value(val.clone())
+            .writable(true)
+            .enumerable(true)
+            .configurable(true);
 
         obj.insert(index, prop);
         index += 1;
@@ -243,14 +245,20 @@ pub fn make_builtin_fn<N>(
             .prototype()
             .into(),
     );
-    let attribute = Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE;
-    function.insert_property("length", length, attribute);
-    function.insert_property("name", name.as_str(), attribute);
+    let attribute = PropertyDescriptor::builder()
+        .writable(false)
+        .enumerable(false)
+        .configurable(true);
+    function.insert_property("length", attribute.clone().value(length));
+    function.insert_property("name", attribute.value(name.as_str()));
 
     parent.clone().insert_property(
         name,
-        function,
-        Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE,
+        PropertyDescriptor::builder()
+            .value(function)
+            .writable(true)
+            .enumerable(false)
+            .configurable(true),
     );
 }
 

boa/src/builtins/iterable/mod.rs

@@ -5,7 +5,7 @@ use crate::{
         SetIterator,
     },
     object::{GcObject, ObjectInitializer},
-    property::{Attribute, DataDescriptor},
+    property::PropertyDescriptor,
     symbol::WellKnownSymbols,
     BoaProfiler, Context, Result, Value,
 };
@@ -89,8 +89,17 @@ impl IteratorPrototypes {
 pub fn create_iter_result_object(context: &mut Context, value: Value, done: bool) -> Value {
     let object = Value::new_object(context);
     // TODO: Fix attributes of value and done
-    let value_property = DataDescriptor::new(value, Attribute::all());
-    let done_property = DataDescriptor::new(done, Attribute::all());
+    // TODO: use CreateDataPropertyOrThrow
+    let value_property = PropertyDescriptor::builder()
+        .value(value)
+        .writable(true)
+        .enumerable(true)
+        .configurable(true);
+    let done_property = PropertyDescriptor::builder()
+        .value(done)
+        .writable(true)
+        .enumerable(true)
+        .configurable(true);
     object.set_property("value", value_property);
     object.set_property("done", done_property);
     object

boa/src/builtins/json/mod.rs

@@ -17,7 +17,7 @@ use crate::{
     builtins::BuiltIn,
     object::Object,
     object::ObjectInitializer,
-    property::{Attribute, DataDescriptor, PropertyKey},
+    property::{Attribute, PropertyDescriptor, PropertyKey},
     symbol::WellKnownSymbols,
     value::IntegerOrInfinity,
     BoaProfiler, Context, Result, Value,
@@ -201,15 +201,16 @@ impl Json {
                         let this_arg = object.clone();
                         object_to_return.set_property(
                             key.to_owned(),
-                            DataDescriptor::new(
-                                context.call(
+                            PropertyDescriptor::builder()
+                                .value(context.call(
                                     replacer,
                                     &this_arg,
                                     &[Value::from(key.clone()), val.clone()],
-                                )?,
-                                Attribute::all(),
-                            ),
-                        );
+                                )?)
+                                .writable(true)
+                                .enumerable(true)
+                                .configurable(true),
+                        )
                     }
                     if let Some(value) = object_to_return.to_json(context)? {
                         Ok(Value::from(json_to_pretty_string(&value, gap)))
@@ -229,9 +230,10 @@ impl Json {
                         replacer
                             .get_property(key)
                             .as_ref()
-                            .and_then(|p| p.as_data_descriptor())
                             .map(|d| d.value())
-                            .unwrap_or_else(Value::undefined),
+                            .flatten()
+                            .cloned()
+                            .unwrap_or_default(),
                     )
                 }
             });

boa/src/builtins/map/map_iterator.rs

@@ -1,7 +1,7 @@
 use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object, Array, Value},
     object::{GcObject, ObjectData},
-    property::{Attribute, DataDescriptor},
+    property::PropertyDescriptor,
     symbol::WellKnownSymbols,
     BoaProfiler, Context, Result,
 };
@@ -154,10 +154,11 @@ impl MapIterator {
         map_iterator.set_prototype_instance(iterator_prototype);
 
         let to_string_tag = WellKnownSymbols::to_string_tag();
-        let to_string_tag_property = DataDescriptor::new(
-            "Map Iterator",
-            Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE,
-        );
+        let to_string_tag_property = PropertyDescriptor::builder()
+            .value("Map Iterator")
+            .writable(false)
+            .enumerable(false)
+            .configurable(true);
         map_iterator.insert(to_string_tag, to_string_tag_property);
         map_iterator
     }