Skip to content

Commit 1594d05

Browse files
committed
Add completed PropertyDescriptor type
1 parent f5e88de commit 1594d05

File tree

28 files changed

+1180
-718
lines changed

28 files changed

+1180
-718
lines changed

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

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ use crate::{
2626
ordinary_get_own_property,
2727
},
2828
},
29-
property::{Attribute, PropertyDescriptor, PropertyKey, PropertyNameKind},
29+
property::{
30+
Attribute, CompletePropertyDescriptor, PropertyDescriptor, PropertyKey, PropertyNameKind,
31+
},
3032
realm::Realm,
3133
string::StaticJsStrings,
3234
symbol::JsSymbol,
@@ -3286,11 +3288,12 @@ impl Array {
32863288
pub(crate) fn unscopables_object() -> JsObject {
32873289
// 1. Let unscopableList be OrdinaryObjectCreate(null).
32883290
let unscopable_list = JsObject::with_null_proto();
3289-
let true_prop = PropertyDescriptor::builder()
3290-
.value(true)
3291-
.writable(true)
3292-
.enumerable(true)
3293-
.configurable(true);
3291+
let true_prop = CompletePropertyDescriptor::Data {
3292+
value: JsValue::new(true),
3293+
writable: true,
3294+
configurable: true,
3295+
enumerable: true,
3296+
};
32943297
{
32953298
let mut obj = unscopable_list.borrow_mut();
32963299
// 2. Perform ! CreateDataPropertyOrThrow(unscopableList, "at", true).
@@ -3493,21 +3496,28 @@ fn array_exotic_define_own_property(
34933496
.expect("the property descriptor must exist");
34943497

34953498
// b. Assert: ! IsDataDescriptor(oldLenDesc) is true.
3496-
debug_assert!(old_len_desc.is_data_descriptor());
3499+
let CompletePropertyDescriptor::Data {
3500+
value,
3501+
writable,
3502+
enumerable,
3503+
configurable,
3504+
} = old_len_desc
3505+
else {
3506+
panic!("length is always a data property descriptor");
3507+
};
34973508

34983509
// c. Assert: oldLenDesc.[[Configurable]] is false.
3499-
debug_assert!(!old_len_desc.expect_configurable());
3510+
debug_assert!(!configurable);
35003511

35013512
// d. Let oldLen be oldLenDesc.[[Value]].
35023513
// e. Assert: oldLen is a non-negative integral Number.
35033514
// f. Let index be ! ToUint32(P).
3504-
let old_len = old_len_desc
3505-
.expect_value()
3515+
let old_len = value
35063516
.to_u32(context)
35073517
.js_expect("this ToUint32 call must not fail")?;
35083518

35093519
// g. If index ≥ oldLen and oldLenDesc.[[Writable]] is false, return false.
3510-
if index >= old_len && !old_len_desc.expect_writable() {
3520+
if index >= old_len && !writable {
35113521
return Ok(false);
35123522
}
35133523

@@ -3518,9 +3528,9 @@ fn array_exotic_define_own_property(
35183528
// i. Set oldLenDesc.[[Value]] to index + 1𝔽.
35193529
let old_len_desc = PropertyDescriptor::builder()
35203530
.value(new_len)
3521-
.maybe_writable(old_len_desc.writable())
3522-
.maybe_enumerable(old_len_desc.enumerable())
3523-
.maybe_configurable(old_len_desc.configurable());
3531+
.writable(writable)
3532+
.enumerable(enumerable)
3533+
.configurable(configurable);
35243534

35253535
// ii. Set succeeded to OrdinaryDefineOwnProperty(A, "length", oldLenDesc).
35263536
let succeeded = ordinary_define_own_property(
@@ -3590,13 +3600,21 @@ fn array_set_length(
35903600
.expect("the property descriptor must exist");
35913601

35923602
// 8. Assert: ! IsDataDescriptor(oldLenDesc) is true.
3593-
debug_assert!(old_len_desc.is_data_descriptor());
3603+
let CompletePropertyDescriptor::Data {
3604+
value,
3605+
writable,
3606+
configurable,
3607+
..
3608+
} = old_len_desc
3609+
else {
3610+
panic!("length is always a data property descriptor");
3611+
};
35943612

35953613
// 9. Assert: oldLenDesc.[[Configurable]] is false.
3596-
debug_assert!(!old_len_desc.expect_configurable());
3614+
debug_assert!(!configurable);
35973615

35983616
// 10. Let oldLen be oldLenDesc.[[Value]].
3599-
let old_len = old_len_desc.expect_value();
3617+
let old_len = value;
36003618

36013619
// 11. If newLen ≥ oldLen, then
36023620
if new_len >= old_len.to_u32(context)? {
@@ -3610,7 +3628,7 @@ fn array_set_length(
36103628
}
36113629

36123630
// 12. If oldLenDesc.[[Writable]] is false, return false.
3613-
if !old_len_desc.expect_writable() {
3631+
if !writable {
36143632
return Ok(false);
36153633
}
36163634

core/engine/src/builtins/builder.rs

Lines changed: 49 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
CONSTRUCTOR, FunctionBinding, JsFunction, JsPrototype, PROTOTYPE,
66
shape::{property_table::PropertyTableInner, slot::SlotAttributes},
77
},
8-
property::{Attribute, PropertyDescriptor, PropertyKey},
8+
property::{Attribute, CompletePropertyDescriptor, PropertyKey},
99
realm::Realm,
1010
string::StaticJsStrings,
1111
};
@@ -67,23 +67,25 @@ impl ApplyToObject for Constructor {
6767
fn apply_to(self, object: &JsObject) {
6868
object.insert(
6969
PROTOTYPE,
70-
PropertyDescriptor::builder()
71-
.value(self.prototype.clone())
72-
.writable(false)
73-
.enumerable(false)
74-
.configurable(false),
70+
CompletePropertyDescriptor::Data {
71+
value: self.prototype.clone().into(),
72+
writable: false,
73+
enumerable: false,
74+
configurable: false,
75+
},
7576
);
7677

7778
{
7879
let mut prototype = self.prototype.borrow_mut();
7980
prototype.set_prototype(self.inherits);
8081
prototype.insert(
8182
CONSTRUCTOR,
82-
PropertyDescriptor::builder()
83-
.value(object.clone())
84-
.writable(self.attributes.writable())
85-
.enumerable(self.attributes.enumerable())
86-
.configurable(self.attributes.configurable()),
83+
CompletePropertyDescriptor::Data {
84+
value: object.clone().into(),
85+
writable: self.attributes.writable(),
86+
enumerable: self.attributes.enumerable(),
87+
configurable: self.attributes.configurable(),
88+
},
8789
);
8890
}
8991
}
@@ -109,19 +111,21 @@ impl<S: ApplyToObject + IsConstructor> ApplyToObject for Callable<S> {
109111
}
110112
object.insert(
111113
StaticJsStrings::LENGTH,
112-
PropertyDescriptor::builder()
113-
.value(self.length)
114-
.writable(false)
115-
.enumerable(false)
116-
.configurable(true),
114+
CompletePropertyDescriptor::Data {
115+
value: self.length.into(),
116+
writable: false,
117+
enumerable: false,
118+
configurable: true,
119+
},
117120
);
118121
object.insert(
119122
js_string!("name"),
120-
PropertyDescriptor::builder()
121-
.value(self.name)
122-
.writable(false)
123-
.enumerable(false)
124-
.configurable(true),
123+
CompletePropertyDescriptor::Data {
124+
value: self.name.into(),
125+
writable: false,
126+
enumerable: false,
127+
configurable: true,
128+
},
125129
);
126130

127131
self.kind.apply_to(object);
@@ -641,11 +645,12 @@ impl<T> BuiltInBuilder<'_, T> {
641645

642646
self.object.insert(
643647
binding.binding,
644-
PropertyDescriptor::builder()
645-
.value(function)
646-
.writable(true)
647-
.enumerable(false)
648-
.configurable(true),
648+
CompletePropertyDescriptor::Data {
649+
value: function.into(),
650+
writable: true,
651+
enumerable: false,
652+
configurable: true,
653+
},
649654
);
650655
self
651656
}
@@ -656,12 +661,15 @@ impl<T> BuiltInBuilder<'_, T> {
656661
K: Into<PropertyKey>,
657662
V: Into<JsValue>,
658663
{
659-
let property = PropertyDescriptor::builder()
660-
.value(value)
661-
.writable(attribute.writable())
662-
.enumerable(attribute.enumerable())
663-
.configurable(attribute.configurable());
664-
self.object.insert(key, property);
664+
self.object.insert(
665+
key,
666+
CompletePropertyDescriptor::Data {
667+
value: value.into(),
668+
writable: attribute.writable(),
669+
enumerable: attribute.enumerable(),
670+
configurable: attribute.configurable(),
671+
},
672+
);
665673
self
666674
}
667675

@@ -676,21 +684,17 @@ impl<T> BuiltInBuilder<'_, T> {
676684
where
677685
K: Into<PropertyKey>,
678686
{
679-
let mut property = PropertyDescriptor::builder()
680-
.enumerable(attribute.enumerable())
681-
.configurable(attribute.configurable());
682-
683-
if let Some(get) = get {
684-
property = property.get(get);
685-
}
686-
687-
if let Some(set) = set {
688-
property = property.set(set);
689-
}
690-
691687
let key = key.into();
692688

693-
self.object.insert(key, property);
689+
self.object.insert(
690+
key,
691+
CompletePropertyDescriptor::Accessor {
692+
get,
693+
set,
694+
enumerable: attribute.enumerable(),
695+
configurable: attribute.configurable(),
696+
},
697+
);
694698

695699
self
696700
}

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

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
ordinary_set, ordinary_try_get,
1111
},
1212
},
13-
property::{DescriptorKind, PropertyDescriptor, PropertyKey},
13+
property::{CompletePropertyDescriptor, DescriptorKind, PropertyDescriptor, PropertyKey},
1414
};
1515
use boa_ast::{function::FormalParameterList, operations::bound_names, scope::Scope};
1616
use boa_gc::{Finalize, Gc, Trace};
@@ -283,7 +283,7 @@ pub(crate) fn arguments_exotic_get_own_property(
283283
obj: &JsObject,
284284
key: &PropertyKey,
285285
context: &mut InternalMethodPropertyContext<'_>,
286-
) -> JsResult<Option<PropertyDescriptor>> {
286+
) -> JsResult<Option<CompletePropertyDescriptor>> {
287287
// 1. Let desc be OrdinaryGetOwnProperty(args, P).
288288
// 2. If desc is undefined, return desc.
289289
let Some(desc) = ordinary_get_own_property(obj, key, context)? else {
@@ -300,14 +300,12 @@ pub(crate) fn arguments_exotic_get_own_property(
300300
.get(index.get())
301301
{
302302
// a. Set desc.[[Value]] to Get(map, P).
303-
return Ok(Some(
304-
PropertyDescriptor::builder()
305-
.value(value)
306-
.maybe_writable(desc.writable())
307-
.maybe_enumerable(desc.enumerable())
308-
.maybe_configurable(desc.configurable())
309-
.build(),
310-
));
303+
return Ok(Some(CompletePropertyDescriptor::Data {
304+
value,
305+
writable: desc.writable().unwrap_or(false),
306+
enumerable: desc.enumerable(),
307+
configurable: desc.configurable(),
308+
}));
311309
}
312310

313311
// 6. Return desc.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ impl ForInIterator {
113113
)?
114114
{
115115
iterator.visited_keys.insert(r.clone());
116-
if desc.expect_enumerable() {
116+
if desc.enumerable() {
117117
return Ok(create_iter_result_object(JsValue::new(r), false, context));
118118
}
119119
}

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

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use super::{
1717
Array, BuiltInBuilder, BuiltInConstructor, Date, IntrinsicObject, RegExp, error::Error,
1818
};
1919
use crate::builtins::function::arguments::{MappedArguments, UnmappedArguments};
20+
use crate::property::CompletePropertyDescriptor;
2021
use crate::value::JsVariant;
2122
use crate::{
2223
Context, JsArgs, JsData, JsExpect, JsResult, JsString,
@@ -36,7 +37,6 @@ use crate::{
3637
};
3738
use boa_gc::{Finalize, Trace};
3839
use boa_macros::js_str;
39-
use tap::{Conv, Pipe};
4040

4141
pub(crate) mod for_in_iterator;
4242
#[cfg(test)]
@@ -378,8 +378,8 @@ impl OrdinaryObject {
378378
// b. If desc is not undefined, then
379379
if let Some(current_desc) = desc {
380380
// i. If IsAccessorDescriptor(desc) is true, return desc.[[Get]].
381-
return if current_desc.is_accessor_descriptor() {
382-
Ok(current_desc.expect_get().clone())
381+
return if let CompletePropertyDescriptor::Accessor { get, .. } = current_desc {
382+
Ok(get.map(JsValue::new).unwrap_or_default())
383383
} else {
384384
// ii. Return undefined.
385385
Ok(JsValue::undefined())
@@ -424,8 +424,8 @@ impl OrdinaryObject {
424424
// b. If desc is not undefined, then
425425
if let Some(current_desc) = desc {
426426
// i. If IsAccessorDescriptor(desc) is true, return desc.[[Set]].
427-
return if current_desc.is_accessor_descriptor() {
428-
Ok(current_desc.expect_set().clone())
427+
return if let CompletePropertyDescriptor::Accessor { set, .. } = current_desc {
428+
Ok(set.map(JsValue::new).unwrap_or_default())
429429
} else {
430430
// ii. Return undefined.
431431
Ok(JsValue::undefined())
@@ -506,7 +506,7 @@ impl OrdinaryObject {
506506
obj.__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?;
507507

508508
// 4. Return FromPropertyDescriptor(desc).
509-
Self::from_property_descriptor(desc, context)
509+
Self::from_property_descriptor(desc.map(Into::into), context)
510510
}
511511

512512
/// `Object.getOwnPropertyDescriptors( object )`
@@ -542,7 +542,8 @@ impl OrdinaryObject {
542542
obj.__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?;
543543

544544
// b. Let descriptor be FromPropertyDescriptor(desc).
545-
let descriptor = Self::from_property_descriptor(desc, context)?;
545+
let descriptor = Self::from_property_descriptor(desc.map(Into::into), context)
546+
.expect("should never fail");
546547

547548
// c. If descriptor is not undefined,
548549
// perform ! CreateDataPropertyOrThrow(descriptors, key, descriptor).
@@ -942,12 +943,10 @@ impl OrdinaryObject {
942943
.to_object(context)?
943944
.__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?;
944945

945-
own_prop
946+
Ok(own_prop
946947
.as_ref()
947-
.and_then(PropertyDescriptor::enumerable)
948-
.unwrap_or_default()
949-
.conv::<JsValue>()
950-
.pipe(Ok)
948+
.is_some_and(CompletePropertyDescriptor::enumerable)
949+
.into())
951950
}
952951

953952
/// `Object.assign( target, ...sources )`
@@ -990,7 +989,7 @@ impl OrdinaryObject {
990989
&mut InternalMethodPropertyContext::new(context),
991990
)? {
992991
// 3.a.iii.2. If desc is not undefined and desc.[[Enumerable]] is true, then
993-
if desc.expect_enumerable() {
992+
if desc.enumerable() {
994993
// 3.a.iii.2.a. Let propValue be ? Get(from, nextKey).
995994
let property = from.get(key.clone(), context)?;
996995
// 3.a.iii.2.b. Perform ? Set(to, nextKey, propValue, true).
@@ -1458,7 +1457,7 @@ fn object_define_properties(
14581457

14591458
if let Some(prop_desc) = props
14601459
.__get_own_property__(&next_key, &mut InternalMethodPropertyContext::new(context))?
1461-
&& prop_desc.expect_enumerable()
1460+
&& prop_desc.enumerable()
14621461
{
14631462
// i. Let descObj be ? Get(props, nextKey).
14641463
let desc_obj = props.get(next_key.clone(), context)?;

0 commit comments

Comments
 (0)