Skip to content

Commit

Permalink
remove unnecessary sys::fun::call helper, and add test case for calli…
Browse files Browse the repository at this point in the history
…ng non-functions with .bind()
  • Loading branch information
dherman committed Sep 9, 2024
1 parent a3f084d commit 589fabc
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 28 deletions.
20 changes: 0 additions & 20 deletions crates/neon/src/sys/fun.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,26 +83,6 @@ where
callback(env, info)
}

pub unsafe fn call(
out: &mut Local,
env: Env,
fun: Local,
this: Local,
argc: i32,
argv: *const c_void,
) -> bool {
let status = napi::call_function(
env,
this,
fun,
argc as usize,
argv as *const _,
out as *mut _,
);

status == napi::Status::Ok
}

pub unsafe fn construct(
out: &mut Local,
env: Env,
Expand Down
35 changes: 27 additions & 8 deletions crates/neon/src/types_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ pub(crate) mod private;
pub(crate) mod utf8;

use std::{
any,
fmt::{self, Debug},
os::raw::c_void,
any, fmt::{self, Debug}, mem::MaybeUninit, os::raw::c_void
};

use smallvec::smallvec;
Expand All @@ -31,7 +29,7 @@ use crate::{
},
object::Object,
result::{JsResult, NeonResult, ResultExt, Throw},
sys::{self, raw},
sys::{self, bindings as napi, raw},
types::{
function::{BindOptions, CallOptions, ConstructOptions},
private::ValueInternal,
Expand Down Expand Up @@ -1165,10 +1163,31 @@ where
AS: AsRef<[Handle<'b, JsValue>]>,
{
let (argc, argv) = unsafe { prepare_call(cx, args.as_ref()) }?;
let env = cx.env().to_raw();
build(cx.env(), |out| unsafe {
sys::fun::call(out, env, callee, this.to_local(), argc, argv)
})
let env = cx.env();
let mut result: MaybeUninit<raw::Local> = MaybeUninit::zeroed();

let status = napi::call_function(
env.to_raw(),
this.to_local(),
callee,
argc as usize,
argv.cast(),
result.as_mut_ptr(),
);

match status {
sys::Status::FunctionExpected => {
return cx.throw_type_error("not a function");
}
sys::Status::PendingException => {
return Err(Throw::new());
}
status => {
assert_eq!(status, sys::Status::Ok);
}
}

Ok(Handle::new_internal(JsValue::from_local(env, result.assume_init())))
}

impl JsFunction {
Expand Down
8 changes: 8 additions & 0 deletions test/napi/lib/objects.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,12 @@ describe("JsObject", function () {
);
assert.strictEqual(obj.toString(), "[object Wonder Woman]");
});

it("throws a TypeError when calling a non-method with .prop()", function () {
const obj = {
number: 42
};

assert.throws(() => { addon.call_non_method_with_prop(obj) }, /failed to downcast/);
});
});
8 changes: 8 additions & 0 deletions test/napi/src/js/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,11 @@ pub fn call_methods_with_prop(mut cx: FunctionContext) -> JsResult<JsString> {
.apply()?;
obj.prop(&mut cx, "toString").bind()?.apply()
}

pub fn call_non_method_with_prop(mut cx: FunctionContext) -> JsResult<JsUndefined> {
let obj: Handle<JsObject> = cx.argument::<JsObject>(0)?;
obj.prop(&mut cx, "number")
.bind()?
.apply()?;
Ok(cx.undefined())
}
1 change: 1 addition & 0 deletions test/napi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> {
cx.export_function("get_property_with_prop", get_property_with_prop)?;
cx.export_function("set_property_with_prop", set_property_with_prop)?;
cx.export_function("call_methods_with_prop", call_methods_with_prop)?;
cx.export_function("call_non_method_with_prop", call_non_method_with_prop)?;

cx.export_function("create_date", create_date)?;
cx.export_function("get_date_value", get_date_value)?;
Expand Down

0 comments on commit 589fabc

Please sign in to comment.