Is this an idiomatic enough approach for an attribute macro?

[fickle moth]

Hi there!

I wrote a small attribute macro, whose purpose is:

1. to add some attributes to a struct
2. to implement a trait for the type

My implementation works, but I have a few questions. First, the code:

#[proc_macro_attribute]
pub fn base_actor(args: TokenStream, input: TokenStream) -> TokenStream {
    let mut ast: DeriveInput = syn::parse(input).unwrap();
    let _ = parse_macro_input!(args as parse::Nothing);

    if let Data::Struct(DataStruct {
        fields: Fields::Named(fields),
        ..
    }) = &mut ast.data
    {
        fields.named.push(
            syn::Field::parse_named
                .parse2(quote! { pub myfield: String })
                .unwrap(),
        );
    } else {
        panic!("Unexpected input (missing curly braces?)");
    }

    let name = &ast.ident;

    let gen = quote! {
        #ast

        impl MyTrait for #name {
            fn print_my_field(&self) {
                println!("myfield: {}", self.myfield);
            }
        }
    };

    gen.into()
}

My first question is: is this an idiomatic enough approach (eg. proper usage of the APIs)?

My second question is: how could I split the two additions (struct modification and trait implementation) into two, possibly pure, functions? Ideally, something like this:

let ast = add_field(ast);
let ast = implement_trait(ast);

quote! { #ast }.into()

Implementing add_field() is not a problem, but I don't know how to easily implement implement_trait. I can think of generating the TokenStream (via quote! { /* impl trait... */ }.into()) and re-parsing it (via syn::parse()), but that sound like a waste.

Thanks!

[lofty edge]

So, first of all, nice job using let _ = parse_macro_input!(args as parse::Nothing); this is a very nice way to future-proof against allowing extra args in the future, without having to go through a major version (which would otherwise be required), and yet not many people think of it, so

Regarding API usage, you have figured out and thus used things quite well; the only exception would be the panic!(): as with classic Rust, panic! should only be used for internal logic bugs or some other catastrophic situation: ideally a proc-macro just kindly bails on incorrect input with a nicer error than "proc-macro panicked".

The way to achieve this easily is through syn's Error type:

let err = Error::new(Span::call_site(), "Unexpected input (missing curly braces?)");

which you can convert into a proc_macro2::TokenStream using .into_compile_error():

err.to_compile_error()
// is kind of equivalent to:
quote_spanned!(Span::call_site()=>
    compile_error! { "Unexpected input (missing curly braces?)" }
)

which you can then .into()-convert into the required TokenStream.

This would thus yield:

    } else {
-       panic!("Unexpected input (missing curly braces?)");
+       return Error::new(Span::call_site(), "Unexpected input (missing curly braces?)")
+                  .into_compile_error().into();
    }

Granted, you could have directly quote!d the compile_error! invocation, but the benefit of using Err is that it plays more nicely into your idea of factoring out code

You use ::syn::Result;, which already bundles its own Error as the parameter, and then can have most of your functions return -> Results, and use ? on call sites as with usual Rust. Even the parsing itself can do this:

let mut ast: DeriveInput = ::syn::parse(input)?;
let _: parse::Nothing = ::syn::parse(args)?;

To do this you put 99% of your logic in a -> Result<TokenStream> inner function, and then have the #[proc_macro…] function call that one and unwrap:

fn unwrap (res: Result<TokenStream2>)
  -> TokenStream
{
    res.unwrap_or_else(Error::into_compile_error).into()
}

#[proc_macro_attribute]
pub fn base_actor(args: TokenStream, input: TokenStream) -> TokenStream {
    unwrap(base_actor_impl(args, input))
}

with:

fn base_actor_impl (
    args: impl Into<TokenStream2>,
    input: impl Into<TokenStream2>,
) -> Result<TokenStream2>
{
    let mut ast: DeriveInput = ::syn::parse2(input.into())?;
    let _: parse::Nothing = ::syn::parse2(args.into())?;

    add_field(&mut ast)?;

    let trait_impl = impl_trait(&ast)?;

    Ok(quote!(
        #ast
        #trait_impl
    ))
}

And so:

fn add_field (input: &'_ mut DeriveInput)
  -> Result<()>
{
    if let Data::Struct(DataStruct {
        fields: Fields::Named(fields),
        ..
    }) = &mut ast.data
    {
        fields.named.push(parse_quote!(
            pub myfield: ::std::string::String
        )));
    } else {
        bail!("Unexpected input (missing curly braces?)");
    }
    Ok(())
}

fn impl_trait (input: &'_ DeriveInput)
  -> Result<TokenStream2> // or `Result<ItemImpl>` if you prefer strongly typed (replace `quote!` with `parse_quote!`
{
    let TyName = &input.ident;
    let (intro_generics, forward_generics, maybe_where_clause) =
        input.generics.split_for_impl()
    ;
    Ok(quote!(
        impl #intro_generics
            ::absolute::path::to::MyTrait
        for
            #TyName #forward_generics
        #maybe_where_clause
        {
            fn print_my_field(&self) {
                ::std::println!("my field: {}", self.my_field);
            }
        }
    ))
}

Where I usually defined that helper bail! macro as:

macro_rules! bail {
    ( $msg:expr $(,)? ) => (
        return ::syn::Result::<_>::Err(::syn::Error::new(
            ::proc_macro2::Span::call_site(),
            &$msg,
        ))
    );

    ( $msg:expr => $spanned:expr $(,)? ) => (
        return ::syn::Result::<_>::Err(::syn::Error::new_spanned(
            &$spanned,
            &$msg,
        ))
    );
}

That second rule is very handy to get nicely spanned error messages. Say, for instance, you wanted to reject the struct name being Foo:

if input.ident == "Foo" {
    bail! {
        "Use a more creative name!" => &input.ident,
    }
}

and you'll see that the error message with ^^^-highlight the Foo identifier

So, to summarize:

• Use ::syn::Error and its .into_compile_error() utility instead of panic!; which becomes easy inside -> Result-returning functions, since you could use bail! in there;
• The #[proc_macro…] function should then just "unwrap" a call to a -> Result-yielding function.
• You can use parse_quote! as a shorthand for parse2(quote!(…)).unwrap()
• make sure to fully qualify the paths you use in your expansion: you can't know if println! is really gonna be the macro you expect it to be, or if MyTrait is necessarily gonna be in scope of the caller. For extra robustness, use the leading :: to disambiguate between a crate and an eponymous module or item in scope

[fickle moth]

Wow, that's amazing!!! Thanks!! 🤩