diff --git a/boa/src/builtins/array/array_iterator.rs b/boa/src/builtins/array/array_iterator.rs index 55593ce96a9..00fe2329d36 100644 --- a/boa/src/builtins/array/array_iterator.rs +++ b/boa/src/builtins/array/array_iterator.rs @@ -1,7 +1,7 @@ use crate::{ builtins::{function::make_builtin_fn, iterable::create_iter_result_object, Array, Value}, object::ObjectData, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, BoaProfiler, Context, Result, }; use gc::{Finalize, Trace}; @@ -131,8 +131,7 @@ impl ArrayIterator { .set_prototype_instance(iterator_prototype); let to_string_tag = ctx.well_known_symbols().to_string_tag_symbol(); - let to_string_tag_property = - Property::data_descriptor(Value::string("Array Iterator"), Attribute::CONFIGURABLE); + let to_string_tag_property = DataDescriptor::new("Array Iterator", Attribute::CONFIGURABLE); array_iterator.set_property(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 eee30daa4f7..999ec1f8c35 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -17,7 +17,7 @@ use crate::{ builtins::array::array_iterator::{ArrayIterationKind, ArrayIterator}, builtins::BuiltIn, object::{ConstructorBuilder, FunctionBuilder, ObjectData, PROTOTYPE}, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, value::{same_value_zero, Value}, BoaProfiler, Context, Result, }; @@ -129,8 +129,8 @@ impl Array { } // finally create length property - let length = Property::data_descriptor( - length.into(), + let length = DataDescriptor::new( + length, Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); @@ -171,8 +171,8 @@ impl Array { } // Create length - let length = Property::data_descriptor( - array_contents.len().into(), + let length = DataDescriptor::new( + array_contents.len(), Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); array_obj_ptr.set_property("length".to_string(), length); diff --git a/boa/src/builtins/date/tests.rs b/boa/src/builtins/date/tests.rs index 0a563fb337c..92a813b55dc 100644 --- a/boa/src/builtins/date/tests.rs +++ b/boa/src/builtins/date/tests.rs @@ -63,12 +63,11 @@ fn date_this_time_value() { let message_property = &error .get_property("message") .expect("Expected 'message' property") - .value; + .as_data_descriptor() + .unwrap() + .value(); - assert_eq!( - &Some(Value::string("\'this\' is not a Date")), - message_property - ); + assert_eq!(Value::string("\'this\' is not a Date"), *message_property); } #[test] diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 365b0ffb8a7..d343a0439d3 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -15,7 +15,7 @@ use crate::{ builtins::{Array, BuiltIn}, environment::lexical_environment::Environment, object::{ConstructorBuilder, FunctionBuilder, Object, ObjectData, PROTOTYPE}, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, syntax::ast::node::{FormalParameter, RcStatementList}, BoaProfiler, Context, Result, Value, }; @@ -174,16 +174,16 @@ pub fn create_unmapped_arguments_object(arguments_list: &[Value]) -> Value { let len = arguments_list.len(); let mut obj = Object::default(); // Set length - let length = Property::data_descriptor( - len.into(), + let length = DataDescriptor::new( + len, Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); // Define length as a property - obj.define_own_property("length", length); + obj.define_own_property("length", length.into()); let mut index: usize = 0; while index < len { let val = arguments_list.get(index).expect("Could not get argument"); - let prop = Property::data_descriptor( + let prop = DataDescriptor::new( val.clone(), Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, ); @@ -222,14 +222,14 @@ pub fn make_constructor_fn( let mut constructor = Object::function(function, global.get_field("Function").get_field(PROTOTYPE)); - let length = Property::data_descriptor( - length.into(), + let length = DataDescriptor::new( + length, Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); constructor.insert("length", length); - let name = Property::data_descriptor( - name.into(), + let name = DataDescriptor::new( + name, Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); constructor.insert("name", name); diff --git a/boa/src/builtins/iterable/mod.rs b/boa/src/builtins/iterable/mod.rs index 3f3ab44209a..1680f0f9f15 100644 --- a/boa/src/builtins/iterable/mod.rs +++ b/boa/src/builtins/iterable/mod.rs @@ -2,7 +2,7 @@ use crate::{ builtins::string::string_iterator::StringIterator, builtins::ArrayIterator, object::{GcObject, ObjectInitializer}, - property::Property, + property::{Attribute, DataDescriptor}, BoaProfiler, Context, Result, Value, }; @@ -47,8 +47,9 @@ impl IteratorPrototypes { /// Generates an object supporting the IteratorResult interface. pub fn create_iter_result_object(ctx: &mut Context, value: Value, done: bool) -> Value { let object = Value::new_object(Some(ctx.global_object())); - let value_property = Property::default().value(value); - let done_property = Property::default().value(Value::boolean(done)); + // TODO: Fix attributes of value and done + let value_property = DataDescriptor::new(value, Attribute::all()); + let done_property = DataDescriptor::new(done, Attribute::all()); object.set_property("value", value_property); object.set_property("done", done_property); object @@ -56,14 +57,15 @@ pub fn create_iter_result_object(ctx: &mut Context, value: Value, done: bool) -> /// Get an iterator record pub fn get_iterator(ctx: &mut Context, iterable: Value) -> Result { + // TODO: Fix the accessor handling let iterator_function = iterable .get_property(ctx.well_known_symbols().iterator_symbol()) - .and_then(|mut p| p.value.take()) + .map(|p| p.as_data_descriptor().unwrap().value()) .ok_or_else(|| ctx.construct_type_error("Not an iterable"))?; let iterator_object = ctx.call(&iterator_function, &iterable, &[])?; let next_function = iterator_object .get_property("next") - .and_then(|mut p| p.value.take()) + .map(|p| p.as_data_descriptor().unwrap().value()) .ok_or_else(|| ctx.construct_type_error("Could not find property `next`"))?; Ok(IteratorRecord::new(iterator_object, next_function)) } @@ -111,14 +113,17 @@ impl IteratorRecord { /// [spec]: https://tc39.es/ecma262/#sec-iteratornext pub(crate) fn next(&self, ctx: &mut Context) -> Result { let next = ctx.call(&self.next_function, &self.iterator_object, &[])?; + // FIXME: handle accessor descriptors let done = next .get_property("done") - .and_then(|mut p| p.value.take()) + .map(|p| p.as_data_descriptor().unwrap().value()) .and_then(|v| v.as_boolean()) .ok_or_else(|| ctx.construct_type_error("Could not find property `done`"))?; + + // FIXME: handle accessor descriptors let next_result = next .get_property("value") - .and_then(|mut p| p.value.take()) + .map(|p| p.as_data_descriptor().unwrap().value()) .unwrap_or_default(); Ok(IteratorResult::new(next_result, done)) } diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index 6bcf0e6b530..c9c9e7543f3 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -16,7 +16,7 @@ use crate::{ builtins::BuiltIn, object::ObjectInitializer, - property::{Attribute, Property, PropertyKey}, + property::{Attribute, DataDescriptor, PropertyKey}, BoaProfiler, Context, Result, Value, }; use serde_json::{self, Value as JSONValue}; @@ -155,16 +155,20 @@ impl Json { let object_to_return = Value::new_object(None); for (key, val) in obj .iter() - .filter_map(|(k, v)| v.value.as_ref().map(|value| (k, value))) + // FIXME: handle accessor descriptors + .map(|(k, v)| (k, v.as_data_descriptor().unwrap().value())) { let this_arg = object.clone(); object_to_return.set_property( key.to_owned(), - Property::default().value(ctx.call( - replacer, - &this_arg, - &[Value::from(key.clone()), val.clone()], - )?), + DataDescriptor::new( + ctx.call( + replacer, + &this_arg, + &[Value::from(key.clone()), val.clone()], + )?, + Attribute::all(), + ), ); } Ok(Value::from(object_to_return.to_json(ctx)?.to_string())) @@ -182,7 +186,8 @@ impl Json { for field in fields { if let Some(value) = object .get_property(field.to_string(ctx)?) - .and_then(|prop| prop.value.as_ref().map(|v| v.to_json(ctx))) + // FIXME: handle accessor descriptors + .map(|prop| prop.as_data_descriptor().unwrap().value().to_json(ctx)) .transpose()? { obj_to_return.insert(field.to_string(ctx)?.to_string(), value); diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index cf925ba40f9..632da4de0e8 100644 --- a/boa/src/builtins/map/mod.rs +++ b/boa/src/builtins/map/mod.rs @@ -3,7 +3,7 @@ use crate::{ builtins::BuiltIn, object::{ConstructorBuilder, ObjectData, PROTOTYPE}, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, BoaProfiler, Context, Result, Value, }; use ordered_map::OrderedMap; @@ -100,8 +100,8 @@ impl Map { /// Helper function to set the size property. fn set_size(this: &Value, size: usize) { - let size = Property::data_descriptor( - size.into(), + let size = DataDescriptor::new( + size, Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); diff --git a/boa/src/builtins/mod.rs b/boa/src/builtins/mod.rs index 594b54f8f25..89096330770 100644 --- a/boa/src/builtins/mod.rs +++ b/boa/src/builtins/mod.rs @@ -43,7 +43,7 @@ pub(crate) use self::{ undefined::Undefined, }; use crate::{ - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, Context, Value, }; @@ -96,7 +96,7 @@ pub fn init(context: &mut Context) { for init in &globals { let (name, value, attribute) = init(context); - let property = Property::data_descriptor(value, attribute); + let property = DataDescriptor::new(value, attribute); global_object.borrow_mut().insert(name, property); } } diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index 011fc2d6074..bf0b031fa9e 100644 --- a/boa/src/builtins/object/mod.rs +++ b/boa/src/builtins/object/mod.rs @@ -16,7 +16,7 @@ use crate::{ builtins::BuiltIn, object::{ConstructorBuilder, Object as BuiltinObject, ObjectData}, - property::{Attribute, Property}, + property::Attribute, value::{same_value, Value}, BoaProfiler, Context, Result, }; @@ -134,13 +134,18 @@ impl Object { } /// Define a property in an object - pub fn define_property(_: &Value, args: &[Value], ctx: &mut Context) -> Result { + pub fn define_property(_: &Value, args: &[Value], context: &mut Context) -> Result { let obj = args.get(0).expect("Cannot get object"); let prop = args .get(1) .expect("Cannot get object") - .to_property_key(ctx)?; - let desc = Property::from(args.get(2).expect("Cannot get object")); + .to_property_key(context)?; + + let desc = if let Value::Object(ref object) = args.get(2).cloned().unwrap_or_default() { + object.to_property_descriptor(context)? + } else { + return context.throw_type_error("Property description must be an object"); + }; obj.set_property(prop, desc); Ok(Value::undefined()) } @@ -246,12 +251,10 @@ impl Object { }; let key = key.to_property_key(ctx)?; - let own_property = this - .to_object(ctx) - .map(|obj| obj.borrow().get_own_property(&key)); + let own_property = this.to_object(ctx)?.borrow().get_own_property(&key); Ok(own_property.map_or(Value::from(false), |own_prop| { - Value::from(own_prop.enumerable_or(false)) + Value::from(own_prop.enumerable()) })) } } diff --git a/boa/src/builtins/regexp/mod.rs b/boa/src/builtins/regexp/mod.rs index 6e6a3e68fb5..eb86947fc75 100644 --- a/boa/src/builtins/regexp/mod.rs +++ b/boa/src/builtins/regexp/mod.rs @@ -13,7 +13,7 @@ use crate::{ builtins::BuiltIn, gc::{empty_trace, Finalize, Trace}, object::{ConstructorBuilder, ObjectData}, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, value::{RcString, Value}, BoaProfiler, Context, Result, }; @@ -370,9 +370,9 @@ impl RegExp { let result = Value::from(result); result.set_property( "index", - Property::default().value(Value::from(m.total().start)), + DataDescriptor::new(m.total().start, Attribute::all()), ); - result.set_property("input", Property::default().value(Value::from(arg_str))); + result.set_property("input", DataDescriptor::new(arg_str, Attribute::all())); result } else { if regex.use_last_index { @@ -473,11 +473,11 @@ impl RegExp { match_val.set_property( "index", - Property::default().value(Value::from(mat.total().start)), + DataDescriptor::new(mat.total().start, Attribute::all()), ); match_val.set_property( "input", - Property::default().value(Value::from(arg_str.clone())), + DataDescriptor::new(arg_str.clone(), Attribute::all()), ); matches.push(match_val); diff --git a/boa/src/builtins/string/string_iterator.rs b/boa/src/builtins/string/string_iterator.rs index c75f6b679ce..1b12d0065ed 100644 --- a/boa/src/builtins/string/string_iterator.rs +++ b/boa/src/builtins/string/string_iterator.rs @@ -2,7 +2,7 @@ use crate::builtins::string::code_point_at; use crate::{ builtins::{function::make_builtin_fn, iterable::create_iter_result_object}, object::ObjectData, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, BoaProfiler, Context, Result, Value, }; use gc::{Finalize, Trace}; @@ -82,7 +82,7 @@ impl StringIterator { let to_string_tag = ctx.well_known_symbols().to_string_tag_symbol(); let to_string_tag_property = - Property::data_descriptor(Value::string("String Iterator"), Attribute::CONFIGURABLE); + DataDescriptor::new("String Iterator", Attribute::CONFIGURABLE); array_iterator.set_property(to_string_tag, to_string_tag_property); array_iterator } diff --git a/boa/src/context.rs b/boa/src/context.rs index 314c37da4a6..e94b1f9eb59 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -10,7 +10,7 @@ use crate::{ class::{Class, ClassBuilder}, exec::Interpreter, object::{GcObject, Object, ObjectData, PROTOTYPE}, - property::{Property, PropertyKey}, + property::{DataDescriptor, PropertyKey}, realm::Realm, syntax::{ ast::{ @@ -608,7 +608,7 @@ impl Context { T::init(&mut class_builder)?; let class = class_builder.build(); - let property = Property::data_descriptor(class.into(), T::ATTRIBUTE); + let property = DataDescriptor::new(class, T::ATTRIBUTE); self.global_object() .as_object_mut() .unwrap() diff --git a/boa/src/environment/global_environment_record.rs b/boa/src/environment/global_environment_record.rs index d0be6cadb61..43695d2bcc7 100644 --- a/boa/src/environment/global_environment_record.rs +++ b/boa/src/environment/global_environment_record.rs @@ -14,7 +14,7 @@ use crate::{ lexical_environment::{Environment, EnvironmentType}, object_environment_record::ObjectEnvironmentRecord, }, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, Value, }; use gc::{Finalize, Trace}; @@ -41,8 +41,8 @@ impl GlobalEnvironmentRecord { let global_object = &self.object_record.bindings; let existing_prop = global_object.get_property(name); match existing_prop { - Some(prop) => { - if prop.value.is_none() || prop.configurable() { + Some(desc) => { + if desc.configurable() { return false; } true @@ -70,21 +70,22 @@ 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); - if let Some(prop) = existing_prop { - if prop.value.is_none() || prop.configurable_or(false) { - let mut property = - Property::data_descriptor(value, Attribute::WRITABLE | Attribute::ENUMERABLE); - property.set_configurable(deletion); - - global_object.update_property(name, property); + 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) } - } else { - let mut property = - Property::data_descriptor(value, Attribute::WRITABLE | Attribute::ENUMERABLE); - property.set_configurable(deletion); + None => DataDescriptor::new(value, Attribute::empty()), + }; - global_object.update_property(name, property); - } + global_object + .as_object_mut() + .expect("global object") + .insert(name, desc); } } diff --git a/boa/src/environment/object_environment_record.rs b/boa/src/environment/object_environment_record.rs index 0741a39f9cf..0983485a2e6 100644 --- a/boa/src/environment/object_environment_record.rs +++ b/boa/src/environment/object_environment_record.rs @@ -11,7 +11,7 @@ use crate::{ environment_record_trait::EnvironmentRecordTrait, lexical_environment::{Environment, EnvironmentType}, }, - property::{Attribute, Property}, + property::{Attribute, DataDescriptor}, Value, }; use gc::{Finalize, Trace}; @@ -39,7 +39,7 @@ 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 = &mut self.bindings; - let mut prop = Property::data_descriptor( + let mut prop = DataDescriptor::new( Value::undefined(), Attribute::WRITABLE | Attribute::ENUMERABLE, ); @@ -63,9 +63,12 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { fn set_mutable_binding(&mut self, name: &str, value: Value, strict: bool) { debug_assert!(value.is_object() || value.is_function()); - let mut property = Property::data_descriptor(value, Attribute::ENUMERABLE); + let mut property = DataDescriptor::new(value, Attribute::ENUMERABLE); property.set_configurable(strict); - self.bindings.update_property(name, property); + self.bindings + .as_object_mut() + .expect("binding object") + .insert(name, property); } fn get_binding_value(&self, name: &str, strict: bool) -> Value { diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index 3776ff12376..18fd06a2ae8 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -10,6 +10,7 @@ use crate::{ environment::{ function_environment_record::BindingStatus, lexical_environment::new_function_environment, }, + property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, syntax::ast::node::RcStatementList, value::PreferredType, Context, Executable, Result, Value, @@ -374,6 +375,81 @@ impl GcObject { Ok(JSONValue::Object(new_obj)) } } + + pub fn to_property_descriptor(&self, context: &mut Context) -> Result { + let mut attribute = Attribute::empty(); + + let enumerable_key = PropertyKey::from("enumerable"); + if self.borrow().has_property(&enumerable_key) + && self.borrow().get(&enumerable_key).to_boolean() + { + attribute |= Attribute::ENUMERABLE; + } + + let configurable_key = PropertyKey::from("configurable"); + if self.borrow().has_property(&configurable_key) + && self.borrow().get(&configurable_key).to_boolean() + { + attribute |= Attribute::CONFIGURABLE; + } + + let mut value = None; + let value_key = PropertyKey::from("value"); + if self.borrow().has_property(&value_key) { + value = Some(self.borrow().get(&value_key)); + } + + let mut has_writable = false; + let writable_key = PropertyKey::from("writable"); + if self.borrow().has_property(&writable_key) { + has_writable = true; + if self.borrow().get(&writable_key).to_boolean() { + attribute |= Attribute::WRITABLE; + } + } + + let mut get = None; + let get_key = PropertyKey::from("get"); + if self.borrow().has_property(&get_key) { + let getter = self.borrow().get(&get_key); + match getter { + Value::Object(ref object) if object.borrow().is_callable() => { + get = Some(object.clone()); + } + _ => { + return Err( + context.construct_type_error("Property descriptor getter must be callable") + ); + } + } + } + + let mut set = None; + let set_key = PropertyKey::from("set"); + if self.borrow().has_property(&set_key) { + let setter = self.borrow().get(&set_key); + match setter { + Value::Object(ref object) if object.borrow().is_callable() => { + set = Some(object.clone()); + } + _ => { + return Err( + context.construct_type_error("Property descriptor setter must be callable") + ); + } + }; + } + + if get.is_some() || set.is_some() { + 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 { + Ok(DataDescriptor::new(value.unwrap_or_else(Value::undefined), attribute).into()) + } + } } impl AsRef> for GcObject { diff --git a/boa/src/object/internal_methods.rs b/boa/src/object/internal_methods.rs index dbb086ac9c0..4da063f3ae5 100644 --- a/boa/src/object/internal_methods.rs +++ b/boa/src/object/internal_methods.rs @@ -6,8 +6,8 @@ //! [spec]: https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots use crate::{ - object::Object, - property::{Attribute, Property, PropertyKey}, + object::{GcObject, Object}, + property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, value::{same_value, Value}, BoaProfiler, Context, Result, }; @@ -19,12 +19,12 @@ impl Object { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-hasproperty-p - pub fn has_property(&self, property_key: &PropertyKey) -> bool { - let prop = self.get_own_property(property_key); + pub fn has_property(&self, key: &PropertyKey) -> bool { + let prop = self.get_own_property(key); if prop.is_none() { let parent = self.get_prototype_of(); return if let Value::Object(ref object) = parent { - object.borrow().has_property(property_key) + object.borrow().has_property(key) } else { false }; @@ -56,92 +56,67 @@ impl Object { } /// Delete property. - pub fn delete(&mut self, property_key: &PropertyKey) -> bool { - let desc = self.get_own_property(property_key); - if desc - .value - .clone() - .expect("unable to get value") - .is_undefined() - { - return true; - } - if desc.configurable_or(false) { - self.remove_property(&property_key); - return true; + pub fn delete(&mut self, key: &PropertyKey) -> bool { + match self.get_own_property(key) { + Some(desc) if desc.configurable() => { + self.remove_property(&key); + true + } + Some(_) => false, + None => true, } - - false } /// [[Get]] /// https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-get-p-receiver - pub fn get(&self, property_key: &PropertyKey) -> Value { - let desc = self.get_own_property(property_key); - if desc.value.clone().is_none() - || desc - .value - .clone() - .expect("Failed to get object") - .is_undefined() - { - // parent will either be null or an Object - let parent = self.get_prototype_of(); - if parent.is_null() { - return Value::undefined(); - } - - return parent.get_field(property_key.clone()); - } - - if desc.is_data_descriptor() { - return desc.value.clone().expect("failed to extract value"); - } + pub fn get(&self, key: &PropertyKey) -> Value { + match self.get_own_property(key) { + None => { + // parent will either be null or an Object + let parent = self.get_prototype_of(); + if parent.is_null() { + return Value::undefined(); + } - let getter = desc.get.clone(); - if getter.is_none() || getter.expect("Failed to get object").is_undefined() { - return Value::undefined(); + parent.get_field(key.clone()) + } + Some(ref desc) => match desc { + PropertyDescriptor::Accessor(_) => todo!(), + PropertyDescriptor::Data(desc) => desc.value(), + }, } - - // TODO: Call getter from here! - Value::undefined() } /// [[Set]] /// - pub fn set(&mut self, property_key: PropertyKey, val: Value) -> bool { + pub fn set(&mut self, key: PropertyKey, val: Value) -> bool { let _timer = BoaProfiler::global().start_event("Object::set", "object"); // Fetch property key - let mut own_desc = self.get_own_property(&property_key); - // [2] - if own_desc.is_none() { + let own_desc = if let Some(desc) = self.get_own_property(&key) { + desc + } else { let parent = self.get_prototype_of(); if !parent.is_null() { // TODO: come back to this } - own_desc = Property::data_descriptor( + DataDescriptor::new( Value::undefined(), Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, - ); - } - // [3] - if own_desc.is_data_descriptor() { - if !own_desc.writable() { - return false; - } + ) + .into() + }; - // Change value on the current descriptor - own_desc = own_desc.value(val); - return self.define_own_property(property_key, own_desc); - } - // [4] - debug_assert!(own_desc.is_accessor_descriptor()); - match own_desc.set { - None => false, - Some(_) => { - unimplemented!(); + match &own_desc { + PropertyDescriptor::Data(desc) => { + if !desc.writable() { + return false; + } + + let desc = DataDescriptor::new(val, own_desc.attributes()).into(); + self.define_own_property(key, desc) } + PropertyDescriptor::Accessor(_) => todo!(), } } @@ -151,102 +126,84 @@ impl Object { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-defineownproperty-p-desc - pub fn define_own_property(&mut self, key: K, desc: Property) -> bool + pub fn define_own_property(&mut self, key: K, desc: PropertyDescriptor) -> bool where K: Into, { let _timer = BoaProfiler::global().start_event("Object::define_own_property", "object"); let key = key.into(); - let mut current = self.get_own_property(&key); let extensible = self.is_extensible(); - // https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor - // There currently isn't a property, lets create a new one - if current.value.is_none() || current.value.as_ref().expect("failed").is_undefined() { + let current = if let Some(desc) = self.get_own_property(&key) { + desc + } else { if !extensible { return false; } self.insert(key, desc); return true; - } - // If every field is absent we don't need to set anything - if desc.is_none() { - return true; - } + }; // 4 - if !current.configurable_or(false) { - if desc.configurable_or(false) { + if !current.configurable() { + if desc.configurable() { return false; } - if desc.enumerable_or(false) != current.enumerable_or(false) { + if desc.enumerable() != current.enumerable() { return false; } } - // 5 - if desc.is_generic_descriptor() { - // 6 - } else if current.is_data_descriptor() != desc.is_data_descriptor() { - // a - if !current.configurable() { - return false; - } - // b - if current.is_data_descriptor() { - // Convert to accessor - current.value = None; - current.attribute.remove(Attribute::WRITABLE); - } else { - // c - // convert to data - current.get = None; - current.set = None; - } + match (¤t, &desc) { + (PropertyDescriptor::Data(current), PropertyDescriptor::Data(desc)) => { + if !current.configurable() && !current.writable() { + if desc.writable() { + return false; + } - self.insert(key, current); - return true; - // 7 - } else if current.is_data_descriptor() && desc.is_data_descriptor() { - // a - if !current.configurable() && !current.writable() { - if desc.writable_or(false) { - return false; + if !same_value(&desc.value(), ¤t.value()) { + return false; + } } - - if desc.value.is_some() - && !same_value( - &desc.value.clone().unwrap(), - ¤t.value.clone().unwrap(), - ) - { + } + (PropertyDescriptor::Data(current), PropertyDescriptor::Accessor(_)) => { + if !current.configurable() { return false; } + let current = AccessorDescriptor::new(None, None, current.attributes()); + self.insert(key, current); return true; } - // 8 - } else { - if !current.configurable() { - if desc.set.is_some() - && !same_value(&desc.set.clone().unwrap(), ¤t.set.clone().unwrap()) - { + (PropertyDescriptor::Accessor(current), PropertyDescriptor::Data(_)) => { + if !current.configurable() { return false; } - if desc.get.is_some() - && !same_value(&desc.get.clone().unwrap(), ¤t.get.clone().unwrap()) - { - return false; + let current = DataDescriptor::new(Value::undefined(), current.attributes()); + self.insert(key, current); + return true; + } + (PropertyDescriptor::Accessor(current), PropertyDescriptor::Accessor(desc)) => { + if !current.configurable() { + if let (Some(current_get), Some(desc_get)) = (current.getter(), desc.getter()) { + if !GcObject::equals(¤t_get, &desc_get) { + return false; + } + } + + if let (Some(current_set), Some(desc_set)) = (current.setter(), desc.setter()) { + if !GcObject::equals(¤t_set, &desc_set) { + return false; + } + } } } - - return true; } - // 9 + self.insert(key, desc); true } @@ -259,27 +216,16 @@ impl Object { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-getownproperty-p - pub fn get_own_property(&self, key: &PropertyKey) -> Property { + pub fn get_own_property(&self, key: &PropertyKey) -> Option { let _timer = BoaProfiler::global().start_event("Object::get_own_property", "object"); - // Prop could either be a String or Symbol let property = match key { PropertyKey::Index(index) => self.indexed_properties.get(&index), PropertyKey::String(ref st) => self.string_properties.get(st), PropertyKey::Symbol(ref symbol) => self.symbol_properties.get(symbol), }; - property.map_or_else(Property::empty, |v| { - let mut d = Property::empty(); - if v.is_data_descriptor() { - d.value = v.value.clone(); - } else { - debug_assert!(v.is_accessor_descriptor()); - d.get = v.get.clone(); - d.set = v.set.clone(); - } - d.attribute = v.attribute; - d - }) + + property.cloned() } /// Essential internal method OwnPropertyKeys @@ -301,14 +247,15 @@ impl Object { pub fn define_properties(&mut self, props: Value, ctx: &mut Context) -> Result<()> { let props = props.to_object(ctx)?; let keys = props.borrow().own_property_keys(); - let mut descriptors: Vec<(PropertyKey, Property)> = Vec::new(); + let mut descriptors: Vec<(PropertyKey, PropertyDescriptor)> = Vec::new(); for next_key in keys { - let prop_desc = props.borrow().get_own_property(&next_key); - if prop_desc.enumerable() { - let desc_obj = props.borrow().get(&next_key); - let desc = Property::from(&desc_obj); - descriptors.push((next_key, desc)); + if let Some(prop_desc) = props.borrow().get_own_property(&next_key) { + if prop_desc.enumerable() { + let desc_obj = props.borrow().get(&next_key); + let desc = desc_obj.to_property_descriptor(ctx)?; + descriptors.push((next_key, desc)); + } } } @@ -319,45 +266,46 @@ impl Object { Ok(()) } - // /// `Object.setPropertyOf(obj, prototype)` - // /// - // /// This method sets the prototype (i.e., the internal `[[Prototype]]` property) - // /// of a specified object to another object or `null`. - // /// - // /// More information: - // /// - [ECMAScript reference][spec] - // /// - [MDN documentation][mdn] - // /// - // /// [spec]: https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-setprototypeof-v - // /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf - // pub fn set_prototype_of(&mut self, val: Value) -> bool { - // debug_assert!(val.is_object() || val.is_null()); - // let current = self.prototype.clone(); - // if same_value(¤t, &val) { - // return true; - // } - // if !self.is_extensible() { - // return false; - // } - // let mut p = val.clone(); - // let mut done = false; - // while !done { - // if p.is_null() { - // done = true - // } else if same_value(&Value::from(self.clone()), &p) { - // return false; - // } else { - // let prototype = p - // .as_object() - // .expect("prototype should be null or object") - // .prototype - // .clone(); - // p = prototype; - // } - // } - // self.prototype = val; - // true - // } + /// `Object.setPropertyOf(obj, prototype)` + /// + /// This method sets the prototype (i.e., the internal `[[Prototype]]` property) + /// of a specified object to another object or `null`. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-setprototypeof-v + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf + pub fn set_prototype_of(&mut self, _val: Value) -> bool { + // debug_assert!(val.is_object() || val.is_null()); + // let current = self.prototype.clone(); + // if same_value(¤t, &val) { + // return true; + // } + // if !self.is_extensible() { + // return false; + // } + // let mut p = val.clone(); + // let mut done = false; + // while !done { + // if p.is_null() { + // done = true + // } else if same_value(&Value::from(self.clone()), &p) { + // return false; + // } else { + // let prototype = p + // .as_object() + // .expect("prototype should be null or object") + // .prototype + // .clone(); + // p = prototype; + // } + // } + // self.prototype = val; + // true + todo!("Object.setPropertyOf(obj, prototype)") + } /// Returns either the prototype or null /// @@ -373,10 +321,12 @@ impl Object { /// Helper function for property insertion. #[inline] - pub(crate) fn insert(&mut self, key: K, property: Property) -> Option + pub(crate) fn insert(&mut self, key: K, property: P) -> Option where K: Into, + P: Into, { + let property = property.into(); match key.into() { PropertyKey::Index(index) => self.indexed_properties.insert(index, property), PropertyKey::String(ref string) => { @@ -390,7 +340,7 @@ impl Object { /// Helper function for property removal. #[inline] - pub(crate) fn remove_property(&mut self, key: &PropertyKey) -> Option { + pub(crate) fn remove_property(&mut self, key: &PropertyKey) -> Option { match key { PropertyKey::Index(index) => self.indexed_properties.remove(&index), PropertyKey::String(ref string) => self.string_properties.remove(string), @@ -408,20 +358,11 @@ impl Object { key: K, value: V, attribute: Attribute, - ) -> Option + ) -> Option where K: Into, V: Into, { - self.insert( - key.into(), - Property::data_descriptor( - value.into(), - attribute - | Attribute::HAS_WRITABLE - | Attribute::HAS_ENUMERABLE - | Attribute::HAS_CONFIGURABLE, - ), - ) + self.insert(key.into(), DataDescriptor::new(value, attribute)) } } diff --git a/boa/src/object/iter.rs b/boa/src/object/iter.rs index 504f88c2945..9eb8cefe04f 100644 --- a/boa/src/object/iter.rs +++ b/boa/src/object/iter.rs @@ -1,4 +1,4 @@ -use super::{Object, Property, PropertyKey}; +use super::{Object, PropertyDescriptor, PropertyKey}; use crate::value::{RcString, RcSymbol}; use std::{collections::hash_map, iter::FusedIterator}; @@ -108,13 +108,13 @@ impl Object { /// An iterator over the property entries of an `Object` #[derive(Debug, Clone)] pub struct Iter<'a> { - indexed_properties: hash_map::Iter<'a, u32, Property>, - string_properties: hash_map::Iter<'a, RcString, Property>, - symbol_properties: hash_map::Iter<'a, RcSymbol, Property>, + indexed_properties: hash_map::Iter<'a, u32, PropertyDescriptor>, + string_properties: hash_map::Iter<'a, RcString, PropertyDescriptor>, + symbol_properties: hash_map::Iter<'a, RcSymbol, PropertyDescriptor>, } impl<'a> Iterator for Iter<'a> { - type Item = (PropertyKey, &'a Property); + type Item = (PropertyKey, &'a PropertyDescriptor); fn next(&mut self) -> Option { if let Some((key, value)) = self.indexed_properties.next() { Some(((*key).into(), value)) @@ -162,7 +162,7 @@ impl FusedIterator for Keys<'_> {} pub struct Values<'a>(Iter<'a>); impl<'a> Iterator for Values<'a> { - type Item = &'a Property; + type Item = &'a PropertyDescriptor; fn next(&mut self) -> Option { let (_, value) = self.0.next()?; Some(value) @@ -180,10 +180,10 @@ impl FusedIterator for Values<'_> {} /// An iterator over the `Symbol` property entries of an `Object` #[derive(Debug, Clone)] -pub struct SymbolProperties<'a>(hash_map::Iter<'a, RcSymbol, Property>); +pub struct SymbolProperties<'a>(hash_map::Iter<'a, RcSymbol, PropertyDescriptor>); impl<'a> Iterator for SymbolProperties<'a> { - type Item = (&'a RcSymbol, &'a Property); + type Item = (&'a RcSymbol, &'a PropertyDescriptor); #[inline] fn next(&mut self) -> Option { @@ -207,7 +207,7 @@ impl FusedIterator for SymbolProperties<'_> {} /// An iterator over the keys (`RcSymbol`) of an `Object`. #[derive(Debug, Clone)] -pub struct SymbolPropertyKeys<'a>(hash_map::Keys<'a, RcSymbol, Property>); +pub struct SymbolPropertyKeys<'a>(hash_map::Keys<'a, RcSymbol, PropertyDescriptor>); impl<'a> Iterator for SymbolPropertyKeys<'a> { type Item = &'a RcSymbol; @@ -234,10 +234,10 @@ impl FusedIterator for SymbolPropertyKeys<'_> {} /// An iterator over the `Symbol` values (`Property`) of an `Object`. #[derive(Debug, Clone)] -pub struct SymbolPropertyValues<'a>(hash_map::Values<'a, RcSymbol, Property>); +pub struct SymbolPropertyValues<'a>(hash_map::Values<'a, RcSymbol, PropertyDescriptor>); impl<'a> Iterator for SymbolPropertyValues<'a> { - type Item = &'a Property; + type Item = &'a PropertyDescriptor; #[inline] fn next(&mut self) -> Option { @@ -261,10 +261,10 @@ impl FusedIterator for SymbolPropertyValues<'_> {} /// An iterator over the indexed property entries of an `Object` #[derive(Debug, Clone)] -pub struct IndexProperties<'a>(hash_map::Iter<'a, u32, Property>); +pub struct IndexProperties<'a>(hash_map::Iter<'a, u32, PropertyDescriptor>); impl<'a> Iterator for IndexProperties<'a> { - type Item = (&'a u32, &'a Property); + type Item = (&'a u32, &'a PropertyDescriptor); #[inline] fn next(&mut self) -> Option { @@ -288,7 +288,7 @@ impl FusedIterator for IndexProperties<'_> {} /// An iterator over the index keys (`u32`) of an `Object`. #[derive(Debug, Clone)] -pub struct IndexPropertyKeys<'a>(hash_map::Keys<'a, u32, Property>); +pub struct IndexPropertyKeys<'a>(hash_map::Keys<'a, u32, PropertyDescriptor>); impl<'a> Iterator for IndexPropertyKeys<'a> { type Item = &'a u32; @@ -315,10 +315,10 @@ impl FusedIterator for IndexPropertyKeys<'_> {} /// An iterator over the index values (`Property`) of an `Object`. #[derive(Debug, Clone)] -pub struct IndexPropertyValues<'a>(hash_map::Values<'a, u32, Property>); +pub struct IndexPropertyValues<'a>(hash_map::Values<'a, u32, PropertyDescriptor>); impl<'a> Iterator for IndexPropertyValues<'a> { - type Item = &'a Property; + type Item = &'a PropertyDescriptor; #[inline] fn next(&mut self) -> Option { @@ -342,10 +342,10 @@ impl FusedIterator for IndexPropertyValues<'_> {} /// An iterator over the `String` property entries of an `Object` #[derive(Debug, Clone)] -pub struct StringProperties<'a>(hash_map::Iter<'a, RcString, Property>); +pub struct StringProperties<'a>(hash_map::Iter<'a, RcString, PropertyDescriptor>); impl<'a> Iterator for StringProperties<'a> { - type Item = (&'a RcString, &'a Property); + type Item = (&'a RcString, &'a PropertyDescriptor); #[inline] fn next(&mut self) -> Option { @@ -369,7 +369,7 @@ impl FusedIterator for StringProperties<'_> {} /// An iterator over the string keys (`RcString`) of an `Object`. #[derive(Debug, Clone)] -pub struct StringPropertyKeys<'a>(hash_map::Keys<'a, RcString, Property>); +pub struct StringPropertyKeys<'a>(hash_map::Keys<'a, RcString, PropertyDescriptor>); impl<'a> Iterator for StringPropertyKeys<'a> { type Item = &'a RcString; @@ -396,10 +396,10 @@ impl FusedIterator for StringPropertyKeys<'_> {} /// An iterator over the string values (`Property`) of an `Object`. #[derive(Debug, Clone)] -pub struct StringPropertyValues<'a>(hash_map::Values<'a, RcString, Property>); +pub struct StringPropertyValues<'a>(hash_map::Values<'a, RcString, PropertyDescriptor>); impl<'a> Iterator for StringPropertyValues<'a> { - type Item = &'a Property; + type Item = &'a PropertyDescriptor; #[inline] fn next(&mut self) -> Option { diff --git a/boa/src/object/mod.rs b/boa/src/object/mod.rs index 655ac5161ef..65b72cd8eea 100644 --- a/boa/src/object/mod.rs +++ b/boa/src/object/mod.rs @@ -10,7 +10,7 @@ use crate::{ }, context::StandardConstructor, gc::{Finalize, Trace}, - property::{Attribute, Property, PropertyKey}, + property::{Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, value::{RcBigInt, RcString, RcSymbol, Value}, BoaProfiler, Context, }; @@ -59,11 +59,11 @@ impl NativeObject for T { pub struct Object { /// The type of the object. pub data: ObjectData, - indexed_properties: FxHashMap, + indexed_properties: FxHashMap, /// Properties - string_properties: FxHashMap, + string_properties: FxHashMap, /// Symbol Properties - symbol_properties: FxHashMap, + symbol_properties: FxHashMap, /// Instance prototype `__proto__`. prototype: Value, /// Whether it can have new properties added to it. @@ -760,7 +760,7 @@ impl<'context> ObjectInitializer<'context> { K: Into, V: Into, { - let property = Property::data_descriptor(value.into(), attribute); + let property = DataDescriptor::new(value, attribute); self.object.borrow_mut().insert(key, property); self } @@ -891,7 +891,7 @@ impl<'context> ConstructorBuilder<'context> { K: Into, V: Into, { - let property = Property::data_descriptor(value.into(), attribute); + let property = DataDescriptor::new(value, attribute); self.prototype.borrow_mut().insert(key, property); self } @@ -903,7 +903,7 @@ impl<'context> ConstructorBuilder<'context> { K: Into, V: Into, { - let property = Property::data_descriptor(value.into(), attribute); + let property = DataDescriptor::new(value, attribute); self.constructor_object.borrow_mut().insert(key, property); self } @@ -971,15 +971,12 @@ impl<'context> ConstructorBuilder<'context> { FunctionFlags::from_parameters(self.callable, self.constructable), ); - let length = Property::data_descriptor( - self.length.into(), + let length = DataDescriptor::new( + self.length, Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); - let name = Property::data_descriptor( - self.name - .take() - .unwrap_or_else(|| String::from("[object]")) - .into(), + let name = DataDescriptor::new( + self.name.take().unwrap_or_else(|| String::from("[object]")), Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, ); diff --git a/boa/src/property/attribute/mod.rs b/boa/src/property/attribute/mod.rs index 476e262fd4f..d6c00c4241e 100644 --- a/boa/src/property/attribute/mod.rs +++ b/boa/src/property/attribute/mod.rs @@ -10,40 +10,31 @@ bitflags! { /// This struct constains the property flags as describen in the ECMAScript specification. /// /// It contains the following flags: - /// - `[[Writable]]` (`WRITABLE`) - If `false`, attempts by ECMAScript code to change the property's `[[Value]]` attribute using `[[Set]]` will not succeed. + /// - `[[Writable]]` (`WRITABLE`) - If `false`, attempts by ECMAScript code to change the property's + /// `[[Value]]` attribute using `[[Set]]` will not succeed. /// - `[[Enumerable]]` (`ENUMERABLE`) - If the property will be enumerated by a for-in enumeration. - /// - `[[Configurable]]` (`CONFIGURABLE`) - If `false`, attempts to delete the property, change the property to be an `accessor property`, or change its attributes (other than `[[Value]]`, or changing `[[Writable]]` to `false`) will fail. - /// - /// Additionaly there are flags for when the flags are defined. + /// - `[[Configurable]]` (`CONFIGURABLE`) - If `false`, attempts to delete the property, + /// change the property to be an `accessor property`, or change its attributes (other than `[[Value]]`, + /// or changing `[[Writable]]` to `false`) will fail. #[derive(Finalize)] pub struct Attribute: u8 { /// The `Writable` attribute decides whether the value associated with the property can be changed or not, from its initial value. - const WRITABLE = 0b0000_0011; - - /// The property is not writable. - const READONLY = 0b0000_0010; - - /// Is the `Writable` flag defined. - const HAS_WRITABLE = 0b0000_0010; + const WRITABLE = 0b0000_0001; /// If the property can be enumerated by a `for-in` loop. - const ENUMERABLE = 0b0000_1100; - - /// The property can not be enumerated in a `for-in` loop. - const NON_ENUMERABLE = 0b0000_1000; - - /// Is the `Enumerable` flag defined. - const HAS_ENUMERABLE = 0b0000_1000; + const ENUMERABLE = 0b0000_0010; /// If the property descriptor can be changed later. - const CONFIGURABLE = 0b0011_0000; + const CONFIGURABLE = 0b0000_0100; - /// The property descriptor cannot be changed. - const PERMANENT = 0b0010_0000; + /// The property is not writable. + const READONLY = 0b0000_0000; - /// Is the `Configurable` flag defined. - const HAS_CONFIGURABLE = 0b0010_0000; + /// The property can not be enumerated in a `for-in` loop. + const NON_ENUMERABLE = 0b0000_0000; + /// The property descriptor cannot be changed. + const PERMANENT = 0b0000_0000; } } @@ -63,12 +54,6 @@ impl Attribute { self.bits = 0; } - /// Is the `writable` flag defined. - #[inline] - pub fn has_writable(self) -> bool { - self.contains(Self::HAS_WRITABLE) - } - /// Sets the `writable` flag. #[inline] pub fn set_writable(&mut self, value: bool) { @@ -82,16 +67,9 @@ impl Attribute { /// Gets the `writable` flag. #[inline] pub fn writable(self) -> bool { - debug_assert!(self.has_writable()); self.contains(Self::WRITABLE) } - /// Is the `enumerable` flag defined. - #[inline] - pub fn has_enumerable(self) -> bool { - self.contains(Self::HAS_ENUMERABLE) - } - /// Sets the `enumerable` flag. #[inline] pub fn set_enumerable(&mut self, value: bool) { @@ -105,16 +83,9 @@ impl Attribute { /// Gets the `enumerable` flag. #[inline] pub fn enumerable(self) -> bool { - debug_assert!(self.has_enumerable()); self.contains(Self::ENUMERABLE) } - /// Is the `configurable` flag defined. - #[inline] - pub fn has_configurable(self) -> bool { - self.contains(Self::HAS_CONFIGURABLE) - } - /// Sets the `configurable` flag. #[inline] pub fn set_configurable(&mut self, value: bool) { @@ -128,7 +99,6 @@ impl Attribute { /// Gets the `configurable` flag. #[inline] pub fn configurable(self) -> bool { - debug_assert!(self.has_configurable()); self.contains(Self::CONFIGURABLE) } } diff --git a/boa/src/property/attribute/tests.rs b/boa/src/property/attribute/tests.rs index e9ced879177..64f8d174318 100644 --- a/boa/src/property/attribute/tests.rs +++ b/boa/src/property/attribute/tests.rs @@ -4,7 +4,6 @@ use super::Attribute; fn writable() { let attribute = Attribute::WRITABLE; - assert!(attribute.has_writable()); assert!(attribute.writable()); } @@ -12,7 +11,6 @@ fn writable() { fn enumerable() { let attribute = Attribute::ENUMERABLE; - assert!(attribute.has_enumerable()); assert!(attribute.enumerable()); } @@ -20,7 +18,6 @@ fn enumerable() { fn configurable() { let attribute = Attribute::CONFIGURABLE; - assert!(attribute.has_configurable()); assert!(attribute.configurable()); } @@ -28,23 +25,17 @@ fn configurable() { fn writable_and_enumerable() { let attribute = Attribute::WRITABLE | Attribute::ENUMERABLE; - assert!(attribute.has_writable()); assert!(attribute.writable()); - assert!(attribute.has_enumerable()); assert!(attribute.enumerable()); - - assert!(!attribute.has_configurable()); } #[test] fn enumerable_configurable() { let attribute = Attribute::ENUMERABLE | Attribute::CONFIGURABLE; - assert!(!attribute.has_writable()); + assert!(!attribute.writable()); - assert!(attribute.has_enumerable()); assert!(attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(attribute.configurable()); } @@ -52,11 +43,8 @@ fn enumerable_configurable() { fn writable_enumerable_configurable() { let attribute = Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE; - assert!(attribute.has_writable()); assert!(attribute.writable()); - assert!(attribute.has_enumerable()); assert!(attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(attribute.configurable()); } @@ -64,9 +52,9 @@ fn writable_enumerable_configurable() { fn default() { let attribute = Attribute::default(); - assert!(attribute.has_writable()); - assert!(attribute.has_enumerable()); - assert!(attribute.has_configurable()); + assert!(!attribute.writable()); + assert!(!attribute.enumerable()); + assert!(!attribute.configurable()); } #[test] @@ -75,9 +63,9 @@ fn clear() { attribute.clear(); - assert!(!attribute.has_writable()); - assert!(!attribute.has_enumerable()); - assert!(!attribute.has_configurable()); + assert!(!attribute.writable()); + assert!(!attribute.enumerable()); + assert!(!attribute.configurable()); assert!(attribute.is_empty()); } @@ -88,11 +76,8 @@ fn set_writable_to_true() { attribute.set_writable(true); - assert!(attribute.has_writable()); assert!(attribute.writable()); - assert!(attribute.has_enumerable()); assert!(!attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(!attribute.configurable()); } @@ -102,11 +87,8 @@ fn set_writable_to_false() { attribute.set_writable(false); - assert!(attribute.has_writable()); assert!(!attribute.writable()); - assert!(attribute.has_enumerable()); assert!(!attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(!attribute.configurable()); } @@ -116,11 +98,8 @@ fn set_enumerable_to_true() { attribute.set_enumerable(true); - assert!(attribute.has_writable()); assert!(!attribute.writable()); - assert!(attribute.has_enumerable()); assert!(attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(!attribute.configurable()); } @@ -130,11 +109,8 @@ fn set_enumerable_to_false() { attribute.set_enumerable(false); - assert!(attribute.has_writable()); assert!(!attribute.writable()); - assert!(attribute.has_enumerable()); assert!(!attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(!attribute.configurable()); } @@ -144,11 +120,8 @@ fn set_configurable_to_true() { attribute.set_configurable(true); - assert!(attribute.has_writable()); assert!(!attribute.writable()); - assert!(attribute.has_enumerable()); assert!(!attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(attribute.configurable()); } @@ -158,10 +131,7 @@ fn set_configurable_to_false() { attribute.set_configurable(false); - assert!(attribute.has_writable()); assert!(!attribute.writable()); - assert!(attribute.has_enumerable()); assert!(!attribute.enumerable()); - assert!(attribute.has_configurable()); assert!(!attribute.configurable()); } diff --git a/boa/src/property/mod.rs b/boa/src/property/mod.rs index 17e36ad3075..5dac93aa4eb 100644 --- a/boa/src/property/mod.rs +++ b/boa/src/property/mod.rs @@ -14,26 +14,17 @@ //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty //! [section]: https://tc39.es/ecma262/#sec-property-attributes -use crate::value::{RcString, RcSymbol, Value}; -use gc::{Finalize, Trace}; -use std::convert::TryFrom; -use std::fmt; +use crate::{ + gc::{Finalize, Trace}, + object::GcObject, + value::{RcString, RcSymbol, Value}, +}; +use std::{convert::TryFrom, fmt}; mod attribute; - pub use attribute::Attribute; -/// 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. -/// -/// Any field in a JavaScript Property may be present or absent. /// /// More information: /// - [MDN documentation][mdn] @@ -41,145 +32,202 @@ 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(Trace, Finalize, Clone, Debug)] -pub struct Property { - pub(crate) attribute: Attribute, - /// The value associated with the property - pub value: Option, - /// The function serving as getter - pub get: Option, - /// The function serving as setter - pub set: Option, +#[derive(Debug, Clone, Trace, Finalize)] +pub struct DataDescriptor { + value: Value, + attributes: Attribute, } -impl Property { - /// Make a new property with the given value - /// The difference between New and Default: - /// - /// New: zeros everything to make an empty object - /// Default: Defaults according to the spec +impl DataDescriptor { + /// Create a new `DataDescriptor`. #[inline] - pub fn new() -> Self { + pub fn new(value: V, attributes: Attribute) -> Self + where + V: Into, + { Self { - attribute: Default::default(), - value: None, - get: None, - set: None, + value: value.into(), + attributes, } } + /// Return the `value` of the data descriptor. #[inline] - pub fn empty() -> Self { - Self { - attribute: Attribute::empty(), - value: None, - get: None, - set: None, - } + pub fn value(&self) -> Value { + self.value.clone() } + /// Return the attributes of the descriptor. #[inline] - pub fn data_descriptor(value: Value, attribute: Attribute) -> Self { - Self { - attribute, - value: Some(value), - get: None, - set: None, - } + pub fn attributes(&self) -> Attribute { + self.attributes } - /// Get the + /// Check whether the descriptor is configurable. #[inline] pub fn configurable(&self) -> bool { - self.attribute.configurable() + self.attributes.configurable() } + /// Set whether the descriptor is configurable. #[inline] pub fn set_configurable(&mut self, configurable: bool) { - self.attribute.set_configurable(configurable) + self.attributes.set_configurable(configurable) } + /// Check whether the descriptor is configurable. #[inline] - pub fn configurable_or(&self, value: bool) -> bool { - if self.attribute.has_configurable() { - self.attribute.configurable() - } else { - value - } + pub fn enumerable(&self) -> bool { + self.attributes.enumerable() } - /// Set enumerable + /// Set whether the descriptor is enumerable. #[inline] - pub fn enumerable(&self) -> bool { - self.attribute.enumerable() + pub fn set_enumerable(&mut self, enumerable: bool) { + self.attributes.set_enumerable(enumerable) } + /// Check whether the descriptor is writable. #[inline] - pub fn enumerable_or(&self, value: bool) -> bool { - if self.attribute.has_enumerable() { - self.attribute.enumerable() - } else { - value - } + pub fn writable(&self) -> bool { + self.attributes.writable() } - /// Set writable + /// Set whether the descriptor is writable. #[inline] - pub fn writable(&self) -> bool { - self.attribute.writable() + pub fn set_writable(&mut self, writable: bool) { + self.attributes.set_writable(writable) } +} +impl From for PropertyDescriptor { #[inline] - pub fn writable_or(&self, value: bool) -> bool { - if self.attribute.has_writable() { - self.attribute.writable() - } else { - value - } + fn from(value: DataDescriptor) -> Self { + Self::Data(value) } +} + +/// 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. + get: Option, + /// The function serving as setter. + set: Option, + /// The attributes of the accessor descriptor. + attributes: Attribute, +} - /// Set value +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 value(mut self, value: Value) -> Self { - self.value = Some(value); - self + 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, + } } - /// Set get + /// Return the getter if it exists. #[inline] - pub fn get(mut self, get: Value) -> Self { - self.get = Some(get); - self + pub fn getter(&self) -> Option<&GcObject> { + self.get.as_ref() } + /// Return the setter if it exists. #[inline] - pub fn has_get(&self) -> bool { - self.get.is_some() + pub fn setter(&self) -> Option<&GcObject> { + self.set.as_ref() } - /// Set set + /// Set the getter of the accessor descriptor. #[inline] - pub fn set(mut self, set: Value) -> Self { - self.set = Some(set); - self + pub fn set_getter(&mut self, get: Option) { + self.get = get; } + /// Set the setter of the accessor descriptor. #[inline] - pub fn has_set(&self) -> bool { - self.set.is_some() + pub fn set_setter(&mut self, set: Option) { + self.set = set; } - /// Is this an empty Property? + /// Return the attributes of the accessor descriptor. /// - /// `true` if all fields are set to none + /// It is guaranteed to not contain a `writable` flag #[inline] - pub fn is_none(&self) -> bool { - self.value.is_none() - && self.attribute.is_empty() - && self.get.is_none() - && self.set.is_none() + pub fn attributes(&self) -> Attribute { + self.attributes } + /// Check whether the descriptor is configurable. + #[inline] + pub fn configurable(&self) -> bool { + self.attributes.configurable() + } + + /// Set whether the descriptor is configurable. + #[inline] + pub fn set_configurable(&mut self, configurable: bool) { + self.attributes.set_configurable(configurable) + } + + /// Check whether the descriptor is enumerable. + #[inline] + pub fn enumerable(&self) -> bool { + self.attributes.enumerable() + } + + /// Set whether the descriptor is enumerable. + #[inline] + pub fn set_enumerable(&mut self, enumerable: bool) { + self.attributes.set_enumerable(enumerable) + } +} + +impl From for PropertyDescriptor { + #[inline] + fn from(value: AccessorDescriptor) -> Self { + Self::Accessor(value) + } +} + +/// 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), +} + +impl PropertyDescriptor { /// An accessor Property Descriptor is one that includes any fields named either [[Get]] or [[Set]]. /// /// More information: @@ -188,7 +236,16 @@ impl Property { /// [spec]: https://tc39.es/ecma262/#sec-isaccessordescriptor #[inline] pub fn is_accessor_descriptor(&self) -> bool { - self.get.is_some() || self.set.is_some() + matches!(self, Self::Accessor(_)) + } + + /// 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, + } } /// A data Property Descriptor is one that includes any fields named either [[Value]] or [[Writable]]. @@ -199,82 +256,42 @@ impl Property { /// [spec]: https://tc39.es/ecma262/#sec-isdatadescriptor #[inline] pub fn is_data_descriptor(&self) -> bool { - self.value.is_some() || self.attribute.has_writable() + matches!(self, Self::Data(_)) } - /// Check if a property is generic descriptor. - /// - /// More information: - /// - [ECMAScript reference][spec] - /// - /// [spec]: https://tc39.es/ecma262/#sec-isgenericdescriptor + /// Return `Some()` if it is a data descriptor, `None` otherwise. #[inline] - pub fn is_generic_descriptor(&self) -> bool { - !self.is_accessor_descriptor() && !self.is_data_descriptor() + pub fn as_data_descriptor(&self) -> Option<&DataDescriptor> { + match self { + Self::Data(ref data) => Some(data), + _ => None, + } } -} -impl Default for Property { - /// Make a default property - /// - /// More information: - /// - [ECMAScript reference][spec] - /// - /// [spec]: https://tc39.es/ecma262/#table-default-attribute-values + /// Check whether the descriptor is enumerable. #[inline] - fn default() -> Self { - Self::new() - } -} - -impl From<&Property> for Value { - fn from(value: &Property) -> Value { - let property = Value::new_object(None); - if value.attribute.has_writable() { - property.set_field("writable", value.attribute.writable()); - } - - if value.attribute.has_enumerable() { - property.set_field("enumerable", value.attribute.enumerable()); - } - - if value.attribute.has_configurable() { - property.set_field("configurable", value.attribute.configurable()); + pub fn enumerable(&self) -> bool { + match self { + Self::Accessor(ref accessor) => accessor.enumerable(), + Self::Data(ref data) => data.enumerable(), } - - property.set_field("value", value.value.clone().unwrap_or_else(Value::null)); - property.set_field("get", value.get.clone().unwrap_or_else(Value::null)); - property.set_field("set", value.set.clone().unwrap_or_else(Value::null)); - property } -} - -impl<'a> From<&'a Value> for Property { - /// Attempt to fetch values "configurable", "enumerable", "writable" from the value, - /// if they're not there default to false - fn from(value: &Value) -> Self { - let mut attribute = Attribute::empty(); - - let writable = value.get_field("writable"); - if !writable.is_undefined() { - attribute.set_writable(bool::from(&writable)); - } - - let enumerable = value.get_field("enumerable"); - if !enumerable.is_undefined() { - attribute.set_enumerable(bool::from(&enumerable)); - } - let configurable = value.get_field("configurable"); - if !configurable.is_undefined() { - attribute.set_configurable(bool::from(&configurable)); + /// 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(), } + } - Self { - attribute, - value: Some(value.get_field("value")), - get: Some(value.get_field("get")), - set: Some(value.get_field("set")), + /// 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(), } } } diff --git a/boa/src/value/conversions.rs b/boa/src/value/conversions.rs index ca4dfaf1439..1a1ae39d442 100644 --- a/boa/src/value/conversions.rs +++ b/boa/src/value/conversions.rs @@ -127,7 +127,7 @@ where fn from(value: &[T]) -> Self { let mut array = Object::default(); for (i, item) in value.iter().enumerate() { - array.insert(i, Property::default().value(item.clone().into())); + array.insert(i, DataDescriptor::new(item.clone(), Attribute::all())); } Self::from(array) } @@ -140,7 +140,7 @@ where fn from(value: Vec) -> Self { let mut array = Object::default(); for (i, item) in value.into_iter().enumerate() { - array.insert(i, Property::default().value(item.into())); + array.insert(i, DataDescriptor::new(item, Attribute::all())); } Value::from(array) } diff --git a/boa/src/value/display.rs b/boa/src/value/display.rs index 9fc535faaf6..4e62b494a21 100644 --- a/boa/src/value/display.rs +++ b/boa/src/value/display.rs @@ -49,9 +49,10 @@ 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)| { let v = &val - .value - .as_ref() - .expect("Could not get the property's value"); + // FIXME: handle accessor descriptors + .as_data_descriptor() + .unwrap() + .value(); format!( "{:>width$}: {}", @@ -94,9 +95,12 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: let len = v .borrow() .get_own_property(&PropertyKey::from("length")) - .value - .clone() - .expect("Could not borrow value") + // TODO: do this in a better way `unwrap` + .unwrap() + // FIXME: handle accessor descriptors + .as_data_descriptor() + .unwrap() + .value() .as_number() .unwrap() as i32; @@ -112,9 +116,12 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: log_string_from( &v.borrow() .get_own_property(&i.into()) - .value - .clone() - .expect("Could not borrow value"), + // TODO: do this in a better way "unwrap" + .unwrap() + // FIXME: handle accessor descriptors + .as_data_descriptor() + .unwrap() + .value(), print_internals, false, ) @@ -131,9 +138,12 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: let size = v .borrow() .get_own_property(&PropertyKey::from("size")) - .value - .clone() - .expect("Could not borrow value") + // TODO: do this in a better way "unwrap" + .unwrap() + // FIXME: handle accessor descriptors + .as_data_descriptor() + .unwrap() + .value() .as_number() .unwrap() as i32; if size == 0 { diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index 154f1042511..132e6fec8b0 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -11,7 +11,7 @@ use crate::{ BigInt, Number, }, object::{GcObject, Object, ObjectData, PROTOTYPE}, - property::{Attribute, Property, PropertyKey}, + property::{Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, BoaProfiler, Context, Result, }; use gc::{Finalize, GcCellRef, GcCellRefMut, Trace}; @@ -183,15 +183,16 @@ impl Value { for (idx, json) in vs.into_iter().enumerate() { new_obj.set_property( idx.to_string(), - Property::data_descriptor( + DataDescriptor::new( Self::from_json(json, context), Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, ), ); } new_obj.set_property( - "length".to_string(), - Property::default().value(Self::from(length)), + "length", + // TODO: Fix length attribute + DataDescriptor::new(length, Attribute::all()), ); new_obj } @@ -201,7 +202,7 @@ impl Value { let value = Self::from_json(json, context); new_obj.set_property( key, - Property::data_descriptor( + DataDescriptor::new( value, Attribute::WRITABLE | Attribute::ENUMERABLE | Attribute::CONFIGURABLE, ), @@ -441,7 +442,7 @@ impl Value { /// Resolve the property in the object. /// /// A copy of the Property is returned. - pub fn get_property(&self, key: Key) -> Option + pub fn get_property(&self, key: Key) -> Option where Key: Into, { @@ -451,8 +452,8 @@ impl Value { Self::Object(ref object) => { let object = object.borrow(); let property = object.get_own_property(&key); - if !property.is_none() { - return Some(property); + if property.is_some() { + return property; } object.prototype_instance().get_property(key) @@ -461,18 +462,6 @@ impl Value { } } - /// update_prop will overwrite individual [Property] fields, unlike - /// Set_prop, which will overwrite prop with a new Property - /// - /// Mostly used internally for now - pub(crate) fn update_property(&self, field: &str, new_property: Property) { - let _timer = BoaProfiler::global().start_event("Value::update_property", "value"); - - if let Some(ref mut object) = self.as_object_mut() { - object.insert(field, new_property); - } - } - /// Resolve the property in the object and get its value, or undefined if this is not an object or the field doesn't exist /// get_field recieves a Property from get_prop(). It should then return the [[Get]] result value if that's set, otherwise fall back to [[Value]] /// TODO: this function should use the get Value if its set @@ -483,24 +472,10 @@ impl Value { let _timer = BoaProfiler::global().start_event("Value::get_field", "value"); let key = key.into(); match self.get_property(key) { - Some(prop) => { - // If the Property has [[Get]] set to a function, we should run that and return the Value - let prop_getter = match prop.get { - Some(_) => None, - None => None, - }; - - // If the getter is populated, use that. If not use [[Value]] instead - if let Some(val) = prop_getter { - val - } else { - let val = prop - .value - .as_ref() - .expect("Could not get property as reference"); - val.clone() - } - } + Some(ref desc) => match desc { + PropertyDescriptor::Accessor(_) => todo!(), + PropertyDescriptor::Data(desc) => desc.value(), + }, None => Value::undefined(), } } @@ -550,14 +525,15 @@ impl Value { } /// Set the property in the value. - pub fn set_property(&self, key: K, property: Property) -> Property + #[inline] + pub fn set_property(&self, key: K, property: P) where K: Into, + P: Into, { if let Some(mut object) = self.as_object_mut() { - object.insert(key.into(), property.clone()); + object.insert(key.into(), property.into()); } - property } /// The abstract operation ToPrimitive takes an input argument and an optional argument PreferredType. @@ -884,6 +860,15 @@ impl Value { Ok(self) } } + + #[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")) + } + } } impl Default for Value { diff --git a/boa/src/value/tests.rs b/boa/src/value/tests.rs index a39fcd61a82..afef1df0a06 100644 --- a/boa/src/value/tests.rs +++ b/boa/src/value/tests.rs @@ -1,3 +1,5 @@ +#![allow(clippy::float_cmp)] + use super::*; use crate::{forward, forward_val, Context}; @@ -557,7 +559,7 @@ mod cyclic_conversions { let value = forward_val(&mut engine, src).unwrap(); let result = value.as_number().unwrap(); - assert_eq!(result, 0.); + assert_eq!(result, 0.0); } #[test]