Skip to content

Commit

Permalink
Use proper span when generating matches token
Browse files Browse the repository at this point in the history
In `struct-opt-derive`, a function is generated with a parameter named
`matches`. Since `quote!` is used to generate the function, the
`matches` token will be resolved using `Span::call_site`.

However, the literal identifier `matches` is also used inside several
`quote_spanned!` expressions. Such a `matches` identifier will be
resolved using the `Span` passed to `quote_spanned!`, which may not be
the same as `Span::call_site`.

Currently, this is difficult to observe in practice, due to
rust-lang/rust#43081 . However, once PR rust-lang/rust#73084
is merged, proc macros will see properly spanned tokens in more cases,
which will cause these incorrect uses of `quote_spanned!` to break.

This PR uses `quote! { matches }` to generate a correctly spanned
`matches` token, which is then include in the `quote_spanned!`
expressions using `#matches`.
  • Loading branch information
Aaron1011 authored and CreepySkeleton committed Jun 12, 2020
1 parent f84e751 commit 0a6bc51
Showing 1 changed file with 13 additions and 12 deletions.
25 changes: 13 additions & 12 deletions structopt-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ fn gen_constructor(fields: &Punctuated<Field, Comma>, parent_attribute: &Attrs)
);
let field_name = field.ident.as_ref().unwrap();
let kind = attrs.kind();
let matches = quote! { matches };
match &*kind {
Kind::ExternalSubcommand => abort!(
kind.span(),
Expand All @@ -265,13 +266,13 @@ fn gen_constructor(fields: &Punctuated<Field, Comma>, parent_attribute: &Attrs)
};
quote_spanned! { kind.span()=>
#field_name: <#subcmd_type as ::structopt::StructOptInternal>::from_subcommand(
matches.subcommand())
#matches.subcommand())
#unwrapper
}
}

Kind::Flatten => quote_spanned! { kind.span()=>
#field_name: ::structopt::StructOpt::from_clap(matches)
#field_name: ::structopt::StructOpt::from_clap(#matches)
},

Kind::Skip(val) => match val {
Expand Down Expand Up @@ -318,45 +319,45 @@ fn gen_constructor(fields: &Punctuated<Field, Comma>, parent_attribute: &Attrs)
let occurrences = *attrs.parser().kind == ParserKind::FromOccurrences;
let name = attrs.cased_name();
let field_value = match **ty {
Ty::Bool => quote_spanned!(ty.span()=> matches.is_present(#name)),
Ty::Bool => quote_spanned!(ty.span()=> #matches.is_present(#name)),

Ty::Option => quote_spanned! { ty.span()=>
matches.#value_of(#name)
#matches.#value_of(#name)
.map(#parse)
},

Ty::OptionOption => quote_spanned! { ty.span()=>
if matches.is_present(#name) {
Some(matches.#value_of(#name).map(#parse))
if #matches.is_present(#name) {
Some(#matches.#value_of(#name).map(#parse))
} else {
None
}
},

Ty::OptionVec => quote_spanned! { ty.span()=>
if matches.is_present(#name) {
Some(matches.#values_of(#name)
if #matches.is_present(#name) {
Some(#matches.#values_of(#name)
.map_or_else(Vec::new, |v| v.map(#parse).collect()))
} else {
None
}
},

Ty::Vec => quote_spanned! { ty.span()=>
matches.#values_of(#name)
#matches.#values_of(#name)
.map_or_else(Vec::new, |v| v.map(#parse).collect())
},

Ty::Other if occurrences => quote_spanned! { ty.span()=>
#parse(matches.#value_of(#name))
#parse(#matches.#value_of(#name))
},

Ty::Other if flag => quote_spanned! { ty.span()=>
#parse(matches.is_present(#name))
#parse(#matches.is_present(#name))
},

Ty::Other => quote_spanned! { ty.span()=>
matches.#value_of(#name)
#matches.#value_of(#name)
.map(#parse)
.unwrap()
},
Expand Down

0 comments on commit 0a6bc51

Please sign in to comment.