From 42dc3a97bafc5a9162112404fa9ebfb5b8fd3555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 20 Feb 2020 13:53:20 +0100 Subject: [PATCH 01/18] napi: add test for a plain object --- test/napi/lib/hello.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/napi/lib/hello.js b/test/napi/lib/hello.js index e01f15707..a9fc850ca 100644 --- a/test/napi/lib/hello.js +++ b/test/napi/lib/hello.js @@ -19,4 +19,11 @@ describe('hello', function() { assert.strictEqual(addon.one, 1); assert.strictEqual(addon.two, 2.1); }); + + it('should be able to create JS objects in rust', function () { + assert.deepEqual(addon.rustCreated, { + a: 1, + whatever: true + }) + }); }); From 071b1da4abdb463bc087a6320e4fc96b97fb982b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 20 Feb 2020 14:03:14 +0100 Subject: [PATCH 02/18] implement empty_object() --- crates/neon-runtime/src/napi/object.rs | 4 +++- crates/neon-sys/native/src/neon.cc | 2 +- crates/neon-sys/native/src/neon.h | 2 +- crates/neon-sys/src/lib.rs | 2 +- src/types/mod.rs | 10 ++++++---- test/napi/native/src/lib.rs | 13 +++++++++++++ 6 files changed, 25 insertions(+), 8 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 0c6aedc79..e2fc9c3f4 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -4,7 +4,9 @@ use nodejs_sys as napi; use raw::{Env, Local}; -pub unsafe extern "C" fn new(_out: &mut Local) { unimplemented!() } +pub unsafe extern "C" fn new(out: &mut Local, env: Env) { + napi::napi_create_object(env, out as *mut Local); +} pub unsafe extern "C" fn get_own_property_names(_out: &mut Local, _object: Local) -> bool { unimplemented!() } diff --git a/crates/neon-sys/native/src/neon.cc b/crates/neon-sys/native/src/neon.cc index 8f7a23ad3..aadb26a88 100644 --- a/crates/neon-sys/native/src/neon.cc +++ b/crates/neon-sys/native/src/neon.cc @@ -48,7 +48,7 @@ extern "C" void Neon_Call_Get(v8::FunctionCallbackInfo *info, int32_t *out = (*info)[i]; } -extern "C" void Neon_Object_New(v8::Local *out) { +extern "C" void Neon_Object_New(v8::Local *out, v8::Isolate *isolate) { *out = Nan::New(); } diff --git a/crates/neon-sys/native/src/neon.h b/crates/neon-sys/native/src/neon.h index 1dcdb98d7..56926f535 100644 --- a/crates/neon-sys/native/src/neon.h +++ b/crates/neon-sys/native/src/neon.h @@ -29,7 +29,7 @@ extern "C" { bool Neon_Primitive_IsUint32(v8::Local p); bool Neon_Primitive_IsInt32(v8::Local p); - void Neon_Object_New(v8::Local *out); + void Neon_Object_New(v8::Local *out, v8::Isolate *isolate); bool Neon_Object_GetOwnPropertyNames(v8::Local *out, v8::Local obj); void *Neon_Object_GetIsolate(v8::Local obj); bool Neon_Object_Get_Index(v8::Local *out, v8::Local object, uint32_t index); diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs index d43d259f6..51079626f 100644 --- a/crates/neon-sys/src/lib.rs +++ b/crates/neon-sys/src/lib.rs @@ -143,7 +143,7 @@ extern "C" { pub fn Neon_Module_ExecCallback(callback: CCallback, exports: Local, vm: *mut c_void); pub fn Neon_Module_GetVersion() -> i32; - pub fn Neon_Object_New(out: &mut Local); + pub fn Neon_Object_New(out: &mut Local, isolate: Isolate); pub fn Neon_Object_GetOwnPropertyNames(out: &mut Local, object: Local) -> bool; pub fn Neon_Object_GetIsolate(obj: Local) -> Isolate; pub fn Neon_Object_Get_Index(out: &mut Local, object: Local, index: u32) -> bool; diff --git a/src/types/mod.rs b/src/types/mod.rs index a88814169..bd355f270 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -452,12 +452,14 @@ impl ValueInternal for JsObject { impl Object for JsObject { } impl JsObject { - pub fn new<'a, C: Context<'a>>(_: &mut C) -> Handle<'a, JsObject> { - JsObject::new_internal() + pub fn new<'a, C: Context<'a>>(c: &mut C) -> Handle<'a, JsObject> { + JsObject::new_internal(c.env()) } - pub(crate) fn new_internal<'a>() -> Handle<'a, JsObject> { - JsObject::build(|out| { unsafe { neon_runtime::object::new(out) } }) + pub(crate) fn new_internal<'a>(env: Env) -> Handle<'a, JsObject> { + JsObject::build(|out| { + unsafe { neon_runtime::object::new(out, env.to_raw()) } + }) } pub(crate) fn build<'a, F: FnOnce(&mut raw::Local)>(init: F) -> Handle<'a, JsObject> { diff --git a/test/napi/native/src/lib.rs b/test/napi/native/src/lib.rs index a2324ba63..49d7a9169 100644 --- a/test/napi/native/src/lib.rs +++ b/test/napi/native/src/lib.rs @@ -29,5 +29,18 @@ register_module!(|mut cx| { cx.export_value("one", one)?; cx.export_value("two", two)?; + // Plain objects. + let mut rust_created = cx.empty_object(); + { + let a = cx.number(1); + rust_created.set(&mut cx, "a", a); + } + { + let whatever = cx.boolean(true); + rust_created.set(&mut cx, "whatever", whatever); + } + + cx.export_value("rustCreated", rust_created); + Ok(()) }); From e2a75c1099b0694166c354f6483a0fd02c15775f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 20 Feb 2020 14:18:05 +0100 Subject: [PATCH 03/18] Implement Object#set() --- crates/neon-runtime/src/napi/object.rs | 6 +++++- crates/neon-sys/native/src/neon.cc | 1 + src/object/mod.rs | 4 ++-- test/napi/native/src/lib.rs | 8 ++++---- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index e2fc9c3f4..d8eed71ba 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -55,4 +55,8 @@ pub unsafe extern "C" fn set_string( pub unsafe extern "C" fn get(_out: &mut Local, _object: Local, _key: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn set(_out: &mut bool, _object: Local, _key: Local, _val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn set(out: &mut bool, env: Env, object: Local, key: Local, val: Local) -> bool { + let status = napi::napi_set_property(env, object, key, val); + *out = status == napi::napi_status::napi_ok; + return *out; +} diff --git a/crates/neon-sys/native/src/neon.cc b/crates/neon-sys/native/src/neon.cc index aadb26a88..b992b66ef 100644 --- a/crates/neon-sys/native/src/neon.cc +++ b/crates/neon-sys/native/src/neon.cc @@ -139,6 +139,7 @@ extern "C" bool Neon_Object_Get(v8::Local *out, v8::Local } extern "C" bool Neon_Object_Set(bool *out, v8::Local obj, v8::Local key, v8::Local val) { + // Only returns `Just(true)` or `Empty()`. Nan::Maybe maybe = Nan::Set(obj, key, val); if (maybe.IsJust()) { *out = maybe.FromJust(); diff --git a/src/object/mod.rs b/src/object/mod.rs index 658cc7c0c..b43c7a06f 100644 --- a/src/object/mod.rs +++ b/src/object/mod.rs @@ -139,12 +139,12 @@ mod traits { unsafe fn set_from<'c, C: Context<'c>>( self, - _cx: &mut C, + cx: &mut C, out: &mut bool, obj: raw::Local, val: raw::Local, ) -> bool { - neon_runtime::object::set(out, obj, self.to_raw(), val) + neon_runtime::object::set(out, cx.env().to_raw(), obj, self.to_raw(), val) } } diff --git a/test/napi/native/src/lib.rs b/test/napi/native/src/lib.rs index 49d7a9169..8266e92ba 100644 --- a/test/napi/native/src/lib.rs +++ b/test/napi/native/src/lib.rs @@ -30,17 +30,17 @@ register_module!(|mut cx| { cx.export_value("two", two)?; // Plain objects. - let mut rust_created = cx.empty_object(); + let rust_created = cx.empty_object(); { let a = cx.number(1); - rust_created.set(&mut cx, "a", a); + rust_created.set(&mut cx, "a", a)?; } { let whatever = cx.boolean(true); - rust_created.set(&mut cx, "whatever", whatever); + rust_created.set(&mut cx, "whatever", whatever)?; } - cx.export_value("rustCreated", rust_created); + cx.export_value("rustCreated", rust_created)?; Ok(()) }); From 70f15f80f21c907ce1e961d63d0258b8f668fba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 20 Feb 2020 14:44:25 +0100 Subject: [PATCH 04/18] Implement Object#get() --- crates/neon-runtime/src/napi/object.rs | 36 +++++++++++++++++++++++--- src/object/mod.rs | 16 ++++++++---- test/napi/native/src/lib.rs | 9 +++++++ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index d8eed71ba..62ee3aef7 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -16,7 +16,32 @@ pub unsafe extern "C" fn get_index(_out: &mut Local, _object: Local, _index: u32 pub unsafe extern "C" fn set_index(_out: &mut bool, _object: Local, _index: u32, _val: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn get_string(_out: &mut Local, _object: Local, _key: *const u8, _len: i32) -> bool { unimplemented!() } +pub unsafe extern "C" fn get_string(env: Env, out: &mut Local, object: Local, key: *const u8, len: i32) -> bool { + let mut key_val = MaybeUninit::uninit(); + + // Not using `crate::string::new()` because it requires a _reference_ to a Local, + // while we only have uninitialized memory. + if napi::napi_create_string_utf8( + env, + key as *const i8, + len as usize, + key_val.as_mut_ptr(), + ) != napi::napi_status::napi_ok { + return false; + } + + // Not using napi_get_named_property() because the `key` may not be null terminated. + if napi::napi_get_property( + env, + object, + key_val.assume_init(), + out as *mut Local, + ) != napi::napi_status::napi_ok { + return false; + } + + true +} pub unsafe extern "C" fn set_string( env: Env, @@ -53,10 +78,15 @@ pub unsafe extern "C" fn set_string( true } -pub unsafe extern "C" fn get(_out: &mut Local, _object: Local, _key: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn get(out: &mut Local, env: Env, object: Local, key: Local) -> bool { + let status = napi::napi_get_property(env, object, key, out as *mut Local); + + status == napi::napi_status::napi_ok +} pub unsafe extern "C" fn set(out: &mut bool, env: Env, object: Local, key: Local, val: Local) -> bool { let status = napi::napi_set_property(env, object, key, val); *out = status == napi::napi_status::napi_ok; - return *out; + + *out } diff --git a/src/object/mod.rs b/src/object/mod.rs index b43c7a06f..2a8fe50aa 100644 --- a/src/object/mod.rs +++ b/src/object/mod.rs @@ -130,11 +130,13 @@ mod traits { impl<'a, K: Value> PropertyKey for Handle<'a, K> { unsafe fn get_from<'c, C: Context<'c>>( self, - _cx: &mut C, + cx: &mut C, out: &mut raw::Local, obj: raw::Local ) -> bool { - neon_runtime::object::get(out, obj, self.to_raw()) + let env = cx.env().to_raw(); + + neon_runtime::object::get(out, env, obj, self.to_raw()) } unsafe fn set_from<'c, C: Context<'c>>( @@ -144,19 +146,23 @@ mod traits { obj: raw::Local, val: raw::Local, ) -> bool { - neon_runtime::object::set(out, cx.env().to_raw(), obj, self.to_raw(), val) + let env = cx.env().to_raw(); + + neon_runtime::object::set(out, env, obj, self.to_raw(), val) } } impl<'a> PropertyKey for &'a str { unsafe fn get_from<'c, C: Context<'c>>( self, - _cx: &mut C, + cx: &mut C, out: &mut raw::Local, obj: raw::Local ) -> bool { let (ptr, len) = Utf8::from(self).into_small_unwrap().lower(); - neon_runtime::object::get_string(out, obj, ptr, len) + let env = cx.env().to_raw(); + + neon_runtime::object::get_string(env, out, obj, ptr, len) } unsafe fn set_from<'c, C: Context<'c>>( diff --git a/test/napi/native/src/lib.rs b/test/napi/native/src/lib.rs index 8266e92ba..65d2bb63d 100644 --- a/test/napi/native/src/lib.rs +++ b/test/napi/native/src/lib.rs @@ -40,6 +40,15 @@ register_module!(|mut cx| { rust_created.set(&mut cx, "whatever", whatever)?; } + assert_eq!({ + let v: Handle = rust_created.get(&mut cx, "a")?.downcast_or_throw(&mut cx)?; + v.value(&mut cx) + }, 1.0f64); + assert_eq!({ + let v: Handle = rust_created.get(&mut cx, "whatever")?.downcast_or_throw(&mut cx)?; + v.value(&mut cx) + }, true); + cx.export_value("rustCreated", rust_created)?; Ok(()) From 3508034c38242ef8cb9b24eb1994b1f768821600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 20 Feb 2020 15:23:03 +0100 Subject: [PATCH 05/18] Implement required tag checks for the object test --- crates/neon-runtime/src/napi/tag.rs | 39 +++++++++++++------ crates/neon-sys/native/src/neon.cc | 22 +++++------ crates/neon-sys/native/src/neon.h | 22 +++++------ crates/neon-sys/src/lib.rs | 22 +++++------ src/handle/mod.rs | 58 ++++++++++++++++++++++++++++- src/object/class/internal.rs | 3 +- src/object/class/mod.rs | 7 +--- src/types/binary.rs | 9 +++-- src/types/error.rs | 5 ++- src/types/internal.rs | 7 ++-- src/types/mod.rs | 34 ++++++++--------- 11 files changed, 149 insertions(+), 79 deletions(-) diff --git a/crates/neon-runtime/src/napi/tag.rs b/crates/neon-runtime/src/napi/tag.rs index 387158847..c481bca8e 100644 --- a/crates/neon-runtime/src/napi/tag.rs +++ b/crates/neon-runtime/src/napi/tag.rs @@ -1,23 +1,38 @@ -use raw::Local; +use raw::{Env, Local}; -pub unsafe extern "C" fn is_undefined(_val: Local) -> bool { unimplemented!() } +use nodejs_sys as napi; -pub unsafe extern "C" fn is_null(_val: Local) -> bool { unimplemented!() } +unsafe fn is_type(env: Env, val: Local, expect: napi::napi_valuetype) -> bool { + let mut actual = napi::napi_valuetype::napi_undefined; + if napi::napi_typeof(env, val, &mut actual as *mut _) == napi::napi_status::napi_ok { + actual == expect + } else { + false + } +} -pub unsafe extern "C" fn is_number(_val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_undefined(_env: Env, _val: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn is_boolean(_val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_null(_env: Env, _val: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn is_string(_val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_number(env: Env, val: Local) -> bool { + is_type(env, val, napi::napi_valuetype::napi_number) +} -pub unsafe extern "C" fn is_object(_val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_boolean(env: Env, val: Local) -> bool { + is_type(env, val, napi::napi_valuetype::napi_boolean) +} -pub unsafe extern "C" fn is_array(_val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_string(_env: Env, _val: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn is_function(_val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_object(_env: Env, _val: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn is_error(_val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_array(_env: Env, _val: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn is_buffer(_obj: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_function(_env: Env, _val: Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn is_arraybuffer(_obj: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_error(_env: Env, _val: Local) -> bool { unimplemented!() } + +pub unsafe extern "C" fn is_buffer(_env: Env, _obj: Local) -> bool { unimplemented!() } + +pub unsafe extern "C" fn is_arraybuffer(_env: Env, _obj: Local) -> bool { unimplemented!() } diff --git a/crates/neon-sys/native/src/neon.cc b/crates/neon-sys/native/src/neon.cc index b992b66ef..0b75f2390 100644 --- a/crates/neon-sys/native/src/neon.cc +++ b/crates/neon-sys/native/src/neon.cc @@ -205,7 +205,7 @@ extern "C" size_t Neon_Buffer_Data(void **base_out, v8::Local obj) { return node::Buffer::Length(obj); } -extern "C" bool Neon_Tag_IsBuffer(v8::Local obj) { +extern "C" bool Neon_Tag_IsBuffer(v8::Isolate *isolate, v8::Local obj) { return node::Buffer::HasInstance(obj); } @@ -222,7 +222,7 @@ extern "C" size_t Neon_ArrayBuffer_Data(void **base_out, v8::Local value) { +extern "C" bool Neon_Tag_IsArrayBuffer(v8::Isolate *isolate, v8::Local value) { return value->IsArrayBuffer(); } @@ -447,39 +447,39 @@ extern "C" bool Neon_Fun_Construct(v8::Local *out, v8::Isolate *isol return maybe_result.ToLocal(out); } -extern "C" bool Neon_Tag_IsUndefined(v8::Local val) { +extern "C" bool Neon_Tag_IsUndefined(v8::Isolate *isolate, v8::Local val) { return val->IsUndefined(); } -extern "C" bool Neon_Tag_IsNull(v8::Local val) { +extern "C" bool Neon_Tag_IsNull(v8::Isolate *isolate, v8::Local val) { return val->IsNull(); } -extern "C" bool Neon_Tag_IsNumber(v8::Local val) { +extern "C" bool Neon_Tag_IsNumber(v8::Isolate *isolate, v8::Local val) { return val->IsNumber(); } -extern "C" bool Neon_Tag_IsBoolean(v8::Local val) { +extern "C" bool Neon_Tag_IsBoolean(v8::Isolate *isolate, v8::Local val) { return val->IsBoolean(); } -extern "C" bool Neon_Tag_IsString(v8::Local val) { +extern "C" bool Neon_Tag_IsString(v8::Isolate *isolate, v8::Local val) { return val->IsString(); } -extern "C" bool Neon_Tag_IsObject(v8::Local val) { +extern "C" bool Neon_Tag_IsObject(v8::Isolate *isolate, v8::Local val) { return val->IsObject(); } -extern "C" bool Neon_Tag_IsArray(v8::Local val) { +extern "C" bool Neon_Tag_IsArray(v8::Isolate *isolate, v8::Local val) { return val->IsArray(); } -extern "C" bool Neon_Tag_IsFunction(v8::Local val) { +extern "C" bool Neon_Tag_IsFunction(v8::Isolate *isolate, v8::Local val) { return val->IsFunction(); } -extern "C" bool Neon_Tag_IsError(v8::Local val) { +extern "C" bool Neon_Tag_IsError(v8::Isolate *isolate, v8::Local val) { return val->IsNativeError(); } diff --git a/crates/neon-sys/native/src/neon.h b/crates/neon-sys/native/src/neon.h index 56926f535..d6868b960 100644 --- a/crates/neon-sys/native/src/neon.h +++ b/crates/neon-sys/native/src/neon.h @@ -109,17 +109,17 @@ extern "C" { uint32_t Neon_Module_GetVersion(); - bool Neon_Tag_IsUndefined(v8::Local val); - bool Neon_Tag_IsNull(v8::Local val); - bool Neon_Tag_IsBoolean(v8::Local val); - bool Neon_Tag_IsNumber(v8::Local val); - bool Neon_Tag_IsString(v8::Local val); - bool Neon_Tag_IsObject(v8::Local val); - bool Neon_Tag_IsArray(v8::Local val); - bool Neon_Tag_IsFunction(v8::Local val); - bool Neon_Tag_IsBuffer(v8::Local obj); - bool Neon_Tag_IsArrayBuffer(v8::Local obj); - bool Neon_Tag_IsError(v8::Local val); + bool Neon_Tag_IsUndefined(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsNull(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsBoolean(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsNumber(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsString(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsObject(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsArray(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsFunction(v8::Isolate *isolate, v8::Local val); + bool Neon_Tag_IsBuffer(v8::Isolate *isolate, v8::Local obj); + bool Neon_Tag_IsArrayBuffer(v8::Isolate *isolate, v8::Local obj); + bool Neon_Tag_IsError(v8::Isolate *isolate, v8::Local val); void Neon_Error_NewError(v8::Local *out, v8::Local msg); void Neon_Error_NewTypeError(v8::Local *out, v8::Local msg); diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs index 51079626f..7d76b95c3 100644 --- a/crates/neon-sys/src/lib.rs +++ b/crates/neon-sys/src/lib.rs @@ -181,17 +181,17 @@ extern "C" { pub fn Neon_String_Utf8Length(str: Local) -> isize; pub fn Neon_String_Data(out: *mut u8, len: isize, str: Local) -> isize; - pub fn Neon_Tag_IsUndefined(val: Local) -> bool; - pub fn Neon_Tag_IsNull(val: Local) -> bool; - pub fn Neon_Tag_IsNumber(val: Local) -> bool; - pub fn Neon_Tag_IsBoolean(val: Local) -> bool; - pub fn Neon_Tag_IsString(val: Local) -> bool; - pub fn Neon_Tag_IsObject(val: Local) -> bool; - pub fn Neon_Tag_IsArray(val: Local) -> bool; - pub fn Neon_Tag_IsFunction(val: Local) -> bool; - pub fn Neon_Tag_IsError(val: Local) -> bool; - pub fn Neon_Tag_IsBuffer(obj: Local) -> bool; - pub fn Neon_Tag_IsArrayBuffer(obj: Local) -> bool; + pub fn Neon_Tag_IsUndefined(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsNull(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsNumber(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsBoolean(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsString(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsObject(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsArray(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsFunction(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsError(isolate: Isolate, val: Local) -> bool; + pub fn Neon_Tag_IsBuffer(isolate: Isolate, obj: Local) -> bool; + pub fn Neon_Tag_IsArrayBuffer(isolate: Isolate, obj: Local) -> bool; pub fn Neon_Task_Schedule(task: *mut c_void, perform: unsafe extern fn(*mut c_void) -> *mut c_void, diff --git a/src/handle/mod.rs b/src/handle/mod.rs index 88aece4ba..32ade0b2d 100644 --- a/src/handle/mod.rs +++ b/src/handle/mod.rs @@ -10,6 +10,7 @@ use neon_runtime; use neon_runtime::raw; use types::Value; use context::Context; +use context::internal::Env; use result::{JsResult, JsResultExt}; use self::internal::SuperType; @@ -102,6 +103,7 @@ impl<'a, T: Value> Handle<'a, T> { Handle::new_internal(SuperType::upcast_internal(self.value)) } + #[cfg(feature = "legacy-runtime")] /// Tests whether this value is an instance of the given type. /// /// # Example: @@ -117,20 +119,64 @@ impl<'a, T: Value> Handle<'a, T> { /// # } /// ``` pub fn is_a(&self) -> bool { - U::is_typeof(self.value) + U::is_typeof(Env::current(), self.value) } + #[cfg(feature = "napi-runtime")] + /// Tests whether this value is an instance of the given type. + /// + /// # Example: + /// + /// ```no_run + /// # use neon::prelude::*; + /// # fn my_neon_function(mut cx: FunctionContext) -> JsResult { + /// let v: Handle = cx.number(17).upcast(&mut cx); + /// v.is_a::(&mut cx); // false + /// v.is_a::(&mut cx); // true + /// v.is_a::(&mut cx); // true + /// # Ok(cx.undefined()) + /// # } + /// ``` + pub fn is_a<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> bool { + U::is_typeof(cx.env(), self.value) + } + + /// Tests whether this value is an instance of the given type. + /// + /// Works with both the legacy and the N-API runtime with the same signature. This allows other + /// parts of the codebase to care less about which version we're in. + pub(crate) fn is_a_compat<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> bool { + #[cfg(feature = "legacy-runtime")] + return self.is_a::(); + #[cfg(feature = "napi-runtime")] + return self.is_a::(cx); + } + + #[cfg(feature = "legacy-runtime")] /// Attempts to downcast a handle to another type, which may fail. A failure /// to downcast **does not** throw a JavaScript exception, so it's OK to /// continue interacting with the JS engine if this method produces an `Err` /// result. pub fn downcast(&self) -> DowncastResult<'a, T, U> { - match U::downcast(self.value) { + match U::downcast(Env::current(), self.value) { Some(v) => Ok(Handle::new_internal(v)), None => Err(DowncastError::new()) } } + #[cfg(feature = "napi-runtime")] + /// Attempts to downcast a handle to another type, which may fail. A failure + /// to downcast **does not** throw a JavaScript exception, so it's OK to + /// continue interacting with the JS engine if this method produces an `Err` + /// result. + pub fn downcast<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> DowncastResult<'a, T, U> { + match U::downcast(cx.env(), self.value) { + Some(v) => Ok(Handle::new_internal(v)), + None => Err(DowncastError::new()) + } + } + + #[cfg(feature = "legacy-runtime")] /// Attempts to downcast a handle to another type, raising a JavaScript `TypeError` /// exception on failure. This method is a convenient shorthand, equivalent to /// `self.downcast::().or_throw::(cx)`. @@ -138,6 +184,14 @@ impl<'a, T: Value> Handle<'a, T> { self.downcast().or_throw(cx) } + #[cfg(feature = "napi-runtime")] + /// Attempts to downcast a handle to another type, raising a JavaScript `TypeError` + /// exception on failure. This method is a convenient shorthand, equivalent to + /// `self.downcast::().or_throw::(cx)`. + pub fn downcast_or_throw<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> JsResult<'a, U> { + self.downcast(cx).or_throw(cx) + } + } impl<'a, T: Managed> Deref for Handle<'a, T> { diff --git a/src/object/class/internal.rs b/src/object/class/internal.rs index c707169cc..1a0f42183 100644 --- a/src/object/class/internal.rs +++ b/src/object/class/internal.rs @@ -20,7 +20,8 @@ impl Callback<()> for MethodCallback { info.with_cx::(|mut cx| { let data = info.data(); let this: Handle = Handle::new_internal(JsValue::from_raw(info.this(&mut cx))); - if !this.is_a::() { + let is_a_t = this.is_a_compat::(&mut cx); + if !is_a_t { if let Ok(metadata) = T::metadata(&mut cx) { neon_runtime::class::throw_this_error(mem::transmute(cx.env()), metadata.pointer); } diff --git a/src/object/class/mod.rs b/src/object/class/mod.rs index 8a06609d3..dd04e196b 100644 --- a/src/object/class/mod.rs +++ b/src/object/class/mod.rs @@ -220,11 +220,8 @@ impl ValueInternal for T { } } - fn is_typeof(value: Other) -> bool { - let mut isolate: Env = unsafe { - mem::transmute(neon_runtime::call::current_isolate()) - }; - let map = isolate.class_map(); + fn is_typeof(mut env: Env, value: Other) -> bool { + let map = env.class_map(); match map.get(&TypeId::of::()) { None => false, Some(ref metadata) => unsafe { diff --git a/src/types/binary.rs b/src/types/binary.rs index 80cd7ef59..e6dcf5d60 100644 --- a/src/types/binary.rs +++ b/src/types/binary.rs @@ -5,6 +5,7 @@ use std::mem::{self, MaybeUninit}; use std::os::raw::c_void; use std::slice; use context::{Context, Lock}; +use context::internal::Env; use borrow::{Borrow, BorrowMut, Ref, RefMut, LoanError}; use borrow::internal::Pointer; use handle::Managed; @@ -42,8 +43,8 @@ impl Managed for JsBuffer { impl ValueInternal for JsBuffer { fn name() -> String { "Buffer".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_buffer(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_buffer(env.to_raw(), other.to_raw()) } } } @@ -74,8 +75,8 @@ impl Managed for JsArrayBuffer { impl ValueInternal for JsArrayBuffer { fn name() -> String { "ArrayBuffer".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_arraybuffer(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_arraybuffer(env.to_raw(), other.to_raw()) } } } diff --git a/src/types/error.rs b/src/types/error.rs index 8b8f3e51e..d3f421979 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -6,6 +6,7 @@ use neon_runtime; use neon_runtime::raw; use context::Context; +use context::internal::Env; use result::{NeonResult, Throw}; use types::{Value, Object, Handle, Managed, build}; use types::internal::ValueInternal; @@ -25,8 +26,8 @@ impl Managed for JsError { impl ValueInternal for JsError { fn name() -> String { "Error".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_error(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_error(env.to_raw(), other.to_raw()) } } } diff --git a/src/types/internal.rs b/src/types/internal.rs index 3675c1e00..880b9d4bd 100644 --- a/src/types/internal.rs +++ b/src/types/internal.rs @@ -3,6 +3,7 @@ use std::os::raw::c_void; use neon_runtime; use neon_runtime::raw; use context::{CallbackInfo, FunctionContext}; +use context::internal::Env; use types::error::convert_panics; use types::{JsObject, Handle, Managed}; use result::JsResult; @@ -12,10 +13,10 @@ use super::Value; pub trait ValueInternal: Managed + 'static { fn name() -> String; - fn is_typeof(other: Other) -> bool; + fn is_typeof(env: Env, other: Other) -> bool; - fn downcast(other: Other) -> Option { - if Self::is_typeof(other) { + fn downcast(env: Env, other: Other) -> Option { + if Self::is_typeof(env, other) { Some(Self::from_raw(other.to_raw())) } else { None diff --git a/src/types/mod.rs b/src/types/mod.rs index bd355f270..44b6cb84a 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -75,7 +75,7 @@ impl Managed for JsValue { impl ValueInternal for JsValue { fn name() -> String { "any".to_string() } - fn is_typeof(_: Other) -> bool { + fn is_typeof(_env: Env, _other: Other) -> bool { true } } @@ -154,8 +154,8 @@ unsafe impl This for JsUndefined { impl ValueInternal for JsUndefined { fn name() -> String { "undefined".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_undefined(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_undefined(env.to_raw(), other.to_raw()) } } } @@ -195,8 +195,8 @@ impl Managed for JsNull { impl ValueInternal for JsNull { fn name() -> String { "null".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_null(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_null(env.to_raw(), other.to_raw()) } } } @@ -245,8 +245,8 @@ impl Managed for JsBoolean { impl ValueInternal for JsBoolean { fn name() -> String { "boolean".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_boolean(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_boolean(env.to_raw(), other.to_raw()) } } } @@ -288,8 +288,8 @@ impl Managed for JsString { impl ValueInternal for JsString { fn name() -> String { "string".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_string(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_string(env.to_raw(), other.to_raw()) } } } @@ -411,8 +411,8 @@ impl Managed for JsNumber { impl ValueInternal for JsNumber { fn name() -> String { "number".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_number(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_number(env.to_raw(), other.to_raw()) } } } @@ -444,8 +444,8 @@ unsafe impl This for JsObject { impl ValueInternal for JsObject { fn name() -> String { "object".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_object(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_object(env.to_raw(), other.to_raw()) } } } @@ -522,8 +522,8 @@ impl Managed for JsArray { impl ValueInternal for JsArray { fn name() -> String { "Array".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_array(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_array(env.to_raw(), other.to_raw()) } } } @@ -615,7 +615,7 @@ impl Managed for JsFunction { impl ValueInternal for JsFunction { fn name() -> String { "function".to_string() } - fn is_typeof(other: Other) -> bool { - unsafe { neon_runtime::tag::is_function(other.to_raw()) } + fn is_typeof(env: Env, other: Other) -> bool { + unsafe { neon_runtime::tag::is_function(env.to_raw(), other.to_raw()) } } } From 6526dd3babba1705d4b580f47c999e77d94bde0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 21 Feb 2020 11:28:59 +0100 Subject: [PATCH 06/18] Implement Object#get_own_property_names() --- crates/neon-runtime/src/napi/object.rs | 7 ++++++- crates/neon-sys/native/src/neon.cc | 2 +- crates/neon-sys/native/src/neon.h | 2 +- crates/neon-sys/src/lib.rs | 2 +- src/object/mod.rs | 15 +++++++++++---- test/napi/native/src/lib.rs | 7 +++++++ 6 files changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 62ee3aef7..0535ba513 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -8,8 +8,13 @@ pub unsafe extern "C" fn new(out: &mut Local, env: Env) { napi::napi_create_object(env, out as *mut Local); } -pub unsafe extern "C" fn get_own_property_names(_out: &mut Local, _object: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, object: Local) -> bool { + let status = napi::napi_get_property_names(env, object, out as *mut Local); + status == napi::napi_status::napi_ok +} + +// Unused. pub unsafe extern "C" fn get_isolate(_obj: Local) -> Env { unimplemented!() } pub unsafe extern "C" fn get_index(_out: &mut Local, _object: Local, _index: u32) -> bool { unimplemented!() } diff --git a/crates/neon-sys/native/src/neon.cc b/crates/neon-sys/native/src/neon.cc index 0b75f2390..2f3f5f152 100644 --- a/crates/neon-sys/native/src/neon.cc +++ b/crates/neon-sys/native/src/neon.cc @@ -52,7 +52,7 @@ extern "C" void Neon_Object_New(v8::Local *out, v8::Isolate *isolate *out = Nan::New(); } -extern "C" bool Neon_Object_GetOwnPropertyNames(v8::Local *out, v8::Local obj) { +extern "C" bool Neon_Object_GetOwnPropertyNames(v8::Local *out, v8::Isolate *isolate, v8::Local obj) { Nan::MaybeLocal maybe = Nan::GetOwnPropertyNames(obj); return maybe.ToLocal(out); } diff --git a/crates/neon-sys/native/src/neon.h b/crates/neon-sys/native/src/neon.h index d6868b960..34f437b84 100644 --- a/crates/neon-sys/native/src/neon.h +++ b/crates/neon-sys/native/src/neon.h @@ -30,7 +30,7 @@ extern "C" { bool Neon_Primitive_IsInt32(v8::Local p); void Neon_Object_New(v8::Local *out, v8::Isolate *isolate); - bool Neon_Object_GetOwnPropertyNames(v8::Local *out, v8::Local obj); + bool Neon_Object_GetOwnPropertyNames(v8::Local *out, v8::Isolate *isolate, v8::Local obj); void *Neon_Object_GetIsolate(v8::Local obj); bool Neon_Object_Get_Index(v8::Local *out, v8::Local object, uint32_t index); bool Neon_Object_Set_Index(bool *out, v8::Local object, uint32_t index, v8::Local val); diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs index 7d76b95c3..a849ffb54 100644 --- a/crates/neon-sys/src/lib.rs +++ b/crates/neon-sys/src/lib.rs @@ -144,7 +144,7 @@ extern "C" { pub fn Neon_Module_GetVersion() -> i32; pub fn Neon_Object_New(out: &mut Local, isolate: Isolate); - pub fn Neon_Object_GetOwnPropertyNames(out: &mut Local, object: Local) -> bool; + pub fn Neon_Object_GetOwnPropertyNames(out: &mut Local, isolate: Isolate, object: Local) -> bool; pub fn Neon_Object_GetIsolate(obj: Local) -> Isolate; pub fn Neon_Object_Get_Index(out: &mut Local, object: Local, index: u32) -> bool; pub fn Neon_Object_Set_Index(out: &mut bool, object: Local, index: u32, val: Local) -> bool; diff --git a/src/object/mod.rs b/src/object/mod.rs index 2a8fe50aa..893456099 100644 --- a/src/object/mod.rs +++ b/src/object/mod.rs @@ -58,8 +58,11 @@ mod traits { build(|out| { unsafe { key.get_from(out, self.to_raw()) } }) } - fn get_own_property_names<'a, C: Context<'a>>(self, _: &mut C) -> JsResult<'a, JsArray> { - build(|out| { unsafe { neon_runtime::object::get_own_property_names(out, self.to_raw()) } }) + fn get_own_property_names<'a, C: Context<'a>>(self, cx: &mut C) -> JsResult<'a, JsArray> { + let env = cx.env(); + build(|out| { + unsafe { neon_runtime::object::get_own_property_names(out, env.to_raw(), self.to_raw()) } + }) } fn set<'a, C: Context<'a>, K: PropertyKey, W: Value>(self, _: &mut C, key: K, val: Handle) -> NeonResult { @@ -185,8 +188,12 @@ mod traits { build(|out| { unsafe { key.get_from(cx, out, self.to_raw()) } }) } - fn get_own_property_names<'a, C: Context<'a>>(self, _: &mut C) -> JsResult<'a, JsArray> { - build(|out| { unsafe { neon_runtime::object::get_own_property_names(out, self.to_raw()) } }) + fn get_own_property_names<'a, C: Context<'a>>(self, cx: &mut C) -> JsResult<'a, JsArray> { + let env = cx.env(); + + build(|out| { + unsafe { neon_runtime::object::get_own_property_names(out, env.to_raw(), self.to_raw()) } + }) } fn set<'a, C: Context<'a>, K: PropertyKey, W: Value>(self, cx: &mut C, key: K, val: Handle) -> NeonResult { diff --git a/test/napi/native/src/lib.rs b/test/napi/native/src/lib.rs index 65d2bb63d..08e9ab687 100644 --- a/test/napi/native/src/lib.rs +++ b/test/napi/native/src/lib.rs @@ -49,6 +49,13 @@ register_module!(|mut cx| { v.value(&mut cx) }, true); + let property_names = rust_created.get_own_property_names(&mut cx)? + .to_vec(&mut cx)? + .into_iter() + .map(|value| value.to_string(&mut cx).unwrap().value(&mut cx)) + .collect::>(); + assert_eq!(property_names, &["a", "whatever"]); + cx.export_value("rustCreated", rust_created)?; Ok(()) From 89bb88f7a0589c0833f3537bfb2c1b4a23577efd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 21 Feb 2020 11:36:15 +0100 Subject: [PATCH 07/18] Implement Array#len --- crates/neon-runtime/src/napi/array.rs | 11 ++++++++++- crates/neon-sys/native/src/neon.cc | 2 +- crates/neon-sys/native/src/neon.h | 2 +- crates/neon-sys/src/lib.rs | 2 +- src/types/mod.rs | 18 ++++++++++++++---- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/neon-runtime/src/napi/array.rs b/crates/neon-runtime/src/napi/array.rs index 753c8baf1..527e77a8f 100644 --- a/crates/neon-runtime/src/napi/array.rs +++ b/crates/neon-runtime/src/napi/array.rs @@ -1,5 +1,14 @@ +//! Facilities for working with Array `napi_value`s. + use raw::{Local, Env}; +use nodejs_sys as napi; + pub unsafe extern "C" fn new(_out: &mut Local, _env: Env, _length: u32) { unimplemented!() } -pub unsafe extern "C" fn len(_array: Local) -> u32 { unimplemented!() } +/// Gets the length of a `napi_value` containing a JavaScript Array. +pub unsafe extern "C" fn len(env: Env, array: Local) -> u32 { + let mut len = 0; + assert_eq!(napi::napi_get_array_length(env, array, &mut len as *mut _), napi::napi_status::napi_ok); + len +} diff --git a/crates/neon-sys/native/src/neon.cc b/crates/neon-sys/native/src/neon.cc index 2f3f5f152..5dc50a80d 100644 --- a/crates/neon-sys/native/src/neon.cc +++ b/crates/neon-sys/native/src/neon.cc @@ -152,7 +152,7 @@ extern "C" void Neon_Array_New(v8::Local *out, v8::Isolate *isolate, *out = v8::Array::New(isolate, length); } -extern "C" uint32_t Neon_Array_Length(v8::Local array) { +extern "C" uint32_t Neon_Array_Length(v8::Isolate *isolate, v8::Local array) { return array->Length(); } diff --git a/crates/neon-sys/native/src/neon.h b/crates/neon-sys/native/src/neon.h index 34f437b84..cf7c42208 100644 --- a/crates/neon-sys/native/src/neon.h +++ b/crates/neon-sys/native/src/neon.h @@ -40,7 +40,7 @@ extern "C" { bool Neon_Object_Set(bool *out, v8::Local obj, v8::Local key, v8::Local val); void Neon_Array_New(v8::Local *out, v8::Isolate *isolate, uint32_t length); - uint32_t Neon_Array_Length(v8::Local array); + uint32_t Neon_Array_Length(v8::Isolate *isolate, v8::Local array); bool Neon_String_New(v8::Local *out, v8::Isolate *isolate, const uint8_t *data, int32_t len); int32_t Neon_String_Utf8Length(v8::Local str); diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs index a849ffb54..15b9002c6 100644 --- a/crates/neon-sys/src/lib.rs +++ b/crates/neon-sys/src/lib.rs @@ -84,7 +84,7 @@ pub struct InheritedHandleScope; extern "C" { pub fn Neon_Array_New(out: &mut Local, isolate: Isolate, length: u32); - pub fn Neon_Array_Length(array: Local) -> u32; + pub fn Neon_Array_Length(isolate: Isolate, array: Local) -> u32; pub fn Neon_ArrayBuffer_New(out: &mut Local, isolate: Isolate, size: u32) -> bool; pub fn Neon_ArrayBuffer_Data<'a, 'b>(base_out: &'a mut *mut c_void, obj: Local) -> usize; diff --git a/src/types/mod.rs b/src/types/mod.rs index 44b6cb84a..cc34d7bea 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -491,12 +491,12 @@ impl JsArray { } pub fn to_vec<'a, C: Context<'a>>(self, cx: &mut C) -> NeonResult>> { - let mut result = Vec::with_capacity(self.len() as usize); + let mut result = Vec::with_capacity(self.len_inner(cx.env()) as usize); let mut i = 0; loop { // Since getting a property can trigger arbitrary code, // we have to re-check the length on every iteration. - if i >= self.len() { + if i >= self.len_inner(cx.env()) { return Ok(result); } result.push(self.get(cx, i)?); @@ -504,11 +504,21 @@ impl JsArray { } } - pub fn len(self) -> u32 { + fn len_inner(self, env: Env) -> u32 { unsafe { - neon_runtime::array::len(self.to_raw()) + neon_runtime::array::len(env.to_raw(), self.to_raw()) } } + + #[cfg(feature = "legacy-runtime")] + pub fn len(self) -> u32 { + self.len_inner(Env::current()) + } + + #[cfg(feature = "napi-runtime")] + pub fn len<'a, C: Context<'a>>(self, cx: &mut C) -> u32 { + self.len_inner(cx.env()) + } } impl Value for JsArray { } From c4fb9fd2ef7ee2dc3caecef8d1a716644ea8ffdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 21 Feb 2020 11:41:48 +0100 Subject: [PATCH 08/18] Implement Object#get_index --- crates/neon-runtime/src/napi/object.rs | 14 +++++++++----- src/object/mod.rs | 4 ++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 0535ba513..745807c3c 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -5,11 +5,11 @@ use nodejs_sys as napi; use raw::{Env, Local}; pub unsafe extern "C" fn new(out: &mut Local, env: Env) { - napi::napi_create_object(env, out as *mut Local); + napi::napi_create_object(env, out as *mut _); } pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, object: Local) -> bool { - let status = napi::napi_get_property_names(env, object, out as *mut Local); + let status = napi::napi_get_property_names(env, object, out as *mut _); status == napi::napi_status::napi_ok } @@ -17,7 +17,11 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec // Unused. pub unsafe extern "C" fn get_isolate(_obj: Local) -> Env { unimplemented!() } -pub unsafe extern "C" fn get_index(_out: &mut Local, _object: Local, _index: u32) -> bool { unimplemented!() } +pub unsafe extern "C" fn get_index(out: &mut Local, env: Env, object: Local, index: u32) -> bool { + let status = napi::napi_get_element(env, object, index, out as *mut _); + + status == napi::napi_status::napi_ok +} pub unsafe extern "C" fn set_index(_out: &mut bool, _object: Local, _index: u32, _val: Local) -> bool { unimplemented!() } @@ -40,7 +44,7 @@ pub unsafe extern "C" fn get_string(env: Env, out: &mut Local, object: Local, ke env, object, key_val.assume_init(), - out as *mut Local, + out as *mut _, ) != napi::napi_status::napi_ok { return false; } @@ -84,7 +88,7 @@ pub unsafe extern "C" fn set_string( } pub unsafe extern "C" fn get(out: &mut Local, env: Env, object: Local, key: Local) -> bool { - let status = napi::napi_get_property(env, object, key, out as *mut Local); + let status = napi::napi_get_property(env, object, key, out as *mut _); status == napi::napi_status::napi_ok } diff --git a/src/object/mod.rs b/src/object/mod.rs index 893456099..52c0ae5b0 100644 --- a/src/object/mod.rs +++ b/src/object/mod.rs @@ -112,11 +112,11 @@ mod traits { impl PropertyKey for u32 { unsafe fn get_from<'c, C: Context<'c>>( self, - _cx: &mut C, + cx: &mut C, out: &mut raw::Local, obj: raw::Local ) -> bool { - neon_runtime::object::get_index(out, obj, self) + neon_runtime::object::get_index(out, cx.env().to_raw(), obj, self) } unsafe fn set_from<'c, C: Context<'c>>( From 65d6d2400697abf38c095f115d668504d005b911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 21 Feb 2020 11:41:54 +0100 Subject: [PATCH 09/18] Implement is_string tag --- crates/neon-runtime/src/napi/tag.rs | 4 +++- test/napi/native/src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/neon-runtime/src/napi/tag.rs b/crates/neon-runtime/src/napi/tag.rs index c481bca8e..75249f1b1 100644 --- a/crates/neon-runtime/src/napi/tag.rs +++ b/crates/neon-runtime/src/napi/tag.rs @@ -23,7 +23,9 @@ pub unsafe extern "C" fn is_boolean(env: Env, val: Local) -> bool { is_type(env, val, napi::napi_valuetype::napi_boolean) } -pub unsafe extern "C" fn is_string(_env: Env, _val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn is_string(env: Env, val: Local) -> bool { + is_type(env, val, napi::napi_valuetype::napi_string) +} pub unsafe extern "C" fn is_object(_env: Env, _val: Local) -> bool { unimplemented!() } diff --git a/test/napi/native/src/lib.rs b/test/napi/native/src/lib.rs index 08e9ab687..4345a72a3 100644 --- a/test/napi/native/src/lib.rs +++ b/test/napi/native/src/lib.rs @@ -52,7 +52,7 @@ register_module!(|mut cx| { let property_names = rust_created.get_own_property_names(&mut cx)? .to_vec(&mut cx)? .into_iter() - .map(|value| value.to_string(&mut cx).unwrap().value(&mut cx)) + .map(|value| value.downcast::(&mut cx).unwrap().value(&mut cx)) .collect::>(); assert_eq!(property_names, &["a", "whatever"]); From 6bca18148a752d2b4bc89231e20517b0d8972401 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 21 Feb 2020 11:45:41 +0100 Subject: [PATCH 10/18] Remove is_a_compat, used in one place --- src/handle/mod.rs | 11 ----------- src/object/class/internal.rs | 7 ++++++- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/handle/mod.rs b/src/handle/mod.rs index 32ade0b2d..f365f30bf 100644 --- a/src/handle/mod.rs +++ b/src/handle/mod.rs @@ -141,17 +141,6 @@ impl<'a, T: Value> Handle<'a, T> { U::is_typeof(cx.env(), self.value) } - /// Tests whether this value is an instance of the given type. - /// - /// Works with both the legacy and the N-API runtime with the same signature. This allows other - /// parts of the codebase to care less about which version we're in. - pub(crate) fn is_a_compat<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> bool { - #[cfg(feature = "legacy-runtime")] - return self.is_a::(); - #[cfg(feature = "napi-runtime")] - return self.is_a::(cx); - } - #[cfg(feature = "legacy-runtime")] /// Attempts to downcast a handle to another type, which may fail. A failure /// to downcast **does not** throw a JavaScript exception, so it's OK to diff --git a/src/object/class/internal.rs b/src/object/class/internal.rs index 1a0f42183..6c564cd90 100644 --- a/src/object/class/internal.rs +++ b/src/object/class/internal.rs @@ -20,7 +20,12 @@ impl Callback<()> for MethodCallback { info.with_cx::(|mut cx| { let data = info.data(); let this: Handle = Handle::new_internal(JsValue::from_raw(info.this(&mut cx))); - let is_a_t = this.is_a_compat::(&mut cx); + + #[cfg(feature = "legacy-runtime")] + let is_a_t = this.is_a::(); + #[cfg(feature = "napi-runtime")] + let is_a_t = this.is_a::(&mut cx); + if !is_a_t { if let Ok(metadata) = T::metadata(&mut cx) { neon_runtime::class::throw_this_error(mem::transmute(cx.env()), metadata.pointer); From a22e51240206f5f35fe5edd2966c7472b21fb676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 21 Feb 2020 12:23:00 +0100 Subject: [PATCH 11/18] Implement Object#set_index --- crates/neon-runtime/src/napi/object.rs | 7 ++++++- src/object/mod.rs | 4 ++-- test/napi/lib/hello.js | 1 + test/napi/native/src/lib.rs | 9 ++++++++- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 745807c3c..0eb400c50 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -23,7 +23,12 @@ pub unsafe extern "C" fn get_index(out: &mut Local, env: Env, object: Local, ind status == napi::napi_status::napi_ok } -pub unsafe extern "C" fn set_index(_out: &mut bool, _object: Local, _index: u32, _val: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn set_index(out: &mut bool, env: Env, object: Local, index: u32, val: Local) -> bool { + let status = napi::napi_set_element(env, object, index, val); + *out = status == napi::napi_status::napi_ok; + + *out +} pub unsafe extern "C" fn get_string(env: Env, out: &mut Local, object: Local, key: *const u8, len: i32) -> bool { let mut key_val = MaybeUninit::uninit(); diff --git a/src/object/mod.rs b/src/object/mod.rs index 52c0ae5b0..04ca1334c 100644 --- a/src/object/mod.rs +++ b/src/object/mod.rs @@ -121,12 +121,12 @@ mod traits { unsafe fn set_from<'c, C: Context<'c>>( self, - _cx: &mut C, + cx: &mut C, out: &mut bool, obj: raw::Local, val: raw::Local, ) -> bool { - neon_runtime::object::set_index(out, obj, self, val) + neon_runtime::object::set_index(out, cx.env().to_raw(), obj, self, val) } } diff --git a/test/napi/lib/hello.js b/test/napi/lib/hello.js index a9fc850ca..1b07c2322 100644 --- a/test/napi/lib/hello.js +++ b/test/napi/lib/hello.js @@ -22,6 +22,7 @@ describe('hello', function() { it('should be able to create JS objects in rust', function () { assert.deepEqual(addon.rustCreated, { + 0: 1, a: 1, whatever: true }) diff --git a/test/napi/native/src/lib.rs b/test/napi/native/src/lib.rs index 4345a72a3..6fc51942c 100644 --- a/test/napi/native/src/lib.rs +++ b/test/napi/native/src/lib.rs @@ -33,7 +33,10 @@ register_module!(|mut cx| { let rust_created = cx.empty_object(); { let a = cx.number(1); + // set at name rust_created.set(&mut cx, "a", a)?; + // set at index + rust_created.set(&mut cx, 0, a)?; } { let whatever = cx.boolean(true); @@ -44,6 +47,10 @@ register_module!(|mut cx| { let v: Handle = rust_created.get(&mut cx, "a")?.downcast_or_throw(&mut cx)?; v.value(&mut cx) }, 1.0f64); + assert_eq!({ + let v: Handle = rust_created.get(&mut cx, 0)?.downcast_or_throw(&mut cx)?; + v.value(&mut cx) + }, 1.0f64); assert_eq!({ let v: Handle = rust_created.get(&mut cx, "whatever")?.downcast_or_throw(&mut cx)?; v.value(&mut cx) @@ -54,7 +61,7 @@ register_module!(|mut cx| { .into_iter() .map(|value| value.downcast::(&mut cx).unwrap().value(&mut cx)) .collect::>(); - assert_eq!(property_names, &["a", "whatever"]); + assert_eq!(property_names, &["0", "a", "whatever"]); cx.export_value("rustCreated", rust_created)?; From 0a31cc2254316edca1774a2f7693878a7ab7ee52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Sun, 1 Mar 2020 13:59:58 +0100 Subject: [PATCH 12/18] Only import Env if it is used --- src/handle/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/handle/mod.rs b/src/handle/mod.rs index f365f30bf..1ef692ef7 100644 --- a/src/handle/mod.rs +++ b/src/handle/mod.rs @@ -10,6 +10,7 @@ use neon_runtime; use neon_runtime::raw; use types::Value; use context::Context; +#[cfg(feature = "legacy-runtime")] use context::internal::Env; use result::{JsResult, JsResultExt}; use self::internal::SuperType; From a4c5a3c831000e4ed70156c0276c102fd84efed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 3 Apr 2020 13:16:29 +0200 Subject: [PATCH 13/18] Document panics for `neon_runtime::napi::array::len` --- crates/neon-runtime/src/napi/array.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/neon-runtime/src/napi/array.rs b/crates/neon-runtime/src/napi/array.rs index 527e77a8f..2c8dbb614 100644 --- a/crates/neon-runtime/src/napi/array.rs +++ b/crates/neon-runtime/src/napi/array.rs @@ -7,6 +7,10 @@ use nodejs_sys as napi; pub unsafe extern "C" fn new(_out: &mut Local, _env: Env, _length: u32) { unimplemented!() } /// Gets the length of a `napi_value` containing a JavaScript Array. +/// +/// # Panics +/// This function panics if `array` is not an Array, or if a previous n-api call caused a pending +/// exception. pub unsafe extern "C" fn len(env: Env, array: Local) -> u32 { let mut len = 0; assert_eq!(napi::napi_get_array_length(env, array, &mut len as *mut _), napi::napi_status::napi_ok); From ff8a4f2606c692c73785d2153b41c66ef228f633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 3 Apr 2020 13:20:53 +0200 Subject: [PATCH 14/18] Document out param in `neon_runtime::nan::object::set_{index,string}`. --- crates/neon-runtime/src/napi/object.rs | 68 +++++++++++++------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 0eb400c50..812708553 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -15,7 +15,9 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec } // Unused. -pub unsafe extern "C" fn get_isolate(_obj: Local) -> Env { unimplemented!() } +pub unsafe extern "C" fn get_isolate(_obj: Local) -> Env { + unimplemented!() +} pub unsafe extern "C" fn get_index(out: &mut Local, env: Env, object: Local, index: u32) -> bool { let status = napi::napi_get_element(env, object, index, out as *mut _); @@ -23,6 +25,13 @@ pub unsafe extern "C" fn get_index(out: &mut Local, env: Env, object: Local, ind status == napi::napi_status::napi_ok } +/// Sets the key value of a `napi_value` at the `index` provided. Returns `true` if the set +/// succeeded. +/// +/// The `out` parameter and the return value contain the same information for historical reasons, +/// see [discussion]. +/// +/// [discussion]: https://github.com/neon-bindings/neon/pull/458#discussion_r344827965 pub unsafe extern "C" fn set_index(out: &mut bool, env: Env, object: Local, index: u32, val: Local) -> bool { let status = napi::napi_set_element(env, object, index, val); *out = status == napi::napi_status::napi_ok; @@ -35,56 +44,43 @@ pub unsafe extern "C" fn get_string(env: Env, out: &mut Local, object: Local, ke // Not using `crate::string::new()` because it requires a _reference_ to a Local, // while we only have uninitialized memory. - if napi::napi_create_string_utf8( - env, - key as *const i8, - len as usize, - key_val.as_mut_ptr(), - ) != napi::napi_status::napi_ok { + if napi::napi_create_string_utf8(env, key as *const i8, len as usize, key_val.as_mut_ptr()) + != napi::napi_status::napi_ok + { return false; } // Not using napi_get_named_property() because the `key` may not be null terminated. - if napi::napi_get_property( - env, - object, - key_val.assume_init(), - out as *mut _, - ) != napi::napi_status::napi_ok { + if napi::napi_get_property(env, object, key_val.assume_init(), out as *mut _) + != napi::napi_status::napi_ok + { return false; } true } -pub unsafe extern "C" fn set_string( - env: Env, - out: &mut bool, - object: Local, - key: *const u8, - len: i32, - val: Local, -) -> bool { +/// Sets the key value of a `napi_value` at a named key. Returns `true` if the set succeeded. +/// +/// The `out` parameter and the return value contain the same information for historical reasons, +/// see [discussion]. +/// +/// [discussion]: https://github.com/neon-bindings/neon/pull/458#discussion_r344827965 +pub unsafe extern "C" fn set_string(env: Env, out: &mut bool, object: Local, key: *const u8, len: i32, val: Local) -> bool { let mut key_val = MaybeUninit::uninit(); *out = true; - if napi::napi_create_string_utf8( - env, - key as *const i8, - len as usize, - key_val.as_mut_ptr(), - ) != napi::napi_status::napi_ok { + if napi::napi_create_string_utf8(env, key as *const i8, len as usize, key_val.as_mut_ptr()) + != napi::napi_status::napi_ok + { *out = false; return false; } - if napi::napi_set_property( - env, - object, - key_val.assume_init(), - val, - ) != napi::napi_status::napi_ok { + if napi::napi_set_property(env, object, key_val.assume_init(), val) + != napi::napi_status::napi_ok + { *out = false; return false; } @@ -98,6 +94,12 @@ pub unsafe extern "C" fn get(out: &mut Local, env: Env, object: Local, key: Loca status == napi::napi_status::napi_ok } +/// Sets the property value of an `napi_value` object, named by another `napi_value` `key`. Returns `true` if the set succeeded. +/// +/// The `out` parameter and the return value contain the same information for historical reasons, +/// see [discussion]. +/// +/// [discussion]: https://github.com/neon-bindings/neon/pull/458#discussion_r344827965 pub unsafe extern "C" fn set(out: &mut bool, env: Env, object: Local, key: Local, val: Local) -> bool { let status = napi::napi_set_property(env, object, key, val); *out = status == napi::napi_status::napi_ok; From 55700eb74c0d94d1160cebe054d431c4b1a03105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Fri, 3 Apr 2020 13:51:02 +0200 Subject: [PATCH 15/18] Document the implemented `neon_runtime::napi` functions. --- crates/neon-runtime/src/napi/object.rs | 7 +++++++ crates/neon-runtime/src/napi/tag.rs | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 812708553..001992387 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -4,10 +4,13 @@ use nodejs_sys as napi; use raw::{Env, Local}; +/// Mutates the `out` argument to refer to a `napi_value` containing a newly created JavaScript Object. pub unsafe extern "C" fn new(out: &mut Local, env: Env) { napi::napi_create_object(env, out as *mut _); } +/// Mutates the `out` argument to refer to a `napi_value` containing the own property names of the +/// `object` as a JavaScript Array. pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, object: Local) -> bool { let status = napi::napi_get_property_names(env, object, out as *mut _); @@ -19,6 +22,7 @@ pub unsafe extern "C" fn get_isolate(_obj: Local) -> Env { unimplemented!() } +/// Mutate the `out` argument to refer to the value at `index` in the given `object`. Returns `false` if the value couldn't be retrieved. pub unsafe extern "C" fn get_index(out: &mut Local, env: Env, object: Local, index: u32) -> bool { let status = napi::napi_get_element(env, object, index, out as *mut _); @@ -39,6 +43,7 @@ pub unsafe extern "C" fn set_index(out: &mut bool, env: Env, object: Local, inde *out } +/// Mutate the `out` argument to refer to the value at a named `key` in the given `object`. Returns `false` if the value couldn't be retrieved. pub unsafe extern "C" fn get_string(env: Env, out: &mut Local, object: Local, key: *const u8, len: i32) -> bool { let mut key_val = MaybeUninit::uninit(); @@ -88,6 +93,8 @@ pub unsafe extern "C" fn set_string(env: Env, out: &mut bool, object: Local, key true } +/// Mutates `out` to refer to the value of the property of `object` named by the `key` value. +/// Returns false if the value couldn't be retrieved. pub unsafe extern "C" fn get(out: &mut Local, env: Env, object: Local, key: Local) -> bool { let status = napi::napi_get_property(env, object, key, out as *mut _); diff --git a/crates/neon-runtime/src/napi/tag.rs b/crates/neon-runtime/src/napi/tag.rs index 75249f1b1..1d51f7a97 100644 --- a/crates/neon-runtime/src/napi/tag.rs +++ b/crates/neon-runtime/src/napi/tag.rs @@ -2,6 +2,7 @@ use raw::{Env, Local}; use nodejs_sys as napi; +/// Return true if an `napi_value` `val` has the expected value type. unsafe fn is_type(env: Env, val: Local, expect: napi::napi_valuetype) -> bool { let mut actual = napi::napi_valuetype::napi_undefined; if napi::napi_typeof(env, val, &mut actual as *mut _) == napi::napi_status::napi_ok { @@ -15,14 +16,17 @@ pub unsafe extern "C" fn is_undefined(_env: Env, _val: Local) -> bool { unimplem pub unsafe extern "C" fn is_null(_env: Env, _val: Local) -> bool { unimplemented!() } +/// Is `val` a JavaScript number? pub unsafe extern "C" fn is_number(env: Env, val: Local) -> bool { is_type(env, val, napi::napi_valuetype::napi_number) } +/// Is `val` a JavaScript boolean? pub unsafe extern "C" fn is_boolean(env: Env, val: Local) -> bool { is_type(env, val, napi::napi_valuetype::napi_boolean) } +/// Is `val` a JavaScript string? pub unsafe extern "C" fn is_string(env: Env, val: Local) -> bool { is_type(env, val, napi::napi_valuetype::napi_string) } From c1fbddcd513db55bacd3b5fb0047bcfbed663ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 8 Apr 2020 16:22:41 +0200 Subject: [PATCH 16/18] Stringify elements of `napi_get_property_names`. --- crates/neon-runtime/src/napi/convert.rs | 10 ++++++-- crates/neon-runtime/src/napi/object.rs | 34 ++++++++++++++++++++++++- crates/neon-sys/native/src/neon.cc | 2 +- crates/neon-sys/native/src/neon.h | 2 +- crates/neon-sys/src/lib.rs | 2 +- src/types/mod.rs | 7 +++-- test/napi/native/src/lib.rs | 7 +++-- 7 files changed, 54 insertions(+), 10 deletions(-) diff --git a/crates/neon-runtime/src/napi/convert.rs b/crates/neon-runtime/src/napi/convert.rs index 45834ec66..2f41e3ba5 100644 --- a/crates/neon-runtime/src/napi/convert.rs +++ b/crates/neon-runtime/src/napi/convert.rs @@ -1,5 +1,11 @@ -use raw::Local; +use nodejs_sys as napi; + +use raw::{Env, Local}; pub unsafe extern "C" fn to_object(_out: &mut Local, _value: &Local) -> bool { unimplemented!() } -pub unsafe extern "C" fn to_string(_out: &mut Local, _value: Local) -> bool { unimplemented!() } +pub unsafe extern "C" fn to_string(out: &mut Local, env: Env, value: Local) -> bool { + let status = napi::napi_coerce_to_string(env, value, out as *mut _); + + status == napi::napi_status::napi_ok +} diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 001992387..f3d2b2451 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -2,6 +2,9 @@ use std::mem::MaybeUninit; use nodejs_sys as napi; +use array; +use convert; +use tag; use raw::{Env, Local}; /// Mutates the `out` argument to refer to a `napi_value` containing a newly created JavaScript Object. @@ -14,7 +17,36 @@ pub unsafe extern "C" fn new(out: &mut Local, env: Env) { pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, object: Local) -> bool { let status = napi::napi_get_property_names(env, object, out as *mut _); - status == napi::napi_status::napi_ok + if status != napi::napi_status::napi_ok { + return false; + } + + // Before https://github.com/nodejs/node/pull/27524, `napi_get_property_names` would return + // numbers for numeric indices instead of strings. + let len = array::len(env, *out); + for index in 0..len { + let mut element: Local = std::mem::zeroed(); + // In general, getters may cause arbitrary JS code to be run, but this is a newly created + // Array from an official internal API so it doesn't do anything strange. + if !get_index(&mut element, env, *out, index) { + continue; + } + if tag::is_string(env, element) { + continue; + } + let mut stringified: Local = std::mem::zeroed(); + // If we can't convert to a string, something went wrong. + if !convert::to_string(&mut stringified, env, element) { + return false; + } + let mut dummy = false; + // If we can't convert assign to this array, something went wrong. + if !set_index(&mut dummy, env, *out, index, stringified) { + return false; + } + } + + true } // Unused. diff --git a/crates/neon-sys/native/src/neon.cc b/crates/neon-sys/native/src/neon.cc index 5dc50a80d..edbc4b3ab 100644 --- a/crates/neon-sys/native/src/neon.cc +++ b/crates/neon-sys/native/src/neon.cc @@ -173,7 +173,7 @@ extern "C" size_t Neon_String_Data(char *out, size_t len, v8::Local s return Nan::DecodeWrite(out, len, str, Nan::UTF8); } -extern "C" bool Neon_Convert_ToString(v8::Local *out, v8::Local value) { +extern "C" bool Neon_Convert_ToString(v8::Local *out, v8::Isolate *isolate, v8::Local value) { Nan::MaybeLocal maybe = Nan::To(value); return maybe.ToLocal(out); } diff --git a/crates/neon-sys/native/src/neon.h b/crates/neon-sys/native/src/neon.h index cf7c42208..2b5cf2aad 100644 --- a/crates/neon-sys/native/src/neon.h +++ b/crates/neon-sys/native/src/neon.h @@ -46,7 +46,7 @@ extern "C" { int32_t Neon_String_Utf8Length(v8::Local str); size_t Neon_String_Data(char *out, size_t len, v8::Local str); - bool Neon_Convert_ToString(v8::Local *out, v8::Local value); + bool Neon_Convert_ToString(v8::Local *out, v8::Isolate *isolate, v8::Local value); bool Neon_Convert_ToObject(v8::Local *out, v8::Local *value); bool Neon_Buffer_New(v8::Local *out, uint32_t size); diff --git a/crates/neon-sys/src/lib.rs b/crates/neon-sys/src/lib.rs index 15b9002c6..136a1b89e 100644 --- a/crates/neon-sys/src/lib.rs +++ b/crates/neon-sys/src/lib.rs @@ -123,7 +123,7 @@ extern "C" { pub fn Neon_Class_GetInstanceInternals(obj: Local) -> *mut c_void; pub fn Neon_Convert_ToObject(out: &mut Local, value: &Local) -> bool; - pub fn Neon_Convert_ToString(out: &mut Local, value: Local) -> bool; + pub fn Neon_Convert_ToString(out: &mut Local, isolate: Isolate, value: Local) -> bool; pub fn Neon_Error_Throw(val: Local); pub fn Neon_Error_NewError(out: &mut Local, msg: Local); diff --git a/src/types/mod.rs b/src/types/mod.rs index cc34d7bea..217534e53 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -50,8 +50,11 @@ impl SuperType for JsObject { /// The trait shared by all JavaScript values. pub trait Value: ValueInternal { - fn to_string<'a, C: Context<'a>>(self, _: &mut C) -> JsResult<'a, JsString> { - build(|out| { unsafe { neon_runtime::convert::to_string(out, self.to_raw()) } }) + fn to_string<'a, C: Context<'a>>(self, cx: &mut C) -> JsResult<'a, JsString> { + let env = cx.env(); + build(|out| { + unsafe { neon_runtime::convert::to_string(out, env.to_raw(), self.to_raw()) } + }) } fn as_value<'a, C: Context<'a>>(self, _: &mut C) -> Handle<'a, JsValue> { diff --git a/test/napi/native/src/lib.rs b/test/napi/native/src/lib.rs index 6fc51942c..f79e1692c 100644 --- a/test/napi/native/src/lib.rs +++ b/test/napi/native/src/lib.rs @@ -59,8 +59,11 @@ register_module!(|mut cx| { let property_names = rust_created.get_own_property_names(&mut cx)? .to_vec(&mut cx)? .into_iter() - .map(|value| value.downcast::(&mut cx).unwrap().value(&mut cx)) - .collect::>(); + .map(|value| { + let string: Handle = value.downcast_or_throw(&mut cx)?; + Ok(string.value(&mut cx)) + }) + .collect::, _>>()?; assert_eq!(property_names, &["0", "a", "whatever"]); cx.export_value("rustCreated", rust_created)?; From f6b1c4f1adb8cd3e957ddab94fd30c9829c406d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 8 Apr 2020 16:58:43 +0200 Subject: [PATCH 17/18] Filter it down to own properties. --- crates/neon-runtime/src/napi/object.rs | 66 +++++++++++++++++++------- 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index f3d2b2451..5494f0aeb 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -15,35 +15,69 @@ pub unsafe extern "C" fn new(out: &mut Local, env: Env) { /// Mutates the `out` argument to refer to a `napi_value` containing the own property names of the /// `object` as a JavaScript Array. pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, object: Local) -> bool { - let status = napi::napi_get_property_names(env, object, out as *mut _); - - if status != napi::napi_status::napi_ok { + // Node.js 13+ have `napi_get_all_property_names`, which does the conversion right and allows + // us to ask for only own properties or prototype properties or anything we like. + // Unfortunately, earlier versions do not support that method, so we have to implement it + // manually. + // + // So we use a temporary array for the raw names: + let mut raw_names = MaybeUninit::uninit(); + if napi::napi_get_property_names(env, object, raw_names.as_mut_ptr()) != napi::napi_status::napi_ok { + return false; + } + // And a "fixed" array for the actual return value: + let mut fixed_names = MaybeUninit::uninit(); + if napi::napi_create_array(env, fixed_names.as_mut_ptr()) != napi::napi_status::napi_ok { return false; } - // Before https://github.com/nodejs/node/pull/27524, `napi_get_property_names` would return - // numbers for numeric indices instead of strings. - let len = array::len(env, *out); - for index in 0..len { - let mut element: Local = std::mem::zeroed(); + let raw_names = raw_names.assume_init(); + let mut fixed_names = fixed_names.assume_init(); + + *out = fixed_names; + + let raw_len = array::len(env, raw_names); + let mut fixed_len = 0; + for index in 0..raw_len { + let mut property_name: Local = std::mem::zeroed(); + // In general, getters may cause arbitrary JS code to be run, but this is a newly created // Array from an official internal API so it doesn't do anything strange. - if !get_index(&mut element, env, *out, index) { - continue; - } - if tag::is_string(env, element) { + if !get_index(&mut property_name, env, raw_names, index) { continue; } - let mut stringified: Local = std::mem::zeroed(); - // If we can't convert to a string, something went wrong. - if !convert::to_string(&mut stringified, env, element) { + + let mut is_own_property = false; + // May return a non-OK status if `key` is not a string or a Symbol, but here it is always + // a string. + if napi::napi_has_own_property(env, object, property_name, &mut is_own_property as *mut _) != napi::napi_status::napi_ok { return false; } + + if !is_own_property { + continue; + } + + // Before https://github.com/nodejs/node/pull/27524, `napi_get_property_names` would return + // numbers for numeric indices instead of strings. + // Make sure we always return strings. + let property_name = if !tag::is_string(env, property_name) { + let mut stringified: Local = std::mem::zeroed(); + // If we can't convert to a string, something went wrong. + if !convert::to_string(&mut stringified, env, property_name) { + return false; + } + stringified + } else { + property_name + }; + let mut dummy = false; // If we can't convert assign to this array, something went wrong. - if !set_index(&mut dummy, env, *out, index, stringified) { + if !set_index(&mut dummy, env, fixed_names, fixed_len, property_name) { return false; } + fixed_len += 1; } true From 9b338f9ed0a8dcb781566b61bc06b67f07ca23a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 8 Apr 2020 17:19:49 +0200 Subject: [PATCH 18/18] Check `hasOwnProperty` after conversion to string I thought I could be clever and micro-optimize this a bit, but `has_own_property` really does require a string argument, so we have to do the conversion first. --- crates/neon-runtime/src/napi/object.rs | 24 ++++++++++++------------ test/napi/lib/hello.js | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/neon-runtime/src/napi/object.rs b/crates/neon-runtime/src/napi/object.rs index 5494f0aeb..5cefb253a 100644 --- a/crates/neon-runtime/src/napi/object.rs +++ b/crates/neon-runtime/src/napi/object.rs @@ -32,7 +32,7 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec } let raw_names = raw_names.assume_init(); - let mut fixed_names = fixed_names.assume_init(); + let fixed_names = fixed_names.assume_init(); *out = fixed_names; @@ -47,17 +47,6 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec continue; } - let mut is_own_property = false; - // May return a non-OK status if `key` is not a string or a Symbol, but here it is always - // a string. - if napi::napi_has_own_property(env, object, property_name, &mut is_own_property as *mut _) != napi::napi_status::napi_ok { - return false; - } - - if !is_own_property { - continue; - } - // Before https://github.com/nodejs/node/pull/27524, `napi_get_property_names` would return // numbers for numeric indices instead of strings. // Make sure we always return strings. @@ -72,6 +61,17 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec property_name }; + let mut is_own_property = false; + // May return a non-OK status if `key` is not a string or a Symbol, but here it is always + // a string. + if napi::napi_has_own_property(env, object, property_name, &mut is_own_property as *mut _) != napi::napi_status::napi_ok { + return false; + } + + if !is_own_property { + continue; + } + let mut dummy = false; // If we can't convert assign to this array, something went wrong. if !set_index(&mut dummy, env, fixed_names, fixed_len, property_name) { diff --git a/test/napi/lib/hello.js b/test/napi/lib/hello.js index 1b07c2322..becef4295 100644 --- a/test/napi/lib/hello.js +++ b/test/napi/lib/hello.js @@ -25,6 +25,6 @@ describe('hello', function() { 0: 1, a: 1, whatever: true - }) + }); }); });