Keywords in meta path positions cause parsing errors

[TedDriggs]

On syn 2.0.16, the following code produces an error:

let _m: syn::Meta = parse_quote!(type = "hi");

This has caused issues for darling users (TedDriggs/darling#238) who are consuming meta items with names such as type or pub.

I think the issue is here where the impl Parse for Meta, impl Parse for MetaList and impl Parse for MetaNameValue all call out to Path::parse_mod_style. Could there be a similar function that for parsing paths in this Meta context uses Ident::peek_any so that keywords are allowed?

[dtolnay]

I think this is behaving correctly. syn::Meta corresponds to Rust's "meta item" syntax (MetaItem in the reference) which does not allow keyword identifiers.

macro_rules! meta {
    ($m:meta) => {};
}

meta!(type = "hi");

error: expected identifier, found keyword `type`
 --> src/lib.rs:5:7
  |
5 | meta!(type = "hi");
  |       ^^^^ expected identifier, found keyword

[TedDriggs]

The inconsistency noted here, where the meta parses fine if it's meta!(hello(type = "world")) is confusing; would you be open to Meta exposing a parse_nested function that allowed keywords in the path position?

[dtolnay]

would you be open to Meta exposing a parse_nested function that allowed keywords in the path position?

I would prefer not to add this. In macro_rules, the only intended use for $:meta is as #[$m:meta]. syn::Meta serves the same role. In general syn::Meta isn't the right data structure for representing anything different from that intended use case, which is top-level attribute content, same as $:meta.

Whether keys can be a keyword is not the only consideration. There is also the fact that Meta::NameValue has a rhs that is required to be an expression. In Rust syntax, #[foo(t = Vec<u8>)] is a syntactically valid attribute but #[t = Vec<u8>] is not. I've found using Meta to represent the content in the parenthesized part of a Meta::List is an antipattern: it leads to things that are not expressions needing to be handled as string literals instead, which is problematic when one needs macro-generated attributes such as #[foo(t = Vec<$T>)], and also problematic for IDE functionality like go-to-definition as each token parsed from the string literal would end up carrying the Span of the entire string literal.

[TedDriggs]

Thanks for the explanation on this; it now makes more sense why NestedMeta got removed in syn v2 - and why attempting to recreate it to maintain compatibility in darling felt awkward.

For the darling::FromMeta use-case, I want that to take an input type that represents the items of a Meta::List. To my knowledge, there are three variants that make sense there:

1. Path, e.g. the First in #[example(derive(First))].
2. NameValue, e.g. the t = Vec<T> in #[example(t = Vec<T>)]
3. List, e.g. the thing(...) in #[example(thing(t = Vec<T>, derive(First))]

Aside: In the paragraph that follows, I'm going to use darling::Meta to refer to a thing that you used to call a NestedMeta. I apologize for any confusion, but since darling's main trait is called FromMeta, I think I'm stuck using the term to mean the nested thing in the context of the darling library.

It'd be a pretty rough breaking change for darling, but if you're willing to lend a hand on writing the Parse impl for it, I'd be willing to introduce darling::Meta to specifically represent the contents inside the parentheses so that I'm not misusing the syn::Meta type across the darling API and all the impls it generates. That would allow using keywords as idents of these nested items, and - if I'm reading your message correctly - would be your recommended long-term supportable approach?

I think it'd be really interesting to make this generic over what's allowed in the RHS position of NameValue, e.g. darling::Meta<RHS = syn::Expr> - that would make it possible to use the type to limit the items that were valid in the value position, while the default value of the type parameter would make for easier compatibility with syn::Meta.

[dtolnay]

I think it is not a matter of writing a smarter Parse impl — parsing an attribute's nested contents into any data structure without regard for what the keys mean is fundamentally problematic. For example, consider what you might do to parse #[foo(return_type = T<K, V>, precondition = A < K, note = "...")]. The meaning of the comma after K is determined by what key is being parsed.

[TedDriggs]

That's a good example.

I'm passionate about preserving the ease-of-use of darling - the first time I added #[derive(Deserialize)] to a struct remains one of my most joyful moments with any programming language, and I want to deliver that experience for proc-macro authors.

Would your recommendation be that darling generates something like syn::Parse impls, taking a ParseStream or a ParseNestedMeta in and returning Result<Self>?

I think I can imagine a lot of darling's current generated FromMeta impls working with this approach - some, like enums, might require lookahead, but that doesn't seem like the end of the world? The breaking change would be quite severe, and the amount of work within the library to support this would be rough, but if that's the long-term right way to do this then it's worth having the discussion about how to execute on it.

[dtolnay]

Deriving a function that operates on ParseNestedMeta would be the winning approach in the long term, I think.

I don't have a complete design with all of the signatures worked out, but following the handwritten ones in colin-kiegel/rust-derive-builder#310 would be a good guide.