Add completed PropertyDescriptor type

core/engine/src/builtins/array/mod.rs

@@ -16,7 +16,7 @@ use crate::{
     Context, JsArgs, JsExpect, JsResult, JsString,
     builtins::{BuiltInObject, Number, iterable::if_abrupt_close_iterator},
     context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
-    error::JsNativeError,
+    error::{JsNativeError, PanicError},
     js_string,
     object::{
         CONSTRUCTOR, IndexedProperties, JsData, JsObject,
@@ -26,7 +26,9 @@ use crate::{
             ordinary_get_own_property,
         },
     },
-    property::{Attribute, PropertyDescriptor, PropertyKey, PropertyNameKind},
+    property::{
+        Attribute, CompletePropertyDescriptor, PropertyDescriptor, PropertyKey, PropertyNameKind,
+    },
     realm::Realm,
     string::StaticJsStrings,
     symbol::JsSymbol,
@@ -3286,11 +3288,12 @@ impl Array {
     pub(crate) fn unscopables_object() -> JsObject {
         // 1. Let unscopableList be OrdinaryObjectCreate(null).
         let unscopable_list = JsObject::with_null_proto();
-        let true_prop = PropertyDescriptor::builder()
-            .value(true)
-            .writable(true)
-            .enumerable(true)
-            .configurable(true);
+        let true_prop = CompletePropertyDescriptor::Data {
+            value: JsValue::new(true),
+            writable: true,
+            configurable: true,
+            enumerable: true,
+        };
         {
             let mut obj = unscopable_list.borrow_mut();
             // 2. Perform ! CreateDataPropertyOrThrow(unscopableList, "at", true).
@@ -3493,21 +3496,28 @@ fn array_exotic_define_own_property(
                     .expect("the property descriptor must exist");
 
             // b. Assert: ! IsDataDescriptor(oldLenDesc) is true.
-            debug_assert!(old_len_desc.is_data_descriptor());
+            let CompletePropertyDescriptor::Data {
+                value,
+                writable,
+                enumerable,
+                configurable,
+            } = old_len_desc
+            else {
+                return Err(PanicError::new("length is always a data property descriptor").into());
+            };
 
             // c. Assert: oldLenDesc.[[Configurable]] is false.
-            debug_assert!(!old_len_desc.expect_configurable());
+            debug_assert!(!configurable);
 
             // d. Let oldLen be oldLenDesc.[[Value]].
             // e. Assert: oldLen is a non-negative integral Number.
             // f. Let index be ! ToUint32(P).
-            let old_len = old_len_desc
-                .expect_value()
+            let old_len = value
                 .to_u32(context)
                 .js_expect("this ToUint32 call must not fail")?;
 
             // g. If index ≥ oldLen and oldLenDesc.[[Writable]] is false, return false.
-            if index >= old_len && !old_len_desc.expect_writable() {
+            if index >= old_len && !writable {
                 return Ok(false);
             }
 
@@ -3518,9 +3528,9 @@ fn array_exotic_define_own_property(
                     // i. Set oldLenDesc.[[Value]] to index + 1𝔽.
                     let old_len_desc = PropertyDescriptor::builder()
                         .value(new_len)
-                        .maybe_writable(old_len_desc.writable())
-                        .maybe_enumerable(old_len_desc.enumerable())
-                        .maybe_configurable(old_len_desc.configurable());
+                        .writable(writable)
+                        .enumerable(enumerable)
+                        .configurable(configurable);
 
                     // ii. Set succeeded to OrdinaryDefineOwnProperty(A, "length", oldLenDesc).
                     let succeeded = ordinary_define_own_property(
@@ -3590,13 +3600,21 @@ fn array_set_length(
         .expect("the property descriptor must exist");
 
     // 8. Assert: ! IsDataDescriptor(oldLenDesc) is true.
-    debug_assert!(old_len_desc.is_data_descriptor());
+    let CompletePropertyDescriptor::Data {
+        value,
+        writable,
+        configurable,
+        ..
+    } = old_len_desc
+    else {
+        panic!("length is always a data property descriptor");
+    };
 
     // 9. Assert: oldLenDesc.[[Configurable]] is false.
-    debug_assert!(!old_len_desc.expect_configurable());
+    debug_assert!(!configurable);
 
     // 10. Let oldLen be oldLenDesc.[[Value]].
-    let old_len = old_len_desc.expect_value();
+    let old_len = value;
 
     // 11. If newLen ≥ oldLen, then
     if new_len >= old_len.to_u32(context)? {
@@ -3610,7 +3628,7 @@ fn array_set_length(
     }
 
     // 12. If oldLenDesc.[[Writable]] is false, return false.
-    if !old_len_desc.expect_writable() {
+    if !writable {
         return Ok(false);
     }
 

core/engine/src/builtins/builder.rs

@@ -5,7 +5,7 @@ use crate::{
         CONSTRUCTOR, FunctionBinding, JsFunction, JsPrototype, PROTOTYPE,
         shape::{property_table::PropertyTableInner, slot::SlotAttributes},
     },
-    property::{Attribute, PropertyDescriptor, PropertyKey},
+    property::{Attribute, CompletePropertyDescriptor, PropertyKey},
     realm::Realm,
     string::StaticJsStrings,
 };
@@ -67,23 +67,25 @@ impl ApplyToObject for Constructor {
     fn apply_to(self, object: &JsObject) {
         object.insert(
             PROTOTYPE,
-            PropertyDescriptor::builder()
-                .value(self.prototype.clone())
-                .writable(false)
-                .enumerable(false)
-                .configurable(false),
+            CompletePropertyDescriptor::Data {
+                value: self.prototype.clone().into(),
+                writable: false,
+                enumerable: false,
+                configurable: false,
+            },
         );
 
         {
             let mut prototype = self.prototype.borrow_mut();
             prototype.set_prototype(self.inherits);
             prototype.insert(
                 CONSTRUCTOR,
-                PropertyDescriptor::builder()
-                    .value(object.clone())
-                    .writable(self.attributes.writable())
-                    .enumerable(self.attributes.enumerable())
-                    .configurable(self.attributes.configurable()),
+                CompletePropertyDescriptor::Data {
+                    value: object.clone().into(),
+                    writable: self.attributes.writable(),
+                    enumerable: self.attributes.enumerable(),
+                    configurable: self.attributes.configurable(),
+                },
             );
         }
     }
@@ -109,19 +111,21 @@ impl<S: ApplyToObject + IsConstructor> ApplyToObject for Callable<S> {
         }
         object.insert(
             StaticJsStrings::LENGTH,
-            PropertyDescriptor::builder()
-                .value(self.length)
-                .writable(false)
-                .enumerable(false)
-                .configurable(true),
+            CompletePropertyDescriptor::Data {
+                value: self.length.into(),
+                writable: false,
+                enumerable: false,
+                configurable: true,
+            },
         );
         object.insert(
             js_string!("name"),
-            PropertyDescriptor::builder()
-                .value(self.name)
-                .writable(false)
-                .enumerable(false)
-                .configurable(true),
+            CompletePropertyDescriptor::Data {
+                value: self.name.into(),
+                writable: false,
+                enumerable: false,
+                configurable: true,
+            },
         );
 
         self.kind.apply_to(object);
@@ -641,11 +645,12 @@ impl<T> BuiltInBuilder<'_, T> {
 
         self.object.insert(
             binding.binding,
-            PropertyDescriptor::builder()
-                .value(function)
-                .writable(true)
-                .enumerable(false)
-                .configurable(true),
+            CompletePropertyDescriptor::Data {
+                value: function.into(),
+                writable: true,
+                enumerable: false,
+                configurable: true,
+            },
         );
         self
     }
@@ -656,12 +661,15 @@ impl<T> BuiltInBuilder<'_, T> {
         K: Into<PropertyKey>,
         V: Into<JsValue>,
     {
-        let property = PropertyDescriptor::builder()
-            .value(value)
-            .writable(attribute.writable())
-            .enumerable(attribute.enumerable())
-            .configurable(attribute.configurable());
-        self.object.insert(key, property);
+        self.object.insert(
+            key,
+            CompletePropertyDescriptor::Data {
+                value: value.into(),
+                writable: attribute.writable(),
+                enumerable: attribute.enumerable(),
+                configurable: attribute.configurable(),
+            },
+        );
         self
     }
 
@@ -676,21 +684,17 @@ impl<T> BuiltInBuilder<'_, T> {
     where
         K: Into<PropertyKey>,
     {
-        let mut property = PropertyDescriptor::builder()
-            .enumerable(attribute.enumerable())
-            .configurable(attribute.configurable());
-
-        if let Some(get) = get {
-            property = property.get(get);
-        }
-
-        if let Some(set) = set {
-            property = property.set(set);
-        }
-
         let key = key.into();
 
-        self.object.insert(key, property);
+        self.object.insert(
+            key,
+            CompletePropertyDescriptor::Accessor {
+                get,
+                set,
+                enumerable: attribute.enumerable(),
+                configurable: attribute.configurable(),
+            },
+        );
 
         self
     }

core/engine/src/builtins/function/arguments.rs

@@ -10,7 +10,7 @@ use crate::{
             ordinary_set, ordinary_try_get,
         },
     },
-    property::{DescriptorKind, PropertyDescriptor, PropertyKey},
+    property::{CompletePropertyDescriptor, DescriptorKind, PropertyDescriptor, PropertyKey},
 };
 use boa_ast::{function::FormalParameterList, operations::bound_names, scope::Scope};
 use boa_gc::{Finalize, Gc, Trace};
@@ -283,7 +283,7 @@ pub(crate) fn arguments_exotic_get_own_property(
     obj: &JsObject,
     key: &PropertyKey,
     context: &mut InternalMethodPropertyContext<'_>,
-) -> JsResult<Option<PropertyDescriptor>> {
+) -> JsResult<Option<CompletePropertyDescriptor>> {
     // 1. Let desc be OrdinaryGetOwnProperty(args, P).
     // 2. If desc is undefined, return desc.
     let Some(desc) = ordinary_get_own_property(obj, key, context)? else {
@@ -300,14 +300,12 @@ pub(crate) fn arguments_exotic_get_own_property(
             .get(index.get())
     {
         // a. Set desc.[[Value]] to Get(map, P).
-        return Ok(Some(
-            PropertyDescriptor::builder()
-                .value(value)
-                .maybe_writable(desc.writable())
-                .maybe_enumerable(desc.enumerable())
-                .maybe_configurable(desc.configurable())
-                .build(),
-        ));
+        return Ok(Some(CompletePropertyDescriptor::Data {
+            value,
+            writable: desc.writable().unwrap_or(false),
+            enumerable: desc.enumerable(),
+            configurable: desc.configurable(),
+        }));
     }
 
     // 6. Return desc.

core/engine/src/builtins/iterable/iterator_constructor.rs

@@ -342,7 +342,7 @@ impl IteratorConstructor {
             };
 
             // 12.c. If desc is not undefined and desc.[[Enumerable]] is true, then
-            if desc.enumerable() != Some(true) {
+            if !desc.enumerable() {
                 continue;
             }
 

core/engine/src/builtins/object/for_in_iterator.rs

@@ -113,7 +113,7 @@ impl ForInIterator {
                     )?
                 {
                     iterator.visited_keys.insert(r.clone());
-                    if desc.expect_enumerable() {
+                    if desc.enumerable() {
                         return Ok(create_iter_result_object(JsValue::new(r), false, context));
                     }
                 }