Skip to content

Commit

Permalink
Fix PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kjvalencik committed May 6, 2024
1 parent 9e38acd commit 9dce892
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 43 deletions.
11 changes: 11 additions & 0 deletions crates/neon-macros/src/export/function/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub(crate) struct Meta {
pub(super) name: Option<syn::LitStr>,
pub(super) json: bool,
pub(super) context: bool,
pub(super) result: bool,
}

#[derive(Default)]
Expand Down Expand Up @@ -37,6 +38,12 @@ impl Meta {
Ok(())
}

fn force_result(&mut self, _meta: syn::meta::ParseNestedMeta) -> syn::Result<()> {
self.result = true;

Ok(())
}

fn make_task(&mut self, meta: syn::meta::ParseNestedMeta) -> syn::Result<()> {
if self.context {
return Err(meta.error(super::TASK_CX_ERROR));
Expand Down Expand Up @@ -68,6 +75,10 @@ impl syn::parse::Parser for Parser {
return attr.force_context(meta);
}

if meta.path.is_ident("result") {
return attr.force_result(meta);
}

if meta.path.is_ident("task") {
return attr.make_task(meta);
}
Expand Down
44 changes: 25 additions & 19 deletions crates/neon-macros/src/export/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,14 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS
let wrapper_name = quote::format_ident!("__NEON_EXPORT_WRAPPER__{name}");

// Determine if the first argument is `FunctionContext`
let has_context = meta.context
|| match has_context_arg(&meta, &sig) {
Ok(has_context) => has_context,
Err(err) => return err.into_compile_error().into(),
};
let has_context = match has_context_arg(&meta, &sig) {
Ok(has_context) => has_context,
Err(err) => return err.into_compile_error().into(),
};

// Retain the context argument, if necessary
let context_arg = has_context.then(|| quote::quote!(&mut cx,));

// Default export name as identity unless a name is provided
let export_name = meta
.name
.map(|name| quote::quote!(#name))
.unwrap_or_else(|| quote::quote!(stringify!(#name)));

// Generate an argument list used when calling the original function
let start = if has_context { 1 } else { 0 };
let args = (start..sig.inputs.len()).map(|i| quote::format_ident!("a{i}"));
Expand All @@ -49,13 +42,19 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS

// If necessary, wrap the return value in `Json` before calling `TryIntoJs`
let json_return = meta.json.then(|| {
is_result_output(&sig.output)
is_result_output(&meta, &sig.output)
// Use `.map(Json)` on a `Result`
.then(|| quote::quote!(let res = res.map(neon::types::extract::Json);))
// Wrap other values with `Json(res)`
.unwrap_or_else(|| quote::quote!(let res = neon::types::extract::Json(res);))
});

// Default export name as identity unless a name is provided
let export_name = meta
.name
.map(|name| quote::quote!(#name))
.unwrap_or_else(|| quote::quote!(stringify!(#name)));

// Generate the call to the original function
let call_body = match meta.kind {
Kind::Normal => quote::quote!(
Expand Down Expand Up @@ -119,7 +118,7 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS
}

// Get the ident for the first argument
fn first_arg_ident(sig: &syn::Signature) -> Option<&syn::Ident> {
fn first_arg_ty(sig: &syn::Signature) -> Option<&syn::Ident> {
let arg = sig.inputs.first()?;
let ty = match arg {
syn::FnArg::Receiver(v) => &*v.ty,
Expand All @@ -136,18 +135,20 @@ fn first_arg_ident(sig: &syn::Signature) -> Option<&syn::Ident> {
_ => return None,
};

let path = match path.path.segments.last() {
Some(path) => path,
None => return None,
};
let path = path.path.segments.last()?;

Some(&path.ident)
}

// Determine if the function has a context argument and if it is allowed
fn has_context_arg(meta: &meta::Meta, sig: &syn::Signature) -> syn::Result<bool> {
// Forced context argument
if meta.context {
return Ok(true);
}

// Return early if no arguments
let first = match first_arg_ident(sig) {
let first = match first_arg_ty(sig) {
Some(first) => first,
None => return Ok(false),
};
Expand All @@ -167,7 +168,12 @@ fn has_context_arg(meta: &meta::Meta, sig: &syn::Signature) -> syn::Result<bool>
}

// Determine if a return type is a `Result`
fn is_result_output(ret: &syn::ReturnType) -> bool {
fn is_result_output(meta: &meta::Meta, ret: &syn::ReturnType) -> bool {
// Forced result output
if meta.result {
return true;
}

let ty = match ret {
syn::ReturnType::Default => return false,
syn::ReturnType::Type(_, ty) => &**ty,
Expand Down
5 changes: 5 additions & 0 deletions crates/neon-macros/src/export/global/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ pub(super) fn export(meta: meta::Meta, name: &syn::Ident, expr: Box<syn::Expr>)

// Generate the function that is registered to create the global on addon initialization.
// Braces are included to prevent names from polluting user code.
//
// N.B.: The `linkme(..)` attribute informs the `distributed_slice(..)` macro where
// to find the `linkme` crate. It is re-exported from neon to avoid dependents from
// needing to adding a direct dependency on `linkme`. It is an undocumented feature.
// https://github.com/dtolnay/linkme/issues/54
let create_fn = quote::quote!({
#[doc(hidden)]
#[neon::macro_internal::linkme::distributed_slice(neon::macro_internal::EXPORTS)]
Expand Down
114 changes: 90 additions & 24 deletions crates/neon/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,30 +70,6 @@ pub use neon_macros::main;
/// }
/// ```
///
/// ### Interact with the JavaScript runtime
///
/// More complex functions may need to interact directly with the JavaScript runtime,
/// for example with [`Context`](crate::context::Context) or handles to JavaScript values.
///
/// Functions may optionally include a [`FunctionContext`](crate::context::FunctionContext) argument. Note
/// that unlike functions created with [`JsFunction::new`](crate::types::JsFunction), exported function
/// receive a borrowed context and may require explicit lifetimes.
///
/// ```
/// # use neon::prelude::*;
/// #[neon::export]
/// fn add<'cx>(
/// cx: &mut FunctionContext<'cx>,
/// a: Handle<JsNumber>,
/// b: Handle<JsNumber>,
/// ) -> JsResult<'cx, JsNumber> {
/// let a = a.value(cx);
/// let b = b.value(cx);
///
/// Ok(cx.number(a + b))
/// }
/// ```
///
/// ### Exporting a function that uses JSON
///
/// The [`Json`](crate::types::extract::Json) wrapper allows ergonomically handling complex
Expand Down Expand Up @@ -145,4 +121,94 @@ pub use neon_macros::main;
/// a + b
/// }
/// ```
///
/// ### Error Handling
///
/// If an exported function returns a [`Result`], a JavaScript exception will be thrown
/// with the [`Err`]. Any error type that implements [`TryIntoJs`](crate::types::extract::TryIntoJs)
/// may be used.
///
/// ```
/// #[neon::export]
/// fn throw(msg: String) -> Result<(), String> {
/// Err(msg)
/// }
/// ```
///
/// The [`Error`](crate::types::extract::Error) type is provided for ergonomic error conversions
/// from most error types using the `?` operator.
///
/// ```
/// use neon::types::extract::Error;
///
/// #[neon::export]
/// fn read_file(path: String) -> Result<String, Error> {
/// let contents = std::fs::read_to_string(path)?;
/// Ok(contents)
/// }
/// ```
///
/// ### Interact with the JavaScript runtime
///
/// More complex functions may need to interact directly with the JavaScript runtime,
/// for example with [`Context`](crate::context::Context) or handles to JavaScript values.
///
/// Functions may optionally include a [`FunctionContext`](crate::context::FunctionContext) argument. Note
/// that unlike functions created with [`JsFunction::new`](crate::types::JsFunction), exported function
/// receive a borrowed context and may require explicit lifetimes.
///
/// ```
/// # use neon::prelude::*;
/// #[neon::export]
/// fn add<'cx>(
/// cx: &mut FunctionContext<'cx>,
/// a: Handle<JsNumber>,
/// b: Handle<JsNumber>,
/// ) -> JsResult<'cx, JsNumber> {
/// let a = a.value(cx);
/// let b = b.value(cx);
///
/// Ok(cx.number(a + b))
/// }
/// ```
///
/// ### Advanced
///
/// The following attributes are for advanced configuration and may not be
/// necessary for most users.
///
/// #### `context`
///
/// The `#[neon::export]` macro looks checks if the first argument has a type of
/// &mut FunctionContext` to determine if the [`Context`](crate::context::Context)
/// should be passed to the function.
///
/// If the type has been renamed when importing, the `context` attribute can be
/// added to force it to be passed.
///
/// ```
/// use neon::context::{FunctionContext as FnCtx};
///
/// #[neon::export(context)]
/// fn add(_cx: &mut FnCtx, a: f64, b: f64) -> f64 {
/// a + b
/// }
/// ```
///
/// ### `result`
///
/// The `#[neon::export]` macro will infer an exported function returns a [`Result`]
/// if the type is named [`Result`], [`NeonResult`](crate::result::NeonResult) or
/// [`JsResult`](crate::result::JsResult).
///
/// If a type alias is used for [`Result`], the `result` attribute can be added to
/// inform the generated code.
///
/// ```
/// use neon::result::{NeonResult as Res};
///
/// fn add(a: f64, b: f64) -> Res<f64> {
/// Ok(a + b)
/// }
/// ```
pub use neon_macros::export;
15 changes: 15 additions & 0 deletions crates/neon/src/types_impl/extract/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,21 @@ type BoxError = Box<dyn error::Error + Send + Sync + 'static>;
#[derive(Debug)]
/// Error that implements [`TryFromJs`] and [`TryIntoJs`] and can produce specific error types
///
/// [`Error`] implements [`From`] for most error types, allowing ergonomic error handling in
/// exported functions with the `?` operator.
///
/// ### Example
///
/// ```
/// use neon::types::extract::Error;
///
/// #[neon::export]
/// fn read_file(path: String) -> Result<String, Error> {
/// let contents = std::fs::read_to_string(path)?;
/// Ok(contents)
/// }
/// ```
///
/// **Note**: Extracting an [`Error`] from a [`JsValue`] with [`TryFromJs`] and converting
/// back to a [`JsError`] with [`TryIntoJs`] is _lossy_. It is not guaranteed that the same
/// type will be returned.
Expand Down

0 comments on commit 9dce892

Please sign in to comment.