From ffe6ce464a34701279e3b99c9eac3104048ba404 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Tue, 1 Oct 2024 13:58:13 -0400 Subject: [PATCH 1/2] feat(neon-macros): Add `Channel` context for async functions and `this` support Additionally, improves error messaging when incorrect types are provided. UI tests, via trybuild, are added to prevent regressions on errors. --- .gitignore | 1 + Cargo.lock | 114 ++++++ .../neon-macros/src/export/function/meta.rs | 27 +- crates/neon-macros/src/export/function/mod.rs | 326 ++++++++++++++---- crates/neon/src/macros.rs | 51 ++- .../src/types_impl/extract/try_into_js.rs | 14 +- test/napi/src/js/boxed.rs | 18 +- test/napi/src/js/export.rs | 20 +- test/napi/src/js/futures.rs | 16 + test/napi/src/js/threads.rs | 21 +- test/ui/Cargo.toml | 17 + test/ui/src/lib.rs | 7 + test/ui/tests/fail/missing-context.rs | 4 + test/ui/tests/fail/missing-context.stderr | 7 + test/ui/tests/fail/need-borrowed-context.rs | 19 + .../tests/fail/need-borrowed-context.stderr | 35 ++ test/ui/tests/fail/unexpected-self.rs | 14 + test/ui/tests/fail/unexpected-self.stderr | 17 + test/ui/tests/fail/unnecessary-attribute.rs | 4 + .../tests/fail/unnecessary-attribute.stderr | 5 + test/ui/tests/fail/unsupported-property.rs | 7 + .../ui/tests/fail/unsupported-property.stderr | 11 + test/ui/tests/fail/wrong-context.rs | 40 +++ test/ui/tests/fail/wrong-context.stderr | 77 +++++ test/ui/tests/pass/context-and-this.rs | 134 +++++++ test/ui/tests/pass/globals.rs | 13 + test/ui/tests/pass/json.rs | 6 + 27 files changed, 944 insertions(+), 81 deletions(-) create mode 100644 test/ui/Cargo.toml create mode 100644 test/ui/src/lib.rs create mode 100644 test/ui/tests/fail/missing-context.rs create mode 100644 test/ui/tests/fail/missing-context.stderr create mode 100644 test/ui/tests/fail/need-borrowed-context.rs create mode 100644 test/ui/tests/fail/need-borrowed-context.stderr create mode 100644 test/ui/tests/fail/unexpected-self.rs create mode 100644 test/ui/tests/fail/unexpected-self.stderr create mode 100644 test/ui/tests/fail/unnecessary-attribute.rs create mode 100644 test/ui/tests/fail/unnecessary-attribute.stderr create mode 100644 test/ui/tests/fail/unsupported-property.rs create mode 100644 test/ui/tests/fail/unsupported-property.stderr create mode 100644 test/ui/tests/fail/wrong-context.rs create mode 100644 test/ui/tests/fail/wrong-context.stderr create mode 100644 test/ui/tests/pass/context-and-this.rs create mode 100644 test/ui/tests/pass/globals.rs create mode 100644 test/ui/tests/pass/json.rs diff --git a/.gitignore b/.gitignore index 740828832..db501ae6c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ target +wip **/*~ **/node_modules **/.DS_Store diff --git a/Cargo.lock b/Cargo.lock index 5884ca49b..8a31c5086 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -168,6 +168,12 @@ dependencies = [ "neon", ] +[[package]] +name = "equivalent" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" + [[package]] name = "errno" version = "0.3.8" @@ -201,6 +207,12 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" + [[package]] name = "hermit-abi" version = "0.3.3" @@ -235,6 +247,16 @@ dependencies = [ "quote", ] +[[package]] +name = "indexmap" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68b900aa2f7301e21c36462b170ee99994de34dff39a4a6a528e80e7376d07e5" +dependencies = [ + "equivalent", + "hashbrown", +] + [[package]] name = "itertools" version = "0.10.5" @@ -707,6 +729,15 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_spanned" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87607cb1398ed59d48732e575a4c28a7a8ebf2454b964fe3f224f2afc07909e1" +dependencies = [ + "serde", +] + [[package]] name = "shlex" version = "1.3.0" @@ -747,6 +778,15 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + [[package]] name = "thiserror" version = "1.0.50" @@ -778,6 +818,62 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "toml" +version = "0.8.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1ed1f98e3fdc28d6d910e6737ae6ab1a93bf1985935a1193e68f93eeb68d24e" +dependencies = [ + "serde", + "serde_spanned", + "toml_datetime", + "toml_edit", +] + +[[package]] +name = "toml_datetime" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0dd7358ecb8fc2f8d014bf86f6f638ce72ba252a2c3a2572f2a795f1d23efb41" +dependencies = [ + "serde", +] + +[[package]] +name = "toml_edit" +version = "0.22.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ae48d6208a266e853d946088ed816055e556cc6028c5e8e2b84d9fa5dd7c7f5" +dependencies = [ + "indexmap", + "serde", + "serde_spanned", + "toml_datetime", + "winnow", +] + +[[package]] +name = "trybuild" +version = "1.0.99" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "207aa50d36c4be8d8c6ea829478be44a372c6a77669937bb39c698e52f1491e8" +dependencies = [ + "glob", + "serde", + "serde_derive", + "serde_json", + "termcolor", + "toml", +] + +[[package]] +name = "ui-test" +version = "0.1.0" +dependencies = [ + "neon", + "trybuild", +] + [[package]] name = "unicode-ident" version = "1.0.12" @@ -830,6 +926,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" +dependencies = [ + "windows-sys 0.52.0", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" @@ -967,3 +1072,12 @@ name = "windows_x86_64_msvc" version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dff9641d1cd4be8d1a070daf9e3773c5f67e78b4d9d42263020c057706765c04" + +[[package]] +name = "winnow" +version = "0.6.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "36c1fec1a2bb5866f07c25f68c26e565c4c200aebb96d7e55710c19d3e8ac49b" +dependencies = [ + "memchr", +] diff --git a/crates/neon-macros/src/export/function/meta.rs b/crates/neon-macros/src/export/function/meta.rs index 948677b0c..cfe2bd776 100644 --- a/crates/neon-macros/src/export/function/meta.rs +++ b/crates/neon-macros/src/export/function/meta.rs @@ -4,6 +4,7 @@ pub(crate) struct Meta { pub(super) name: Option, pub(super) json: bool, pub(super) context: bool, + pub(super) this: bool, } #[derive(Default)] @@ -28,21 +29,21 @@ impl Meta { Ok(()) } - fn force_context(&mut self, meta: syn::meta::ParseNestedMeta) -> syn::Result<()> { - match self.kind { - Kind::Normal | Kind::AsyncFn => {} - Kind::Async => return Err(meta.error(super::ASYNC_CX_ERROR)), - Kind::Task => return Err(meta.error(super::TASK_CX_ERROR)), - } - + fn force_context(&mut self, _meta: syn::meta::ParseNestedMeta) -> syn::Result<()> { self.context = true; Ok(()) } + fn force_this(&mut self, _meta: syn::meta::ParseNestedMeta) -> syn::Result<()> { + self.this = true; + + Ok(()) + } + fn make_async(&mut self, meta: syn::meta::ParseNestedMeta) -> syn::Result<()> { if matches!(self.kind, Kind::AsyncFn) { - return Err(meta.error(super::ASYNC_FN_ERROR)); + return Err(meta.error("`async` attribute should not be used with an `async fn`")); } self.kind = Kind::Async; @@ -50,11 +51,7 @@ impl Meta { Ok(()) } - fn make_task(&mut self, meta: syn::meta::ParseNestedMeta) -> syn::Result<()> { - if self.context { - return Err(meta.error(super::TASK_CX_ERROR)); - } - + fn make_task(&mut self, _meta: syn::meta::ParseNestedMeta) -> syn::Result<()> { self.kind = Kind::Task; Ok(()) @@ -93,6 +90,10 @@ impl syn::parse::Parser for Parser { return attr.force_context(meta); } + if meta.path.is_ident("this") { + return attr.force_this(meta); + } + if meta.path.is_ident("async") { return attr.make_async(meta); } diff --git a/crates/neon-macros/src/export/function/mod.rs b/crates/neon-macros/src/export/function/mod.rs index 7f84479df..5ba8b57b1 100644 --- a/crates/neon-macros/src/export/function/mod.rs +++ b/crates/neon-macros/src/export/function/mod.rs @@ -1,11 +1,9 @@ +use syn::spanned::Spanned; + use crate::export::function::meta::Kind; pub(crate) mod meta; -static ASYNC_CX_ERROR: &str = "`FunctionContext` is not allowed in async functions"; -static ASYNC_FN_ERROR: &str = "`async` attribute should not be used with an `async fn`"; -static TASK_CX_ERROR: &str = "`FunctionContext` is not allowed with `task` attribute"; - pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenStream { let syn::ItemFn { attrs, @@ -16,24 +14,25 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS let name = &sig.ident; - // Name for the registered create function - let create_name = quote::format_ident!("__NEON_EXPORT_CREATE__{name}"); - - // Name for the function that is wrapped by `JsFunction`. Delegates to the original. - let wrapper_name = quote::format_ident!("__NEON_EXPORT_WRAPPER__{name}"); - - // Determine if the first argument is `FunctionContext` - let has_context = match has_context_arg(&meta, &sig) { - Ok(has_context) => has_context, + // Generate the context or channel argument for the function + let (context_extract, context_arg) = match context_parse(&meta, &sig) { + Ok(arg) => arg, Err(err) => return err.into_compile_error().into(), }; - // Retain the context argument, if necessary - let context_arg = has_context.then(|| quote::quote!(&mut cx,)); + // Extract `this` if necessary + let has_this = check_this(&meta, &sig, context_arg.is_some()); + let this_arg = has_this.then(|| quote::quote!(this,)); + let this_extract = has_this.then(|| { + quote::quote!( + let this = cx.this()?; + let this = neon::types::extract::TryFromJs::from_js(&mut cx, this)?; + ) + }); // 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}")); + let num_args = count_args(&sig, context_arg.is_some(), has_this); + let args = (0..num_args).map(|i| quote::format_ident!("a{i}")); // Generate the tuple fields used to destructure `cx.args()`. Wrap in `Json` if necessary. let tuple_fields = args.clone().map(|name| { @@ -57,17 +56,13 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS (&res).to_neon_marker::().neon_into_js(&mut cx, 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::Async | Kind::AsyncFn => quote::quote!( + Kind::Async => quote::quote!( + #context_extract + #this_extract let (#(#tuple_fields,)*) = cx.args()?; - let fut = #name(#context_arg #(#args),*); + let fut = #name(#context_arg #this_arg #(#args),*); let fut = { use neon::macro_internal::{ToNeonMarker, NeonValueTag}; @@ -76,15 +71,27 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS neon::macro_internal::spawn(&mut cx, fut, |mut cx, res| #result_extract) ), + Kind::AsyncFn => quote::quote!( + #context_extract + #this_extract + let (#(#tuple_fields,)*) = cx.args()?; + let fut = #name(#context_arg #this_arg #(#args),*); + + neon::macro_internal::spawn(&mut cx, fut, |mut cx, res| #result_extract) + ), Kind::Normal => quote::quote!( + #context_extract + #this_extract let (#(#tuple_fields,)*) = cx.args()?; - let res = #name(#context_arg #(#args),*); + let res = #name(#context_arg #this_arg #(#args),*); #result_extract ), Kind::Task => quote::quote!( + #context_extract + #this_extract let (#(#tuple_fields,)*) = cx.args()?; - let promise = neon::context::Context::task(&mut cx, move || #name(#context_arg #(#args),*)) + let promise = neon::context::Context::task(&mut cx, move || #name(#context_arg #this_arg #(#args),*)) .promise(|mut cx, res| #result_extract); Ok(neon::handle::Handle::upcast(&promise)) @@ -92,6 +99,7 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS }; // Generate the wrapper function + let wrapper_name = quote::format_ident!("__NEON_EXPORT_WRAPPER__{name}"); let wrapper_fn = quote::quote!( #[doc(hidden)] fn #wrapper_name(mut cx: neon::context::FunctionContext) -> neon::result::JsResult { @@ -99,8 +107,15 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS } ); + // 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 function that is registered to create the function on addon initialization. // Braces are included to prevent names from polluting user code. + let create_name = quote::format_ident!("__NEON_EXPORT_CREATE__{name}"); let create_fn = quote::quote!({ #[doc(hidden)] #[neon::macro_internal::linkme::distributed_slice(neon::macro_internal::EXPORTS)] @@ -130,53 +145,244 @@ pub(super) fn export(meta: meta::Meta, input: syn::ItemFn) -> proc_macro::TokenS .into() } -// Get the ident for the first argument -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, - syn::FnArg::Typed(v) => &*v.ty, - }; +// Determine the number of arguments to the function +fn count_args(sig: &syn::Signature, has_context: bool, has_this: bool) -> usize { + let n = sig.inputs.len(); - let ty = match ty { - syn::Type::Reference(ty) => &*ty.elem, - _ => return None, + match (has_context, has_this) { + (true, true) => n - 2, + (false, false) => n, + _ => n - 1, + } +} + +// Generate the context extraction and argument for the function +fn context_parse( + opts: &meta::Meta, + sig: &syn::Signature, +) -> syn::Result<( + Option, + Option, +)> { + match opts.kind { + // Allow borrowing from context + Kind::Async | Kind::Normal if check_context(opts, sig)? => { + Ok((None, Some(quote::quote!(&mut cx,)))) + } + + // Require `'static` arguments + Kind::AsyncFn | Kind::Task if check_channel(opts, sig)? => Ok(( + Some(quote::quote!(let ch = neon::context::Context::channel(&mut cx);)), + Some(quote::quote!(ch,)), + )), + + _ => Ok((None, None)), + } +} + +// Checks if a _sync_ function has a context argument and if it is valid +// * If the `context` attribute is included, must be at least one argument +// * Inferred to be context if named `FunctionContext` or `Cx` +// * Context argument must be a `&mut` reference +// * First argument must not be `Channel` +// * Must not be a `self` receiver +fn check_context(opts: &meta::Meta, sig: &syn::Signature) -> syn::Result { + // Extract the first argument + let ty = match first_arg(opts, sig)? { + Some(arg) => arg, + None => return Ok(false), }; - let path = match ty { - syn::Type::Path(path) => path, - _ => return None, + // Extract the reference type + let ty = match &*ty.ty { + // Tried to use a borrowed Channel + syn::Type::Reference(ty) if !opts.context && is_channel_type(&ty.elem) => { + return Err(syn::Error::new( + ty.elem.span(), + "Expected `&mut Cx` instead of a `Channel` reference.", + )) + } + + syn::Type::Reference(ty) => ty, + + // Context needs to be a reference + _ if opts.context || is_context_type(&ty.ty) => { + return Err(syn::Error::new( + ty.ty.span(), + "Context must be a `&mut` reference.", + )) + } + + // Hint that `Channel` should be swapped for `&mut Cx` + _ if is_channel_type(&ty.ty) => { + return Err(syn::Error::new( + ty.ty.span(), + "Expected `&mut Cx` instead of `Channel`.", + )) + } + + _ => return Ok(false), }; - let path = path.path.segments.last()?; + // Not a forced or inferred context + if !opts.context && !is_context_type(&ty.elem) { + return Ok(false); + } + + // Context argument must be mutable + if ty.mutability.is_none() { + return Err(syn::Error::new(ty.span(), "Must be a `&mut` reference.")); + } - Some(&path.ident) + // All tests passed! + Ok(true) } -// 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 { - // Forced context argument - if meta.context { - return Ok(true); +// Checks if a _async_ function has a Channel argument and if it is valid +// * If the `context` attribute is included, must be at least one argument +// * Inferred to be channel if named `Channel` +// * Channel argument must not be a reference +// * First argument must not be `FunctionContext` or `Cx` +// * Must not be a `self` receiver +fn check_channel(opts: &meta::Meta, sig: &syn::Signature) -> syn::Result { + // Extract the first argument + let ty = match first_arg(opts, sig)? { + Some(arg) => arg, + None => return Ok(false), + }; + + // Check the type + match &*ty.ty { + // Provided `&mut Channel` instead of `Channel` + syn::Type::Reference(ty) if opts.context || is_channel_type(&ty.elem) => { + Err(syn::Error::new( + ty.span(), + "Expected an owned `Channel` instead of a reference.", + )) + } + + // Provided a `&mut Cx` instead of a `Channel` + syn::Type::Reference(ty) if is_context_type(&ty.elem) => Err(syn::Error::new( + ty.elem.span(), + "Expected an owned `Channel` instead of a context reference.", + )), + + // Found a `Channel` + _ if opts.context || is_channel_type(&ty.ty) => Ok(true), + + // Tried to use an owned `Cx` + _ if is_context_type(&ty.ty) => Err(syn::Error::new( + ty.ty.span(), + "Context is not available in async functions. Try a `Channel` instead.", + )), + + _ => Ok(false), } +} - // Return early if no arguments - let first = match first_arg_ty(sig) { - Some(first) => first, - None => return Ok(false), +// Extract the first argument, that may be a context, of a function +fn first_arg<'a>( + opts: &meta::Meta, + sig: &'a syn::Signature, +) -> syn::Result> { + // Extract the first argument + let arg = match sig.inputs.first() { + Some(arg) => arg, + + // If context was forced, error to let the user know the mistake + None if opts.context => { + return Err(syn::Error::new( + sig.inputs.span(), + "Expected a context argument. Try removing the `context` attribute.", + )) + } + + None => return Ok(None), }; - // First argument isn't context - if first != "FunctionContext" && first != "Cx" { - return Ok(false); + // Expect a typed pattern; self receivers are not supported + match arg { + syn::FnArg::Typed(ty) => Ok(Some(ty)), + syn::FnArg::Receiver(arg) => Err(syn::Error::new( + arg.span(), + "Exported functions cannot receive `self`.", + )), } +} + +fn is_context_type(ty: &syn::Type) -> bool { + let ident = match type_path_ident(ty) { + Some(ident) => ident, + None => return false, + }; + + ident == "FunctionContext" || ident == "Cx" +} - // Context is only allowed for normal functions - match meta.kind { - Kind::Normal | Kind::Async => {} - Kind::AsyncFn => return Err(syn::Error::new(first.span(), ASYNC_CX_ERROR)), - Kind::Task => return Err(syn::Error::new(first.span(), TASK_CX_ERROR)), +fn is_channel_type(ty: &syn::Type) -> bool { + let ident = match type_path_ident(ty) { + Some(ident) => ident, + None => return false, + }; + + ident == "Channel" +} + +// Extract the identifier from the last segment of a type's path +fn type_path_ident(ty: &syn::Type) -> Option<&syn::Ident> { + let segment = match ty { + syn::Type::Path(ty) => ty.path.segments.last()?, + _ => return None, + }; + + Some(&segment.ident) +} + +// Determine if the function has a `this` argument. It will be either the `0th` element +// or, if a context argument is included, the `1st`. +fn check_this(opts: &meta::Meta, sig: &syn::Signature, has_context: bool) -> bool { + static THIS: &str = "this"; + + // Forced `this` argument + if opts.this { + return true; } - Ok(true) + // Get the first argument, skipping context + let first = if has_context { + sig.inputs.iter().nth(1) + } else { + sig.inputs.first() + }; + + // No other arguments; return early + let first = match first { + Some(first) => first, + None => return false, + }; + + // Ignore `self` type receivers; those aren't used for `this` + let ty = match first { + syn::FnArg::Receiver(_) => return false, + syn::FnArg::Typed(ty) => ty, + }; + + // Check for `this` ident or a tuple struct + let pat = match &*ty.pat { + syn::Pat::Ident(ident) if ident.ident == THIS => return true, + syn::Pat::TupleStruct(pat) => pat, + _ => return false, + }; + + // Expect exactly one element in the tuple struct + let elem = match pat.elems.first() { + Some(elem) if pat.elems.len() == 1 => elem, + _ => return false, + }; + + // Must be an identifier named `this` + match elem { + syn::Pat::Ident(ident) => ident.ident == THIS, + _ => false, + } } diff --git a/crates/neon/src/macros.rs b/crates/neon/src/macros.rs index 343d1cebb..e6e950649 100644 --- a/crates/neon/src/macros.rs +++ b/crates/neon/src/macros.rs @@ -237,19 +237,62 @@ pub use neon_macros::main; /// /// #### `context` /// -/// The `#[neon::export]` macro looks checks if the first argument has a type of -/// `&mut Cx` or `&mut FunctionContext` to determine if the [`Context`](crate::context::Context) -/// should be passed to the function. +/// The `#[neon::export]` uses a heuristic to determine if the first argument +/// to a function is a _context_ argument. +/// +/// * In a function executed on the JavaScript main thread, it looks for `&mut Cx` +/// or `&mut FunctionContext` to determine if the [`Context`](crate::context::Context) +/// should be passed. +/// * In a function executed on another thread, it looks for [`Channel`](crate::event::Channel). /// /// 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}; +/// use neon::event::Channel as Ch; +/// use neon::context::FunctionContext as FnCtx; /// /// #[neon::export(context)] /// fn add(_cx: &mut FnCtx, a: f64, b: f64) -> f64 { /// a + b /// } +/// +/// #[neon::export(context)] +/// async fn div(_ch: Ch, a: f64, b: f64) -> f64 { +/// a / b +/// } +/// ``` +/// +/// #### `this` +/// +/// The `#[neon::export]` uses a heuristic to determine if an argument to this function is +/// referring to [`this`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this). +/// +/// 1. If the first argument is a [context](#context), use the 0th argument, otherwise use the 1st. +/// 2. If the argument binding is named `this` +/// 3. Or if it is a tuple struct pattern with an element named `this` +/// +/// ``` +/// use neon::types::extract::Boxed; +/// +/// #[neon::export] +/// fn buffer_clone(this: Vec) -> Vec { +/// this +/// } +/// +/// #[neon::export] +/// fn box_to_string(Boxed(this): Boxed) -> String { +/// this +/// } +/// ``` +/// +/// If the function uses a variable name other than `this`, the `this` attribute may +/// be added. +/// +/// ``` +/// #[neon::export(this)] +/// fn buffer_clone(me: Vec) -> Vec { +/// me +/// } /// ``` pub use neon_macros::export; diff --git a/crates/neon/src/types_impl/extract/try_into_js.rs b/crates/neon/src/types_impl/extract/try_into_js.rs index 9f452684d..e1151a746 100644 --- a/crates/neon/src/types_impl/extract/try_into_js.rs +++ b/crates/neon/src/types_impl/extract/try_into_js.rs @@ -1,8 +1,9 @@ use std::sync::Arc; +use crate::prelude::Object; use crate::{ context::{Context, Cx}, - handle::Handle, + handle::{Handle, Root}, result::{JsResult, ResultExt, Throw}, types::{ buffer::Binary, @@ -23,6 +24,17 @@ where } } +impl<'cx, O> TryIntoJs<'cx> for Root +where + O: Object, +{ + type Value = O; + + fn try_into_js(self, cx: &mut Cx<'cx>) -> JsResult<'cx, Self::Value> { + Ok(self.into_inner(cx)) + } +} + impl<'cx, T, E> TryIntoJs<'cx> for Result where T: TryIntoJs<'cx>, diff --git a/test/napi/src/js/boxed.rs b/test/napi/src/js/boxed.rs index 1c24e6c45..9cfeb6ff3 100644 --- a/test/napi/src/js/boxed.rs +++ b/test/napi/src/js/boxed.rs @@ -1,6 +1,6 @@ use std::cell::RefCell; -use neon::prelude::*; +use neon::{prelude::*, types::extract::Boxed}; pub struct Person { name: String, @@ -72,3 +72,19 @@ pub fn ref_person_fail(mut cx: FunctionContext) -> JsResult { pub fn external_unit(mut cx: FunctionContext) -> JsResult> { Ok(cx.boxed(())) } + +#[neon::export] +fn create_boxed_string(s: String) -> Boxed { + Boxed(s) +} + +#[neon::export] +fn boxed_string_concat(Boxed(this): Boxed, rhs: String) -> String { + this + &rhs +} + +#[neon::export] +// N.B.: Intentionally including unused `cx` and not using tuple struct pattern to test the macro +fn boxed_string_repeat(_cx: &mut FunctionContext, this: Boxed, n: f64) -> String { + this.0.repeat(n as usize) +} diff --git a/test/napi/src/js/export.rs b/test/napi/src/js/export.rs index d5d1267e8..36f0b75c7 100644 --- a/test/napi/src/js/export.rs +++ b/test/napi/src/js/export.rs @@ -1,4 +1,7 @@ -use neon::{prelude::*, types::extract::Error}; +use neon::{ + prelude::*, + types::extract::{Boxed, Error}, +}; #[neon::export] const NUMBER: u8 = 42; @@ -93,3 +96,18 @@ fn sleep_task(ms: f64) { fn number_with_cx<'cx>(cx: &mut Cx<'cx>, n: f64) -> Handle<'cx, JsNumber> { cx.number(n) } + +#[neon::export] +fn simple_self(this: Handle) -> Handle { + this +} + +#[neon::export] +fn boxed_self(Boxed(this): Boxed) -> String { + this +} + +#[neon::export] +fn boxed_string(s: String) -> Boxed { + Boxed(s) +} diff --git a/test/napi/src/js/futures.rs b/test/napi/src/js/futures.rs index 3b02c7e4a..acbba29cb 100644 --- a/test/napi/src/js/futures.rs +++ b/test/napi/src/js/futures.rs @@ -135,3 +135,19 @@ fn async_with_events( }) }) } + +#[neon::export] +async fn await_callback(ch: Channel, cb: Root) -> Result, Error> { + let res = ch + .send(move |mut cx| { + let this = cx.undefined(); + + cb.into_inner(&mut cx) + .call(&mut cx, this, []) + .and_then(|v| v.downcast_or_throw::(&mut cx)) + .map(|v| v.root(&mut cx)) + }) + .await?; + + Ok(res) +} diff --git a/test/napi/src/js/threads.rs b/test/napi/src/js/threads.rs index a55a656fc..cd8b8a59c 100644 --- a/test/napi/src/js/threads.rs +++ b/test/napi/src/js/threads.rs @@ -1,6 +1,9 @@ use std::{cell::RefCell, sync::Arc, time::Duration}; -use neon::{prelude::*, types::buffer::TypedArray}; +use neon::{ + prelude::*, + types::{buffer::TypedArray, extract::Error}, +}; pub fn useless_root(mut cx: FunctionContext) -> JsResult { let object = cx.argument::(0)?; @@ -463,3 +466,19 @@ pub fn deferred_settle_with_panic_throw(mut cx: FunctionContext) -> JsResult) -> Result, Error> { + let res = ch + .send(move |mut cx| { + let this = cx.undefined(); + + cb.into_inner(&mut cx) + .call(&mut cx, this, []) + .and_then(|v| v.downcast_or_throw::(&mut cx)) + .map(|v| v.root(&mut cx)) + }) + .join()?; + + Ok(res) +} diff --git a/test/ui/Cargo.toml b/test/ui/Cargo.toml new file mode 100644 index 000000000..83b074886 --- /dev/null +++ b/test/ui/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "ui-test" +version = "0.1.0" +edition = "2021" +authors = ["The Neon Community"] +license = "MIT/Apache-2.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies.neon] +version = "1.0.0" +path = "../../crates/neon" +features = ["futures", "napi-experimental", "serde"] + +[dev-dependencies] +trybuild = "1" diff --git a/test/ui/src/lib.rs b/test/ui/src/lib.rs new file mode 100644 index 000000000..f120f3f48 --- /dev/null +++ b/test/ui/src/lib.rs @@ -0,0 +1,7 @@ +#[test] +fn ui() { + let t = trybuild::TestCases::new(); + + t.compile_fail("tests/fail/*.rs"); + t.pass("tests/pass/*.rs"); +} diff --git a/test/ui/tests/fail/missing-context.rs b/test/ui/tests/fail/missing-context.rs new file mode 100644 index 000000000..27295bc91 --- /dev/null +++ b/test/ui/tests/fail/missing-context.rs @@ -0,0 +1,4 @@ +#[neon::export(context)] +fn missing_context() {} + +fn main() {} diff --git a/test/ui/tests/fail/missing-context.stderr b/test/ui/tests/fail/missing-context.stderr new file mode 100644 index 000000000..b00ee44a2 --- /dev/null +++ b/test/ui/tests/fail/missing-context.stderr @@ -0,0 +1,7 @@ +error: Expected a context argument. Try removing the `context` attribute. + --> tests/fail/missing-context.rs:1:1 + | +1 | #[neon::export(context)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the attribute macro `neon::export` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/test/ui/tests/fail/need-borrowed-context.rs b/test/ui/tests/fail/need-borrowed-context.rs new file mode 100644 index 000000000..141bfa8a7 --- /dev/null +++ b/test/ui/tests/fail/need-borrowed-context.rs @@ -0,0 +1,19 @@ +#[neon::export] +fn owned_cx(_cx: Cx) {} + +#[neon::export] +fn owned_function_cx(_cx: FunctionContext) {} + +#[neon::export] +fn ref_cx(_cx: &Cx) {} + +#[neon::export] +fn ref_function_cx(_cx: &FunctionContext) {} + +#[neon::export(context)] +fn forced_cx(_cx: String) {} + +#[neon::export(context)] +fn forced_ref_cx(_cx: &String) {} + +fn main() {} diff --git a/test/ui/tests/fail/need-borrowed-context.stderr b/test/ui/tests/fail/need-borrowed-context.stderr new file mode 100644 index 000000000..1a807a3a6 --- /dev/null +++ b/test/ui/tests/fail/need-borrowed-context.stderr @@ -0,0 +1,35 @@ +error: Context must be a `&mut` reference. + --> tests/fail/need-borrowed-context.rs:2:18 + | +2 | fn owned_cx(_cx: Cx) {} + | ^^ + +error: Context must be a `&mut` reference. + --> tests/fail/need-borrowed-context.rs:5:27 + | +5 | fn owned_function_cx(_cx: FunctionContext) {} + | ^^^^^^^^^^^^^^^ + +error: Must be a `&mut` reference. + --> tests/fail/need-borrowed-context.rs:8:16 + | +8 | fn ref_cx(_cx: &Cx) {} + | ^ + +error: Must be a `&mut` reference. + --> tests/fail/need-borrowed-context.rs:11:25 + | +11 | fn ref_function_cx(_cx: &FunctionContext) {} + | ^ + +error: Context must be a `&mut` reference. + --> tests/fail/need-borrowed-context.rs:14:19 + | +14 | fn forced_cx(_cx: String) {} + | ^^^^^^ + +error: Must be a `&mut` reference. + --> tests/fail/need-borrowed-context.rs:17:23 + | +17 | fn forced_ref_cx(_cx: &String) {} + | ^ diff --git a/test/ui/tests/fail/unexpected-self.rs b/test/ui/tests/fail/unexpected-self.rs new file mode 100644 index 000000000..1c25d252b --- /dev/null +++ b/test/ui/tests/fail/unexpected-self.rs @@ -0,0 +1,14 @@ +struct Example; + +impl Example { + #[neon::export] + fn borrow(&self) {} + + #[neon::export] + fn borrow_mut(&mut self) {} + + #[neon::export] + fn owned(self) {} +} + +fn main() {} diff --git a/test/ui/tests/fail/unexpected-self.stderr b/test/ui/tests/fail/unexpected-self.stderr new file mode 100644 index 000000000..e60386084 --- /dev/null +++ b/test/ui/tests/fail/unexpected-self.stderr @@ -0,0 +1,17 @@ +error: Exported functions cannot receive `self`. + --> tests/fail/unexpected-self.rs:5:15 + | +5 | fn borrow(&self) {} + | ^ + +error: Exported functions cannot receive `self`. + --> tests/fail/unexpected-self.rs:8:19 + | +8 | fn borrow_mut(&mut self) {} + | ^ + +error: Exported functions cannot receive `self`. + --> tests/fail/unexpected-self.rs:11:14 + | +11 | fn owned(self) {} + | ^^^^ diff --git a/test/ui/tests/fail/unnecessary-attribute.rs b/test/ui/tests/fail/unnecessary-attribute.rs new file mode 100644 index 000000000..c89d83cce --- /dev/null +++ b/test/ui/tests/fail/unnecessary-attribute.rs @@ -0,0 +1,4 @@ +#[neon::export(async)] +async fn async_with_async() {} + +fn main() {} diff --git a/test/ui/tests/fail/unnecessary-attribute.stderr b/test/ui/tests/fail/unnecessary-attribute.stderr new file mode 100644 index 000000000..9b931ade6 --- /dev/null +++ b/test/ui/tests/fail/unnecessary-attribute.stderr @@ -0,0 +1,5 @@ +error: `async` attribute should not be used with an `async fn` + --> tests/fail/unnecessary-attribute.rs:1:16 + | +1 | #[neon::export(async)] + | ^^^^^ diff --git a/test/ui/tests/fail/unsupported-property.rs b/test/ui/tests/fail/unsupported-property.rs new file mode 100644 index 000000000..3c21c7bac --- /dev/null +++ b/test/ui/tests/fail/unsupported-property.rs @@ -0,0 +1,7 @@ +#[neon::export(foo)] +static STRING: &str = ""; + +#[neon::export(foo)] +fn unsupported() {} + +fn main() {} diff --git a/test/ui/tests/fail/unsupported-property.stderr b/test/ui/tests/fail/unsupported-property.stderr new file mode 100644 index 000000000..d2a3a1f45 --- /dev/null +++ b/test/ui/tests/fail/unsupported-property.stderr @@ -0,0 +1,11 @@ +error: unsupported property + --> tests/fail/unsupported-property.rs:1:16 + | +1 | #[neon::export(foo)] + | ^^^ + +error: unsupported property + --> tests/fail/unsupported-property.rs:4:16 + | +4 | #[neon::export(foo)] + | ^^^ diff --git a/test/ui/tests/fail/wrong-context.rs b/test/ui/tests/fail/wrong-context.rs new file mode 100644 index 000000000..d83b6322a --- /dev/null +++ b/test/ui/tests/fail/wrong-context.rs @@ -0,0 +1,40 @@ +#[neon::export] +fn sync_channel(_ch: Channel) {} + +#[neon::export] +fn sync_borrow_channel(_ch: &mut Channel) {} + +#[neon::export(async)] +fn async_channel(_ch: Channel) {} + +#[neon::export(async)] +fn async_borrow_channel(_ch: &mut Channel) {} + +#[neon::export] +async fn async_cx(_cx: Cx) {} + +#[neon::export] +async fn async_function_context(_cx: FunctionContext) {} + +#[neon::export] +async fn async_cx_ref(_cx: &Cx) {} + +#[neon::export] +async fn async_borrow_channel(_cx: &Channel) {} + +#[neon::export(context)] +async fn async_borrow_forced_channel(_cx: &String) {} + +#[neon::export] +async fn async_function_context_ref(_cx: &FunctionContext) {} + +#[neon::export(task)] +fn task_function_context(_cx: FunctionContext) {} + +#[neon::export(task)] +fn task_cx_ref(_cx: &Cx) {} + +#[neon::export(task)] +fn task_function_context_ref(_cx: &FunctionContext) {} + +fn main() {} diff --git a/test/ui/tests/fail/wrong-context.stderr b/test/ui/tests/fail/wrong-context.stderr new file mode 100644 index 000000000..293873bf5 --- /dev/null +++ b/test/ui/tests/fail/wrong-context.stderr @@ -0,0 +1,77 @@ +error: Expected `&mut Cx` instead of `Channel`. + --> tests/fail/wrong-context.rs:2:22 + | +2 | fn sync_channel(_ch: Channel) {} + | ^^^^^^^ + +error: Expected `&mut Cx` instead of a `Channel` reference. + --> tests/fail/wrong-context.rs:5:34 + | +5 | fn sync_borrow_channel(_ch: &mut Channel) {} + | ^^^^^^^ + +error: Expected `&mut Cx` instead of `Channel`. + --> tests/fail/wrong-context.rs:8:23 + | +8 | fn async_channel(_ch: Channel) {} + | ^^^^^^^ + +error: Expected `&mut Cx` instead of a `Channel` reference. + --> tests/fail/wrong-context.rs:11:35 + | +11 | fn async_borrow_channel(_ch: &mut Channel) {} + | ^^^^^^^ + +error: Context is not available in async functions. Try a `Channel` instead. + --> tests/fail/wrong-context.rs:14:24 + | +14 | async fn async_cx(_cx: Cx) {} + | ^^ + +error: Context is not available in async functions. Try a `Channel` instead. + --> tests/fail/wrong-context.rs:17:38 + | +17 | async fn async_function_context(_cx: FunctionContext) {} + | ^^^^^^^^^^^^^^^ + +error: Expected an owned `Channel` instead of a context reference. + --> tests/fail/wrong-context.rs:20:29 + | +20 | async fn async_cx_ref(_cx: &Cx) {} + | ^^ + +error: Expected an owned `Channel` instead of a reference. + --> tests/fail/wrong-context.rs:23:36 + | +23 | async fn async_borrow_channel(_cx: &Channel) {} + | ^ + +error: Expected an owned `Channel` instead of a reference. + --> tests/fail/wrong-context.rs:26:43 + | +26 | async fn async_borrow_forced_channel(_cx: &String) {} + | ^ + +error: Expected an owned `Channel` instead of a context reference. + --> tests/fail/wrong-context.rs:29:43 + | +29 | async fn async_function_context_ref(_cx: &FunctionContext) {} + | ^^^^^^^^^^^^^^^ + +error: Context is not available in async functions. Try a `Channel` instead. + --> tests/fail/wrong-context.rs:32:31 + | +32 | fn task_function_context(_cx: FunctionContext) {} + | ^^^^^^^^^^^^^^^ + +error: Expected an owned `Channel` instead of a context reference. + --> tests/fail/wrong-context.rs:35:22 + | +35 | fn task_cx_ref(_cx: &Cx) {} + | ^^ + +error: Expected an owned `Channel` instead of a context reference. + --> tests/fail/wrong-context.rs:38:36 + | +38 | fn task_function_context_ref(_cx: &FunctionContext) {} + | ^^^^^^^^^^^^^^^ diff --git a/test/ui/tests/pass/context-and-this.rs b/test/ui/tests/pass/context-and-this.rs new file mode 100644 index 000000000..1697bd3be --- /dev/null +++ b/test/ui/tests/pass/context-and-this.rs @@ -0,0 +1,134 @@ +use std::future::Future; + +use neon::{ + context::{Context, Cx, FunctionContext}, + event::Channel, + handle::Handle, + types::{extract::Boxed, JsString}, +}; + +type Ch = Channel; +type FnCx<'cx> = FunctionContext<'cx>; + +#[neon::export] +fn sync_nothing() {} + +#[neon::export] +fn sync_function_context(_cx: &mut FunctionContext) {} + +#[neon::export] +fn sync_cx(_cx: &mut Cx) {} + +#[neon::export(context)] +fn sync_cx_forced(_cx: &mut FnCx) {} + +#[neon::export] +fn sync_cx_lifetimes<'cx>(cx: &mut Cx<'cx>) -> Handle<'cx, JsString> { + cx.string("Hello, World!") +} + +#[neon::export] +fn sync_this(this: Vec) { + let _ = this; +} + +#[neon::export(this)] +fn sync_this_forced(_this: Vec) {} + +#[neon::export] +fn sync_cx_and_this(_cx: &mut Cx, this: Vec) { + let _ = this; +} + +#[neon::export] +fn sync_cx_and_this_and_args(_cx: &mut Cx, this: Vec, _a: String) { + let _ = this; +} + +#[neon::export] +fn boxed_this(Boxed(this): Boxed) { + let _ = this; +} + +#[neon::export] +async fn async_nothing() {} + +#[neon::export] +async fn async_channel(_ch: Channel) {} + +#[neon::export(context)] +async fn async_channel_forced(_ch: Ch) {} + +#[neon::export] +async fn async_channel_and_arg(_ch: Channel, _a: String) {} + +#[neon::export] +async fn async_no_channel(_a: String) {} + +#[neon::export] +async fn async_this(this: Vec) { + let _ = this; +} + +#[neon::export(this)] +async fn async_this_forced(_this: Vec) {} + +#[neon::export] +async fn async_this_args(this: Vec, _a: String) { + let _ = this; +} + +#[neon::export] +async fn async_this_and_channel(_ch: Channel, this: Vec) { + let _ = this; +} + +#[neon::export] +async fn async_this_and_channel_args(_ch: Channel, this: Vec, _a: String, _b: String) { + let _ = this; +} + +#[neon::export(task)] +fn task_nothing() {} + +#[neon::export(task)] +fn task_channel(_ch: Channel) {} + +#[neon::export(context, task)] +fn task_channel_forced(_ch: Ch) {} + +#[neon::export(task)] +fn task_channel_and_arg(_ch: Channel, _a: String) {} + +#[neon::export(task)] +fn task_no_channel(_a: String) {} + +#[neon::export(task)] +fn task_this(this: Vec) { + let _ = this; +} + +#[neon::export(task, this)] +fn task_this_forced(_this: Vec) {} + +#[neon::export(task)] +fn task_this_args(this: Vec, _a: String) { + let _ = this; +} + +#[neon::export(task)] +fn task_this_and_channel(_ch: Channel, this: Vec) { + let _ = this; +} + +#[neon::export(task)] +fn task_this_and_channel_args(_ch: Channel, this: Vec, _a: String, _b: String) { + let _ = this; +} + +#[neon::export(async)] +fn impl_async_context(_cx: &mut Cx) -> impl Future { + async {} +} + +fn main() {} diff --git a/test/ui/tests/pass/globals.rs b/test/ui/tests/pass/globals.rs new file mode 100644 index 000000000..124d7e1df --- /dev/null +++ b/test/ui/tests/pass/globals.rs @@ -0,0 +1,13 @@ +#[neon::export] +static STATIC_STRING: &str = ""; + +#[neon::export] +const CONST_NUMBER: f64 = 42.0; + +#[neon::export] +static STATIC_ARR: &[f64] = &[42.0]; + +#[neon::export(json)] +static ARR_OF_ARR: &[&[f64]] = &[&[42.0]]; + +fn main() {} diff --git a/test/ui/tests/pass/json.rs b/test/ui/tests/pass/json.rs new file mode 100644 index 000000000..0d963e9d0 --- /dev/null +++ b/test/ui/tests/pass/json.rs @@ -0,0 +1,6 @@ +#[neon::export(json)] +fn wrap_with_json(v: Vec) -> Vec { + v +} + +fn main() {} From 4c77978e6aa3102679ca944b7c392d16b7ad3bb9 Mon Sep 17 00:00:00 2001 From: "K.J. Valencik" Date: Mon, 7 Oct 2024 16:47:15 -0400 Subject: [PATCH 2/2] Add comments to .gitignore --- .gitignore | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index db501ae6c..43948c23d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,15 +1,26 @@ +# Rust target +rls*.log + +# Created by `trybuild` ui tests wip **/*~ + +# Node **/node_modules -**/.DS_Store +npm-debug.log + +# JS build **/build +**/dist +cli/lib +test/cli/lib + +# Neon build **/index.node **/artifacts.json -cli/lib -**/dist pkgs/create-neon/create-neon-test-project pkgs/create-neon/create-neon-manual-test-project -test/cli/lib -npm-debug.log -rls*.log + +# System +**/.DS_Store