Merge pull request #918 from neon-bindings/kv/remove-this

breaking: Remove `trait This`

crates/neon/src/context/mod.rs

@@ -10,9 +10,8 @@
 //! execution context. All interaction with the JavaScript engine in Neon code is mediated
 //! through instances of this trait.
 //!
-//! One particularly useful context type is [`CallContext`](CallContext), which is passed
-//! to all Neon functions as their initial execution context (or [`FunctionContext`](FunctionContext),
-//! a convenient shorthand for `CallContext<JsObject>`):
+//! One particularly useful context type is [`FunctionContext`](FunctionContext), which is passed
+//! to all Neon functions as their initial execution context.
 //!
 //! ```
 //! # use neon::prelude::*;
@@ -149,7 +148,7 @@ pub use crate::types::buffer::lock::Lock;
 use crate::{
     event::TaskBuilder,
     handle::{Handle, Managed},
-    object::{Object, This},
+    object::Object,
     result::{JsResult, NeonResult, Throw},
     sys::{
         self, raw,
@@ -602,32 +601,30 @@ impl<'a, 'b> Context<'a> for ComputeContext<'a, 'b> {}
 /// An execution context of a function call.
 ///
 /// The type parameter `T` is the type of the `this`-binding.
-pub struct CallContext<'a, T: This> {
+pub struct FunctionContext<'a> {
     env: Env,
     info: &'a CallbackInfo<'a>,
 
     arguments: Option<sys::call::Arguments>,
-    phantom_type: PhantomData<T>,
 }
 
-impl<'a, T: This> UnwindSafe for CallContext<'a, T> {}
+impl<'a> UnwindSafe for FunctionContext<'a> {}
 
-impl<'a, T: This> CallContext<'a, T> {
+impl<'a> FunctionContext<'a> {
     /// Indicates whether the function was called with `new`.
     pub fn kind(&self) -> CallKind {
         self.info.kind(self)
     }
 
-    pub(crate) fn with<U, F: for<'b> FnOnce(CallContext<'b, T>) -> U>(
+    pub(crate) fn with<U, F: for<'b> FnOnce(FunctionContext<'b>) -> U>(
         env: Env,
         info: &'a CallbackInfo<'a>,
         f: F,
     ) -> U {
-        f(CallContext {
+        f(FunctionContext {
             env,
             info,
             arguments: None,
-            phantom_type: PhantomData,
         })
     }
 
@@ -662,27 +659,27 @@ impl<'a, T: This> CallContext<'a, T> {
         }
     }
 
-    /// Produces a handle to the `this`-binding.
-    pub fn this(&mut self) -> Handle<'a, T> {
-        let this = T::as_this(self.env(), self.info.this(self));
+    /// Produces a handle to the `this`-binding and attempts to downcast as a specific type.
+    /// Equivalent to calling `cx.this_value().downcast_or_throw(&mut cx)`.
+    ///
+    /// Throws an exception if the value is a different type.
+    pub fn this<T: Value>(&mut self) -> JsResult<'a, T> {
+        self.this_value().downcast_or_throw(self)
+    }
 
-        Handle::new_internal(this)
+    /// Produces a handle to the function's [`this`-binding](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this#function_context).
+    pub fn this_value(&mut self) -> Handle<'a, JsValue> {
+        JsValue::new_internal(self.info.this(self))
     }
 }
 
-impl<'a, T: This> ContextInternal<'a> for CallContext<'a, T> {
+impl<'a> ContextInternal<'a> for FunctionContext<'a> {
     fn env(&self) -> Env {
         self.env
     }
 }
 
-impl<'a, T: This> Context<'a> for CallContext<'a, T> {}
-
-/// A shorthand for a [`CallContext`](CallContext) with `this`-type [`JsObject`](crate::types::JsObject).
-pub type FunctionContext<'a> = CallContext<'a, JsObject>;
-
-/// An alias for [`CallContext`](CallContext), useful for indicating that the function is a method of a class.
-pub type MethodContext<'a, T> = CallContext<'a, T>;
+impl<'a> Context<'a> for FunctionContext<'a> {}
 
 /// An execution context of a task completion callback.
 pub struct TaskContext<'a> {

crates/neon/src/object/mod.rs

@@ -33,8 +33,8 @@
 //! [symbol]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol
 
 use crate::{
-    context::{internal::Env, Context},
-    handle::{Handle, Managed, Root},
+    context::Context,
+    handle::{Handle, Root},
     result::{NeonResult, Throw},
     sys::{self, raw},
     types::{build, function::CallOptions, utf8::Utf8, JsFunction, JsUndefined, JsValue, Value},
@@ -242,9 +242,3 @@ pub trait Object: Value {
         Ok(options)
     }
 }
-
-/// The trait of types that can be a function's `this` binding.
-pub unsafe trait This: Managed {
-    #[allow(clippy::wrong_self_convention)]
-    fn as_this(env: Env, h: raw::Local) -> Self;
-}

crates/neon/src/prelude.rs

@@ -3,8 +3,8 @@
 #[doc(no_inline)]
 pub use crate::{
     context::{
-        CallContext, CallKind, ComputeContext, Context, ExecuteContext, FunctionContext,
-        MethodContext, ModuleContext, TaskContext,
+        CallKind, ComputeContext, Context, ExecuteContext, FunctionContext, ModuleContext,
+        TaskContext,
     },
     handle::{Handle, Root},
     object::Object,

crates/neon/src/sys/call.rs

@@ -52,7 +52,7 @@ pub unsafe fn this(env: Env, info: FunctionCallbackInfo, out: &mut Local) {
 }
 
 /// Gets the number of arguments passed to the function.
-// TODO: Remove this when `CallContext` is refactored to get call info upfront.
+// TODO: Remove this when `FunctionContext` is refactored to get call info upfront.
 pub unsafe fn len(env: Env, info: FunctionCallbackInfo) -> usize {
     let mut argc = 0usize;
     let status = napi::get_cb_info(

crates/neon/src/types/mod.rs

@@ -97,7 +97,7 @@ use crate::{
         internal::{SuperType, TransparentNoCopyWrapper},
         Handle, Managed,
     },
-    object::{Object, This},
+    object::Object,
     result::{JsResult, NeonResult, ResultExt, Throw},
     sys::{self, raw},
     types::{
@@ -195,12 +195,6 @@ impl private::ValueInternal for JsValue {
     }
 }
 
-unsafe impl This for JsValue {
-    fn as_this(_env: Env, h: raw::Local) -> Self {
-        JsValue(h)
-    }
-}
-
 impl JsValue {
     pub(crate) fn new_internal<'a>(value: raw::Local) -> Handle<'a, JsValue> {
         Handle::new_internal(JsValue(value))
@@ -224,15 +218,6 @@ impl JsUndefined {
             Handle::new_internal(JsUndefined(local))
         }
     }
-
-    #[allow(clippy::wrong_self_convention)]
-    fn as_this_compat(env: Env, _: raw::Local) -> Self {
-        unsafe {
-            let mut local: raw::Local = std::mem::zeroed();
-            sys::primitive::undefined(&mut local, env.to_raw());
-            JsUndefined(local)
-        }
-    }
 }
 
 impl Value for JsUndefined {}
@@ -255,12 +240,6 @@ impl Managed for JsUndefined {
     }
 }
 
-unsafe impl This for JsUndefined {
-    fn as_this(env: Env, h: raw::Local) -> Self {
-        JsUndefined::as_this_compat(env, h)
-    }
-}
-
 impl private::ValueInternal for JsUndefined {
     fn name() -> String {
         "undefined".to_string()
@@ -560,12 +539,6 @@ impl Managed for JsObject {
     }
 }
 
-unsafe impl This for JsObject {
-    fn as_this(_env: Env, h: raw::Local) -> Self {
-        JsObject(h)
-    }
-}
-
 impl private::ValueInternal for JsObject {
     fn name() -> String {
         "object".to_string()

test/napi/src/js/functions.rs

@@ -1,4 +1,4 @@
-use neon::{object::This, prelude::*};
+use neon::prelude::*;
 
 fn add1(mut cx: FunctionContext) -> JsResult<JsNumber> {
     let x = cx.argument::<JsNumber>(0)?.value(&mut cx);
@@ -138,19 +138,9 @@ pub fn construct_js_function_with_overloaded_result(mut cx: FunctionContext) ->
         .apply(&mut cx)
 }
 
-trait CheckArgument<'a> {
-    fn check_argument<V: Value>(&mut self, i: usize) -> JsResult<'a, V>;
-}
-
-impl<'a, T: This> CheckArgument<'a> for CallContext<'a, T> {
-    fn check_argument<V: Value>(&mut self, i: usize) -> JsResult<'a, V> {
-        self.argument::<V>(i)
-    }
-}
-
 pub fn check_string_and_number(mut cx: FunctionContext) -> JsResult<JsUndefined> {
-    cx.check_argument::<JsString>(0)?;
-    cx.check_argument::<JsNumber>(1)?;
+    cx.argument::<JsString>(0)?;
+    cx.argument::<JsNumber>(1)?;
     Ok(cx.undefined())
 }
 
@@ -170,12 +160,11 @@ pub fn num_arguments(mut cx: FunctionContext) -> JsResult<JsNumber> {
 }
 
 pub fn return_this(mut cx: FunctionContext) -> JsResult<JsValue> {
-    Ok(cx.this().upcast())
+    Ok(cx.this()?)
 }
 
 pub fn require_object_this(mut cx: FunctionContext) -> JsResult<JsUndefined> {
-    let this = cx.this();
-    let this = this.downcast::<JsObject, _>(&mut cx).or_throw(&mut cx)?;
+    let this = cx.this::<JsObject>()?;
     let t = cx.boolean(true);
     this.set(&mut cx, "modified", t)?;
     Ok(cx.undefined())
@@ -241,7 +230,7 @@ pub fn get_number_or_default(mut cx: FunctionContext) -> JsResult<JsNumber> {
 }
 
 pub fn is_construct(mut cx: FunctionContext) -> JsResult<JsObject> {
-    let this = cx.this();
+    let this = cx.this::<JsObject>()?;
     let construct = matches!(cx.kind(), CallKind::Construct);
     let construct = cx.boolean(construct);
     this.set(&mut cx, "wasConstructed", construct)?;