From a110490e64fb39b4480f8e228121f945cc897f30 Mon Sep 17 00:00:00 2001 From: Chris Bajorin Date: Thu, 24 Jun 2021 18:27:19 -0400 Subject: [PATCH 1/4] JsSymbol primitive implementation --- Cargo.toml | 4 + .../src/napi/bindings/functions.rs | 2 + crates/neon-runtime/src/napi/primitive.rs | 6 ++ crates/neon-runtime/src/napi/tag.rs | 5 ++ src/context/mod.rs | 10 +++ src/prelude.rs | 2 + src/types/mod.rs | 26 +++--- src/types/symbol.rs | 86 +++++++++++++++++++ test/napi/Cargo.toml | 2 +- test/napi/lib/objects.js | 9 ++ test/napi/lib/symbols.js | 31 +++++++ test/napi/lib/types.js | 13 +++ test/napi/src/js/objects.rs | 7 ++ test/napi/src/js/symbols.rs | 22 +++++ test/napi/src/js/types.rs | 6 ++ test/napi/src/lib.rs | 12 +++ 16 files changed, 232 insertions(+), 11 deletions(-) create mode 100644 src/types/symbol.rs create mode 100644 test/napi/lib/symbols.js create mode 100644 test/napi/src/js/symbols.rs diff --git a/Cargo.toml b/Cargo.toml index 5c09b2958..25c237e74 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,6 +75,10 @@ event-queue-api = [] # Feature flag to include procedural macros proc-macros = ["neon-macros"] +# Feature flag to enable the `JsSymbol` API of RFC 38 +# https://github.com/neon-bindings/rfcs/pull/38 +symbol-primitive-api = [] + [package.metadata.docs.rs] features = ["docs-only", "event-handler-api", "proc-macros", "try-catch-api"] diff --git a/crates/neon-runtime/src/napi/bindings/functions.rs b/crates/neon-runtime/src/napi/bindings/functions.rs index b8d29b16e..7690c9e00 100644 --- a/crates/neon-runtime/src/napi/bindings/functions.rs +++ b/crates/neon-runtime/src/napi/bindings/functions.rs @@ -198,6 +198,8 @@ mod napi1 { ) -> Status; fn run_script(env: Env, script: Value, result: *mut Value) -> Status; + + fn create_symbol(env: Env, description: Value, result: *mut Value) -> Status; } ); } diff --git a/crates/neon-runtime/src/napi/primitive.rs b/crates/neon-runtime/src/napi/primitive.rs index 2df6f19ee..f8e51e11c 100644 --- a/crates/neon-runtime/src/napi/primitive.rs +++ b/crates/neon-runtime/src/napi/primitive.rs @@ -43,3 +43,9 @@ pub unsafe fn number_value(env: Env, p: Local) -> f64 { ); value } + +/// Mutates the `out` argument provided to refer to a newly created `Local` containing a +/// JavaScript symbol. +pub unsafe fn symbol(out: &mut Local, env: Env, desc: Local) { + napi::create_symbol(env, desc, out as *mut Local); +} \ No newline at end of file diff --git a/crates/neon-runtime/src/napi/tag.rs b/crates/neon-runtime/src/napi/tag.rs index d0cd048b3..25bbd9988 100644 --- a/crates/neon-runtime/src/napi/tag.rs +++ b/crates/neon-runtime/src/napi/tag.rs @@ -34,6 +34,11 @@ pub unsafe fn is_string(env: Env, val: Local) -> bool { is_type(env, val, napi::ValueType::String) } +/// Is `val` a JavaScript symbol? +pub unsafe fn is_symbol(env: Env, val: Local) -> bool { + is_type(env, val, napi::ValueType::Symbol) +} + pub unsafe fn is_object(env: Env, val: Local) -> bool { is_type(env, val, napi::ValueType::Object) } diff --git a/src/context/mod.rs b/src/context/mod.rs index 325c005a4..b11c86c7d 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -168,6 +168,8 @@ use crate::types::{ JsArray, JsBoolean, JsFunction, JsNull, JsNumber, JsObject, JsString, JsUndefined, JsValue, StringResult, Value, }; +#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +use crate::types::symbol::JsSymbol; use neon_runtime; use neon_runtime::raw; #[cfg(feature = "napi-1")] @@ -438,6 +440,14 @@ pub trait Context<'a>: ContextInternal<'a> { JsString::try_new(self, s) } + /// Convenience method for creating a `JsSymbol` value. + /// + /// If the string exceeds the limits of the JS engine, this method panics. + #[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] + fn symbol>(&mut self, s: S) -> Handle<'a, JsSymbol> { + JsSymbol::with_description(self, s) + } + /// Convenience method for creating a `JsNull` value. fn null(&mut self) -> Handle<'a, JsNull> { #[cfg(feature = "legacy-runtime")] diff --git a/src/prelude.rs b/src/prelude.rs index ee29de95d..ea68693ae 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -31,3 +31,5 @@ pub use crate::{ handle::Root, types::boxed::{Finalize, JsBox}, }; +#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +pub use crate::types::symbol::JsSymbol; diff --git a/src/types/mod.rs b/src/types/mod.rs index 0bfa4c219..8d10d81f3 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -67,7 +67,8 @@ //! of custom objects that own Rust data structures. //! - **Primitive types:** These are the built-in JavaScript datatypes that are not //! object types: [`JsNumber`](JsNumber), [`JsBoolean`](JsBoolean), -//! [`JsString`](JsString), [`JsNull`](JsNull), and [`JsUndefined`](JsUndefined). +//! [`JsString`](JsString), [`JsNull`](JsNull), [`JsSymbol`](JsSymbol), +//! and [`JsUndefined`](JsUndefined). //! //! [types]: https://raw.githubusercontent.com/neon-bindings/neon/main/doc/types.jpg //! [unknown]: https://mariusschulz.com/blog/the-unknown-type-in-typescript#the-unknown-type @@ -80,6 +81,8 @@ pub(crate) mod date; pub(crate) mod error; pub(crate) mod internal; +#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +pub(crate) mod symbol; pub(crate) mod utf8; use self::internal::{FunctionCallback, ValueInternal}; @@ -434,16 +437,19 @@ impl JsString { #[cfg(feature = "napi-1")] pub fn value<'a, C: Context<'a>>(self, cx: &mut C) -> String { - let env = cx.env().to_raw(); + unsafe { JsString::value_internal(cx.env().to_raw(), self.to_raw()) } + } - unsafe { - let capacity = neon_runtime::string::utf8_len(env, self.to_raw()) + 1; - let mut buffer: Vec = Vec::with_capacity(capacity as usize); - let p = buffer.as_mut_ptr(); - std::mem::forget(buffer); - let len = neon_runtime::string::data(env, p, capacity, self.to_raw()); - String::from_raw_parts(p, len as usize, capacity as usize) - } + /// Invariants: + /// - `local` has an underlying value type of `napi::ValueType::String` + #[cfg(feature = "napi-1")] + pub(crate) unsafe fn value_internal(env: raw::Env, local: raw::Local) -> String { + let capacity = neon_runtime::string::utf8_len(env, local) + 1; + let mut buffer: Vec = Vec::with_capacity(capacity as usize); + let p = buffer.as_mut_ptr(); + std::mem::forget(buffer); + let len = neon_runtime::string::data(env, p, capacity, local); + String::from_raw_parts(p, len as usize, capacity as usize) } pub fn new<'a, C: Context<'a>, S: AsRef>(cx: &mut C, val: S) -> Handle<'a, JsString> { diff --git a/src/types/symbol.rs b/src/types/symbol.rs new file mode 100644 index 000000000..de8ab3ee7 --- /dev/null +++ b/src/types/symbol.rs @@ -0,0 +1,86 @@ +use crate::context::Context; +use crate::handle::{Handle, Managed}; +use crate::types::internal::ValueInternal; +use crate::types::utf8::Utf8; +use crate::types::{Env, JsString, Value}; + +use neon_runtime::raw; + +/// A JavaScript symbol primitive value. +#[repr(C)] +#[derive(Clone, Copy)] +pub struct JsSymbol(raw::Local); + +impl JsSymbol { + /// Create a new symbol. + /// Equivalent to calling `Symbol()` in JavaScript + pub fn new<'a, C: Context<'a>>(cx: &mut C) -> Handle<'a, JsSymbol> { + JsSymbol::new_internal(cx.env(), None::<&str>) + } + + /// Create a new symbol with a description. + /// Equivalent to calling `Symbol(desc)` in JavaScript + pub fn with_description<'a, C: Context<'a>, S: AsRef>( + cx: &mut C, + desc: S, + ) -> Handle<'a, JsSymbol> { + JsSymbol::new_internal(cx.env(), Some(desc)) + } + + /// Get the optional symbol description, where `None` represents an undefined description. + pub fn description<'a, C: Context<'a>>(self, cx: &mut C) -> Option { + let env = cx.env().to_raw(); + let (desc_ptr, desc_len) = Utf8::from("description").into_small_unwrap().lower(); + + unsafe { + let mut local = std::mem::zeroed(); + if !neon_runtime::object::get_string(env, &mut local, self.to_raw(), desc_ptr, desc_len) + { + return None; + } + + if neon_runtime::tag::is_string(env, local) { + Some(JsString::value_internal(env, local)) + } else { + None + } + } + } + + pub(crate) fn new_internal<'a, S: AsRef>( + env: Env, + desc: Option, + ) -> Handle<'a, JsSymbol> { + unsafe { + let desc_local = desc + .and_then(|d| JsString::new_internal(env, d.as_ref())) + .map_or_else(|| std::mem::zeroed(), |h| h.to_raw()); + + let mut sym_local = std::mem::zeroed(); + neon_runtime::primitive::symbol(&mut sym_local, env.to_raw(), desc_local); + Handle::new_internal(JsSymbol(sym_local)) + } + } +} + +impl Value for JsSymbol {} + +impl Managed for JsSymbol { + fn to_raw(self) -> raw::Local { + self.0 + } + + fn from_raw(_: Env, h: raw::Local) -> Self { + JsSymbol(h) + } +} + +impl ValueInternal for JsSymbol { + fn name() -> String { + "symbol".to_string() + } + + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_symbol(env.to_raw(), other.to_raw()) } + } +} diff --git a/test/napi/Cargo.toml b/test/napi/Cargo.toml index 0981e244d..a8fed4ec0 100644 --- a/test/napi/Cargo.toml +++ b/test/napi/Cargo.toml @@ -13,4 +13,4 @@ crate-type = ["cdylib"] version = "*" path = "../.." default-features = false -features = ["default-panic-hook", "napi-6", "try-catch-api", "event-queue-api"] +features = ["default-panic-hook", "napi-6", "try-catch-api", "event-queue-api", "symbol-primitive-api"] diff --git a/test/napi/lib/objects.js b/test/napi/lib/objects.js index 18d11f77e..607f0a446 100644 --- a/test/napi/lib/objects.js +++ b/test/napi/lib/objects.js @@ -22,6 +22,15 @@ describe('JsObject', function() { assert.deepEqual({number: 9000, string: 'hello node'}, addon.return_js_object_with_mixed_content()); }); + it('return a JsObject with a symbol property key', function () { + const obj = addon.return_js_object_with_symbol_property_key(); + const propertySymbols = Object.getOwnPropertySymbols(obj); + assert.equal(propertySymbols.length, 1); + const sym = propertySymbols[0]; + assert.equal(typeof sym, "symbol"); + assert.equal(obj[sym], sym); + }) + it('gets a 16-byte, zeroed ArrayBuffer', function() { var b = addon.return_array_buffer(); assert.equal(b.byteLength, 16); diff --git a/test/napi/lib/symbols.js b/test/napi/lib/symbols.js new file mode 100644 index 000000000..59763cc9e --- /dev/null +++ b/test/napi/lib/symbols.js @@ -0,0 +1,31 @@ +const addon = require('..'); +const { assert } = require('chai'); + +describe('JsSymbol', function() { + it('should return a JsSymbol with a description built in Rust', function () { + const sym = addon.return_js_symbol_with_description(); + assert.equal(typeof sym, 'symbol'); + assert.equal(sym.description, "neon:description"); + }); + it('should return a JsSymbol without a description built in Rust', function () { + const sym = addon.return_js_symbol(); + assert.equal(typeof sym, 'symbol'); + assert.equal(sym.description, undefined); + }); + it('should read the description property in Rust', function () { + const sym = Symbol('neon:description'); + const description = addon.read_js_symbol_description(sym); + assert.equal(description, 'neon:description'); + }); + it('should read an undefined description property in Rust', function () { + const sym = Symbol(); + const description = addon.read_js_symbol_description(sym); + assert.equal(description, undefined); + }); + it('accepts and returns symbols', function () { + const symDesc = Symbol('neon:description'); + const symNoDesc = Symbol(); + assert.equal(addon.accept_and_return_js_symbol(symDesc), symDesc); + assert.equal(addon.accept_and_return_js_symbol(symNoDesc), symNoDesc); + }); +}); diff --git a/test/napi/lib/types.js b/test/napi/lib/types.js index 7b963840b..eaf8231f1 100644 --- a/test/napi/lib/types.js +++ b/test/napi/lib/types.js @@ -70,6 +70,15 @@ describe('type checks', function() { assert(!addon.is_string(new String('1'))); }); + it('is_symbol', function () { + assert(addon.is_symbol(Symbol())); + assert(addon.is_symbol(Symbol("unique symbol"))); + assert(addon.is_symbol(Symbol.for('neon:description'))); + assert(addon.is_symbol(Symbol.toStringTag)); + assert(!addon.is_symbol(undefined)); + assert(!addon.is_symbol("anything other than symbol")); + }); + it('is_undefined', function () { assert(addon.is_undefined(undefined)); assert(!addon.is_undefined(null)); @@ -84,5 +93,9 @@ describe('type checks', function() { assert(addon.strict_equals(o1, o1)); assert(!addon.strict_equals(o1, o2)); assert(!addon.strict_equals(o1, 17)); + let s1 = Symbol(); + let s2 = Symbol(); + assert(addon.strict_equals(s1, s1)); + assert(!addon.strict_equals(s1, s2)); }); }); diff --git a/test/napi/src/js/objects.rs b/test/napi/src/js/objects.rs index dbb4db792..5c5c074c8 100644 --- a/test/napi/src/js/objects.rs +++ b/test/napi/src/js/objects.rs @@ -31,6 +31,13 @@ pub fn return_js_object_with_string(mut cx: FunctionContext) -> JsResult JsResult { + let js_object: Handle = cx.empty_object(); + let s = cx.symbol("neon:description"); + js_object.set(&mut cx, s, s)?; + Ok(js_object) +} + pub fn return_array_buffer(mut cx: FunctionContext) -> JsResult { let b: Handle = cx.array_buffer(16)?; Ok(b) diff --git a/test/napi/src/js/symbols.rs b/test/napi/src/js/symbols.rs new file mode 100644 index 000000000..6ad0b4cf6 --- /dev/null +++ b/test/napi/src/js/symbols.rs @@ -0,0 +1,22 @@ +use neon::prelude::*; + +pub fn return_js_symbol_with_description(mut cx: FunctionContext) -> JsResult { + Ok(cx.symbol("neon:description")) +} + +pub fn return_js_symbol(mut cx: FunctionContext) -> JsResult { + Ok(JsSymbol::new(&mut cx)) +} + +pub fn read_js_symbol_description(mut cx: FunctionContext) -> JsResult { + let symbol: Handle = cx.argument(0)?; + match symbol.description(&mut cx) { + None => Ok(cx.undefined().upcast()), + Some(s) => Ok(cx.string(s).upcast()), + } +} + +pub fn accept_and_return_js_symbol(mut cx: FunctionContext) -> JsResult { + let sym: Handle = cx.argument(0)?; + Ok(sym) +} diff --git a/test/napi/src/js/types.rs b/test/napi/src/js/types.rs index 9c75fa05a..be1528523 100644 --- a/test/napi/src/js/types.rs +++ b/test/napi/src/js/types.rs @@ -54,6 +54,12 @@ pub fn is_object(mut cx: FunctionContext) -> JsResult { Ok(cx.boolean(result)) } +pub fn is_symbol(mut cx: FunctionContext) -> JsResult { + let val: Handle = cx.argument(0)?; + let result = val.is_a::(&mut cx); + Ok(cx.boolean(result)) +} + pub fn is_undefined(mut cx: FunctionContext) -> JsResult { let val: Handle = cx.argument(0)?; let is_string = val.is_a::(&mut cx); diff --git a/test/napi/src/lib.rs b/test/napi/src/lib.rs index f081c07a1..3300d2a97 100644 --- a/test/napi/src/lib.rs +++ b/test/napi/src/lib.rs @@ -10,6 +10,7 @@ mod js { pub mod numbers; pub mod objects; pub mod strings; + pub mod symbols; pub mod threads; pub mod types; } @@ -23,6 +24,7 @@ use js::functions::*; use js::numbers::*; use js::objects::*; use js::strings::*; +use js::symbols::*; use js::threads::*; use js::types::*; @@ -164,6 +166,10 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { cx.export_function("return_js_object", return_js_object)?; cx.export_function("return_js_object_with_number", return_js_object_with_number)?; cx.export_function("return_js_object_with_string", return_js_object_with_string)?; + cx.export_function( + "return_js_object_with_symbol_property_key", + return_js_object_with_symbol_property_key + )?; cx.export_function( "return_js_object_with_mixed_content", return_js_object_with_mixed_content, @@ -218,6 +224,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { cx.export_function("is_number", is_number)?; cx.export_function("is_object", is_object)?; cx.export_function("is_string", is_string)?; + cx.export_function("is_symbol", is_symbol)?; cx.export_function("is_undefined", is_undefined)?; cx.export_function("strict_equals", strict_equals)?; @@ -258,5 +265,10 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { cx.export_function("leak_channel", leak_channel)?; cx.export_function("drop_global_queue", drop_global_queue)?; + cx.export_function("return_js_symbol_with_description", return_js_symbol_with_description)?; + cx.export_function("return_js_symbol", return_js_symbol)?; + cx.export_function("read_js_symbol_description", read_js_symbol_description)?; + cx.export_function("accept_and_return_js_symbol", accept_and_return_js_symbol)?; + Ok(()) } From 53ad83357d1e45a3e417544ecb22451b297cbe0a Mon Sep 17 00:00:00 2001 From: Chris Bajorin Date: Fri, 25 Jun 2021 19:36:41 -0400 Subject: [PATCH 2/4] update symbol description representation to Option> --- crates/neon-runtime/src/napi/primitive.rs | 2 +- src/context/mod.rs | 9 +++++---- src/prelude.rs | 4 ++-- src/types/mod.rs | 21 +++++++++----------- src/types/symbol.rs | 24 +++++++++++------------ test/napi/src/js/symbols.rs | 2 +- test/napi/src/js/types.rs | 2 +- test/napi/src/lib.rs | 7 +++++-- 8 files changed, 36 insertions(+), 35 deletions(-) diff --git a/crates/neon-runtime/src/napi/primitive.rs b/crates/neon-runtime/src/napi/primitive.rs index f8e51e11c..f20d9095a 100644 --- a/crates/neon-runtime/src/napi/primitive.rs +++ b/crates/neon-runtime/src/napi/primitive.rs @@ -48,4 +48,4 @@ pub unsafe fn number_value(env: Env, p: Local) -> f64 { /// JavaScript symbol. pub unsafe fn symbol(out: &mut Local, env: Env, desc: Local) { napi::create_symbol(env, desc, out as *mut Local); -} \ No newline at end of file +} diff --git a/src/context/mod.rs b/src/context/mod.rs index b11c86c7d..1fdf1a814 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -164,12 +164,12 @@ use crate::types::boxed::{Finalize, JsBox}; #[cfg(feature = "napi-5")] use crate::types::date::{DateError, JsDate}; use crate::types::error::JsError; +#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +use crate::types::symbol::JsSymbol; use crate::types::{ JsArray, JsBoolean, JsFunction, JsNull, JsNumber, JsObject, JsString, JsUndefined, JsValue, StringResult, Value, }; -#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] -use crate::types::symbol::JsSymbol; use neon_runtime; use neon_runtime::raw; #[cfg(feature = "napi-1")] @@ -444,8 +444,9 @@ pub trait Context<'a>: ContextInternal<'a> { /// /// If the string exceeds the limits of the JS engine, this method panics. #[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] - fn symbol>(&mut self, s: S) -> Handle<'a, JsSymbol> { - JsSymbol::with_description(self, s) + fn symbol>(&mut self, description: S) -> Handle<'a, JsSymbol> { + let desc = self.string(description); + JsSymbol::with_description(self, desc) } /// Convenience method for creating a `JsNull` value. diff --git a/src/prelude.rs b/src/prelude.rs index ea68693ae..04ec2c041 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -22,6 +22,8 @@ pub use crate::register_module; pub use crate::result::{JsResult, JsResultExt, NeonResult}; #[cfg(feature = "legacy-runtime")] pub use crate::task::Task; +#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +pub use crate::types::symbol::JsSymbol; pub use crate::types::{ BinaryData, JsArray, JsArrayBuffer, JsBoolean, JsBuffer, JsError, JsFunction, JsNull, JsNumber, JsObject, JsString, JsUndefined, JsValue, Value, @@ -31,5 +33,3 @@ pub use crate::{ handle::Root, types::boxed::{Finalize, JsBox}, }; -#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] -pub use crate::types::symbol::JsSymbol; diff --git a/src/types/mod.rs b/src/types/mod.rs index 8d10d81f3..eb0478843 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -437,19 +437,16 @@ impl JsString { #[cfg(feature = "napi-1")] pub fn value<'a, C: Context<'a>>(self, cx: &mut C) -> String { - unsafe { JsString::value_internal(cx.env().to_raw(), self.to_raw()) } - } + let env = cx.env().to_raw(); - /// Invariants: - /// - `local` has an underlying value type of `napi::ValueType::String` - #[cfg(feature = "napi-1")] - pub(crate) unsafe fn value_internal(env: raw::Env, local: raw::Local) -> String { - let capacity = neon_runtime::string::utf8_len(env, local) + 1; - let mut buffer: Vec = Vec::with_capacity(capacity as usize); - let p = buffer.as_mut_ptr(); - std::mem::forget(buffer); - let len = neon_runtime::string::data(env, p, capacity, local); - String::from_raw_parts(p, len as usize, capacity as usize) + unsafe { + let capacity = neon_runtime::string::utf8_len(env, self.to_raw()) + 1; + let mut buffer: Vec = Vec::with_capacity(capacity as usize); + let p = buffer.as_mut_ptr(); + std::mem::forget(buffer); + let len = neon_runtime::string::data(env, p, capacity, self.to_raw()); + String::from_raw_parts(p, len as usize, capacity as usize) + } } pub fn new<'a, C: Context<'a>, S: AsRef>(cx: &mut C, val: S) -> Handle<'a, JsString> { diff --git a/src/types/symbol.rs b/src/types/symbol.rs index de8ab3ee7..c18af75e3 100644 --- a/src/types/symbol.rs +++ b/src/types/symbol.rs @@ -15,20 +15,20 @@ impl JsSymbol { /// Create a new symbol. /// Equivalent to calling `Symbol()` in JavaScript pub fn new<'a, C: Context<'a>>(cx: &mut C) -> Handle<'a, JsSymbol> { - JsSymbol::new_internal(cx.env(), None::<&str>) + JsSymbol::new_internal(cx.env(), None) } /// Create a new symbol with a description. - /// Equivalent to calling `Symbol(desc)` in JavaScript - pub fn with_description<'a, C: Context<'a>, S: AsRef>( + /// Equivalent to calling `Symbol(description)` in JavaScript + pub fn with_description<'a, C: Context<'a>>( cx: &mut C, - desc: S, + desc: Handle<'a, JsString>, ) -> Handle<'a, JsSymbol> { JsSymbol::new_internal(cx.env(), Some(desc)) } /// Get the optional symbol description, where `None` represents an undefined description. - pub fn description<'a, C: Context<'a>>(self, cx: &mut C) -> Option { + pub fn description<'a, C: Context<'a>>(self, cx: &mut C) -> Option> { let env = cx.env().to_raw(); let (desc_ptr, desc_len) = Utf8::from("description").into_small_unwrap().lower(); @@ -40,22 +40,22 @@ impl JsSymbol { } if neon_runtime::tag::is_string(env, local) { - Some(JsString::value_internal(env, local)) + Some(Handle::new_internal(JsString(local))) } else { None } } } - pub(crate) fn new_internal<'a, S: AsRef>( + pub(crate) fn new_internal<'a>( env: Env, - desc: Option, + desc: Option>, ) -> Handle<'a, JsSymbol> { unsafe { - let desc_local = desc - .and_then(|d| JsString::new_internal(env, d.as_ref())) - .map_or_else(|| std::mem::zeroed(), |h| h.to_raw()); - + let desc_local = match desc { + None => std::mem::zeroed(), + Some(h) => h.to_raw(), + }; let mut sym_local = std::mem::zeroed(); neon_runtime::primitive::symbol(&mut sym_local, env.to_raw(), desc_local); Handle::new_internal(JsSymbol(sym_local)) diff --git a/test/napi/src/js/symbols.rs b/test/napi/src/js/symbols.rs index 6ad0b4cf6..714487dd1 100644 --- a/test/napi/src/js/symbols.rs +++ b/test/napi/src/js/symbols.rs @@ -12,7 +12,7 @@ pub fn read_js_symbol_description(mut cx: FunctionContext) -> JsResult let symbol: Handle = cx.argument(0)?; match symbol.description(&mut cx) { None => Ok(cx.undefined().upcast()), - Some(s) => Ok(cx.string(s).upcast()), + Some(s) => Ok(s.upcast()), } } diff --git a/test/napi/src/js/types.rs b/test/napi/src/js/types.rs index be1528523..1dd1bcbcc 100644 --- a/test/napi/src/js/types.rs +++ b/test/napi/src/js/types.rs @@ -56,7 +56,7 @@ pub fn is_object(mut cx: FunctionContext) -> JsResult { pub fn is_symbol(mut cx: FunctionContext) -> JsResult { let val: Handle = cx.argument(0)?; - let result = val.is_a::(&mut cx); + let result = val.is_a::(&mut cx); Ok(cx.boolean(result)) } diff --git a/test/napi/src/lib.rs b/test/napi/src/lib.rs index 3300d2a97..97a245a02 100644 --- a/test/napi/src/lib.rs +++ b/test/napi/src/lib.rs @@ -168,7 +168,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { cx.export_function("return_js_object_with_string", return_js_object_with_string)?; cx.export_function( "return_js_object_with_symbol_property_key", - return_js_object_with_symbol_property_key + return_js_object_with_symbol_property_key, )?; cx.export_function( "return_js_object_with_mixed_content", @@ -265,7 +265,10 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { cx.export_function("leak_channel", leak_channel)?; cx.export_function("drop_global_queue", drop_global_queue)?; - cx.export_function("return_js_symbol_with_description", return_js_symbol_with_description)?; + cx.export_function( + "return_js_symbol_with_description", + return_js_symbol_with_description, + )?; cx.export_function("return_js_symbol", return_js_symbol)?; cx.export_function("read_js_symbol_description", read_js_symbol_description)?; cx.export_function("accept_and_return_js_symbol", accept_and_return_js_symbol)?; From 23a797c90cea1681f492cfba356edc6d57c3c8da Mon Sep 17 00:00:00 2001 From: Chris Bajorin Date: Mon, 12 Jul 2021 20:52:19 -0400 Subject: [PATCH 3/4] rename symbol feature flag, add test coverage, remove out param from neon_runtime --- Cargo.toml | 2 +- crates/neon-runtime/src/napi/primitive.rs | 14 ++++++++++---- src/context/mod.rs | 4 ++-- src/prelude.rs | 2 +- src/types/mod.rs | 4 +++- src/types/symbol.rs | 5 ++--- test/napi/Cargo.toml | 2 +- test/napi/lib/objects.js | 4 ++-- test/napi/lib/symbols.js | 21 ++++++++++++++++----- test/napi/lib/types.js | 2 +- test/napi/src/js/objects.rs | 2 +- test/napi/src/js/symbols.rs | 15 ++++++++++----- test/napi/src/lib.rs | 4 ++++ 13 files changed, 54 insertions(+), 27 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 25c237e74..a36df9b90 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,7 +77,7 @@ proc-macros = ["neon-macros"] # Feature flag to enable the `JsSymbol` API of RFC 38 # https://github.com/neon-bindings/rfcs/pull/38 -symbol-primitive-api = [] +symbol-api = [] [package.metadata.docs.rs] features = ["docs-only", "event-handler-api", "proc-macros", "try-catch-api"] diff --git a/crates/neon-runtime/src/napi/primitive.rs b/crates/neon-runtime/src/napi/primitive.rs index f20d9095a..8fea1f812 100644 --- a/crates/neon-runtime/src/napi/primitive.rs +++ b/crates/neon-runtime/src/napi/primitive.rs @@ -1,5 +1,6 @@ use crate::napi::bindings as napi; use crate::raw::{Env, Local}; +use std::mem::MaybeUninit; /// Mutates the `out` argument provided to refer to the global `undefined` object. pub unsafe fn undefined(out: &mut Local, env: Env) { @@ -44,8 +45,13 @@ pub unsafe fn number_value(env: Env, p: Local) -> f64 { value } -/// Mutates the `out` argument provided to refer to a newly created `Local` containing a -/// JavaScript symbol. -pub unsafe fn symbol(out: &mut Local, env: Env, desc: Local) { - napi::create_symbol(env, desc, out as *mut Local); +/// Returns a newly created `Local` containing a JavaScript symbol. +/// Panics if `desc` is not a `Local` representing a `ValueType::String` or a null pointer. +pub unsafe fn symbol(env: Env, desc: Local) -> Local { + let mut local = MaybeUninit::uninit(); + assert_eq!( + napi::create_symbol(env, desc, local.as_mut_ptr()), + napi::Status::Ok + ); + local.assume_init() } diff --git a/src/context/mod.rs b/src/context/mod.rs index 1fdf1a814..cfe6ba3bd 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -164,7 +164,7 @@ use crate::types::boxed::{Finalize, JsBox}; #[cfg(feature = "napi-5")] use crate::types::date::{DateError, JsDate}; use crate::types::error::JsError; -#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +#[cfg(all(feature = "napi-1", feature = "symbol-api"))] use crate::types::symbol::JsSymbol; use crate::types::{ JsArray, JsBoolean, JsFunction, JsNull, JsNumber, JsObject, JsString, JsUndefined, JsValue, @@ -443,7 +443,7 @@ pub trait Context<'a>: ContextInternal<'a> { /// Convenience method for creating a `JsSymbol` value. /// /// If the string exceeds the limits of the JS engine, this method panics. - #[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] + #[cfg(all(feature = "napi-1", feature = "symbol-api"))] fn symbol>(&mut self, description: S) -> Handle<'a, JsSymbol> { let desc = self.string(description); JsSymbol::with_description(self, desc) diff --git a/src/prelude.rs b/src/prelude.rs index 04ec2c041..13decee8a 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -22,7 +22,7 @@ pub use crate::register_module; pub use crate::result::{JsResult, JsResultExt, NeonResult}; #[cfg(feature = "legacy-runtime")] pub use crate::task::Task; -#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +#[cfg(all(feature = "napi-1", feature = "symbol-api"))] pub use crate::types::symbol::JsSymbol; pub use crate::types::{ BinaryData, JsArray, JsArrayBuffer, JsBoolean, JsBuffer, JsError, JsFunction, JsNull, JsNumber, diff --git a/src/types/mod.rs b/src/types/mod.rs index eb0478843..eed2c000c 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -81,7 +81,7 @@ pub(crate) mod date; pub(crate) mod error; pub(crate) mod internal; -#[cfg(all(feature = "napi-1", feature = "symbol-primitive-api"))] +#[cfg(all(feature = "napi-1", feature = "symbol-api"))] pub(crate) mod symbol; pub(crate) mod utf8; @@ -108,6 +108,8 @@ pub use self::boxed::JsBox; #[cfg(feature = "napi-5")] pub use self::date::{DateError, DateErrorKind, JsDate}; pub use self::error::JsError; +#[cfg(all(feature = "napi-1", feature = "symbol-api"))] +pub use self::symbol::JsSymbol; pub(crate) fn build<'a, T: Managed, F: FnOnce(&mut raw::Local) -> bool>( env: Env, diff --git a/src/types/symbol.rs b/src/types/symbol.rs index c18af75e3..f45ca7535 100644 --- a/src/types/symbol.rs +++ b/src/types/symbol.rs @@ -53,11 +53,10 @@ impl JsSymbol { ) -> Handle<'a, JsSymbol> { unsafe { let desc_local = match desc { - None => std::mem::zeroed(), + None => std::ptr::null_mut(), Some(h) => h.to_raw(), }; - let mut sym_local = std::mem::zeroed(); - neon_runtime::primitive::symbol(&mut sym_local, env.to_raw(), desc_local); + let sym_local = neon_runtime::primitive::symbol(env.to_raw(), desc_local); Handle::new_internal(JsSymbol(sym_local)) } } diff --git a/test/napi/Cargo.toml b/test/napi/Cargo.toml index a8fed4ec0..a8501bd63 100644 --- a/test/napi/Cargo.toml +++ b/test/napi/Cargo.toml @@ -13,4 +13,4 @@ crate-type = ["cdylib"] version = "*" path = "../.." default-features = false -features = ["default-panic-hook", "napi-6", "try-catch-api", "event-queue-api", "symbol-primitive-api"] +features = ["default-panic-hook", "napi-6", "try-catch-api", "event-queue-api", "symbol-api"] diff --git a/test/napi/lib/objects.js b/test/napi/lib/objects.js index 607f0a446..265884709 100644 --- a/test/napi/lib/objects.js +++ b/test/napi/lib/objects.js @@ -26,8 +26,8 @@ describe('JsObject', function() { const obj = addon.return_js_object_with_symbol_property_key(); const propertySymbols = Object.getOwnPropertySymbols(obj); assert.equal(propertySymbols.length, 1); - const sym = propertySymbols[0]; - assert.equal(typeof sym, "symbol"); + const [sym] = propertySymbols; + assert.typeOf(sym, "symbol"); assert.equal(obj[sym], sym); }) diff --git a/test/napi/lib/symbols.js b/test/napi/lib/symbols.js index 59763cc9e..2fb8f6430 100644 --- a/test/napi/lib/symbols.js +++ b/test/napi/lib/symbols.js @@ -1,27 +1,38 @@ const addon = require('..'); const { assert } = require('chai'); -describe('JsSymbol', function() { +describe('JsSymbol', function () { + it('should return a JsSymbol with a description built with the context helper in Rust', function () { + const sym = addon.return_js_symbol_from_context_helper(); + assert.typeOf(sym, 'symbol'); + assert.equal(sym.description, 'neon:context_helper'); + }); + it('should return a JsSymbol with a description built in Rust', function () { - const sym = addon.return_js_symbol_with_description(); - assert.equal(typeof sym, 'symbol'); - assert.equal(sym.description, "neon:description"); + const description = 'neon:description' + const sym = addon.return_js_symbol_with_description(description); + assert.typeOf(sym, 'symbol'); + assert.equal(sym.description, description); }); + it('should return a JsSymbol without a description built in Rust', function () { const sym = addon.return_js_symbol(); - assert.equal(typeof sym, 'symbol'); + assert.typeOf(sym, 'symbol'); assert.equal(sym.description, undefined); }); + it('should read the description property in Rust', function () { const sym = Symbol('neon:description'); const description = addon.read_js_symbol_description(sym); assert.equal(description, 'neon:description'); }); + it('should read an undefined description property in Rust', function () { const sym = Symbol(); const description = addon.read_js_symbol_description(sym); assert.equal(description, undefined); }); + it('accepts and returns symbols', function () { const symDesc = Symbol('neon:description'); const symNoDesc = Symbol(); diff --git a/test/napi/lib/types.js b/test/napi/lib/types.js index eaf8231f1..18380c316 100644 --- a/test/napi/lib/types.js +++ b/test/napi/lib/types.js @@ -74,7 +74,7 @@ describe('type checks', function() { assert(addon.is_symbol(Symbol())); assert(addon.is_symbol(Symbol("unique symbol"))); assert(addon.is_symbol(Symbol.for('neon:description'))); - assert(addon.is_symbol(Symbol.toStringTag)); + assert(addon.is_symbol(Symbol.iterator)); assert(!addon.is_symbol(undefined)); assert(!addon.is_symbol("anything other than symbol")); }); diff --git a/test/napi/src/js/objects.rs b/test/napi/src/js/objects.rs index 5c5c074c8..458cfdcb3 100644 --- a/test/napi/src/js/objects.rs +++ b/test/napi/src/js/objects.rs @@ -32,7 +32,7 @@ pub fn return_js_object_with_string(mut cx: FunctionContext) -> JsResult JsResult { - let js_object: Handle = cx.empty_object(); + let js_object = cx.empty_object(); let s = cx.symbol("neon:description"); js_object.set(&mut cx, s, s)?; Ok(js_object) diff --git a/test/napi/src/js/symbols.rs b/test/napi/src/js/symbols.rs index 714487dd1..1aebdba81 100644 --- a/test/napi/src/js/symbols.rs +++ b/test/napi/src/js/symbols.rs @@ -1,7 +1,12 @@ use neon::prelude::*; +pub fn return_js_symbol_from_context_helper(mut cx: FunctionContext) -> JsResult { + Ok(cx.symbol("neon:context_helper")) +} + pub fn return_js_symbol_with_description(mut cx: FunctionContext) -> JsResult { - Ok(cx.symbol("neon:description")) + let description: Handle = cx.argument(0)?; + Ok(JsSymbol::with_description(&mut cx, description)) } pub fn return_js_symbol(mut cx: FunctionContext) -> JsResult { @@ -10,10 +15,10 @@ pub fn return_js_symbol(mut cx: FunctionContext) -> JsResult { pub fn read_js_symbol_description(mut cx: FunctionContext) -> JsResult { let symbol: Handle = cx.argument(0)?; - match symbol.description(&mut cx) { - None => Ok(cx.undefined().upcast()), - Some(s) => Ok(s.upcast()), - } + symbol + .description(&mut cx) + .map(|v| Ok(v.upcast())) + .unwrap_or_else(|| Ok(cx.undefined().upcast())) } pub fn accept_and_return_js_symbol(mut cx: FunctionContext) -> JsResult { diff --git a/test/napi/src/lib.rs b/test/napi/src/lib.rs index 97a245a02..86e148c53 100644 --- a/test/napi/src/lib.rs +++ b/test/napi/src/lib.rs @@ -265,6 +265,10 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { cx.export_function("leak_channel", leak_channel)?; cx.export_function("drop_global_queue", drop_global_queue)?; + cx.export_function( + "return_js_symbol_from_context_helper", + return_js_symbol_from_context_helper, + )?; cx.export_function( "return_js_symbol_with_description", return_js_symbol_with_description, From cf740fd3c2936eb0269b9031bafae7f372333b39 Mon Sep 17 00:00:00 2001 From: Chris Bajorin Date: Mon, 2 Aug 2021 11:50:26 -0400 Subject: [PATCH 4/4] added get_named_property --- .../src/napi/bindings/functions.rs | 7 ++++++ crates/neon-runtime/src/napi/object.rs | 17 +++++++++++-- src/types/symbol.rs | 24 ++++++++----------- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/crates/neon-runtime/src/napi/bindings/functions.rs b/crates/neon-runtime/src/napi/bindings/functions.rs index 7690c9e00..af2c056c2 100644 --- a/crates/neon-runtime/src/napi/bindings/functions.rs +++ b/crates/neon-runtime/src/napi/bindings/functions.rs @@ -151,6 +151,13 @@ mod napi1 { fn get_property(env: Env, object: Value, key: Value, result: *mut Value) -> Status; + fn get_named_property( + env: Env, + object: Value, + utf8name: *const c_char, + result: *mut Value, + ) -> Status; + fn set_element(env: Env, object: Value, index: u32, value: Value) -> Status; fn get_element(env: Env, object: Value, index: u32, result: *mut Value) -> Status; diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index b5e96953a..305e2c320 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -1,7 +1,7 @@ -use std::mem::MaybeUninit; - use crate::napi::bindings as napi; use crate::raw::{Env, Local}; +use std::mem::MaybeUninit; +use std::os::raw::c_char; /// Mutates the `out` argument to refer to a `napi_value` containing a newly created JavaScript Object. pub unsafe fn new(out: &mut Local, env: Env) { @@ -131,3 +131,16 @@ pub unsafe fn set(out: &mut bool, env: Env, object: Local, key: Local, val: Loca *out } + +/// Returns an `Option` representing the value of the property of `object` named by +/// the `key` value, where `key` is a pointer to a null-terminated byte string. Returns `None` if +/// the value couldn't be retrieved. +pub unsafe fn get_named(env: Env, object: Local, key: *const u8) -> Option { + let mut local = MaybeUninit::uninit(); + if napi::get_named_property(env, object, key as *const _, local.as_mut_ptr()) + != napi::Status::Ok + { + return None; + } + Some(local.assume_init()) +} diff --git a/src/types/symbol.rs b/src/types/symbol.rs index f45ca7535..03aa5760e 100644 --- a/src/types/symbol.rs +++ b/src/types/symbol.rs @@ -1,7 +1,6 @@ use crate::context::Context; use crate::handle::{Handle, Managed}; use crate::types::internal::ValueInternal; -use crate::types::utf8::Utf8; use crate::types::{Env, JsString, Value}; use neon_runtime::raw; @@ -30,20 +29,17 @@ impl JsSymbol { /// Get the optional symbol description, where `None` represents an undefined description. pub fn description<'a, C: Context<'a>>(self, cx: &mut C) -> Option> { let env = cx.env().to_raw(); - let (desc_ptr, desc_len) = Utf8::from("description").into_small_unwrap().lower(); - + const DESCRIPTION_KEY: &[u8] = b"description\0"; unsafe { - let mut local = std::mem::zeroed(); - if !neon_runtime::object::get_string(env, &mut local, self.to_raw(), desc_ptr, desc_len) - { - return None; - } - - if neon_runtime::tag::is_string(env, local) { - Some(Handle::new_internal(JsString(local))) - } else { - None - } + neon_runtime::object::get_named(env, self.to_raw(), DESCRIPTION_KEY.as_ptr()).and_then( + |local| { + if neon_runtime::tag::is_string(env, local) { + Some(Handle::new_internal(JsString(local))) + } else { + None + } + }, + ) } }