Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(clap_derive): Unwrap syn::TypeGroup when checking field types #1992

Merged
merged 1 commit into from
Jun 30, 2020

Conversation

Aaron1011
Copy link
Contributor

Due to macro expansions, a syn type may be wrapped in multiple
'layers' of syn::Type::Group. However, clap_derive currently does
not check for syn::Type::Group, which will cause an Option (along
with other matched types) to fail to be detected when it results from a
macro expansion.

This commit 'unwraps' outer type groups before checking the
user-provided types against well-known types. Currently, these groups
may not be present due to a rustc bug (rust-lang/rust#43081)

However, once rust-lang/rust#73084 is merged,
these groups will be present in more cases. This commit makes clap
compatible with both older and newer versions of rustc.

Due to macro expansions, a `syn` type may be wrapped in multiple
'layers' of `syn::Type::Group`. However, `clap_derive` currently does
not check for `syn::Type::Group`, which will cause an `Option` (along
with other matched types) to fail to be detected when it results from a
macro expansion.

This commit 'unwraps' outer type groups before checking the
user-provided types against well-known types. Currently, these groups
may not be present due to a rustc bug (rust-lang/rust#43081)

However, once rust-lang/rust#73084 is merged,
these groups will be present in more cases. This commit makes `clap`
compatible with both older and newer versions of rustc.
@CreepySkeleton
Copy link
Contributor

bors r+

Thank you, @Aaron1011 !

@bors
Copy link
Contributor

bors bot commented Jun 30, 2020

Build succeeded:

@bors bors bot merged commit e926c29 into clap-rs:master Jun 30, 2020
@Aaron1011
Copy link
Contributor Author

@CreepySkeleton: Could you make a new point release? That will make it easy for any crates broken by rust-lang/rust#73084 to keep compiling.

@pksunkara
Copy link
Member

This branch is still in beta, we can't do a patch release.

@CreepySkeleton
Copy link
Contributor

@pksunkara Why exactly we can't have a new beta.2 release?

@pksunkara
Copy link
Member

clishe is the only crate that was broken and it's using beta.1. We should ideally do an rc release instead of beta.2 since we still have time before the Rust PR lands into stable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants