chore(neon): Improve ValueInternal safety

ValueInternal::is_typeof and ValueInternal::downcast accept raw `Env`, but can trivially accept `Cx` instead.
neon-bindings · Oct 11, 2024 · 539e720 · 539e720
1 parent 1efcd4e
commit 539e720
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 50 deletions.
diff --git a/crates/neon/src/handle/mod.rs b/crates/neon/src/handle/mod.rs
@@ -156,15 +156,15 @@ impl<'a, T: Value> Handle<'a, T> {
     /// # }
     /// ```
     pub fn is_a<'b, U: Value, C: Context<'b>>(&self, cx: &mut C) -> bool {
-        U::is_typeof(cx.env(), self.deref())
+        U::is_typeof(cx.cx_mut(), self.deref())
     }
 
     /// 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.deref()) {
+        match U::downcast(cx.cx_mut(), self.deref()) {
             Some(v) => Ok(Handle::new_internal(v)),
             None => Err(DowncastError::new()),
         }

diff --git a/crates/neon/src/types_impl/bigint.rs b/crates/neon/src/types_impl/bigint.rs
@@ -3,7 +3,10 @@
 use std::{error, fmt, mem::MaybeUninit};
 
 use crate::{
-    context::{internal::Env, Context},
+    context::{
+        internal::{ContextInternal, Env},
+        Context, Cx,
+    },
     handle::{internal::TransparentNoCopyWrapper, Handle},
     result::{NeonResult, ResultExt},
     sys::{self, raw},
@@ -432,8 +435,8 @@ impl private::ValueInternal for JsBigInt {
         "BigInt"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_bigint(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_bigint(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {

diff --git a/crates/neon/src/types_impl/boxed.rs b/crates/neon/src/types_impl/boxed.rs
@@ -4,7 +4,10 @@ use std::{
 };
 
 use crate::{
-    context::{internal::Env, Context, Cx},
+    context::{
+        internal::{ContextInternal, Env},
+        Context, Cx,
+    },
     handle::{internal::TransparentNoCopyWrapper, Handle},
     object::Object,
     sys::{external, raw},
@@ -187,15 +190,15 @@ impl<T: 'static> ValueInternal for JsBox<T> {
         any::type_name::<Self>()
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        let data = unsafe { maybe_external_deref(env, other.to_local()) };
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        let data = unsafe { maybe_external_deref(cx.env(), other.to_local()) };
 
         data.map(|v| v.is::<T>()).unwrap_or(false)
     }
 
-    fn downcast<Other: Value>(env: Env, other: &Other) -> Option<Self> {
+    fn downcast<Other: Value>(cx: &mut Cx, other: &Other) -> Option<Self> {
         let local = other.to_local();
-        let data = unsafe { maybe_external_deref(env, local) };
+        let data = unsafe { maybe_external_deref(cx.env(), local) };
 
         // Attempt to downcast the `Option<&BoxAny>` to `Option<*const T>`
         data.and_then(|v| v.downcast_ref())

diff --git a/crates/neon/src/types_impl/buffer/types.rs b/crates/neon/src/types_impl/buffer/types.rs
@@ -1,7 +1,10 @@
 use std::{marker::PhantomData, slice};
 
 use crate::{
-    context::{internal::Env, Context},
+    context::{
+        internal::{ContextInternal, Env},
+        Context, Cx,
+    },
     handle::{internal::TransparentNoCopyWrapper, Handle},
     object::Object,
     result::{JsResult, Throw},
@@ -128,8 +131,8 @@ impl ValueInternal for JsBuffer {
         "Buffer"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_buffer(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_buffer(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -345,8 +348,8 @@ impl ValueInternal for JsArrayBuffer {
         "JsArrayBuffer"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_arraybuffer(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_arraybuffer(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -804,8 +807,8 @@ macro_rules! impl_typed_array {
                 stringify!($typ)
             }
 
-            fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-                let env = env.to_raw();
+            fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+                let env = cx.env().to_raw();
                 let other = other.to_local();
 
                 if unsafe { !sys::tag::is_typedarray(env, other) } {

diff --git a/crates/neon/src/types_impl/date.rs b/crates/neon/src/types_impl/date.rs
@@ -6,7 +6,10 @@ use std::{
 use super::{private::ValueInternal, Value};
 
 use crate::{
-    context::{internal::Env, Context},
+    context::{
+        internal::{ContextInternal, Env},
+        Context, Cx,
+    },
     handle::{internal::TransparentNoCopyWrapper, Handle},
     object::Object,
     result::{JsResult, ResultExt},
@@ -164,8 +167,8 @@ impl ValueInternal for JsDate {
         "object"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_date(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_date(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {

diff --git a/crates/neon/src/types_impl/error.rs b/crates/neon/src/types_impl/error.rs
@@ -3,7 +3,10 @@
 use std::panic::{catch_unwind, UnwindSafe};
 
 use crate::{
-    context::{internal::Env, Context},
+    context::{
+        internal::{ContextInternal, Env},
+        Context, Cx,
+    },
     handle::{internal::TransparentNoCopyWrapper, Handle},
     object::Object,
     result::{NeonResult, Throw},
@@ -49,8 +52,8 @@ impl ValueInternal for JsError {
         "Error"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_error(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_error(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {

diff --git a/crates/neon/src/types_impl/mod.rs b/crates/neon/src/types_impl/mod.rs
@@ -24,7 +24,10 @@ use private::prepare_call;
 use smallvec::smallvec;
 
 use crate::{
-    context::{internal::Env, Context, Cx, FunctionContext},
+    context::{
+        internal::{ContextInternal, Env},
+        Context, Cx, FunctionContext,
+    },
     handle::{
         internal::{SuperType, TransparentNoCopyWrapper},
         Handle,
@@ -173,7 +176,7 @@ impl ValueInternal for JsValue {
         "any"
     }
 
-    fn is_typeof<Other: Value>(_env: Env, _other: &Other) -> bool {
+    fn is_typeof<Other: Value>(_cx: &mut Cx, _other: &Other) -> bool {
         true
     }
 
@@ -251,8 +254,8 @@ impl ValueInternal for JsUndefined {
         "undefined"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_undefined(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_undefined(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -320,8 +323,8 @@ impl ValueInternal for JsNull {
         "null"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_null(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_null(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -393,8 +396,8 @@ impl ValueInternal for JsBoolean {
         "boolean"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_boolean(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_boolean(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -464,8 +467,8 @@ impl ValueInternal for JsString {
         "string"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_string(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_string(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -738,8 +741,8 @@ impl ValueInternal for JsNumber {
         "number"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_number(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_number(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -792,8 +795,8 @@ impl ValueInternal for JsObject {
         "object"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_object(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_object(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -930,8 +933,8 @@ impl ValueInternal for JsArray {
         "Array"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_array(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_array(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {
@@ -1246,8 +1249,8 @@ impl ValueInternal for JsFunction {
         "function"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_function(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_function(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {

diff --git a/crates/neon/src/types_impl/private.rs b/crates/neon/src/types_impl/private.rs
@@ -1,7 +1,10 @@
 use std::{ffi::c_void, mem::MaybeUninit};
 
 use crate::{
-    context::{internal::Env, Context},
+    context::{
+        internal::{ContextInternal, Env},
+        Context, Cx,
+    },
     handle::{internal::TransparentNoCopyWrapper, Handle},
     result::{JsResult, NeonResult, Throw},
     sys::{self, bindings as napi, raw},
@@ -31,13 +34,13 @@ pub(crate) unsafe fn prepare_call<'a, 'b, C: Context<'a>>(
 pub trait ValueInternal: TransparentNoCopyWrapper + 'static {
     fn name() -> &'static str;
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool;
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool;
 
-    fn downcast<Other: Value>(env: Env, other: &Other) -> Option<Self> {
-        if Self::is_typeof(env, other) {
+    fn downcast<Other: Value>(cx: &mut Cx, other: &Other) -> Option<Self> {
+        if Self::is_typeof(cx, other) {
             // # Safety
             // `is_typeof` check ensures this is the correct JavaScript type
-            Some(unsafe { Self::from_local(env, other.to_local()) })
+            Some(unsafe { Self::from_local(cx.env(), other.to_local()) })
         } else {
             None
         }

diff --git a/crates/neon/src/types_impl/promise.rs b/crates/neon/src/types_impl/promise.rs
@@ -1,7 +1,10 @@
 use std::ptr;
 
 use crate::{
-    context::{internal::Env, Context},
+    context::{
+        internal::{ContextInternal, Env},
+        Context,
+    },
     handle::{internal::TransparentNoCopyWrapper, Handle},
     object::Object,
     result::JsResult,
@@ -23,7 +26,6 @@ use crate::{
 
 #[cfg(all(feature = "napi-5", feature = "futures"))]
 use {
-    crate::context::internal::ContextInternal,
     crate::event::{JoinError, SendThrow},
     crate::result::NeonResult,
     crate::types::{JsFunction, JsValue},
@@ -253,8 +255,8 @@ impl ValueInternal for JsPromise {
         "Promise"
     }
 
-    fn is_typeof<Other: Value>(env: Env, other: &Other) -> bool {
-        unsafe { sys::tag::is_promise(env.to_raw(), other.to_local()) }
+    fn is_typeof<Other: Value>(cx: &mut Cx, other: &Other) -> bool {
+        unsafe { sys::tag::is_promise(cx.env().to_raw(), other.to_local()) }
     }
 
     fn to_local(&self) -> raw::Local {