colin-kiegel / rust-derive-builder

derive builder implementation for rust structs
https://colin-kiegel.github.io/rust-derive-builder/
Apache License 2.0
1.32k stars 88 forks source link

Update syn dependency? #292

Closed jtran closed 8 months ago

jtran commented 1 year ago

Hello,

Thank you for making this project.

I was wondering if there was any plan to upgrade syn to v2 anytime soon. My motivation is that I have multiple versions of syn in my dependency tree, and I'm trying to get it down to one to reduce build times.

TedDriggs commented 1 year ago

I expect right now we’d be blocked on TedDriggs/darling#238, given that this crate uses pub to mark field and setter visibility. It doesn’t look like syn will revisit its behavior change on the subject - see dtolnay/syn#1414 - and I’m reluctant to update derive_builder in such a disruptive manner.

TedDriggs commented 9 months ago

I took another look at this, and hit a couple specific issues:

jtran commented 9 months ago

That is unfortunate. Without seeing the branch, it's hard for me to understand the reason behind these things.

But if you're okay with a minor version bump, the rename sounds simple enough to migrate to, and maybe the error message can be improved in a future release.

TedDriggs commented 9 months ago

https://github.com/colin-kiegel/rust-derive-builder/tree/darling-0.20.4

We have to rename type because syn decided to stop allowing keywords as meta-item identifiers. The two leading candidates for the rename would be ty or r#type; of those, I think ty is the substantially better option. Unfortunately, the error message won't be able to tell people about the rename since it'll be a parsing error rather than an unknown field error.

I'm quite stumped on the whole problem with the diagnostic messages; other crates using darling 0.20.x don't have the same behavior.

derive-builder unhelpful diagnostic message
TedDriggs commented 9 months ago

Okay, I figured out the issue with the diagnostic messages by bumping all the crates to the 2018 edition. Now things seem to be on a better path; I think I need to do an update of darling to address an issue where the Span for darling::util::Flag is not accessible, but that's a small task.

TedDriggs commented 9 months ago

Okay, it looks like all the tests on the branch linked above are passing again, so as long as all the various no_std and no_alloc test crates are properly running in CI we should be good on that front.

TedDriggs commented 9 months ago

I've created #306 as an option for giving people a source-compatible migration path. That updates the syn 1 version of the crate to accept field(ty = "...") or field(type = "..."). The idea is that people can update to that, make the change to ty, verify everything still works, and then won't need to make any source code changes when they do the second update to the version of derive_builder that uses syn 2.

@jtran can you have a look at that PR and see if there are any issues?

TedDriggs commented 8 months ago

@jtran PR for this is now up, if you can give it a look that would be much appreciated.

TedDriggs commented 8 months ago

This is now live as 0.20.0