Use error instead of panic! in extendr-macros (extendr#634)

* Use error instead of panic! in extendr-macros

* Derive macros

* Support impl

extendr-macros/Cargo.toml

@@ -19,3 +19,4 @@ proc-macro2 = { version = "1.0" }
 extendr-api = { path = "../extendr-api" }
 extendr-engine = { path = "../extendr-engine" }
 libR-sys = { workspace = true }
+trybuild = "1.0"

extendr-macros/src/extendr_function.rs

@@ -6,7 +6,12 @@ use syn::{meta::ParseNestedMeta, ItemFn, Lit, LitBool};
 /// Generate bindings for a single function.
 pub fn extendr_function(mut func: ItemFn, opts: &wrappers::ExtendrOptions) -> TokenStream {
     let mut wrappers: Vec<ItemFn> = Vec::new();
-    wrappers::make_function_wrappers(opts, &mut wrappers, "", &func.attrs, &mut func.sig, None);
+
+    let res =
+        wrappers::make_function_wrappers(opts, &mut wrappers, "", &func.attrs, &mut func.sig, None);
+    if let Err(e) = res {
+        return e.into_compile_error().into();
+    };
 
     TokenStream::from(quote! {
         #func
@@ -25,45 +30,46 @@ impl wrappers::ExtendrOptions {
     /// - `use_rng = bool` ensures the RNG-state is pulled and pushed
     ///
     pub fn parse(&mut self, meta: ParseNestedMeta) -> syn::parse::Result<()> {
-        fn help_message() -> ! {
-            panic!("expected #[extendr(use_try_from = bool, r_name = \"name\", mod_name = \"r_mod_name\", use_rng = bool)]");
-        }
-
-        let value = match meta.value() {
-            Ok(value) => value,
-            Err(_) => help_message(),
-        };
+        let value = meta.value()?;
+        let path = meta
+            .path
+            .get_ident()
+            .ok_or(meta.error("Unexpected syntax"))?;
 
-        if meta.path.is_ident("use_try_from") {
-            if let Ok(LitBool { value, .. }) = value.parse() {
-                self.use_try_from = value;
-                Ok(())
-            } else {
-                help_message();
+        match path.to_string().as_str() {
+            "use_try_from" => {
+                if let Ok(LitBool { value, .. }) = value.parse() {
+                    self.use_try_from = value;
+                    Ok(())
+                } else {
+                    Err(value.error("`use_try_from` must be `true` or `false`"))
+                }
             }
-        } else if meta.path.is_ident("r_name") {
-            if let Ok(Lit::Str(litstr)) = value.parse() {
-                self.r_name = Some(litstr.value());
-                Ok(())
-            } else {
-                help_message();
+            "r_name" => {
+                if let Ok(Lit::Str(litstr)) = value.parse() {
+                    self.r_name = Some(litstr.value());
+                    Ok(())
+                } else {
+                    Err(value.error("`r_name` must be a string literal"))
+                }
             }
-        } else if meta.path.is_ident("mod_name") {
-            if let Ok(Lit::Str(litstr)) = value.parse() {
-                self.mod_name = Some(litstr.value());
-                Ok(())
-            } else {
-                help_message();
+            "mod_name" => {
+                if let Ok(Lit::Str(litstr)) = value.parse() {
+                    self.mod_name = Some(litstr.value());
+                    Ok(())
+                } else {
+                    Err(value.error("`mod_name` must be a string literal"))
+                }
             }
-        } else if meta.path.is_ident("use_rng") {
-            if let Ok(LitBool { value, .. }) = value.parse() {
-                self.use_rng = value;
-                Ok(())
-            } else {
-                help_message();
+            "use_rng" => {
+                if let Ok(LitBool { value, .. }) = value.parse() {
+                    self.use_rng = value;
+                    Ok(())
+                } else {
+                    Err(value.error("`use_rng` must be `true` or `false`"))
+                }
             }
-        } else {
-            help_message();
+            _ => Err(syn::Error::new_spanned(meta.path, "Unexpected key")),
         }
     }
 }

extendr-macros/src/extendr_impl.rs

@@ -36,30 +36,45 @@ use crate::wrappers;
 ///     fn aux_func;
 /// }
 /// ```
-pub fn extendr_impl(mut item_impl: ItemImpl) -> TokenStream {
+pub fn extendr_impl(mut item_impl: ItemImpl) -> syn::Result<TokenStream> {
     // Only `impl name { }` allowed
     if item_impl.defaultness.is_some() {
-        return quote! { compile_error!("default not allowed in #[extendr] impl"); }.into();
+        return Err(syn::Error::new_spanned(
+            item_impl,
+            "default not allowed in #[extendr] impl",
+        ));
     }
 
     if item_impl.unsafety.is_some() {
-        return quote! { compile_error!("unsafe not allowed in #[extendr] impl"); }.into();
+        return Err(syn::Error::new_spanned(
+            item_impl,
+            "unsafe not allowed in #[extendr] impl",
+        ));
     }
 
     if item_impl.generics.const_params().count() != 0 {
-        return quote! { compile_error!("const params not allowed in #[extendr] impl"); }.into();
+        return Err(syn::Error::new_spanned(
+            item_impl,
+            "const params not allowed in #[extendr] impl",
+        ));
     }
 
     if item_impl.generics.type_params().count() != 0 {
-        return quote! { compile_error!("type params not allowed in #[extendr] impl"); }.into();
+        return Err(syn::Error::new_spanned(
+            item_impl,
+            "type params not allowed in #[extendr] impl",
+        ));
     }
 
     // if item_impl.generics.lifetimes().count() != 0 {
     //     return quote! { compile_error!("lifetime params not allowed in #[extendr] impl"); }.into();
     // }
 
     if item_impl.generics.where_clause.is_some() {
-        return quote! { compile_error!("where clause not allowed in #[extendr] impl"); }.into();
+        return Err(syn::Error::new_spanned(
+            item_impl,
+            "where clause not allowed in #[extendr] impl",
+        ));
     }
 
     let opts = wrappers::ExtendrOptions::default();
@@ -97,7 +112,7 @@ pub fn extendr_impl(mut item_impl: ItemImpl) -> TokenStream {
                 &method.attrs,
                 &mut method.sig,
                 Some(self_ty),
-            );
+            )?;
         }
     }
 
@@ -187,7 +202,7 @@ pub fn extendr_impl(mut item_impl: ItemImpl) -> TokenStream {
     });
 
     //eprintln!("{}", expanded);
-    expanded
+    Ok(expanded)
 }
 
 // This structure contains parameters parsed from the #[extendr_module] definition.

extendr-macros/src/lib.rs

@@ -80,7 +80,10 @@ pub fn extendr(attr: TokenStream, item: TokenStream) -> TokenStream {
 
     match parse_macro_input!(item as Item) {
         Item::Fn(func) => extendr_function::extendr_function(func, &opts),
-        Item::Impl(item_impl) => extendr_impl::extendr_impl(item_impl),
+        Item::Impl(item_impl) => match extendr_impl::extendr_impl(item_impl) {
+            Ok(result) => result,
+            Err(e) => e.into_compile_error().into(),
+        },
         other_item => TokenStream::from(quote! {#other_item}),
     }
 }
@@ -203,7 +206,10 @@ pub fn Rraw(item: TokenStream) -> TokenStream {
 /// ```
 #[proc_macro_derive(TryFromRobj)]
 pub fn derive_try_from_robj(item: TokenStream) -> TokenStream {
-    list_struct::derive_try_from_robj(item)
+    match list_struct::derive_try_from_robj(item) {
+        Ok(result) => result,
+        Err(e) => e.into_compile_error().into(),
+    }
 }
 
 /// Derives an implementation of `From<Struct> for Robj` and `From<&Struct> for Robj` on this struct.
@@ -241,7 +247,10 @@ pub fn derive_try_from_robj(item: TokenStream) -> TokenStream {
 /// and converting it back to Rust will produce a copy of the original struct.
 #[proc_macro_derive(IntoRobj)]
 pub fn derive_into_robj(item: TokenStream) -> TokenStream {
-    list_struct::derive_into_robj(item)
+    match list_struct::derive_into_robj(item) {
+        Ok(result) => result,
+        Err(e) => e.into_compile_error().into(),
+    }
 }
 
 /// Enable the construction of dataframes from arrays of structures.

extendr-macros/src/list_struct.rs

@@ -1,19 +1,18 @@
 use proc_macro::TokenStream;
 use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
-use syn::{parse_macro_input, Data, DeriveInput};
+use syn::{Data, DeriveInput};
 
 /// Implementation of the TryFromRobj macro. Refer to the documentation there
-pub fn derive_try_from_robj(item: TokenStream) -> TokenStream {
+pub fn derive_try_from_robj(item: TokenStream) -> syn::parse::Result<TokenStream> {
     // Parse the tokens into a Struct
-    let ast: DeriveInput = parse_macro_input!(item as DeriveInput);
-    let struct_name = ast.ident;
-    let inside;
-    if let Data::Struct(inner) = ast.data {
-        inside = inner;
+    let ast = syn::parse::<DeriveInput>(item)?;
+    let inside = if let Data::Struct(inner) = ast.data {
+        inner
     } else {
-        panic!("This is a derive macro, only use it on a struct")
+        return Err(syn::Error::new_spanned(ast, "Only struct is supported"));
     };
+    let struct_name = ast.ident;
 
     // Iterate each struct field and capture a conversion from Robj for each field
     let mut tokens = Vec::<TokenStream2>::with_capacity(inside.fields.len());
@@ -27,7 +26,7 @@ pub fn derive_try_from_robj(item: TokenStream) -> TokenStream {
     }
 
     // Emit the conversion trait impl
-    TokenStream::from(quote!(
+    Ok(TokenStream::from(quote!(
         impl std::convert::TryFrom<&Robj> for #struct_name {
             type Error = extendr_api::Error;
 
@@ -47,20 +46,19 @@ pub fn derive_try_from_robj(item: TokenStream) -> TokenStream {
                 })
             }
         }
-    ))
+    )))
 }
 
 /// Implementation of the IntoRobj macro. Refer to the documentation there
-pub fn derive_into_robj(item: TokenStream) -> TokenStream {
+pub fn derive_into_robj(item: TokenStream) -> syn::parse::Result<TokenStream> {
     // Parse the tokens into a Struct
-    let ast: DeriveInput = parse_macro_input!(item as DeriveInput);
-    let struct_name = ast.ident;
-    let inside;
-    if let Data::Struct(inner) = ast.data {
-        inside = inner;
+    let ast = syn::parse::<DeriveInput>(item)?;
+    let inside = if let Data::Struct(inner) = ast.data {
+        inner
     } else {
-        panic!("This is a derive macro, only use it on a struct")
+        return Err(syn::Error::new_spanned(ast, "Only struct is supported"));
     };
+    let struct_name = ast.ident;
 
     // Iterate each struct field and capture a token that creates a KeyValue pair (tuple) for
     // each field
@@ -75,7 +73,7 @@ pub fn derive_into_robj(item: TokenStream) -> TokenStream {
     }
 
     // The only thing we emit from this macro is the conversion trait impl
-    TokenStream::from(quote!(
+    Ok(TokenStream::from(quote!(
         impl std::convert::From<&#struct_name> for Robj {
             fn from(value: &#struct_name) -> Self {
                 extendr_api::List::from_pairs([#(#tokens),*]).into()
@@ -86,5 +84,5 @@ pub fn derive_into_robj(item: TokenStream) -> TokenStream {
                 extendr_api::List::from_pairs([#(#tokens),*]).into()
             }
         }
-    ))
+    )))
 }