Proposal of new PropertyDescriptor design (#1432)

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,
@@ -225,11 +225,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)
     }
@@ -242,11 +247,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
     }
 
@@ -268,14 +277,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)
     }
@@ -389,7 +409,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(

boa/src/builtins/array/tests.rs

@@ -1546,5 +1546,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/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
     }

boa/src/builtins/map/mod.rs

@@ -15,7 +15,7 @@
 use crate::{
     builtins::BuiltIn,
     object::{ConstructorBuilder, FunctionBuilder, ObjectData, PROTOTYPE},
-    property::{Attribute, DataDescriptor},
+    property::{Attribute, PropertyDescriptor},
     symbol::WellKnownSymbols,
     BoaProfiler, Context, Result, Value,
 };
@@ -217,12 +217,13 @@ impl Map {
 
     /// Helper function to set the size property.
     fn set_size(this: &Value, size: usize) {
-        let size = DataDescriptor::new(
-            size,
-            Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
-        );
+        let size = PropertyDescriptor::builder()
+            .value(size)
+            .writable(false)
+            .enumerable(false)
+            .configurable(false);
 
-        this.set_property("size".to_string(), size);
+        this.set_property("size", size);
     }
 
     /// `Map.prototype.set( key, value )`

boa/src/builtins/mod.rs

@@ -53,7 +53,7 @@ pub(crate) use self::{
     undefined::Undefined,
 };
 use crate::{
-    property::{Attribute, DataDescriptor},
+    property::{Attribute, PropertyDescriptor},
     Context, Value,
 };
 
@@ -104,7 +104,11 @@ pub fn init(context: &mut Context) {
 
     for init in &globals {
         let (name, value, attribute) = init(context);
-        let property = DataDescriptor::new(value, attribute);
+        let property = PropertyDescriptor::builder()
+            .value(value)
+            .writable(attribute.writable())
+            .enumerable(attribute.enumerable())
+            .configurable(attribute.configurable());
         global_object.borrow_mut().insert(name, property);
     }
 }

boa/src/builtins/object/for_in_iterator.rs

@@ -2,8 +2,8 @@ use crate::{
     builtins::{function::make_builtin_fn, iterable::create_iter_result_object},
     gc::{Finalize, Trace},
     object::{GcObject, ObjectData},
+    property::PropertyDescriptor,
     property::PropertyKey,
-    property::{Attribute, DataDescriptor},
     symbol::WellKnownSymbols,
     BoaProfiler, Context, JsString, Result, Value,
 };
@@ -90,7 +90,7 @@ impl ForInIterator {
                                 object.__get_own_property__(&PropertyKey::from(r.clone()))
                             {
                                 iterator.visited_keys.insert(r.clone());
-                                if desc.enumerable() {
+                                if desc.expect_enumerable() {
                                     return Ok(create_iter_result_object(
                                         context,
                                         Value::from(r.to_string()),
@@ -134,10 +134,11 @@ impl ForInIterator {
         for_in_iterator.set_prototype_instance(iterator_prototype);
 
         let to_string_tag = WellKnownSymbols::to_string_tag();
-        let to_string_tag_property = DataDescriptor::new(
-            "For In Iterator",
-            Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE,
-        );
+        let to_string_tag_property = PropertyDescriptor::builder()
+            .value("For In Iterator")
+            .writable(false)
+            .enumerable(false)
+            .configurable(true);
         for_in_iterator.insert(to_string_tag, to_string_tag_property);
         for_in_iterator
     }