Skip to content

Commit

Permalink
add a test case and get rid of unneeded sys helper
Browse files Browse the repository at this point in the history
  • Loading branch information
dherman committed Sep 8, 2024
1 parent cda8c46 commit 9c018a7
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 25 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
31 changes: 26 additions & 5 deletions crates/neon/src/types_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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 +1165,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 result: raw::Local = std::mem::zeroed();

let status = napi::call_function(
env.to_raw(),
this.to_local(),
callee,
argc as usize,
argv as *const _,
result as *mut _,
);

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)))
}

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) }, /not a function/);
})
});
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 9c018a7

Please sign in to comment.