From 6efb395fdc7049a8b8fd1c7db53bbeb57f4130af Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 20 Jan 2020 17:16:24 +0100 Subject: [PATCH] Add Object::define_own_property() (#228) This commit introduces the NONE, READ_ONLY, DONT_ENUM and DONT_DELETE property attributes. --- src/binding.cc | 13 ++++-- src/lib.rs | 2 + src/object.rs | 26 +++++++++++ src/property_attribute.rs | 94 +++++++++++++++++++++++++++++++++++++++ src/template.rs | 23 ++++++++-- tests/test_api.rs | 30 +++++++++++-- 6 files changed, 178 insertions(+), 10 deletions(-) create mode 100644 src/property_attribute.rs diff --git a/src/binding.cc b/src/binding.cc index 50f69cdf..9734d4d6 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -626,9 +626,8 @@ int v8__String__WriteUtf8(const v8::String& self, v8::Isolate* isolate, } void v8__Template__Set(v8::Template& self, v8::Local key, - v8::Local value) { - // TODO(bnoordhuis) Allow setting PropertyAttributes. - self.Set(key, value); + v8::Local value, v8::PropertyAttribute attr) { + self.Set(key, value, attr); } v8::ObjectTemplate* v8__ObjectTemplate__New( @@ -673,6 +672,14 @@ MaybeBool v8__Object__CreateDataProperty(v8::Object& self, return maybe_to_maybe_bool(self.CreateDataProperty(context, key, value)); } +MaybeBool v8__Object__DefineOwnProperty(v8::Object& self, + v8::Local context, + v8::Local key, + v8::Local value, + v8::PropertyAttribute attr) { + return maybe_to_maybe_bool(self.DefineOwnProperty(context, key, value, attr)); +} + MaybeBool v8__Object__SetAccessor(v8::Object& self, v8::Local context, v8::Local key, diff --git a/src/lib.rs b/src/lib.rs index 1b0d8297..28eae18e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,6 +59,7 @@ mod platform; mod primitive_array; mod primitives; mod promise; +mod property_attribute; mod scope_traits; mod script; mod script_or_module; @@ -107,6 +108,7 @@ pub use platform::Task; pub use primitive_array::PrimitiveArray; pub use primitives::*; pub use promise::{PromiseRejectEvent, PromiseRejectMessage, PromiseState}; +pub use property_attribute::*; pub use scope::CallbackScope; pub use scope::ContextScope; pub use scope::FunctionCallbackScope; diff --git a/src/object.rs b/src/object.rs index c7b57201..b89e449e 100644 --- a/src/object.rs +++ b/src/object.rs @@ -8,6 +8,7 @@ use crate::Context; use crate::Local; use crate::Name; use crate::Object; +use crate::PropertyAttribute; use crate::ToLocal; use crate::Value; @@ -44,6 +45,13 @@ extern "C" { key: *const Name, value: *const Value, ) -> MaybeBool; + fn v8__Object__DefineOwnProperty( + object: &Object, + context: *const Context, + key: *const Name, + value: *const Value, + attr: PropertyAttribute, + ) -> MaybeBool; fn v8__Object__GetIdentityHash(object: &Object) -> int; fn v8__Array__New(isolate: *mut Isolate, length: int) -> *mut Array; @@ -120,6 +128,24 @@ impl Object { unsafe { v8__Object__CreateDataProperty(self, &*context, &*key, &*value) } } + /// Implements DefineOwnProperty. + /// + /// In general, CreateDataProperty will be faster, however, does not allow + /// for specifying attributes. + /// + /// Returns true on success. + pub fn define_own_property( + &self, + context: Local, + key: Local, + value: Local, + attr: PropertyAttribute, + ) -> MaybeBool { + unsafe { + v8__Object__DefineOwnProperty(self, &*context, &*key, &*value, attr) + } + } + pub fn get<'a>( &self, scope: &mut impl ToLocal<'a>, diff --git a/src/property_attribute.rs b/src/property_attribute.rs new file mode 100644 index 00000000..0b3267f7 --- /dev/null +++ b/src/property_attribute.rs @@ -0,0 +1,94 @@ +#[repr(C)] +#[derive(Debug, Eq, PartialEq)] +pub struct PropertyAttribute(u32); + +/// No property attributes. +pub const NONE: PropertyAttribute = PropertyAttribute(0); + +/// Not writable. Corresponds to +/// `Object.defineProperty(o, "p", { writable: false })`. +pub const READ_ONLY: PropertyAttribute = PropertyAttribute(1); + +/// Not enumerable. Corresponds to +/// `Object.defineProperty(o, "p", { enumerable: false })`. +pub const DONT_ENUM: PropertyAttribute = PropertyAttribute(2); + +/// Not configurable. Corresponds to +/// `Object.defineProperty(o, "p", { configurable: false })`. +pub const DONT_DELETE: PropertyAttribute = PropertyAttribute(4); + +impl PropertyAttribute { + /// Test if no property attributes are set. + pub fn is_none(&self) -> bool { + *self == NONE + } + + /// Test if the read-only property attribute is set. + pub fn is_read_only(&self) -> bool { + self.has(READ_ONLY) + } + + /// Test if the non-enumerable property attribute is set. + pub fn is_dont_enum(&self) -> bool { + self.has(DONT_ENUM) + } + + /// Test if the non-configurable property attribute is set. + pub fn is_dont_delete(&self) -> bool { + self.has(DONT_DELETE) + } + + fn has(&self, that: Self) -> bool { + let Self(lhs) = self; + let Self(rhs) = that; + 0 != lhs & rhs + } +} + +// Identical to #[derive(Default)] but arguably clearer when made explicit. +impl Default for PropertyAttribute { + fn default() -> Self { + NONE + } +} + +impl std::ops::Add for PropertyAttribute { + type Output = Self; + + fn add(self, Self(rhs): Self) -> Self { + let Self(lhs) = self; + Self(lhs + rhs) + } +} + +#[test] +fn test_attr() { + assert!(NONE.is_none()); + assert!(!NONE.is_read_only()); + assert!(!NONE.is_dont_enum()); + assert!(!NONE.is_dont_delete()); + + assert!(!READ_ONLY.is_none()); + assert!(READ_ONLY.is_read_only()); + assert!(!READ_ONLY.is_dont_enum()); + assert!(!READ_ONLY.is_dont_delete()); + + assert!(!DONT_ENUM.is_none()); + assert!(!DONT_ENUM.is_read_only()); + assert!(DONT_ENUM.is_dont_enum()); + assert!(!DONT_ENUM.is_dont_delete()); + + assert!(!DONT_DELETE.is_none()); + assert!(!DONT_DELETE.is_read_only()); + assert!(!DONT_DELETE.is_dont_enum()); + assert!(DONT_DELETE.is_dont_delete()); + + assert_eq!(NONE, Default::default()); + assert_eq!(READ_ONLY, NONE + READ_ONLY); + + let attr = READ_ONLY + DONT_ENUM; + assert!(!attr.is_none()); + assert!(attr.is_read_only()); + assert!(attr.is_dont_enum()); + assert!(!attr.is_dont_delete()); +} diff --git a/src/template.rs b/src/template.rs index ac05cb3e..7bb9132e 100644 --- a/src/template.rs +++ b/src/template.rs @@ -10,11 +10,18 @@ use crate::Function; use crate::FunctionCallback; use crate::Local; use crate::Object; +use crate::PropertyAttribute; use crate::String; use crate::ToLocal; +use crate::NONE; extern "C" { - fn v8__Template__Set(self_: &Template, key: *const Name, value: *const Data); + fn v8__Template__Set( + self_: &Template, + key: *const Name, + value: *const Data, + attr: PropertyAttribute, + ); fn v8__FunctionTemplate__New( isolate: &Isolate, @@ -42,8 +49,18 @@ extern "C" { impl Template { /// Adds a property to each instance created by this template. pub fn set(&self, key: Local, value: Local) { - // TODO(bnoordhuis) Allow setting PropertyAttributes. - unsafe { v8__Template__Set(self, &*key, &*value) } + self.set_with_attr(key, value, NONE) + } + + /// Adds a property to each instance created by this template with + /// the specified property attributes. + pub fn set_with_attr( + &self, + key: Local, + value: Local, + attr: PropertyAttribute, + ) { + unsafe { v8__Template__Set(self, &*key, &*value, attr) } } } diff --git a/tests/test_api.rs b/tests/test_api.rs index 7870b14f..b237b982 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -869,19 +869,41 @@ fn object_template() { let object_templ = v8::ObjectTemplate::new(scope); let function_templ = v8::FunctionTemplate::new(scope, fortytwo_callback); let name = v8_str(scope, "f"); - object_templ.set(name.into(), function_templ.into()); + let attr = v8::READ_ONLY + v8::DONT_ENUM + v8::DONT_DELETE; + object_templ.set_with_attr(name.into(), function_templ.into(), attr); let context = v8::Context::new(scope); let mut cs = v8::ContextScope::new(scope, context); let scope = cs.enter(); let object = object_templ.new_instance(scope, context).unwrap(); assert!(!object.is_null_or_undefined()); let name = v8_str(scope, "g"); - context - .global(scope) - .set(context, name.into(), object.into()); + context.global(scope).define_own_property( + context, + name.into(), + object.into(), + v8::DONT_ENUM, + ); + let source = r#" + { + const d = Object.getOwnPropertyDescriptor(globalThis, "g"); + [d.configurable, d.enumerable, d.writable].toString() + } + "#; + let actual = eval(scope, context, source).unwrap(); + let expected = v8_str(scope, "true,false,true"); + assert!(expected.strict_equals(actual)); let actual = eval(scope, context, "g.f()").unwrap(); let expected = v8::Integer::new(scope, 42); assert!(expected.strict_equals(actual)); + let source = r#" + { + const d = Object.getOwnPropertyDescriptor(g, "f"); + [d.configurable, d.enumerable, d.writable].toString() + } + "#; + let actual = eval(scope, context, source).unwrap(); + let expected = v8_str(scope, "false,false,false"); + assert!(expected.strict_equals(actual)); } }