Upgrade syn to v2

[jonasbb]

Hi, syn just got a new v2 release and I wanted to see how much work it would be to make this crate compatible with it.

I got it to compile and pass the tests in darling.

Please don't hesitate to close or edit this PR in any way you see fit. The main changes I did were:

1. syn::spanned::Spanned is now a sealed trait and cannot be implemented directly. This only affects Flag and SpannedValue. The only way now is to use the blanket implementation via quote::ToTokens.
2. Attributes no longer need parse_meta(), instead they have a meta field now.
3. The NestedMeta type got removed. Since this crate relies so heavily on it, I created a replacement in darling::ast::NestedMeta and replaced all occurrences of syn::NestedMeta with it.
4. Some types got renamed and some enums became non-exhaustive.
   • LifetimeDef and functions are renamed to LifetimeParam to follow syn.

I just wanted to share the progress. Anyone should feel welcome to finish the code.

[jonasbb]

I think the failures I see with serde_with are because of syn, not darling. Namely this issue: dtolnay/syn#1408.
as is a keyword, so it fails to parse the Path, since a Path contains multiple Idents.

This PR should then be mostly ready, besides some small cleanup.

[TedDriggs]

Thanks for starting on this - I'm going to need to dig into the changes more deeply over the coming week.

[TedDriggs]

Does this test as-written even make sense any more? It was guarding against attributes that can't be expressed as Meta, but that seems to have largely gone away?

[Pat-Lafon]

Hello, saw this pr as I'm also looking to upgrade my projects dependencies on syn. I believe some of the docs need to be updated? For example, the README.md uses AttributeArgs which was removed in syn's 2.0 release.

[jonasbb]

Thanks for the information. I updated the example in the README. Right now, I am not sure this PR is on the right track, given the comments about the non-exhaustive enums and NestedMeta.

[TedDriggs]

@jonasbb I think we're on the right track here - I was traveling last week, and need to get back to this this week.

[TedDriggs]

@jonasbb I'd like your opinion on what to do about impl FromMeta for syn::Expr when it receives a string.

There seem to be two options:

1. Preserve backwards compatibility for crates that consume darling-using macros by parsing the string contents as an expression
2. Avoid complexity for new users of darling (and for darling maintainers) by having syn::Expr treat a string literal no differently from any other literal.

The commit I pushed to the branch implements option 1, and while I think that's fine I'm not enthused about it long-term. This approach creates a weird hole for anyone whose attribute takes an expression where a string literal would be valid input, and there's currently no escape hatch.

[jonasbb]

If I understand the commit correctly, it is about supporting non-string arguments in attributes. If so, it is related to #96 and #216? Darling only supported string arguments, so far. Being backwards compatible with that is important I think. This functionality probably doesn't have to go into FromMeta::from_expr directly, but could be included as new functions, extension traits or in the derive macros.

Maybe something like this as a new function.

impl FromMeta for syn::Expr {
    fn from_str_lit_or_expr(expr: &Expr) -> Result<Self> {
        if let syn::Expr::Lit(expr_lit) = expr {
            if let syn::Lit::Str(s) = &expr_lit.lit {
                return Self::from_string(s);
            }
        }
        Self::from_expr(expr)
    }
    // ...
}

Changes to the derive macros could look something like this. The new from_str likely needs three states. 1) Only accept string literals (current behavior of darling) 2) Accept string literal first and parse it, fallback to keeping the string literal as an expression and 3) never parse the string literal.

#[derive(FromMeta)]
struct SerdeContainerOptions {
    #[darling(rename = "crate", from_str=true)]
    alt_crate_path: Option<Path>,
}

[jonasbb]

If from_value returns Err you could consider returning Ok(expr.clone()) here too.

[TedDriggs]

The idea here is that if we see a literal string we'll always assign it the v1 behavior of parsing the contents? I wish there was a good way to deprecate that behavior, but I don't see a means of emitting a deprecation warning that would show up to the user of the crate that is using darling.

[TedDriggs]

@jonasbb I'm planning to merge and release this tomorrow; if you have any last-second edits or concerns, now's the time to voice them.

[jonasbb]

Maybe increasing the version of syn makes sense already. I don't think this PR uses anything beyond what is in 2.0.0. But syn is already at version 2.0.15 and dtolnay doesn't adhere to semantic versioning, so there might be new features in it.

The code should be good to release to enable syn v2 migrations. But further refactoring around NestedMeta might be good for a next version.

I checked the PR again and I think these are all the breaking changes:

• Replace all occurrences of syn::NestedMeta with darling::ast::NestedMeta.

• Replacement for the deprecated AttributeArgs:
  old:

  parse_macro_input!(args as AttributeArgs);

  new:

  match NestedMeta::parse_meta_list(args) {
      Ok(v) => v,
      Err(e) => { return TokenStream::from(Error::from(e).write_errors()); }
  };

• In GenericParamExt rename LifetimeDef to LifetimeParam.
  as_lifetime_def is renamed to as_lifetime_param.

• Flag and SpannedValue can no longer implement syn::spanned::Spanned.

• Bumped MSRV to 1.56, because of syn.

[TedDriggs]

@jonasbb I just saw your comment about the backwards-compatibility situation; I think your idea of an additional meta option in darling is interesting, though I worry it might be hard to tailor it to only the right field types.

I'm wondering instead if a better approach is to add darling::util::parse_str_content so that people can instead write...

#[derive(FromMeta)]
pub struct Example {
    #[darling(with = "darling::util::parse_str_content")]
    foo: Expr,
}

I need to check whether the generated code would do well with type inference there, but it seems like a good way to allow people to opt into backwards-compatible behavior in cases where ambiguity exists, without needlessly forcing changes to code where the quotation marks don't introduce ambiguity (like Path)

[jonasbb]

@TedDriggs The proposed darling::util::parse_str_content should work. It is a bit more "low level". This could be a problem with type inference or make writing custom deserialization code a bit more complicated. Could, really doesn't have to. I do like the idea of shipping these utility functions directly with darling, even if they are trivial itself. This makes it much easier to adapt and tweak the behavior, at least for all cases that don't have a with attribute already.

[TedDriggs]

@jonasbb let's move that out to its own issue - this branch handles the backwards compatibility case natively, and I think it's probably time to ship this as v0.15.0

[TedDriggs]

Last-second update; this is going to go out as v0.20.0; that way it'll be clearer to people there's been a significant change, and it leaves room for servicing the syn 1 branch