Span-of-buffer and better error messages at eof

[dhardy]

So, if a proc-macro expects some input (in this case a trailing ;) which is missing at the end of input, this can result in bad error messages:

error: expected `;`
   --> crates/kas-widgets/src/label.rs:189:1
    |
189 | / impl_scope! {
190 | |     /// A label supporting an accelerator key
191 | |     ///
192 | |     /// Accelerator keys are not useful on plain labels. To be useful, a parent
...   |
304 | |     }
305 | | }
    | |_^

Um... where exactly is that ; missing from? (Hint: not the end of the whole input. The relevant line isn't even shown.)

Why? Because parsing a token group such as { .. }works by constructing a new ParseBuffer over that group's content. Generating an error message invokes ParseBuffer::span (and thus Cursor::span), which must return something when the buffer is empty, and so returns Span::call_site().

So how could we solve this?

I first looked at a specific case where this error is omitted: an input group (or "item") is expected to be terminated with ;. Idea: I can highlight the whole item, which is part of the current ParseBuffer. To do this, I would want fn ParseBuffer::span (or another method) to return the span of the whole buffer, not just the next token. This functionality is missing (though possible to emulate by joining spans, as done in Tokio here).

More general solutions may be possible without external changes (i.e. only changing syn, probably changes may be considered API breaking (more specifically, behaviour of existing methods changes)):

• Make Cursor::span at eof return an empty span at the end of the buffer. (Are empty spans supported? Could they be?)
• Make ParseBuffer::span at eof return the span of the next token. Problem: the buffer does not know about the next token. Solution: add the next token's span to the ParseBuffer as a field (somehow), added at construction time. Probably at the end of the whole macro's input this will still need to resolve to Span::call_site (or an empty span at the end).

[dtolnay]

Syn already behaves pretty much exactly how you described. See below for demo.

I skimmed through your macro and got a little lost, but I suspect the behavior you are seeing is just your macro being poorly behaved — is it converting #[widget{ derive = self.0 }] (where the {} is correctly spanned with the incoming span) into some other representation that throws out the {} tokens entirely?

// [dependencies]
// syn = "1"

use proc_macro::TokenStream;
use syn::parse::{Parse, ParseStream, Result};
use syn::{braced, bracketed, parse_macro_input, DeriveInput, LitInt, Token};

mod kw {
    syn::custom_keyword!(derive);
    syn::custom_keyword!(widget);
}

#[proc_macro]
pub fn demo1(input: TokenStream) -> TokenStream {
    parse_macro_input!(input as DeriveInput)
        .attrs[0]
        .parse_args_with(|input: ParseStream| {
            input.parse::<kw::derive>()?;
            input.parse::<Token![=]>()?;
            input.parse::<Token![self]>()?;
            input.parse::<Token![.]>()?;
            input.parse::<LitInt>()?;
            input.parse::<Token![;]>()?;
            Ok(())
        })
        .unwrap_err()
        .to_compile_error()
        .into()
}

#[proc_macro]
pub fn demo2(input: TokenStream) -> TokenStream {
    struct Manual;
    impl Parse for Manual {
        fn parse(input: ParseStream) -> Result<Self> {
            input.parse::<Token![#]>()?;
            let bracketed;
            bracketed!(bracketed in input);
            bracketed.parse::<kw::widget>()?;
            let braced;
            braced!(braced in bracketed);
            braced.parse::<kw::derive>()?;
            braced.parse::<Token![=]>()?;
            braced.parse::<Token![self]>()?;
            braced.parse::<Token![.]>()?;
            braced.parse::<LitInt>()?;
            braced.parse::<Token![;]>()?;
            unreachable!()
        }
    }
    parse_macro_input!(input as Manual);
    unreachable!()
}

repro::demo1! {
    #[widget{ derive = self.0 }]
    struct S;
}

repro::demo2! {
    #[widget{ derive = self.0 }]
    struct S;
}

fn main() {}

$ cargo check

error: expected `;`
 --> src/main.rs:2:31
  |
2 |     #[widget{ derive = self.0 }]
  |                               ^

error: expected `;`
 --> src/main.rs:7:31
  |
7 |     #[widget{ derive = self.0 }]
  |                               ^

[dhardy]

is it converting #[widget{ derive = self.0 }] (where the {} is correctly spanned with the incoming span) into some other representation that throws out the {} tokens entirely?

Yes, actually. This was intentional to mirror the behaviour of #[proc_macro_attribute] — although forms like #[doc = "Some doc"] are legal from the point of view of attributes, proc_macro_attribute accepts only #[PATH GROUP] forms then passes only the contents of the GROUP token — the impl can't even detect whether {}, [] or () is used.

I'm not sure whether this design is an oversight, but I copied it for use by impl_scope — see ScopeAttr::apply and parse_attr_group (which should refer to #[proc_macro_attribute]). I guess I should use Attribute::parse_args (_with); I wasn't aware of these methods.

[dhardy]

Thanks for the suggestions; I've modified ScopeAttr::apply to take the Attribute as an argument, allowing usage of Attribute::parse_args.

There's one minor issue: parse_args does not consider #[my_attr] an acceptable alternative to #[my_attr()], which is the case for #[proc_macro_attribute]. Is this a bug (is parse_args supposed to emulate #[proc_macro_attribute])? It would certainly help me if this were the case, and would be a non-breaking change (unless anyone relies on attributes without args being an error, though given that this is undocumented they shouldn't).

[dtolnay]

I prefer the current behavior of parse_args. For attributes that want to support #[my_attr] as a synonym for #[my_attr()], that can be implemented by matching on Meta.

let attr: &Attribute;

match &attr.meta {
    Meta::Path(_) => Default::default(),  // or syn::parse2(TokenStream::new())?
    Meta::List(meta) => meta.parse_args()?,
    Meta::NameValue(meta) => {
        return Err(Error::new_spanned(meta, "expected #[my_attr] or #[my_attr(...)]"));
    }
}