From 6c0998bf023068be9c6a905164efaf171196d66f Mon Sep 17 00:00:00 2001 From: tofpie Date: Sun, 3 Jan 2021 13:48:51 +0100 Subject: [PATCH 1/5] Implement Reflect built-in object --- boa/src/builtins/function/mod.rs | 7 +- boa/src/builtins/mod.rs | 3 + boa/src/builtins/reflect/mod.rs | 374 +++++++++++++++++++++++++++++ boa/src/builtins/reflect/tests.rs | 191 +++++++++++++++ boa/src/context.rs | 50 +--- boa/src/object/gcobject.rs | 2 +- boa/src/object/internal_methods.rs | 40 ++- 7 files changed, 613 insertions(+), 54 deletions(-) create mode 100644 boa/src/builtins/reflect/mod.rs create mode 100644 boa/src/builtins/reflect/tests.rs diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index c63a81940a5..cb1af64abc6 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -308,9 +308,10 @@ impl BuiltInFunctionObject { // TODO?: 3.a. PrepareForTailCall return context.call(this, &this_arg, &[]); } - let arg_list = context - .extract_array_properties(&arg_array)? - .map_err(|()| arg_array)?; + let arg_array = arg_array.as_object().ok_or_else(|| { + context.construct_type_error("argList must be null, undefined or an object") + })?; + let arg_list = arg_array.create_list_from_array_like(&[], context)?; // TODO?: 5. PrepareForTailCall context.call(this, &this_arg, &arg_list) } diff --git a/boa/src/builtins/mod.rs b/boa/src/builtins/mod.rs index 79d19fc87b3..2e1699d4463 100644 --- a/boa/src/builtins/mod.rs +++ b/boa/src/builtins/mod.rs @@ -17,6 +17,7 @@ pub mod math; pub mod nan; pub mod number; pub mod object; +pub mod reflect; pub mod regexp; pub mod string; pub mod symbol; @@ -39,6 +40,7 @@ pub(crate) use self::{ number::Number, object::for_in_iterator::ForInIterator, object::Object as BuiltInObjectObject, + reflect::Reflect, regexp::RegExp, string::String, symbol::Symbol, @@ -86,6 +88,7 @@ pub fn init(context: &mut Context) { SyntaxError::init, EvalError::init, UriError::init, + Reflect::init, #[cfg(feature = "console")] console::Console::init, ]; diff --git a/boa/src/builtins/reflect/mod.rs b/boa/src/builtins/reflect/mod.rs new file mode 100644 index 00000000000..2a535a550fc --- /dev/null +++ b/boa/src/builtins/reflect/mod.rs @@ -0,0 +1,374 @@ +//! This module implements the global `Reflect` object. +//! +//! The `Reflect` global object is a built-in object that provides methods for interceptable +//! JavaScript operations. +//! +//! More information: +//! - [ECMAScript reference][spec] +//! - [MDN documentation][mdn] +//! +//! [spec]: https://tc39.es/ecma262/#sec-reflect-object +//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect + +use crate::{ + builtins::{self, BuiltIn}, + object::{Object, ObjectData, ObjectInitializer}, + property::Attribute, + BoaProfiler, Context, Result, Value, +}; + +#[cfg(test)] +mod tests; + +/// Javascript `Reflect` object. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct Reflect; + +impl BuiltIn for Reflect { + const NAME: &'static str = "Reflect"; + + fn attribute() -> Attribute { + Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE + } + + fn init(context: &mut Context) -> (&'static str, Value, Attribute) { + let _timer = BoaProfiler::global().start_event(Self::NAME, "init"); + + let to_string_tag = context.well_known_symbols().to_string_tag_symbol(); + + let object = ObjectInitializer::new(context) + .function(Self::apply, "apply", 3) + .function(Self::construct, "construct", 2) + .function(Self::define_property, "defineProperty", 3) + .function(Self::delete_property, "deleteProperty", 2) + .function(Self::get, "get", 2) + .function( + Self::get_own_property_descriptor, + "getOwnPropertyDescriptor", + 2, + ) + .function(Self::get_prototype_of, "getPrototypeOf", 1) + .function(Self::has, "has", 2) + .function(Self::is_extensible, "isExtensible", 1) + .function(Self::own_keys, "ownKeys", 1) + .function(Self::prevent_extensions, "preventExtensions", 1) + .function(Self::set, "set", 3) + .function(Self::set_prototype_of, "setPrototypeOf", 3) + .property( + to_string_tag, + Reflect::NAME, + Attribute::READONLY | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, + ) + .build(); + (Self::NAME, object.into(), Self::attribute()) + } +} + +impl Reflect { + /// Calls a target function with arguments. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.apply + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/apply + pub(crate) fn apply(_: &Value, args: &[Value], context: &mut Context) -> Result { + let undefined = Value::undefined(); + let target = args + .get(0) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("target must be a function"))?; + let this_arg = args.get(1).unwrap_or(&undefined); + let args_list = args + .get(2) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("args list must be an object"))?; + + if !target.is_callable() { + return Err(context.construct_type_error("target must be a function")); + } + let args = args_list.create_list_from_array_like(&[], context)?; + target.call(this_arg, &args, context) + } + + /// Calls a target function as a constructor with arguments. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.construct + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/construct + pub(crate) fn construct(_: &Value, args: &[Value], context: &mut Context) -> Result { + let mut target = args + .get(0) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("target must be a function"))?; + let args_list = args + .get(1) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("args list must be an object"))?; + + if let Some(new_target) = args.get(2) { + target = new_target + .as_object() + .ok_or_else(|| context.construct_type_error("newTarget must be an object"))?; + } + + if !target.is_constructable() { + return Err(context.construct_type_error("target must be a constructor")); + } + let args = args_list.create_list_from_array_like(&[], context)?; + target.construct(&args, context) + } + + /// Defines a property on an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.defineProperty + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/defineProperty + pub(crate) fn define_property( + _: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { + let undefined = Value::undefined(); + let mut target = args + .get(0) + .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 + .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)?; + + Ok(target.define_own_property(key, prop_desc).into()) + } + + /// Defines a property on an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.deleteproperty + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/deleteProperty + pub(crate) fn delete_property( + _: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { + let undefined = Value::undefined(); + let mut target = args + .get(0) + .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)?; + + Ok(target.delete(&key).into()) + } + + /// Gets a property of an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.get + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/get + pub(crate) fn get(_: &Value, args: &[Value], context: &mut Context) -> Result { + let undefined = Value::undefined(); + let mut target = args + .get(0) + .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)?; + if let Some(receiver) = args.get(2) { + target = receiver + .as_object() + .ok_or_else(|| context.construct_type_error("receiver must be an object"))?; + } + target.get(&key, context) + } + + /// Gets a property of an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.getownpropertydescriptor + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/getOwnPropertyDescriptor + pub(crate) fn get_own_property_descriptor( + _: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { + match args.get(0) { + Some(v) if v.is_object() => (), + _ => return context.throw_type_error("target must be an object"), + } + + builtins::object::Object::get_own_property_descriptor(&Value::undefined(), args, context) + } + + /// Gets the prototype of an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.getprototypeof + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/getPrototypeOf + pub(crate) fn get_prototype_of( + _: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { + match args.get(0) { + Some(v) if v.is_object() => (), + _ => return context.throw_type_error("target must be an object"), + } + builtins::object::Object::get_prototype_of(&Value::undefined(), args, context) + } + + /// Returns `true` if the object has the property, `false` otherwise. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.has + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/has + pub(crate) fn has(_: &Value, args: &[Value], context: &mut Context) -> Result { + let target = args + .get(0) + .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(&Value::undefined()) + .to_property_key(context)?; + Ok(target.has_property(&key).into()) + } + + /// Returns `true` if the object is extensible, `false` otherwise. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.isextensible + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/isExtensible + pub(crate) fn is_extensible(_: &Value, args: &[Value], context: &mut Context) -> Result { + let target = args + .get(0) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("target must be an object"))?; + Ok(target.is_extensible().into()) + } + + /// Returns an array of object own property keys. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.ownkeys + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/ownKeys + pub(crate) fn own_keys(_: &Value, args: &[Value], context: &mut Context) -> Result { + let target = args + .get(0) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("target must be an object"))?; + let array_prototype = context.standard_objects().array_object().prototype(); + let result: Value = + Object::with_prototype(array_prototype.into(), ObjectData::Array).into(); + result.set_field("length", 0, context)?; + + let keys = target.own_property_keys(); + for (i, k) in keys.iter().enumerate() { + result.set_field(i, k, context)?; + } + + Ok(result) + } + + /// Prevents new properties from ever being added to an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.preventextensions + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/preventExtensions + pub(crate) fn prevent_extensions( + _: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { + let mut target = args + .get(0) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("target must be an object"))?; + + Ok(target.prevent_extensions().into()) + } + + /// Sets a property of an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.set + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/set + pub(crate) fn set(_: &Value, args: &[Value], context: &mut Context) -> Result { + let undefined = Value::undefined(); + let mut target = args + .get(0) + .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 value = args.get(2).unwrap_or(&undefined); + if let Some(receiver) = args.get(3) { + target = receiver + .as_object() + .ok_or_else(|| context.construct_type_error("receiver must be an object"))?; + } + Ok(target.set(key, value.clone(), context)?.into()) + } + + /// Sets the prototype of an object. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-reflect.setprototypeof + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/setPrototypeOf + pub(crate) fn set_prototype_of( + _: &Value, + args: &[Value], + context: &mut Context, + ) -> Result { + let mut target = args + .get(0) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("target must be an object"))?; + let proto = args + .get(1) + .ok_or_else(|| context.construct_type_error("proto cannot be undefined"))?; + if !proto.is_null() && !proto.is_object() { + return Err(context.construct_type_error("proto must be an object or null")); + } + target.set_prototype_instance(proto.clone()); + Ok(Value::Boolean(true)) + } +} diff --git a/boa/src/builtins/reflect/tests.rs b/boa/src/builtins/reflect/tests.rs new file mode 100644 index 00000000000..a5b892efb7a --- /dev/null +++ b/boa/src/builtins/reflect/tests.rs @@ -0,0 +1,191 @@ +use crate::{forward, Context}; + +#[test] +fn apply() { + let mut context = Context::new(); + + let init = r#" + var called = {}; + function f(n) { called.result = n }; + Reflect.apply(f, undefined, [42]); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "called.result"), "42"); +} + +#[test] +fn construct() { + let mut context = Context::new(); + + let init = r#" + var called = {}; + function f(n) { called.result = n }; + Reflect.construct(f, [42]); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "called.result"), "42"); +} + +#[test] +fn define_property() { + let mut context = Context::new(); + + let init = r#" + let obj = {}; + Reflect.defineProperty(obj, 'p', { value: 42 }); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "obj.p"), "42"); +} + +#[test] +fn delete_property() { + let mut context = Context::new(); + + let init = r#" + let obj = { p: 42 }; + let deleted = Reflect.deleteProperty(obj, 'p'); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "obj.p"), "undefined"); + assert_eq!(forward(&mut context, "deleted"), "true"); +} + +#[test] +fn get() { + let mut context = Context::new(); + + let init = r#" + let obj = { p: 42 } + let p = Reflect.get(obj, 'p'); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "p"), "42"); +} + +#[test] +fn get_own_property_descriptor() { + let mut context = Context::new(); + + let init = r#" + let obj = { p: 42 }; + let desc = Reflect.getOwnPropertyDescriptor(obj, 'p'); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "desc.value"), "42"); +} + +#[test] +fn get_prototype_of() { + let mut context = Context::new(); + + let init = r#" + function F() { this.p = 42 }; + let f = new F(); + let proto = Reflect.getPrototypeOf(f); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "proto.constructor.name"), "\"F\""); +} + +#[test] +fn has() { + let mut context = Context::new(); + + let init = r#" + let obj = { p: 42 }; + let hasP = Reflect.has(obj, 'p'); + let hasP2 = Reflect.has(obj, 'p2'); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "hasP"), "true"); + assert_eq!(forward(&mut context, "hasP2"), "false"); +} + +#[test] +fn is_extensible() { + let mut context = Context::new(); + + let init = r#" + let obj = { p: 42 }; + let isExtensible = Reflect.isExtensible(obj); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "isExtensible"), "true"); +} + +#[test] +fn own_keys() { + let mut context = Context::new(); + + let init = r#" + let obj = { p: 42 }; + let ownKeys = Reflect.ownKeys(obj); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "ownKeys"), r#"[ "p" ]"#); +} + +#[test] +fn prevent_extensions() { + let mut context = Context::new(); + + let init = r#" + let obj = { p: 42 }; + let r = Reflect.preventExtensions(obj); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "r"), "true"); +} + +#[test] +fn set() { + let mut context = Context::new(); + + let init = r#" + let obj = {}; + Reflect.set(obj, 'p', 42); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "obj.p"), "42"); +} + +#[test] +fn set_prototype_of() { + let mut context = Context::new(); + + let init = r#" + function F() { this.p = 42 }; + let obj = {} + Reflect.setPrototypeOf(obj, F); + let p = Reflect.getPrototypeOf(obj); + "#; + + forward(&mut context, init); + + assert_eq!(forward(&mut context, "p.name"), "\"F\""); +} diff --git a/boa/src/context.rs b/boa/src/context.rs index 0c835132d89..2efe8b83d78 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -9,7 +9,7 @@ use crate::{ }, class::{Class, ClassBuilder}, exec::Interpreter, - object::{GcObject, Object, ObjectData, PROTOTYPE}, + object::{GcObject, Object, PROTOTYPE}, property::{Attribute, DataDescriptor, PropertyKey}, realm::Realm, syntax::{ @@ -25,7 +25,6 @@ use crate::{ value::{RcString, RcSymbol, Value}, BoaProfiler, Executable, Result, }; -use std::result::Result as StdResult; #[cfg(feature = "console")] use crate::builtins::console::Console; @@ -555,53 +554,6 @@ impl Context { Ok(()) } - /// Converts an array object into a rust vector of values. - /// - /// This is useful for the spread operator, for any other object an `Err` is returned - /// TODO: Not needed for spread of arrays. Check in the future for Map and remove if necessary - pub(crate) fn extract_array_properties( - &mut self, - value: &Value, - ) -> Result, ()>> { - if let Value::Object(ref x) = value { - // Check if object is array - if let ObjectData::Array = x.borrow().data { - let length = value.get_field("length", self)?.as_number().unwrap() as i32; - let values = (0..length) - .map(|idx| value.get_field(idx, self)) - .collect::>>()?; - return Ok(Ok(values)); - } - // Check if object is a Map - else if let ObjectData::Map(ref map) = x.borrow().data { - let values = map - .iter() - .map(|(key, value)| { - // Construct a new array containing the key-value pair - let array = Value::new_object(self); - array.set_data(ObjectData::Array); - array.as_object().expect("object").set_prototype_instance( - self.realm() - .environment - .get_binding_value("Array") - .expect("Array was not initialized") - .get_field(PROTOTYPE, self)?, - ); - array.set_field(0, key, self)?; - array.set_field(1, value, self)?; - array.set_field("length", Value::from(2), self)?; - Ok(array) - }) - .collect::>>()?; - return Ok(Ok(values)); - } - - return Ok(Err(())); - } - - Ok(Err(())) - } - /// #[inline] pub(crate) fn has_property(&self, obj: &Value, key: &PropertyKey) -> bool { diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index 9a49c8f18c0..dee50f826ab 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -399,7 +399,7 @@ impl GcObject { } } - /// Convert the object to a `PropertyDescritptor` + /// Convert the object to a `PropertyDescriptor` /// /// # Panics /// diff --git a/boa/src/object/internal_methods.rs b/boa/src/object/internal_methods.rs index 56b0505ff0d..c51d5d92ed7 100644 --- a/boa/src/object/internal_methods.rs +++ b/boa/src/object/internal_methods.rs @@ -8,7 +8,7 @@ use crate::{ object::{GcObject, Object}, property::{AccessorDescriptor, Attribute, DataDescriptor, PropertyDescriptor, PropertyKey}, - value::{same_value, Value}, + value::{same_value, Type, Value}, BoaProfiler, Context, Result, }; @@ -395,6 +395,44 @@ impl GcObject { pub fn is_constructable(&self) -> bool { self.borrow().is_constructable() } + + /// It is used to create List value whose elements are provided by the indexed properties of + /// self. + /// + /// More information: + /// - [EcmaScript reference][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-createlistfromarraylike + pub(crate) fn create_list_from_array_like( + &self, + element_types: &[Type], + context: &mut Context, + ) -> Result> { + let types = if element_types.is_empty() { + &[ + Type::Undefined, + Type::Null, + Type::Boolean, + Type::String, + Type::Symbol, + Type::Number, + Type::BigInt, + Type::Symbol, + ] + } else { + element_types + }; + let len = self.get(&"length".into(), context)?.to_length(context)?; + let mut list = Vec::with_capacity(len); + for index in 0..len { + let next = self.get(&index.into(), context)?; + if !types.contains(&next.get_type()) { + return Err(context.construct_type_error("bad type")); + } + list.push(next.clone()); + } + Ok(list) + } } impl Object { From 62af9c5d42ff6729b4214c0408ff214395c2585c Mon Sep 17 00:00:00 2001 From: tofpie Date: Sun, 3 Jan 2021 20:46:35 +0100 Subject: [PATCH 2/5] Implement suggestions from review --- boa/src/builtins/reflect/mod.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/boa/src/builtins/reflect/mod.rs b/boa/src/builtins/reflect/mod.rs index 2a535a550fc..019ec9cace3 100644 --- a/boa/src/builtins/reflect/mod.rs +++ b/boa/src/builtins/reflect/mod.rs @@ -214,7 +214,8 @@ impl Reflect { Some(v) if v.is_object() => (), _ => return context.throw_type_error("target must be an object"), } - + // This function is the same as Object.prototype.getOwnPropertyDescriptor, that why + // it is invoked here. builtins::object::Object::get_own_property_descriptor(&Value::undefined(), args, context) } @@ -231,11 +232,11 @@ impl Reflect { args: &[Value], context: &mut Context, ) -> Result { - match args.get(0) { - Some(v) if v.is_object() => (), - _ => return context.throw_type_error("target must be an object"), - } - builtins::object::Object::get_prototype_of(&Value::undefined(), args, context) + let target = args + .get(0) + .and_then(|v| v.as_object()) + .ok_or_else(|| context.construct_type_error("target must be an object"))?; + Ok(target.get_prototype_of()) } /// Returns `true` if the object has the property, `false` otherwise. @@ -358,17 +359,15 @@ impl Reflect { args: &[Value], context: &mut Context, ) -> Result { + let undefined = Value::undefined(); let mut target = args .get(0) .and_then(|v| v.as_object()) .ok_or_else(|| context.construct_type_error("target must be an object"))?; - let proto = args - .get(1) - .ok_or_else(|| context.construct_type_error("proto cannot be undefined"))?; + let proto = args.get(1).unwrap_or(&undefined); if !proto.is_null() && !proto.is_object() { return Err(context.construct_type_error("proto must be an object or null")); } - target.set_prototype_instance(proto.clone()); - Ok(Value::Boolean(true)) + Ok(target.set_prototype_instance(proto.clone()).into()) } } From bd487dcd5650edd634d6b108f6b912ce65d64f02 Mon Sep 17 00:00:00 2001 From: tofpie Date: Mon, 4 Jan 2021 23:34:29 +0100 Subject: [PATCH 3/5] Implement suggestions from review --- boa/src/builtins/reflect/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boa/src/builtins/reflect/mod.rs b/boa/src/builtins/reflect/mod.rs index 019ec9cace3..66db4e29135 100644 --- a/boa/src/builtins/reflect/mod.rs +++ b/boa/src/builtins/reflect/mod.rs @@ -86,7 +86,7 @@ impl Reflect { .ok_or_else(|| context.construct_type_error("args list must be an object"))?; if !target.is_callable() { - return Err(context.construct_type_error("target must be a function")); + return context.throw_type_error("target must be a function"); } let args = args_list.create_list_from_array_like(&[], context)?; target.call(this_arg, &args, context) @@ -117,7 +117,7 @@ impl Reflect { } if !target.is_constructable() { - return Err(context.construct_type_error("target must be a constructor")); + return context.throw_type_error("target must be a constructor"); } let args = args_list.create_list_from_array_like(&[], context)?; target.construct(&args, context) @@ -366,7 +366,7 @@ impl Reflect { .ok_or_else(|| context.construct_type_error("target must be an object"))?; let proto = args.get(1).unwrap_or(&undefined); if !proto.is_null() && !proto.is_object() { - return Err(context.construct_type_error("proto must be an object or null")); + return context.throw_type_error("proto must be an object or null"); } Ok(target.set_prototype_instance(proto.clone()).into()) } From 82a48f3310e84ccf14128ec0500b409f6a930a08 Mon Sep 17 00:00:00 2001 From: tofpie Date: Fri, 8 Jan 2021 00:04:48 +0100 Subject: [PATCH 4/5] Use receiver in get and set --- boa/src/builtins/reflect/mod.rs | 42 ++++++++++++++++++------------ boa/src/object/internal_methods.rs | 6 +++-- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/boa/src/builtins/reflect/mod.rs b/boa/src/builtins/reflect/mod.rs index 66db4e29135..116bfde23d6 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, + property::{Attribute, DataDescriptor}, BoaProfiler, Context, Result, Value, }; @@ -148,7 +148,9 @@ impl Reflect { .ok_or_else(|| context.construct_type_error("property descriptor must be an object"))? .to_property_descriptor(context)?; - Ok(target.define_own_property(key, prop_desc).into()) + target + .define_own_property(key, prop_desc, context) + .map(|b| b.into()) } /// Defines a property on an object. @@ -184,17 +186,17 @@ impl Reflect { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/get pub(crate) fn get(_: &Value, args: &[Value], context: &mut Context) -> Result { let undefined = Value::undefined(); - let mut target = args + let target = args .get(0) .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)?; - if let Some(receiver) = args.get(2) { - target = receiver - .as_object() - .ok_or_else(|| context.construct_type_error("receiver must be an object"))?; - } - target.get(&key, context) + let receiver = if let Some(receiver) = args.get(2).cloned() { + receiver + } else { + target.clone().into() + }; + target.get(&key, receiver, context) } /// Gets a property of an object. @@ -291,7 +293,13 @@ impl Reflect { let array_prototype = context.standard_objects().array_object().prototype(); let result: Value = Object::with_prototype(array_prototype.into(), ObjectData::Array).into(); - result.set_field("length", 0, context)?; + result.set_property( + "length", + DataDescriptor::new( + 0, + Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT, + ), + ); let keys = target.own_property_keys(); for (i, k) in keys.iter().enumerate() { @@ -338,12 +346,12 @@ impl Reflect { .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 value = args.get(2).unwrap_or(&undefined); - if let Some(receiver) = args.get(3) { - target = receiver - .as_object() - .ok_or_else(|| context.construct_type_error("receiver must be an object"))?; - } - Ok(target.set(key, value.clone(), context)?.into()) + let receiver = if let Some(receiver) = args.get(3).cloned() { + receiver + } else { + target.clone().into() + }; + Ok(target.set(key, value.clone(), receiver, context)?.into()) } /// Sets the prototype of an object. @@ -368,6 +376,6 @@ impl Reflect { if !proto.is_null() && !proto.is_object() { return context.throw_type_error("proto must be an object or null"); } - Ok(target.set_prototype_instance(proto.clone()).into()) + Ok(target.set_prototype_of(proto.clone()).into()) } } diff --git a/boa/src/object/internal_methods.rs b/boa/src/object/internal_methods.rs index 963cb1f22cb..0eb7baf149d 100644 --- a/boa/src/object/internal_methods.rs +++ b/boa/src/object/internal_methods.rs @@ -582,10 +582,12 @@ impl GcObject { } else { element_types }; - let len = self.get(&"length".into(), context)?.to_length(context)?; + let len = self + .get(&"length".into(), self.clone().into(), context)? + .to_length(context)?; let mut list = Vec::with_capacity(len); for index in 0..len { - let next = self.get(&index.into(), context)?; + let next = self.get(&index.into(), self.clone().into(), context)?; if !types.contains(&next.get_type()) { return Err(context.construct_type_error("bad type")); } From 7e178f74297ee2fa9df4b8ec57c79d85475fadc4 Mon Sep 17 00:00:00 2001 From: tofpie Date: Tue, 12 Jan 2021 17:08:08 +0100 Subject: [PATCH 5/5] Implement Reflect.construct with newTarget --- boa/src/builtins/reflect/mod.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/boa/src/builtins/reflect/mod.rs b/boa/src/builtins/reflect/mod.rs index 116bfde23d6..9f44fcb2e62 100644 --- a/boa/src/builtins/reflect/mod.rs +++ b/boa/src/builtins/reflect/mod.rs @@ -101,7 +101,7 @@ impl Reflect { /// [spec]: https://tc39.es/ecma262/#sec-reflect.construct /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect/construct pub(crate) fn construct(_: &Value, args: &[Value], context: &mut Context) -> Result { - let mut target = args + let target = args .get(0) .and_then(|v| v.as_object()) .ok_or_else(|| context.construct_type_error("target must be a function"))?; @@ -110,17 +110,21 @@ impl Reflect { .and_then(|v| v.as_object()) .ok_or_else(|| context.construct_type_error("args list must be an object"))?; - if let Some(new_target) = args.get(2) { - target = new_target - .as_object() - .ok_or_else(|| context.construct_type_error("newTarget must be an object"))?; - } - if !target.is_constructable() { return context.throw_type_error("target must be a constructor"); } + + let new_target = if let Some(new_target) = args.get(2) { + if new_target.as_object().map(|o| o.is_constructable()) != Some(true) { + return context.throw_type_error("newTarget must be constructor"); + } + new_target.clone() + } else { + target.clone().into() + }; + let args = args_list.create_list_from_array_like(&[], context)?; - target.construct(&args, context) + target.construct(&args, new_target, context) } /// Defines a property on an object.