diff --git a/boa/src/builtins/array/array_iterator.rs b/boa/src/builtins/array/array_iterator.rs index 8c38d4f8161..d0a12d30db2 100644 --- a/boa/src/builtins/array/array_iterator.rs +++ b/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 } diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 35197c144c5..756ed1b1800 100644 --- a/boa/src/builtins/array/mod.rs +++ b/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( diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index 0bab8e2b86c..4e9fa676679 100644 --- a/boa/src/builtins/array/tests.rs +++ b/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()); } diff --git a/boa/src/builtins/date/tests.rs b/boa/src/builtins/date/tests.rs index fa47954a705..bf78e129e87 100644 --- a/boa/src/builtins/date/tests.rs +++ b/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); } diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index f7a800a17e3..2d74eeddd72 100644 --- a/boa/src/builtins/function/mod.rs +++ b/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( .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), ); } diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 0fb9526c833..89bead0fa33 100644 --- a/boa/src/builtins/json/mod.rs +++ b/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(), ) } }); diff --git a/boa/src/builtins/map/map_iterator.rs b/boa/src/builtins/map/map_iterator.rs index b3c600193e8..7231337543c 100644 --- a/boa/src/builtins/map/map_iterator.rs +++ b/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 } diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index f2aee3d1388..aed07c7413c 100644 --- a/boa/src/builtins/map/mod.rs +++ b/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 )` diff --git a/boa/src/builtins/mod.rs b/boa/src/builtins/mod.rs index 5e7014f551c..7652c513c51 100644 --- a/boa/src/builtins/mod.rs +++ b/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); } } diff --git a/boa/src/builtins/object/for_in_iterator.rs b/boa/src/builtins/object/for_in_iterator.rs index fa00592275d..7449d6b4ef6 100644 --- a/boa/src/builtins/object/for_in_iterator.rs +++ b/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 } diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index e2b6978c352..e677bb43772 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -18,9 +18,7 @@ use crate::{ object::{ ConstructorBuilder, Object as BuiltinObject, ObjectData, ObjectInitializer, PROTOTYPE, }, - property::Attribute, - property::DataDescriptor, - property::PropertyDescriptor, + property::{Attribute, DescriptorKind, PropertyDescriptor}, symbol::WellKnownSymbols, value::{Type, Value}, BoaProfiler, Context, Result, @@ -205,7 +203,11 @@ impl Object { if !descriptor.is_undefined() { descriptors.borrow_mut().insert( key, - PropertyDescriptor::from(DataDescriptor::new(descriptor, Attribute::all())), + PropertyDescriptor::builder() + .value(descriptor) + .writable(true) + .enumerable(true) + .configurable(true), ); } } @@ -221,37 +223,35 @@ impl Object { fn from_property_descriptor(desc: PropertyDescriptor, context: &mut Context) -> Value { let mut descriptor = ObjectInitializer::new(context); - if let PropertyDescriptor::Data(data_desc) = &desc { - descriptor.property("value", data_desc.value(), Attribute::all()); - } + // TODO: use CreateDataPropertyOrThrow - if let PropertyDescriptor::Accessor(accessor_desc) = &desc { - if let Some(setter) = accessor_desc.setter() { - descriptor.property("set", Value::Object(setter.to_owned()), Attribute::all()); + match desc.kind() { + DescriptorKind::Data { value, writable } => { + if let Some(value) = value { + descriptor.property("value", value.clone(), Attribute::all()); + } + if let Some(writable) = writable { + descriptor.property("writable", *writable, Attribute::all()); + } } - if let Some(getter) = accessor_desc.getter() { - descriptor.property("get", Value::Object(getter.to_owned()), Attribute::all()); + DescriptorKind::Accessor { get, set } => { + if let Some(get) = get { + descriptor.property("get", get.clone(), Attribute::all()); + } + if let Some(set) = set { + descriptor.property("set", set.clone(), Attribute::all()); + } } + _ => {} } - let writable = if let PropertyDescriptor::Data(data_desc) = &desc { - data_desc.writable() - } else { - false - }; + if let Some(enumerable) = desc.enumerable() { + descriptor.property("enumerable", enumerable, Attribute::all()); + } - descriptor - .property("writable", Value::from(writable), Attribute::all()) - .property( - "enumerable", - Value::from(desc.enumerable()), - Attribute::all(), - ) - .property( - "configurable", - Value::from(desc.configurable()), - Attribute::all(), - ); + if let Some(configurable) = desc.configurable() { + descriptor.property("configurable", configurable, Attribute::all()); + } descriptor.build().into() } @@ -363,11 +363,11 @@ impl Object { if let Some(object) = object.as_object() { let key = args .get(1) - .unwrap_or(&Value::undefined()) + .unwrap_or(&Value::Undefined) .to_property_key(context)?; let desc = args .get(2) - .unwrap_or(&Value::undefined()) + .unwrap_or(&Value::Undefined) .to_property_descriptor(context)?; object.define_property_or_throw(key, desc, context)?; @@ -548,7 +548,7 @@ impl Object { // 3.a.iii.1. Let desc be ? from.[[GetOwnProperty]](nextKey). if let Some(desc) = from.__get_own_property__(&key) { // 3.a.iii.2. If desc is not undefined and desc.[[Enumerable]] is true, then - if desc.enumerable() { + if desc.expect_enumerable() { // 3.a.iii.2.a. Let propValue be ? Get(from, nextKey). let property = from.get(key.clone(), context)?; // 3.a.iii.2.b. Perform ? Set(to, nextKey, propValue, true). diff --git a/boa/src/builtins/reflect/mod.rs b/boa/src/builtins/reflect/mod.rs index de17acc59f1..4848cc367fb 100644 --- a/boa/src/builtins/reflect/mod.rs +++ b/boa/src/builtins/reflect/mod.rs @@ -13,7 +13,7 @@ use crate::{ builtins::{self, BuiltIn}, object::{Object, ObjectData, ObjectInitializer}, - property::{Attribute, DataDescriptor}, + property::{Attribute, PropertyDescriptor}, symbol::WellKnownSymbols, BoaProfiler, Context, Result, Value, }; @@ -147,14 +147,14 @@ impl Reflect { .and_then(|v| v.as_object()) .ok_or_else(|| context.construct_type_error("target must be an object"))?; let key = args.get(1).unwrap_or(&undefined).to_property_key(context)?; - let prop_desc = args + let prop_desc: Value = args .get(2) .and_then(|v| v.as_object()) .ok_or_else(|| context.construct_type_error("property descriptor must be an object"))? - .to_property_descriptor(context)?; + .into(); target - .__define_own_property__(key, prop_desc, context) + .__define_own_property__(key, prop_desc.to_property_descriptor(context)?, context) .map(|b| b.into()) } @@ -305,10 +305,11 @@ impl Reflect { Object::with_prototype(array_prototype.into(), ObjectData::Array).into(); result.set_property( "length", - DataDescriptor::new( - 0, - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, - ), + PropertyDescriptor::builder() + .value(0) + .writable(true) + .enumerable(false) + .configurable(false), ); let keys = target.own_property_keys(); diff --git a/boa/src/builtins/regexp/regexp_string_iterator.rs b/boa/src/builtins/regexp/regexp_string_iterator.rs index 52e2e097f77..f076c395109 100644 --- a/boa/src/builtins/regexp/regexp_string_iterator.rs +++ b/boa/src/builtins/regexp/regexp_string_iterator.rs @@ -15,7 +15,7 @@ use crate::{ builtins::{function::make_builtin_fn, iterable::create_iter_result_object, regexp}, gc::{Finalize, Trace}, object::{GcObject, ObjectData}, - property::{Attribute, DataDescriptor}, + property::PropertyDescriptor, symbol::WellKnownSymbols, BoaProfiler, Context, JsString, Result, Value, }; @@ -162,10 +162,11 @@ impl RegExpStringIterator { result.set_prototype_instance(iterator_prototype); let to_string_tag = WellKnownSymbols::to_string_tag(); - let to_string_tag_property = DataDescriptor::new( - "RegExp String Iterator", - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); + let to_string_tag_property = PropertyDescriptor::builder() + .value("RegExp String Iterator") + .writable(false) + .enumerable(false) + .configurable(true); result.insert(to_string_tag, to_string_tag_property); result } diff --git a/boa/src/builtins/regexp/tests.rs b/boa/src/builtins/regexp/tests.rs index 788b75e2f74..90e19a6e026 100644 --- a/boa/src/builtins/regexp/tests.rs +++ b/boa/src/builtins/regexp/tests.rs @@ -56,7 +56,6 @@ fn species() { "\"function\"" ); assert_eq!(forward(&mut context, "descriptor.enumerable"), "false"); - assert_eq!(forward(&mut context, "descriptor.writable"), "false"); assert_eq!(forward(&mut context, "descriptor.configurable"), "true"); } diff --git a/boa/src/builtins/set/mod.rs b/boa/src/builtins/set/mod.rs index 1a52eddf9d3..4c6c2966140 100644 --- a/boa/src/builtins/set/mod.rs +++ b/boa/src/builtins/set/mod.rs @@ -232,7 +232,7 @@ impl Set { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/clear pub(crate) fn clear(this: &Value, _: &[Value], context: &mut Context) -> Result { if let Some(object) = this.as_object() { - if object.borrow_mut().is_set() { + if object.borrow().is_set() { this.set_data(ObjectData::Set(OrderedSet::new())); Ok(Value::Undefined) } else { diff --git a/boa/src/builtins/set/set_iterator.rs b/boa/src/builtins/set/set_iterator.rs index b2dbefc4a28..185aa732752 100644 --- a/boa/src/builtins/set/set_iterator.rs +++ b/boa/src/builtins/set/set_iterator.rs @@ -4,7 +4,7 @@ use crate::{ builtins::Array, builtins::Value, object::{GcObject, ObjectData}, - property::{Attribute, DataDescriptor}, + property::PropertyDescriptor, symbol::WellKnownSymbols, BoaProfiler, Context, Result, }; @@ -144,10 +144,11 @@ impl SetIterator { set_iterator.set_prototype_instance(iterator_prototype); let to_string_tag = WellKnownSymbols::to_string_tag(); - let to_string_tag_property = DataDescriptor::new( - "Set Iterator", - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); + let to_string_tag_property = PropertyDescriptor::builder() + .value("Set Iterator") + .writable(false) + .enumerable(false) + .configurable(true); set_iterator.insert(to_string_tag, to_string_tag_property); set_iterator } diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index 6df02d01f03..873df40f712 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -15,11 +15,10 @@ mod tests; use crate::builtins::Symbol; use crate::object::PROTOTYPE; -use crate::property::DataDescriptor; use crate::{ builtins::{string::string_iterator::StringIterator, Array, BuiltIn, RegExp}, object::{ConstructorBuilder, ObjectData}, - property::Attribute, + property::{Attribute, PropertyDescriptor}, symbol::WellKnownSymbols, BoaProfiler, Context, JsString, Result, Value, }; @@ -191,11 +190,14 @@ impl String { .expect("this should be an object") .set_prototype_instance(prototype.into()); - let length = DataDescriptor::new( - Value::from(string.encode_utf16().count()), - Attribute::NON_ENUMERABLE, + this.set_property( + "length", + PropertyDescriptor::builder() + .value(string.encode_utf16().count()) + .writable(false) + .enumerable(false) + .configurable(false), ); - this.set_property("length", length); this.set_data(ObjectData::String(string)); diff --git a/boa/src/builtins/string/string_iterator.rs b/boa/src/builtins/string/string_iterator.rs index 95a4f2558a8..e760cc5f531 100644 --- a/boa/src/builtins/string/string_iterator.rs +++ b/boa/src/builtins/string/string_iterator.rs @@ -4,7 +4,7 @@ use crate::{ }, gc::{Finalize, Trace}, object::{GcObject, ObjectData}, - property::{Attribute, DataDescriptor}, + property::PropertyDescriptor, symbol::WellKnownSymbols, BoaProfiler, Context, Result, Value, }; @@ -79,10 +79,11 @@ impl StringIterator { array_iterator.set_prototype_instance(iterator_prototype); let to_string_tag = WellKnownSymbols::to_string_tag(); - let to_string_tag_property = DataDescriptor::new( - "String Iterator", - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); + let to_string_tag_property = PropertyDescriptor::builder() + .value("String Iterator") + .writable(false) + .enumerable(false) + .configurable(true); array_iterator.insert(to_string_tag, to_string_tag_property); array_iterator } diff --git a/boa/src/class.rs b/boa/src/class.rs index a28b1324400..16396c26e4d 100644 --- a/boa/src/class.rs +++ b/boa/src/class.rs @@ -74,7 +74,7 @@ pub trait Class: NativeObject + Sized { /// The amount of arguments the class `constructor` takes, default is `0`. const LENGTH: usize = 0; /// The attibutes the class will be binded with, default is `writable`, `enumerable`, `configurable`. - const ATTRIBUTE: Attribute = Attribute::all(); + const ATTRIBUTES: Attribute = Attribute::all(); /// The constructor of the class. fn constructor(this: &Value, args: &[Value], context: &mut Context) -> Result; diff --git a/boa/src/context.rs b/boa/src/context.rs index 2ce7c401600..45d6fe2ba52 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -9,7 +9,7 @@ use crate::{ class::{Class, ClassBuilder}, exec::Interpreter, object::{FunctionBuilder, GcObject, Object, PROTOTYPE}, - property::{Attribute, DataDescriptor, PropertyKey}, + property::{Attribute, PropertyDescriptor, PropertyKey}, realm::Realm, syntax::{ ast::{ @@ -223,7 +223,7 @@ impl StandardObjects { /// ## Execute Function of Script File /// /// ```rust -/// use boa::{Context, object::ObjectInitializer, property::Attribute}; +/// use boa::{Context, object::ObjectInitializer, property::{Attribute, PropertyDescriptor}}; /// /// let script = r#" /// function test(arg1) { @@ -243,7 +243,11 @@ impl StandardObjects { /// let arg = ObjectInitializer::new(&mut context) /// .property("x", 12, Attribute::READONLY) /// .build(); -/// context.register_global_property("arg", arg, Attribute::all()); +/// context.register_global_property( +/// "arg", +/// arg, +/// Attribute::all() +/// ); /// /// let value = context.eval("test(arg)").unwrap(); /// @@ -517,26 +521,32 @@ impl Context { let function = GcObject::new(Object::function(func, function_prototype)); // Set constructor field to the newly created Value (function object) - let constructor = DataDescriptor::new( - function.clone(), - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); + let constructor = PropertyDescriptor::builder() + .value(function.clone()) + .writable(true) + .enumerable(false) + .configurable(true); prototype.define_property_or_throw("constructor", constructor, self)?; - let prototype = DataDescriptor::new( - prototype, - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, - ); + let prototype = PropertyDescriptor::builder() + .value(prototype) + .writable(true) + .enumerable(false) + .configurable(false); function.define_property_or_throw(PROTOTYPE, prototype, self)?; - let length = DataDescriptor::new( - params_len, - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); + + let length = PropertyDescriptor::builder() + .value(params_len) + .writable(false) + .enumerable(false) + .configurable(true); function.define_property_or_throw("length", length, self)?; - let name = DataDescriptor::new( - name, - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); + + let name = PropertyDescriptor::builder() + .value(name) + .writable(false) + .enumerable(false) + .configurable(true); function.define_property_or_throw("name", name, self)?; Ok(function.into()) @@ -572,8 +582,11 @@ impl Context { self.global_object().insert_property( name, - function, - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + PropertyDescriptor::builder() + .value(function) + .writable(true) + .enumerable(false) + .configurable(true), ); Ok(()) } @@ -603,8 +616,11 @@ impl Context { self.global_object().insert_property( name, - function, - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + PropertyDescriptor::builder() + .value(function) + .writable(true) + .enumerable(false) + .configurable(true), ); Ok(()) } @@ -664,7 +680,11 @@ impl Context { T::init(&mut class_builder)?; let class = class_builder.build(); - let property = DataDescriptor::new(class, T::ATTRIBUTE); + let property = PropertyDescriptor::builder() + .value(class) + .writable(T::ATTRIBUTES.writable()) + .enumerable(T::ATTRIBUTES.enumerable()) + .configurable(T::ATTRIBUTES.configurable()); self.global_object().insert(T::NAME, property); Ok(()) } @@ -673,17 +693,33 @@ impl Context { /// /// # Example /// ``` - /// use boa::{Context, property::Attribute, object::ObjectInitializer}; + /// use boa::{Context, property::{Attribute, PropertyDescriptor}, object::ObjectInitializer}; /// /// let mut context = Context::new(); /// - /// context.register_global_property("myPrimitiveProperty", 10, Attribute::all()); + /// context.register_global_property( + /// "myPrimitiveProperty", + /// 10, + /// Attribute::all() + /// ); /// /// let object = ObjectInitializer::new(&mut context) - /// .property("x", 0, Attribute::all()) - /// .property("y", 1, Attribute::all()) + /// .property( + /// "x", + /// 0, + /// Attribute::all() + /// ) + /// .property( + /// "y", + /// 1, + /// Attribute::all() + /// ) /// .build(); - /// context.register_global_property("myObjectProperty", object, Attribute::all()); + /// context.register_global_property( + /// "myObjectProperty", + /// object, + /// Attribute::all() + /// ); /// ``` #[inline] pub fn register_global_property(&mut self, key: K, value: V, attribute: Attribute) @@ -691,8 +727,14 @@ impl Context { K: Into, V: Into, { - let property = DataDescriptor::new(value, attribute); - self.global_object().insert(key, property); + self.global_object().insert( + key, + PropertyDescriptor::builder() + .value(value) + .writable(attribute.writable()) + .enumerable(attribute.enumerable()) + .configurable(attribute.configurable()), + ); } /// Evaluates the given code. diff --git a/boa/src/environment/global_environment_record.rs b/boa/src/environment/global_environment_record.rs index 15a9a56ac8c..bc8f41d5859 100644 --- a/boa/src/environment/global_environment_record.rs +++ b/boa/src/environment/global_environment_record.rs @@ -16,7 +16,7 @@ use crate::{ }, gc::{Finalize, Trace}, object::GcObject, - property::{Attribute, DataDescriptor}, + property::PropertyDescriptor, Context, Result, Value, }; use gc::{Gc, GcCell}; @@ -66,7 +66,7 @@ impl GlobalEnvironmentRecord { let existing_prop = global_object.get_property(name); match existing_prop { Some(desc) => { - if desc.configurable() { + if desc.expect_configurable() { return false; } true @@ -89,11 +89,11 @@ impl GlobalEnvironmentRecord { let existing_prop = global_object.get_property(name); match existing_prop { Some(prop) => { - if prop.configurable() { - true - } else { - prop.is_data_descriptor() && prop.attributes().writable() && prop.enumerable() - } + prop.expect_configurable() + || prop + .enumerable() + .zip(prop.writable()) + .map_or(false, |(a, b)| a && b) } None => global_object.is_extensible(), } @@ -124,18 +124,18 @@ impl GlobalEnvironmentRecord { pub fn create_global_function_binding(&mut self, name: &str, value: Value, deletion: bool) { let global_object = &mut self.object_record.bindings; let existing_prop = global_object.get_property(name); + // TODO: change to desc.is_undefined() let desc = match existing_prop { - Some(desc) if desc.configurable() => DataDescriptor::new(value, Attribute::empty()), - Some(_) => { - let mut attributes = Attribute::WRITABLE | Attribute::ENUMERABLE; - if deletion { - attributes |= Attribute::CONFIGURABLE; - } - DataDescriptor::new(value, attributes) - } - None => DataDescriptor::new(value, Attribute::empty()), + Some(desc) if desc.expect_configurable() => PropertyDescriptor::builder().value(value), + Some(_) => PropertyDescriptor::builder() + .value(value) + .writable(true) + .enumerable(true) + .configurable(deletion), + None => PropertyDescriptor::builder().value(value), }; + // TODO: fix spec by adding Set and append name to varDeclaredNames global_object .as_object() .expect("global object") diff --git a/boa/src/environment/object_environment_record.rs b/boa/src/environment/object_environment_record.rs index 8add8e3aae1..04b9042ee5d 100644 --- a/boa/src/environment/object_environment_record.rs +++ b/boa/src/environment/object_environment_record.rs @@ -16,7 +16,6 @@ use crate::{ gc::{Finalize, Trace}, object::GcObject, property::PropertyDescriptor, - property::{Attribute, DataDescriptor}, Context, Result, Value, }; @@ -65,11 +64,11 @@ 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 = &self.bindings; - let mut prop = DataDescriptor::new( - Value::undefined(), - Attribute::WRITABLE | Attribute::ENUMERABLE, - ); - prop.set_configurable(deletion); + let prop = PropertyDescriptor::builder() + .value(Value::undefined()) + .writable(true) + .enumerable(true) + .configurable(deletion); bindings.set_property(name, prop); Ok(()) @@ -100,8 +99,10 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { _context: &mut Context, ) -> Result<()> { debug_assert!(value.is_object() || value.is_function()); - let mut property = DataDescriptor::new(value, Attribute::ENUMERABLE); - property.set_configurable(strict); + let property = PropertyDescriptor::builder() + .value(value) + .enumerable(true) + .configurable(strict); self.bindings .as_object() .expect("binding object") @@ -111,10 +112,13 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { fn get_binding_value(&self, name: &str, strict: bool, context: &mut Context) -> Result { if self.bindings.has_field(name) { - match self.bindings.get_property(name) { - Some(PropertyDescriptor::Data(ref d)) => Ok(d.value()), - _ => Ok(Value::undefined()), - } + Ok(self + .bindings + .get_property(name) + .as_ref() + .and_then(|prop| prop.value()) + .cloned() + .unwrap_or(Value::Undefined)) } else if strict { context.throw_reference_error(format!("{} has no binding", name)) } else { diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index 84cad486c8f..1e74ec91738 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -12,7 +12,7 @@ use crate::{ function_environment_record::{BindingStatus, FunctionEnvironmentRecord}, lexical_environment::Environment, }, - property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, + property::{PropertyDescriptor, PropertyKey}, symbol::WellKnownSymbols, syntax::ast::node::RcStatementList, value::PreferredType, @@ -456,114 +456,6 @@ impl GcObject { } } - /// Convert the object to a `PropertyDescriptor` - /// - /// # Panics - /// - /// Panics if the object is currently mutably borrowed. - pub fn to_property_descriptor(&self, context: &mut Context) -> Result { - // 1. If Type(Obj) is not Object, throw a TypeError exception. - // 2. Let desc be a new Property Descriptor that initially has no fields. - - let mut attribute = Attribute::empty(); - - // 3. Let hasEnumerable be ? HasProperty(Obj, "enumerable"). - let has_enumerable = self.has_property("enumerable", context)?; - // 4. If hasEnumerable is true, then - // a. Let enumerable be ! ToBoolean(? Get(Obj, "enumerable")). - // b. Set desc.[[Enumerable]] to enumerable. - if has_enumerable && self.get("enumerable", context)?.to_boolean() { - attribute |= Attribute::ENUMERABLE; - } - - // 5. Let hasConfigurable be ? HasProperty(Obj, "configurable"). - let has_configurable = self.has_property("configurable", context)?; - // 6. If hasConfigurable is true, then - // a. Let configurable be ! ToBoolean(? Get(Obj, "configurable")). - // b. Set desc.[[Configurable]] to configurable. - if has_configurable && self.get("configurable", context)?.to_boolean() { - attribute |= Attribute::CONFIGURABLE; - } - - let mut value = None; - // 7. Let hasValue be ? HasProperty(Obj, "value"). - let has_value = self.has_property("value", context)?; - // 8. If hasValue is true, then - if has_value { - // a. Let value be ? Get(Obj, "value"). - // b. Set desc.[[Value]] to value. - value = Some(self.get("value", context)?); - } - - // 9. Let hasWritable be ? HasProperty(Obj, ). - let has_writable = self.has_property("writable", context)?; - // 10. If hasWritable is true, then - if has_writable { - // a. Let writable be ! ToBoolean(? Get(Obj, "writable")). - if self.get("writable", context)?.to_boolean() { - // b. Set desc.[[Writable]] to writable. - attribute |= Attribute::WRITABLE; - } - } - - // 11. Let hasGet be ? HasProperty(Obj, "get"). - let has_get = self.has_property("get", context)?; - // 12. If hasGet is true, then - let mut get = None; - if has_get { - // a. Let getter be ? Get(Obj, "get"). - let getter = self.get("get", context)?; - // b. If IsCallable(getter) is false and getter is not undefined, throw a TypeError exception. - match getter { - Value::Object(ref object) if object.is_callable() => { - // c. Set desc.[[Get]] to getter. - get = Some(object.clone()); - } - _ => { - return Err( - context.construct_type_error("Property descriptor getter must be callable") - ); - } - } - } - - // 13. Let hasSet be ? HasProperty(Obj, "set"). - let has_set = self.has_property("set", context)?; - // 14. If hasSet is true, then - let mut set = None; - if has_set { - // 14.a. Let setter be ? Get(Obj, "set"). - let setter = self.get("set", context)?; - // 14.b. If IsCallable(setter) is false and setter is not undefined, throw a TypeError exception. - match setter { - Value::Object(ref object) if object.is_callable() => { - // 14.c. Set desc.[[Set]] to setter. - set = Some(object.clone()); - } - _ => { - return Err( - context.construct_type_error("Property descriptor setter must be callable") - ); - } - }; - } - - // 15. If desc.[[Get]] is present or desc.[[Set]] is present, then - // 16. Return desc. - if get.is_some() || set.is_some() { - // 15.a. If desc.[[Value]] is present or desc.[[Writable]] is present, throw a TypeError exception. - if value.is_some() || has_writable { - return Err(context.construct_type_error("Invalid property descriptor. Cannot both specify accessors and a value or writable attribute")); - } - - Ok(AccessorDescriptor::new(get, set, attribute).into()) - } else if let Some(v) = value { - Ok(DataDescriptor::new(v, attribute).into()) - } else { - Ok(DataDescriptor::new_without_value(attribute).into()) - } - } - /// Return `true` if it is a native object and the native type is `T`. /// /// # Panics @@ -912,6 +804,95 @@ impl GcObject { context.throw_type_error("property 'constructor' is not an object") } } + + pub fn to_property_descriptor(&self, context: &mut Context) -> Result { + // 1 is implemented on the method `to_property_descriptor` of value + + // 2. Let desc be a new Property Descriptor that initially has no fields. + let mut desc = PropertyDescriptor::builder(); + + // 3. Let hasEnumerable be ? HasProperty(Obj, "enumerable"). + // 4. If hasEnumerable is true, then ... + if self.has_property("enumerable", context)? { + // a. Let enumerable be ! ToBoolean(? Get(Obj, "enumerable")). + // b. Set desc.[[Enumerable]] to enumerable. + desc = desc.enumerable(self.get("enumerable", context)?.to_boolean()); + } + + // 5. Let hasConfigurable be ? HasProperty(Obj, "configurable"). + // 6. If hasConfigurable is true, then ... + if self.has_property("configurable", context)? { + // a. Let configurable be ! ToBoolean(? Get(Obj, "configurable")). + // b. Set desc.[[Configurable]] to configurable. + desc = desc.configurable(self.get("configurable", context)?.to_boolean()); + } + + // 7. Let hasValue be ? HasProperty(Obj, "value"). + // 8. If hasValue is true, then ... + if self.has_property("value", context)? { + // a. Let value be ? Get(Obj, "value"). + // b. Set desc.[[Value]] to value. + desc = desc.value(self.get("value", context)?); + } + + // 9. Let hasWritable be ? HasProperty(Obj, ). + // 10. If hasWritable is true, then ... + if self.has_property("writable", context)? { + // a. Let writable be ! ToBoolean(? Get(Obj, "writable")). + // b. Set desc.[[Writable]] to writable. + desc = desc.writable(self.get("writable", context)?.to_boolean()); + } + + // 11. Let hasGet be ? HasProperty(Obj, "get"). + // 12. If hasGet is true, then + let get = if self.has_property("get", context)? { + // a. Let getter be ? Get(Obj, "get"). + let getter = self.get("get", context)?; + // b. If IsCallable(getter) is false and getter is not undefined, throw a TypeError exception. + // todo: extract IsCallable to be callable from Value + if !getter.is_undefined() && getter.as_object().map_or(true, |o| !o.is_callable()) { + return Err( + context.construct_type_error("Property descriptor getter must be callable") + ); + } + // c. Set desc.[[Get]] to getter. + Some(getter) + } else { + None + }; + + // 13. Let hasSet be ? HasProperty(Obj, "set"). + // 14. If hasSet is true, then + let set = if self.has_property("set", context)? { + // 14.a. Let setter be ? Get(Obj, "set"). + let setter = self.get("set", context)?; + // 14.b. If IsCallable(setter) is false and setter is not undefined, throw a TypeError exception. + // todo: extract IsCallable to be callable from Value + if !setter.is_undefined() && setter.as_object().map_or(true, |o| !o.is_callable()) { + return Err( + context.construct_type_error("Property descriptor setter must be callable") + ); + } + // 14.c. Set desc.[[Set]] to setter. + Some(setter) + } else { + None + }; + + // 15. If desc.[[Get]] is present or desc.[[Set]] is present, then ... + // a. If desc.[[Value]] is present or desc.[[Writable]] is present, throw a TypeError exception. + if get.as_ref().or_else(|| set.as_ref()).is_some() && desc.inner().is_data_descriptor() { + return Err(context.construct_type_error( + "Invalid property descriptor.\ + Cannot both specify accessors and a value or writable attribute", + )); + } + + desc = desc.maybe_get(get).maybe_set(set); + + // 16. Return desc. + Ok(desc.build()) + } } impl AsRef> for GcObject { diff --git a/boa/src/object/internal_methods.rs b/boa/src/object/internal_methods.rs index c3495ae1f97..8244235c759 100644 --- a/boa/src/object/internal_methods.rs +++ b/boa/src/object/internal_methods.rs @@ -7,7 +7,7 @@ use crate::{ object::{GcObject, Object, ObjectData}, - property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, + property::{DescriptorKind, PropertyDescriptor, PropertyKey}, value::{Type, Value}, BoaProfiler, Context, Result, }; @@ -193,10 +193,11 @@ impl GcObject { // 1. Assert: Type(O) is Object. // 2. Assert: IsPropertyKey(P) is true. // 3. Let newDesc be the PropertyDescriptor { [[Value]]: V, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true }. - let new_desc = DataDescriptor::new( - value, - Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, - ); + let new_desc = PropertyDescriptor::builder() + .value(value) + .writable(true) + .enumerable(true) + .configurable(true); // 4. Return ? O.[[DefineOwnProperty]](P, newDesc). self.__define_own_property__(key.into(), new_desc.into(), context) } @@ -272,7 +273,7 @@ impl GcObject { #[inline] pub(crate) fn __delete__(&self, key: &PropertyKey) -> bool { match self.__get_own_property__(key) { - Some(desc) if desc.configurable() => { + Some(desc) if desc.expect_configurable() => { self.remove(key); true } @@ -297,10 +298,12 @@ impl GcObject { Ok(Value::undefined()) } } - Some(ref desc) => match desc { - PropertyDescriptor::Data(desc) => Ok(desc.value()), - PropertyDescriptor::Accessor(AccessorDescriptor { get: Some(get), .. }) => { - get.call(&receiver, &[], context) + Some(ref desc) => match desc.kind() { + DescriptorKind::Data { + value: Some(value), .. + } => Ok(value.clone()), + DescriptorKind::Accessor { get: Some(get), .. } if !get.is_undefined() => { + context.call(get, &receiver, &[]) } _ => Ok(Value::undefined()), }, @@ -323,43 +326,44 @@ impl GcObject { } else if let Some(ref mut parent) = self.__get_prototype_of__().as_object() { return parent.__set__(key, value, receiver, context); } else { - DataDescriptor::new(Value::undefined(), Attribute::all()).into() + PropertyDescriptor::builder() + .value(Value::undefined()) + .writable(true) + .enumerable(true) + .configurable(true) + .build() }; - match &own_desc { - PropertyDescriptor::Data(desc) => { - if !desc.writable() { + if own_desc.is_data_descriptor() { + if !own_desc.expect_writable() { + return Ok(false); + } + + let receiver = match receiver.as_object() { + Some(obj) => obj, + _ => return Ok(false), + }; + + if let Some(ref existing_desc) = receiver.__get_own_property__(&key) { + if existing_desc.is_accessor_descriptor() { return Ok(false); } - if let Some(ref mut receiver) = receiver.as_object() { - if let Some(ref existing_desc) = receiver.__get_own_property__(&key) { - match existing_desc { - PropertyDescriptor::Accessor(_) => Ok(false), - PropertyDescriptor::Data(existing_data_desc) => { - if !existing_data_desc.writable() { - return Ok(false); - } - receiver.__define_own_property__( - key, - DataDescriptor::new(value, existing_data_desc.attributes()) - .into(), - context, - ) - } - } - } else { - receiver.__define_own_property__( - key, - DataDescriptor::new(value, Attribute::all()).into(), - context, - ) - } - } else { - Ok(false) + if !existing_desc.expect_writable() { + return Ok(false); } + return receiver.__define_own_property__( + key, + PropertyDescriptor::builder().value(value).build(), + context, + ); + } else { + return receiver.create_data_property(key, value, context); } - PropertyDescriptor::Accessor(AccessorDescriptor { set: Some(set), .. }) => { - set.call(&receiver, &[value], context)?; + } + + match own_desc.set() { + Some(set) if !set.is_undefined() => { + context.call(set, &receiver, &[value])?; Ok(true) } _ => Ok(false), @@ -391,102 +395,77 @@ impl GcObject { let extensible = self.__is_extensible__(); - let current = if let Some(desc) = self.__get_own_property__(&key) { - desc + let mut current = if let Some(own) = self.__get_own_property__(&key) { + own } else { if !extensible { return false; } - self.insert(key, desc); + self.insert( + key, + if desc.is_generic_descriptor() || desc.is_data_descriptor() { + desc.into_data_defaulted() + } else { + desc.into_accessor_defaulted() + }, + ); return true; }; + // 3 + if desc.is_empty() { + return true; + } + // 4 - if !current.configurable() { - if desc.configurable() { + if !current.expect_configurable() { + if matches!(desc.configurable(), Some(true)) { return false; } - if desc.enumerable() != current.enumerable() { + if matches!(desc.enumerable(), Some(desc_enum) if desc_enum != current.expect_enumerable()) + { return false; } } - match (¤t, &desc) { - ( - PropertyDescriptor::Data(current), - PropertyDescriptor::Accessor(AccessorDescriptor { get, set, .. }), - ) => { - // 6. b - if !current.configurable() { + // 5 + if desc.is_generic_descriptor() { + // no further validation required + } else if current.is_data_descriptor() != desc.is_data_descriptor() { + if !current.expect_configurable() { + return false; + } + if current.is_data_descriptor() { + current = current.into_accessor_defaulted(); + } else { + current = current.into_data_defaulted(); + } + } else if current.is_data_descriptor() && desc.is_data_descriptor() { + if !current.expect_configurable() && !current.expect_writable() { + if matches!(desc.writable(), Some(true)) { return false; } - - let current = - AccessorDescriptor::new(get.clone(), set.clone(), current.attributes()); - self.insert(key, current); - return true; - } - ( - PropertyDescriptor::Accessor(current), - PropertyDescriptor::Data(DataDescriptor { value, .. }), - ) => { - // 6. c - if !current.configurable() { + if matches!(desc.value(), Some(value) if !Value::same_value(value, current.expect_value())) + { return false; } - - self.insert(key, DataDescriptor::new(value, current.attributes())); - return true; } - (PropertyDescriptor::Data(current), PropertyDescriptor::Data(desc)) => { - // 7. - if !current.configurable() && !current.writable() { - if desc.writable() { - return false; - } - - if !Value::same_value(&desc.value(), ¤t.value()) { - return false; - } - } + } else if !current.expect_configurable() { + if matches!(desc.set(), Some(set) if !Value::same_value(set, current.expect_set())) { + return false; } - (PropertyDescriptor::Accessor(current), PropertyDescriptor::Accessor(desc)) => { - // 8. - if !current.configurable() { - if let (Some(current_get), Some(desc_get)) = (current.getter(), desc.getter()) { - if !GcObject::equals(current_get, desc_get) { - return false; - } - } - - if let (Some(current_set), Some(desc_set)) = (current.setter(), desc.setter()) { - if !GcObject::equals(current_set, desc_set) { - return false; - } - } - } + if matches!(desc.get(), Some(get) if !Value::same_value(get, current.expect_get())) { + return false; } + return true; } - match (¤t, &desc) { - (PropertyDescriptor::Data(current_data), PropertyDescriptor::Data(desc_data)) => { - if desc_data.has_value() { - self.insert(key, desc); - } else { - self.insert( - key, - DataDescriptor::new(current_data.value.clone(), desc_data.attributes()), - ); - } - } - _ => { - self.insert(key, desc); - } - } + current.fill_with(desc); + self.insert(key, current); true } @@ -505,99 +484,95 @@ impl GcObject { ) -> Result { match key { PropertyKey::String(ref s) if s == "length" => { - match desc { - PropertyDescriptor::Accessor(_) => { - return Ok(self.ordinary_define_own_property("length".into(), desc)) - } - PropertyDescriptor::Data(ref d) => { - if d.value().is_undefined() { - return Ok(self.ordinary_define_own_property("length".into(), desc)); - } - let new_len = d.value().to_u32(context)?; - let number_len = d.value().to_number(context)?; - #[allow(clippy::float_cmp)] - if new_len as f64 != number_len { - return Err(context.construct_range_error("bad length for array")); - } - let mut new_len_desc = - PropertyDescriptor::Data(DataDescriptor::new(new_len, d.attributes())); - let old_len_desc = self.__get_own_property__(&"length".into()).unwrap(); - let old_len_desc = old_len_desc.as_data_descriptor().unwrap(); - let old_len = old_len_desc.value(); - if new_len >= old_len.to_u32(context)? { - return Ok( - self.ordinary_define_own_property("length".into(), new_len_desc) + let new_len_val = match desc.value() { + Some(value) => value, + _ => return Ok(self.ordinary_define_own_property("length".into(), desc)), + }; + + let new_len = new_len_val.to_u32(context)?; + let number_len = new_len_val.to_number(context)?; + + #[allow(clippy::float_cmp)] + if new_len as f64 != number_len { + return Err(context.construct_range_error("bad length for array")); + } + + let mut new_len_desc = PropertyDescriptor::builder() + .value(new_len) + .maybe_writable(desc.writable()) + .maybe_enumerable(desc.enumerable()) + .maybe_configurable(desc.configurable()); + let old_len_desc = self.__get_own_property__(&"length".into()).unwrap(); + let old_len = old_len_desc.expect_value(); + if new_len >= old_len.to_u32(context)? { + return Ok( + self.ordinary_define_own_property("length".into(), new_len_desc.build()) + ); + } + + if !old_len_desc.expect_writable() { + return Ok(false); + } + + let new_writable = if new_len_desc.inner().writable().unwrap_or(true) { + true + } else { + new_len_desc = new_len_desc.writable(true); + false + }; + + if !self.ordinary_define_own_property("length".into(), new_len_desc.clone().build()) + { + return Ok(false); + } + + let max_value = self.borrow().index_property_keys().max().copied(); + + if let Some(mut index) = max_value { + while index >= new_len { + let contains_index = self.borrow().indexed_properties.contains_key(&index); + if contains_index && !self.__delete__(&index.into()) { + new_len_desc = new_len_desc.value(index + 1); + if !new_writable { + new_len_desc = new_len_desc.writable(false); + } + self.ordinary_define_own_property( + "length".into(), + new_len_desc.build(), ); - } - if !old_len_desc.writable() { return Ok(false); } - let new_writable = if new_len_desc.attributes().writable() { - true + + index = if let Some(sub) = index.checked_sub(1) { + sub } else { - let mut new_attributes = new_len_desc.attributes(); - new_attributes.set_writable(true); - new_len_desc = PropertyDescriptor::Data(DataDescriptor::new( - new_len, - new_attributes, - )); - false - }; - if !self.ordinary_define_own_property("length".into(), new_len_desc.clone()) - { - return Ok(false); - } - let keys_to_delete = { - let obj = self.borrow(); - let mut keys = obj - .index_property_keys() - .filter(|&&k| k >= new_len) - .cloned() - .collect::>(); - keys.sort_unstable(); - keys - }; - for key in keys_to_delete.into_iter().rev() { - if !self.__delete__(&key.into()) { - let mut new_len_desc_attribute = new_len_desc.attributes(); - if !new_writable { - new_len_desc_attribute.set_writable(false); - } - let new_len_desc = PropertyDescriptor::Data(DataDescriptor::new( - key + 1, - new_len_desc_attribute, - )); - self.ordinary_define_own_property("length".into(), new_len_desc); - return Ok(false); - } - } - if !new_writable { - let mut new_desc_attr = new_len_desc.attributes(); - new_desc_attr.set_writable(false); - let new_desc = PropertyDescriptor::Data(DataDescriptor::new( - new_len, - new_desc_attr, - )); - self.ordinary_define_own_property("length".into(), new_desc); + break; } } } + + if !new_writable { + self.ordinary_define_own_property( + "length".into(), + PropertyDescriptor::builder().writable(false).build(), + ); + } Ok(true) } PropertyKey::Index(index) => { let old_len_desc = self.__get_own_property__(&"length".into()).unwrap(); - let old_len_data_desc = old_len_desc.as_data_descriptor().unwrap(); - let old_len = old_len_data_desc.value().to_u32(context)?; - if index >= old_len && !old_len_data_desc.writable() { + let old_len = old_len_desc.expect_value().to_u32(context)?; + if index >= old_len && !old_len_desc.expect_writable() { return Ok(false); } if self.ordinary_define_own_property(key, desc) { if index >= old_len && index < u32::MAX { - let desc = PropertyDescriptor::Data(DataDescriptor::new( - index + 1, - old_len_data_desc.attributes(), - )); - self.ordinary_define_own_property("length".into(), desc); + let desc = PropertyDescriptor::builder() + .value(index + 1) + .maybe_writable(old_len_desc.writable()) + .maybe_enumerable(old_len_desc.enumerable()) + .maybe_configurable(old_len_desc.configurable()); + self.ordinary_define_own_property("length".into(), desc.into()); } Ok(true) } else { @@ -645,10 +620,12 @@ impl GcObject { .map_or_else(|| Value::from(format!("\\u{:x}", utf16_val)), Value::from) })?; - let desc = PropertyDescriptor::from(DataDescriptor::new( - result_str, - Attribute::READONLY | Attribute::ENUMERABLE | Attribute::PERMANENT, - )); + let desc = PropertyDescriptor::builder() + .value(result_str) + .writable(false) + .enumerable(true) + .configurable(false) + .build(); Some(desc) } @@ -719,8 +696,8 @@ impl GcObject { for next_key in keys { if let Some(prop_desc) = props.__get_own_property__(&next_key) { - if prop_desc.enumerable() { - let desc_obj = props.__get__(&next_key, props.clone().into(), context)?; + if prop_desc.expect_enumerable() { + let desc_obj = props.get(next_key.clone(), context)?; let desc = desc_obj.to_property_descriptor(context)?; descriptors.push((next_key, desc)); } @@ -728,7 +705,7 @@ impl GcObject { } for (p, d) in descriptors { - self.__define_own_property__(p, d, context)?; + self.define_property_or_throw(p, d, context)?; } Ok(()) @@ -811,17 +788,12 @@ impl GcObject { /// If a field was already in the object with the same name that a `Some` is returned /// with that field, otherwise None is returned. #[inline] - pub fn insert_property( - &self, - key: K, - value: V, - attribute: Attribute, - ) -> Option + pub fn insert_property(&self, key: K, property: P) -> Option where K: Into, - V: Into, + P: Into, { - self.insert(key.into(), DataDescriptor::new(value, attribute)) + self.insert(key.into(), property) } /// It determines if Object is a callable function with a `[[Call]]` internal method. @@ -950,16 +922,11 @@ impl Object { /// If a field was already in the object with the same name that a `Some` is returned /// with that field, otherwise None is retuned. #[inline] - pub fn insert_property( - &mut self, - key: K, - value: V, - attribute: Attribute, - ) -> Option + pub fn insert_property(&mut self, key: K, property: P) -> Option where K: Into, - V: Into, + P: Into, { - self.insert(key.into(), DataDescriptor::new(value, attribute)) + self.insert(key, property) } } diff --git a/boa/src/object/mod.rs b/boa/src/object/mod.rs index ec75c87f897..a09a5551909 100644 --- a/boa/src/object/mod.rs +++ b/boa/src/object/mod.rs @@ -14,7 +14,7 @@ use crate::{ }, context::StandardConstructor, gc::{Finalize, Trace}, - property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, + property::{Attribute, PropertyDescriptor, PropertyKey}, BoaProfiler, Context, JsBigInt, JsString, JsSymbol, Value, }; use rustc_hash::FxHashMap; @@ -767,9 +767,12 @@ impl<'context> FunctionBuilder<'context> { .prototype() .into(), ); - let attribute = Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE; - function.insert_property("name", self.name.clone(), attribute); - function.insert_property("length", self.length, attribute); + let property = PropertyDescriptor::builder() + .writable(false) + .enumerable(false) + .configurable(true); + function.insert_property("name", property.clone().value(self.name.clone())); + function.insert_property("length", property.value(self.length)); GcObject::new(function) } @@ -785,9 +788,13 @@ impl<'context> FunctionBuilder<'context> { .prototype() .into(), ); - let attribute = Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE; - object.insert_property("name", self.name.clone(), attribute); - object.insert_property("length", self.length, attribute); + + let property = PropertyDescriptor::builder() + .writable(false) + .enumerable(false) + .configurable(true); + object.insert_property("name", property.clone().value(self.name.clone())); + object.insert_property("length", property.value(self.length)); } } @@ -799,8 +806,16 @@ impl<'context> FunctionBuilder<'context> { /// # use boa::{Context, Value, object::ObjectInitializer, property::Attribute}; /// let mut context = Context::new(); /// let object = ObjectInitializer::new(&mut context) -/// .property("hello", "world", Attribute::all()) -/// .property(1, 1, Attribute::all()) +/// .property( +/// "hello", +/// "world", +/// Attribute::all() +/// ) +/// .property( +/// 1, +/// 1, +/// Attribute::all() +/// ) /// .function(|_, _, _| Ok(Value::undefined()), "func", 0) /// .build(); /// ``` @@ -842,8 +857,11 @@ impl<'context> ObjectInitializer<'context> { self.object.borrow_mut().insert_property( binding.binding, - function, - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + PropertyDescriptor::builder() + .value(function) + .writable(true) + .enumerable(false) + .configurable(true), ); self } @@ -855,7 +873,11 @@ impl<'context> ObjectInitializer<'context> { K: Into, V: Into, { - let property = DataDescriptor::new(value, attribute); + let property = PropertyDescriptor::builder() + .value(value) + .writable(attribute.writable()) + .enumerable(attribute.enumerable()) + .configurable(attribute.configurable()); self.object.borrow_mut().insert(key, property); self } @@ -945,8 +967,11 @@ impl<'context> ConstructorBuilder<'context> { self.prototype.borrow_mut().insert_property( binding.binding, - function, - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + PropertyDescriptor::builder() + .value(function) + .writable(true) + .enumerable(false) + .configurable(true), ); self } @@ -971,8 +996,11 @@ impl<'context> ConstructorBuilder<'context> { self.constructor_object.borrow_mut().insert_property( binding.binding, - function, - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + PropertyDescriptor::builder() + .value(function) + .writable(true) + .enumerable(false) + .configurable(true), ); self } @@ -984,7 +1012,11 @@ impl<'context> ConstructorBuilder<'context> { K: Into, V: Into, { - let property = DataDescriptor::new(value, attribute); + let property = PropertyDescriptor::builder() + .value(value) + .writable(attribute.writable()) + .enumerable(attribute.enumerable()) + .configurable(attribute.configurable()); self.prototype.borrow_mut().insert(key, property); self } @@ -996,7 +1028,11 @@ impl<'context> ConstructorBuilder<'context> { K: Into, V: Into, { - let property = DataDescriptor::new(value, attribute); + let property = PropertyDescriptor::builder() + .value(value) + .writable(attribute.writable()) + .enumerable(attribute.enumerable()) + .configurable(attribute.configurable()); self.constructor_object.borrow_mut().insert(key, property); self } @@ -1013,7 +1049,11 @@ impl<'context> ConstructorBuilder<'context> { where K: Into, { - let property = AccessorDescriptor::new(get, set, attribute); + let property = PropertyDescriptor::builder() + .maybe_get(get) + .maybe_set(set) + .enumerable(attribute.enumerable()) + .configurable(attribute.configurable()); self.prototype.borrow_mut().insert(key, property); self } @@ -1030,7 +1070,11 @@ impl<'context> ConstructorBuilder<'context> { where K: Into, { - let property = AccessorDescriptor::new(get, set, attribute); + let property = PropertyDescriptor::builder() + .maybe_get(get) + .maybe_set(set) + .enumerable(attribute.enumerable()) + .configurable(attribute.configurable()); self.constructor_object.borrow_mut().insert(key, property); self } @@ -1122,14 +1166,16 @@ impl<'context> ConstructorBuilder<'context> { constructable: self.constructable, }; - let length = DataDescriptor::new( - self.length, - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); - let name = DataDescriptor::new( - self.name.clone(), - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, - ); + let length = PropertyDescriptor::builder() + .value(self.length) + .writable(false) + .enumerable(false) + .configurable(true); + let name = PropertyDescriptor::builder() + .value(self.name.clone()) + .writable(false) + .enumerable(false) + .configurable(true); { let mut constructor = self.constructor_object.borrow_mut(); @@ -1147,8 +1193,11 @@ impl<'context> ConstructorBuilder<'context> { constructor.insert_property( PROTOTYPE, - self.prototype.clone(), - Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + PropertyDescriptor::builder() + .value(self.prototype.clone()) + .writable(false) + .enumerable(false) + .configurable(false), ); } @@ -1156,8 +1205,11 @@ impl<'context> ConstructorBuilder<'context> { let mut prototype = self.prototype.borrow_mut(); prototype.insert_property( "constructor", - self.constructor_object.clone(), - Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + PropertyDescriptor::builder() + .value(self.constructor_object.clone()) + .writable(true) + .enumerable(false) + .configurable(true), ); if let Some(proto) = self.inherit.take() { diff --git a/boa/src/property/mod.rs b/boa/src/property/mod.rs index bb40cf088d3..a8943236c75 100644 --- a/boa/src/property/mod.rs +++ b/boa/src/property/mod.rs @@ -16,7 +16,6 @@ use crate::{ gc::{Finalize, Trace}, - object::GcObject, JsString, JsSymbol, Value, }; use std::{convert::TryFrom, fmt}; @@ -24,7 +23,21 @@ use std::{convert::TryFrom, fmt}; mod attribute; pub use attribute::Attribute; -/// A data descriptor is a property that has a value, which may or may not be writable. +/// This represents a JavaScript Property AKA The Property Descriptor. +/// +/// Property descriptors present in objects come in three main flavors: +/// - data descriptors +/// - accessor descriptors +/// - generic descriptor +/// +/// A data Property Descriptor is one that includes any fields named either +/// \[\[Value\]\] or \[\[Writable\]\]. +/// +/// An accessor Property Descriptor is one that includes any fields named either +/// \[\[Get\]\] or \[\[Set\]\]. +/// +/// A generic Property Descriptor is a Property Descriptor value that is neither +/// a data Property Descriptor nor an accessor Property Descriptor. /// /// More information: /// - [MDN documentation][mdn] @@ -32,285 +45,423 @@ pub use attribute::Attribute; /// /// [spec]: https://tc39.es/ecma262/#sec-property-descriptor-specification-type /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty +#[derive(Default, Debug, Clone, Trace, Finalize)] +pub struct PropertyDescriptor { + enumerable: Option, + configurable: Option, + kind: DescriptorKind, +} + #[derive(Debug, Clone, Trace, Finalize)] -pub struct DataDescriptor { - pub(crate) value: Value, - attributes: Attribute, - has_value: bool, +pub enum DescriptorKind { + Data { + value: Option, + writable: Option, + }, + Accessor { + get: Option, + set: Option, + }, + Generic, } -impl DataDescriptor { - /// Create a new `DataDescriptor`. - #[inline] - pub fn new(value: V, attributes: Attribute) -> Self - where - V: Into, - { - Self { - value: value.into(), - attributes, - has_value: true, - } +impl Default for DescriptorKind { + fn default() -> Self { + Self::Generic } +} - /// Create a new `DataDescriptor` without a value. +impl PropertyDescriptor { + /// An accessor Property Descriptor is one that includes any fields named either `[[Get]]` or `[[Set]]`. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-isaccessordescriptor #[inline] - pub fn new_without_value(attributes: Attribute) -> Self { - Self { - value: Value::undefined(), - attributes, - has_value: false, - } + pub fn is_accessor_descriptor(&self) -> bool { + matches!(self.kind, DescriptorKind::Accessor { .. }) } - /// Return the `value` of the data descriptor. + /// A data Property Descriptor is one that includes any fields named either `[[Value]]` or `[[Writable]]`. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-isdatadescriptor #[inline] - pub fn value(&self) -> Value { - self.value.clone() + pub fn is_data_descriptor(&self) -> bool { + matches!(self.kind, DescriptorKind::Data { .. }) } - /// Check whether the data descriptor has a value. + /// A generic Property Descriptor is one that is neither a data descriptor nor an accessor descriptor. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-isgenericdescriptor #[inline] - pub fn has_value(&self) -> bool { - self.has_value + pub fn is_generic_descriptor(&self) -> bool { + matches!(self.kind, DescriptorKind::Generic) } - /// Return the attributes of the descriptor. #[inline] - pub fn attributes(&self) -> Attribute { - self.attributes + pub fn is_empty(&self) -> bool { + self.is_generic_descriptor() && self.enumerable.is_none() && self.configurable.is_none() } - /// Check whether the descriptor is configurable. #[inline] - pub fn configurable(&self) -> bool { - self.attributes.configurable() + pub fn enumerable(&self) -> Option { + self.enumerable } - /// Set whether the descriptor is configurable. #[inline] - pub fn set_configurable(&mut self, configurable: bool) { - self.attributes.set_configurable(configurable) + pub fn configurable(&self) -> Option { + self.configurable } - /// Check whether the descriptor is enumerable. #[inline] - pub fn enumerable(&self) -> bool { - self.attributes.enumerable() + pub fn writable(&self) -> Option { + match self.kind { + DescriptorKind::Data { writable, .. } => writable, + _ => None, + } } - /// Set whether the descriptor is enumerable. #[inline] - pub fn set_enumerable(&mut self, enumerable: bool) { - self.attributes.set_enumerable(enumerable) + pub fn value(&self) -> Option<&Value> { + match &self.kind { + DescriptorKind::Data { value, .. } => value.as_ref(), + _ => None, + } } - /// Check whether the descriptor is writable. #[inline] - pub fn writable(&self) -> bool { - self.attributes.writable() + pub fn get(&self) -> Option<&Value> { + match &self.kind { + DescriptorKind::Accessor { get, .. } => get.as_ref(), + _ => None, + } } - /// Set whether the descriptor is writable. #[inline] - pub fn set_writable(&mut self, writable: bool) { - self.attributes.set_writable(writable) + pub fn set(&self) -> Option<&Value> { + match &self.kind { + DescriptorKind::Accessor { set, .. } => set.as_ref(), + _ => None, + } } -} -impl From for PropertyDescriptor { #[inline] - fn from(value: DataDescriptor) -> Self { - Self::Data(value) + pub fn expect_enumerable(&self) -> bool { + if let Some(enumerable) = self.enumerable { + enumerable + } else { + panic!("[[enumerable]] field not in property descriptor") + } } -} - -/// An accessor descriptor is a property described by a getter-setter pair of functions. -/// -/// More information: -/// - [MDN documentation][mdn] -/// - [ECMAScript reference][spec] -/// -/// [spec]: https://tc39.es/ecma262/#sec-property-descriptor-specification-type -/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty -#[derive(Debug, Clone, Trace, Finalize)] -pub struct AccessorDescriptor { - /// The function serving as getter. - pub(crate) get: Option, - /// The function serving as setter. - pub(crate) set: Option, - /// The attributes of the accessor descriptor. - pub(crate) attributes: Attribute, -} -impl AccessorDescriptor { - /// Create a new `AccessorDescriptor`. - /// - /// If the `attributes` argument contains a `writable` flag, it will be removed so only `enumerable` - /// and `configurable` remains. #[inline] - pub fn new(get: Option, set: Option, mut attributes: Attribute) -> Self { - // Accessors can not have writable attribute. - attributes.remove(Attribute::WRITABLE); - Self { - get, - set, - attributes, + pub fn expect_configurable(&self) -> bool { + if let Some(configurable) = self.configurable { + configurable + } else { + panic!("[[configurable]] field not in property descriptor") } } - /// Return the getter if it exists. #[inline] - pub fn getter(&self) -> Option<&GcObject> { - self.get.as_ref() + pub fn expect_writable(&self) -> bool { + if let Some(writable) = self.writable() { + writable + } else { + panic!("[[writable]] field not in property descriptor") + } } - /// Return the setter if it exists. #[inline] - pub fn setter(&self) -> Option<&GcObject> { - self.set.as_ref() + pub fn expect_value(&self) -> &Value { + if let Some(value) = self.value() { + value + } else { + panic!("[[value]] field not in property descriptor") + } } - /// Set the getter of the accessor descriptor. #[inline] - pub fn set_getter(&mut self, get: Option) { - self.get = get; + pub fn expect_get(&self) -> &Value { + if let Some(get) = self.get() { + get + } else { + panic!("[[get]] field not in property descriptor") + } } - /// Set the setter of the accessor descriptor. #[inline] - pub fn set_setter(&mut self, set: Option) { - self.set = set; + pub fn expect_set(&self) -> &Value { + if let Some(set) = self.set() { + set + } else { + panic!("[[set]] field not in property descriptor") + } } - /// Return the attributes of the accessor descriptor. - /// - /// It is guaranteed to not contain a `writable` flag #[inline] - pub fn attributes(&self) -> Attribute { - self.attributes + pub fn kind(&self) -> &DescriptorKind { + &self.kind } - /// Check whether the descriptor is configurable. #[inline] - pub fn configurable(&self) -> bool { - self.attributes.configurable() + pub fn builder() -> PropertyDescriptorBuilder { + PropertyDescriptorBuilder::new() } - /// Set whether the descriptor is configurable. #[inline] - pub fn set_configurable(&mut self, configurable: bool) { - self.attributes.set_configurable(configurable) + pub fn into_accessor_defaulted(mut self) -> Self { + self.kind = DescriptorKind::Accessor { + get: self.get().cloned(), + set: self.set().cloned(), + }; + PropertyDescriptorBuilder { inner: self } + .complete_with_defaults() + .build() } - /// Check whether the descriptor is enumerable. - #[inline] - pub fn enumerable(&self) -> bool { - self.attributes.enumerable() + pub fn into_data_defaulted(mut self) -> Self { + self.kind = DescriptorKind::Data { + value: self.value().cloned(), + writable: self.writable(), + }; + PropertyDescriptorBuilder { inner: self } + .complete_with_defaults() + .build() } - /// Set whether the descriptor is enumerable. #[inline] - pub fn set_enumerable(&mut self, enumerable: bool) { - self.attributes.set_enumerable(enumerable) + pub fn complete_property_descriptor(self) -> Self { + PropertyDescriptorBuilder { inner: self } + .complete_with_defaults() + .build() } -} -impl From for PropertyDescriptor { #[inline] - fn from(value: AccessorDescriptor) -> Self { - Self::Accessor(value) + pub fn fill_with(&mut self, desc: Self) { + match (&mut self.kind, &desc.kind) { + ( + DescriptorKind::Data { value, writable }, + DescriptorKind::Data { + value: desc_value, + writable: desc_writable, + }, + ) => { + if let Some(desc_value) = desc_value { + *value = Some(desc_value.clone()) + } + if let Some(desc_writable) = desc_writable { + *writable = Some(*desc_writable) + } + } + ( + DescriptorKind::Accessor { get, set }, + DescriptorKind::Accessor { + get: desc_get, + set: desc_set, + }, + ) => { + if let Some(desc_get) = desc_get { + *get = Some(desc_get.clone()) + } + if let Some(desc_set) = desc_set { + *set = Some(desc_set.clone()) + } + } + (_, DescriptorKind::Generic) => {} + _ => panic!("Tried to fill a descriptor with an incompatible descriptor"), + } + + if let Some(enumerable) = desc.enumerable { + self.enumerable = Some(enumerable) + } + if let Some(configurable) = desc.configurable { + self.configurable = Some(configurable) + } } } -/// This represents a JavaScript Property AKA The Property Descriptor. -/// -/// Property descriptors present in objects come in two main flavors: -/// - data descriptors -/// - accessor descriptors -/// -/// A data descriptor is a property that has a value, which may or may not be writable. -/// An accessor descriptor is a property described by a getter-setter pair of functions. -/// A descriptor must be one of these two flavors; it cannot be both. -/// -/// More information: -/// - [MDN documentation][mdn] -/// - [ECMAScript reference][spec] -/// -/// [spec]: https://tc39.es/ecma262/#sec-property-descriptor-specification-type -/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty -#[derive(Debug, Clone, Trace, Finalize)] -pub enum PropertyDescriptor { - Accessor(AccessorDescriptor), - Data(DataDescriptor), +#[derive(Default, Debug, Clone)] +pub struct PropertyDescriptorBuilder { + inner: PropertyDescriptor, } -impl PropertyDescriptor { - /// An accessor Property Descriptor is one that includes any fields named either `[[Get]]` or `[[Set]]`. - /// - /// More information: - /// - [ECMAScript reference][spec] - /// - /// [spec]: https://tc39.es/ecma262/#sec-isaccessordescriptor - #[inline] - pub fn is_accessor_descriptor(&self) -> bool { - matches!(self, Self::Accessor(_)) +impl PropertyDescriptorBuilder { + pub fn new() -> Self { + Self::default() + } + + pub fn value>(mut self, value: V) -> Self { + match self.inner.kind { + DescriptorKind::Data { + value: ref mut v, .. + } => *v = Some(value.into()), + // TODO: maybe panic when trying to convert accessor to data? + _ => { + self.inner.kind = DescriptorKind::Data { + value: Some(value.into()), + writable: None, + } + } + } + self + } + + pub fn writable(mut self, writable: bool) -> Self { + match self.inner.kind { + DescriptorKind::Data { + writable: ref mut w, + .. + } => *w = Some(writable), + // TODO: maybe panic when trying to convert accessor to data? + _ => { + self.inner.kind = DescriptorKind::Data { + value: None, + writable: Some(writable), + } + } + } + self } - /// Return `Some()` if it is a accessor descriptor, `None` otherwise. - #[inline] - pub fn as_accessor_descriptor(&self) -> Option<&AccessorDescriptor> { - match self { - Self::Accessor(ref accessor) => Some(accessor), - _ => None, + pub fn get>(mut self, get: V) -> Self { + match self.inner.kind { + DescriptorKind::Accessor { get: ref mut g, .. } => *g = Some(get.into()), + // TODO: maybe panic when trying to convert data to accessor? + _ => { + self.inner.kind = DescriptorKind::Accessor { + get: Some(get.into()), + set: None, + } + } } + self } - /// A data Property Descriptor is one that includes any fields named either `[[Value]]` or `[[Writable]]`. - /// - /// More information: - /// - [ECMAScript reference][spec] - /// - /// [spec]: https://tc39.es/ecma262/#sec-isdatadescriptor - #[inline] - pub fn is_data_descriptor(&self) -> bool { - matches!(self, Self::Data(_)) + pub fn set>(mut self, set: V) -> Self { + match self.inner.kind { + DescriptorKind::Accessor { set: ref mut s, .. } => *s = Some(set.into()), + // TODO: maybe panic when trying to convert data to accessor? + _ => { + self.inner.kind = DescriptorKind::Accessor { + set: Some(set.into()), + get: None, + } + } + } + self } - /// Return `Some()` if it is a data descriptor, `None` otherwise. - #[inline] - pub fn as_data_descriptor(&self) -> Option<&DataDescriptor> { - match self { - Self::Data(ref data) => Some(data), - _ => None, + pub fn maybe_enumerable(mut self, enumerable: Option) -> Self { + if let Some(enumerable) = enumerable { + self = self.enumerable(enumerable); } + self } - /// Check whether the descriptor is enumerable. - #[inline] - pub fn enumerable(&self) -> bool { - match self { - Self::Accessor(ref accessor) => accessor.enumerable(), - Self::Data(ref data) => data.enumerable(), + pub fn maybe_configurable(mut self, configurable: Option) -> Self { + if let Some(configurable) = configurable { + self = self.configurable(configurable); } + self } - /// Check whether the descriptor is configurable. - #[inline] - pub fn configurable(&self) -> bool { - match self { - Self::Accessor(ref accessor) => accessor.configurable(), - Self::Data(ref data) => data.configurable(), + pub fn maybe_value>(mut self, value: Option) -> Self { + if let Some(value) = value { + self = self.value(value); } + self } - /// Return the attributes of the descriptor. - #[inline] - pub fn attributes(&self) -> Attribute { - match self { - Self::Accessor(ref accessor) => accessor.attributes(), - Self::Data(ref data) => data.attributes(), + pub fn maybe_writable(mut self, writable: Option) -> Self { + if let Some(writable) = writable { + self = self.writable(writable); } + self + } + + pub fn maybe_get>(mut self, get: Option) -> Self { + if let Some(get) = get { + self = self.get(get); + } + self + } + + pub fn maybe_set>(mut self, set: Option) -> Self { + if let Some(set) = set { + self = self.set(set); + } + self + } + + pub fn enumerable(mut self, enumerable: bool) -> Self { + self.inner.enumerable = Some(enumerable); + self + } + pub fn configurable(mut self, configurable: bool) -> Self { + self.inner.configurable = Some(configurable); + self + } + + pub fn complete_with_defaults(mut self) -> Self { + match self.inner.kind { + DescriptorKind::Generic => { + self.inner.kind = DescriptorKind::Data { + value: Some(Value::undefined()), + writable: Some(false), + } + } + DescriptorKind::Data { + ref mut value, + ref mut writable, + } => { + if value.is_none() { + *value = Some(Value::undefined()) + } + if writable.is_none() { + *writable = Some(false) + } + } + DescriptorKind::Accessor { + ref mut set, + ref mut get, + } => { + if set.is_none() { + *set = Some(Value::undefined()) + } + if get.is_none() { + *get = Some(Value::undefined()) + } + } + } + if self.inner.configurable.is_none() { + self.inner.configurable = Some(false); + } + if self.inner.enumerable.is_none() { + self.inner.enumerable = Some(false); + } + self + } + + pub fn inner(&self) -> &PropertyDescriptor { + &self.inner + } + + pub fn build(self) -> PropertyDescriptor { + self.inner + } +} + +impl From for PropertyDescriptor { + fn from(builder: PropertyDescriptorBuilder) -> Self { + builder.build() } } diff --git a/boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs b/boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs index 8e17489b0cf..d1a3e8ab007 100644 --- a/boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs +++ b/boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs @@ -92,7 +92,9 @@ impl Executable for ForInLoop { let for_in_iterator = ForInIterator::create_for_in_iterator(context, Value::from(object)); let next_function = for_in_iterator .get_property("next") - .map(|p| p.as_data_descriptor().unwrap().value()) + .as_ref() + .map(|p| p.expect_value()) + .cloned() .ok_or_else(|| context.construct_type_error("Could not find property `next`"))?; let iterator = IteratorRecord::new(for_in_iterator, next_function); diff --git a/boa/src/syntax/ast/node/object/mod.rs b/boa/src/syntax/ast/node/object/mod.rs index f469a988388..5cc1f8a29e0 100644 --- a/boa/src/syntax/ast/node/object/mod.rs +++ b/boa/src/syntax/ast/node/object/mod.rs @@ -3,7 +3,7 @@ use crate::{ exec::Executable, gc::{Finalize, Trace}, - property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor}, + property::PropertyDescriptor, syntax::ast::node::{join_nodes, MethodDefinitionKind, Node, PropertyDefinition}, BoaProfiler, Context, Result, Value, }; @@ -97,54 +97,52 @@ impl Executable for Object { PropertyDefinition::Property(key, value) => { obj.set_property( key.clone(), - PropertyDescriptor::Data(DataDescriptor::new( - value.run(context)?, - Attribute::all(), - )), + PropertyDescriptor::builder() + .value(value.run(context)?) + .writable(true) + .enumerable(true) + .configurable(true), ); } PropertyDefinition::MethodDefinition(kind, name, func) => match kind { MethodDefinitionKind::Ordinary => { obj.set_property( name.clone(), - PropertyDescriptor::Data(DataDescriptor::new( - func.run(context)?, - Attribute::all(), - )), + PropertyDescriptor::builder() + .value(func.run(context)?) + .writable(true) + .enumerable(true) + .configurable(true), ); } MethodDefinitionKind::Get => { let set = obj .get_property(name.clone()) .as_ref() - .and_then(|p| p.as_accessor_descriptor()) - .and_then(|a| a.setter().cloned()); + .and_then(|a| a.set()) + .cloned(); obj.set_property( name.clone(), - PropertyDescriptor::Accessor(AccessorDescriptor { - get: func.run(context)?.as_object(), - set, - attributes: Attribute::WRITABLE - | Attribute::ENUMERABLE - | Attribute::CONFIGURABLE, - }), + PropertyDescriptor::builder() + .maybe_get(func.run(context)?.as_object()) + .maybe_set(set) + .enumerable(true) + .configurable(true), ) } MethodDefinitionKind::Set => { let get = obj .get_property(name.clone()) .as_ref() - .and_then(|p| p.as_accessor_descriptor()) - .and_then(|a| a.getter().cloned()); + .and_then(|a| a.get()) + .cloned(); obj.set_property( name.clone(), - PropertyDescriptor::Accessor(AccessorDescriptor { - get, - set: func.run(context)?.as_object(), - attributes: Attribute::WRITABLE - | Attribute::ENUMERABLE - | Attribute::CONFIGURABLE, - }), + PropertyDescriptor::builder() + .maybe_get(get) + .maybe_set(func.run(context)?.as_object()) + .enumerable(true) + .configurable(true), ) } }, diff --git a/boa/src/value/conversions.rs b/boa/src/value/conversions.rs index cab3cfc9151..abb4a03185e 100644 --- a/boa/src/value/conversions.rs +++ b/boa/src/value/conversions.rs @@ -146,7 +146,14 @@ where fn from(value: &[T]) -> Self { let mut array = Object::default(); for (i, item) in value.iter().enumerate() { - array.insert(i, DataDescriptor::new(item.clone(), Attribute::all())); + array.insert( + i, + PropertyDescriptor::builder() + .value(item.clone()) + .writable(true) + .enumerable(true) + .configurable(true), + ); } Self::from(array) } @@ -159,7 +166,14 @@ where fn from(value: Vec) -> Self { let mut array = Object::default(); for (i, item) in value.into_iter().enumerate() { - array.insert(i, DataDescriptor::new(item, Attribute::all())); + array.insert( + i, + PropertyDescriptor::builder() + .value(item) + .writable(true) + .enumerable(true) + .configurable(true), + ); } Value::from(array) } diff --git a/boa/src/value/display.rs b/boa/src/value/display.rs index ce7c53e81e5..af18c8a79b9 100644 --- a/boa/src/value/display.rs +++ b/boa/src/value/display.rs @@ -49,10 +49,7 @@ macro_rules! print_obj_value { (props of $obj:expr, $display_fn:ident, $indent:expr, $encounters:expr, $print_internals:expr) => { print_obj_value!(impl $obj, |(key, val)| { if val.is_data_descriptor() { - let v = &val - .as_data_descriptor() - .unwrap() - .value(); + let v = &val.expect_value(); format!( "{:>width$}: {}", key, @@ -60,8 +57,7 @@ macro_rules! print_obj_value { width = $indent, ) } else { - let accessor = val.as_accessor_descriptor().unwrap(); - let display = match (accessor.setter().is_some(), accessor.getter().is_some()) { + let display = match (val.set().is_some(), val.get().is_some()) { (true, true) => "Getter & Setter", (true, false) => "Setter", (false, true) => "Getter", @@ -106,9 +102,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: // TODO: do this in a better way `unwrap` .unwrap() // FIXME: handle accessor descriptors - .as_data_descriptor() - .unwrap() - .value() + .expect_value() .as_number() .map(|n| n as i32) .unwrap_or_default(); @@ -123,10 +117,11 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: // Introduce recursive call to stringify any objects // which are part of the Array log_string_from( - &v.__get_own_property__(&i.into()) + v.__get_own_property__(&i.into()) + .as_ref() // FIXME: handle accessor descriptors - .and_then(|p| p.as_data_descriptor().map(|d| d.value())) - .unwrap_or_default(), + .and_then(|p| p.value()) + .unwrap_or(&Value::Undefined), print_internals, false, ) @@ -204,16 +199,18 @@ pub(crate) fn display_obj(v: &Value, print_internals: bool) -> String { let name = v .get_property("name") .as_ref() - .and_then(|p| p.as_data_descriptor()) - .map(|d| d.value()) - .unwrap_or_else(Value::undefined); + .and_then(|d| d.value()) + .unwrap_or(&Value::Undefined) + .display() + .to_string(); let message = v .get_property("message") .as_ref() - .and_then(|p| p.as_data_descriptor()) - .map(|d| d.value()) - .unwrap_or_else(Value::undefined); - return format!("{}: {}", name.display(), message.display()); + .and_then(|d| d.value()) + .unwrap_or(&Value::Undefined) + .display() + .to_string(); + return format!("{}: {}", name, message); } } diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index 765512db43c..0a6072911e0 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -12,7 +12,7 @@ use crate::{ Number, }, object::{GcObject, Object, ObjectData}, - property::{Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, + property::{PropertyDescriptor, PropertyKey}, symbol::{JsSymbol, WellKnownSymbols}, BoaProfiler, Context, JsBigInt, JsString, Result, }; @@ -190,16 +190,21 @@ impl Value { for (idx, json) in vs.into_iter().enumerate() { new_obj.set_property( idx.to_string(), - DataDescriptor::new( - Self::from_json(json, context), - Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, - ), + PropertyDescriptor::builder() + .value(Self::from_json(json, context)) + .writable(true) + .enumerable(true) + .configurable(true), ); } new_obj.set_property( "length", // TODO: Fix length attribute - DataDescriptor::new(length, Attribute::all()), + PropertyDescriptor::builder() + .value(length) + .writable(true) + .enumerable(true) + .configurable(true), ); new_obj } @@ -209,10 +214,11 @@ impl Value { let value = Self::from_json(json, context); new_obj.set_property( key, - DataDescriptor::new( - value, - Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, - ), + PropertyDescriptor::builder() + .value(value) + .writable(true) + .enumerable(true) + .configurable(true), ); } new_obj @@ -704,9 +710,12 @@ impl Value { )); // Make sure the correct length is set on our new string object object.insert_property( - PropertyKey::String("length".into()), - Value::from(string.encode_utf16().count()), - Attribute::NON_ENUMERABLE, + "length", + PropertyDescriptor::builder() + .value(string.encode_utf16().count()) + .writable(false) + .enumerable(false) + .configurable(false), ); Ok(object) } @@ -925,10 +934,11 @@ impl Value { #[inline] pub fn to_property_descriptor(&self, context: &mut Context) -> Result { - if let Self::Object(ref object) = self { - object.to_property_descriptor(context) - } else { - Err(context.construct_type_error("Property description must be an object")) + // 1. If Type(Obj) is not Object, throw a TypeError exception. + match self { + Value::Object(ref obj) => obj.to_property_descriptor(context), + _ => Err(context + .construct_type_error("Cannot construct a property descriptor from a non-object")), } } diff --git a/boa/src/value/tests.rs b/boa/src/value/tests.rs index 18ac1e7a696..1323aa344e9 100644 --- a/boa/src/value/tests.rs +++ b/boa/src/value/tests.rs @@ -269,7 +269,7 @@ fn string_length_is_not_enumerable() { let length_desc = object .__get_own_property__(&PropertyKey::from("length")) .unwrap(); - assert!(!length_desc.enumerable()); + assert!(!length_desc.expect_enumerable()); } #[test] @@ -283,9 +283,7 @@ fn string_length_is_in_utf16_codeunits() { .unwrap(); assert_eq!( length_desc - .as_data_descriptor() - .unwrap() - .value() + .expect_value() .to_integer_or_infinity(&mut context) .unwrap(), IntegerOrInfinity::Integer(2) diff --git a/boa_tester/src/exec/js262.rs b/boa_tester/src/exec/js262.rs index 67168a311d2..a4c22863502 100644 --- a/boa_tester/src/exec/js262.rs +++ b/boa_tester/src/exec/js262.rs @@ -16,7 +16,7 @@ pub(super) fn init(context: &mut Context) -> GcObject { // .property("agent", agent, Attribute::default()) .build(); - context.register_global_property("$262", obj.clone(), Attribute::default()); + context.register_global_property("$262", obj.clone(), Attribute::empty()); obj }