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.

Sign up for GitHub

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

Jump to bottom

feat(derive): Support Option when flattening #4350

Merged
merged 5 commits into from Oct 5, 2022
Merged

feat(derive): Support Option when flattening #4350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4f17f99
fix(derive): Better flatten type errors
epage Oct 5, 2022
f121a8b
fix(derive): Better subcommand type errors for Vec
epage Oct 5, 2022
33db640
refactor(derive): Make flatten's update prepped for Option
epage Oct 5, 2022
06d2049
feat(derive): Support 'Option' when flattening
epage Oct 5, 2022
78676f5
fix(derive): Make mismatched behavior more obvious
epage Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
87 changes: 76 additions & 11 deletions clap_derive/src/derives/args.rs
View file
Open in desktop
Expand Up @@ -202,23 +202,27 @@ pub fn gen_augment(
#implicit_methods;
})
}
Kind::Flatten => {
let ty = &field.ty;
Kind::Flatten(ty) => {
let inner_type = match (**ty, sub_type(&field.ty)) {
(Ty::Option, Some(sub_type)) => sub_type,
_ => &field.ty,
};

let next_help_heading = item.next_help_heading();
let next_display_order = item.next_display_order();
if override_required {
Some(quote_spanned! { kind.span()=>
let #app_var = #app_var
#next_help_heading
#next_display_order;
let #app_var = <#ty as clap::Args>::augment_args_for_update(#app_var);
let #app_var = <#inner_type as clap::Args>::augment_args_for_update(#app_var);
})
} else {
Some(quote_spanned! { kind.span()=>
let #app_var = #app_var
#next_help_heading
#next_display_order;
let #app_var = <#ty as clap::Args>::augment_args(#app_var);
let #app_var = <#inner_type as clap::Args>::augment_args(#app_var);
})
}
}
Expand Down Expand Up @@ -350,7 +354,7 @@ pub fn gen_augment(
.iter()
.filter(|(_field, item)| {
let kind = item.kind();
matches!(*kind, Kind::Flatten)
matches!(*kind, Kind::Flatten(_))
})
.count();
if 0 < possible_group_members_len {
Expand Down Expand Up @@ -410,14 +414,14 @@ pub fn gen_constructor(fields: &[(&Field, Item)]) -> TokenStream {
}
}
},
Ty::Vec |
Ty::Other => {
quote_spanned! { kind.span()=>
#field_name: {
<#subcmd_type as clap::FromArgMatches>::from_arg_matches_mut(#arg_matches)?
}
}
},
Ty::Vec |
Ty::OptionOption |
Ty::OptionVec => {
abort!(
Expand All @@ -429,8 +433,42 @@ pub fn gen_constructor(fields: &[(&Field, Item)]) -> TokenStream {
}
}

Kind::Flatten => quote_spanned! { kind.span()=>
#field_name: clap::FromArgMatches::from_arg_matches_mut(#arg_matches)?
Kind::Flatten(ty) => {
let inner_type = match (**ty, sub_type(&field.ty)) {
(Ty::Option, Some(sub_type)) => sub_type,
_ => &field.ty,
};
match **ty {
Ty::Other => {
quote_spanned! { kind.span()=>
#field_name: <#inner_type as clap::FromArgMatches>::from_arg_matches_mut(#arg_matches)?
}
},
Ty::Option => {
quote_spanned! { kind.span()=>
#field_name: {
let group_id = <#inner_type as clap::Args>::group_id()
.expect("`#[arg(flatten)]`ed field type implements `Args::group_id`");
if #arg_matches.contains_id(group_id.as_str()) {
Some(
<#inner_type as clap::FromArgMatches>::from_arg_matches_mut(#arg_matches)?
)
} else {
None
}
}
}
},
Ty::Vec |
Ty::OptionOption |
Ty::OptionVec => {
abort!(
ty.span(),
"{} types are not supported for flatten",
ty.as_str()
);
}
}
},

Kind::Skip(val, _) => match val {
Expand Down Expand Up @@ -506,9 +544,36 @@ pub fn gen_updater(fields: &[(&Field, Item)], use_self: bool) -> TokenStream {
}
}

Kind::Flatten => quote_spanned! { kind.span()=> {
#access
clap::FromArgMatches::update_from_arg_matches_mut(#field_name, #arg_matches)?;
Kind::Flatten(ty) => {
let inner_type = match (**ty, sub_type(&field.ty)) {
(Ty::Option, Some(sub_type)) => sub_type,
_ => &field.ty,
};

let updater = quote_spanned! { ty.span()=>
<#inner_type as clap::FromArgMatches>::update_from_arg_matches_mut(#field_name, #arg_matches)?;
};

let updater = match **ty {
Ty::Option => quote_spanned! { kind.span()=>
if let Some(#field_name) = #field_name.as_mut() {
#updater
} else {
*#field_name = Some(<#inner_type as clap::FromArgMatches>::from_arg_matches_mut(
#arg_matches
)?);
}
},
_ => quote_spanned! { kind.span()=>
#updater
},
};

quote_spanned! { kind.span()=>
{
#access
#updater
}
}
},

Expand Down
8 changes: 4 additions & 4 deletions clap_derive/src/derives/subcommand.rs
View file
Open in desktop
Expand Up @@ -173,7 +173,7 @@ fn gen_augment(
Some(subcommand)
}

Kind::Flatten => match variant.fields {
Kind::Flatten(_) => match variant.fields {
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
let deprecations = if !override_required {
Expand Down Expand Up @@ -363,7 +363,7 @@ fn gen_has_subcommand(variants: &[(&Variant, Item)]) -> TokenStream {
})
.partition(|(_, item)| {
let kind = item.kind();
matches!(&*kind, Kind::Flatten)
matches!(&*kind, Kind::Flatten(_))
});

let subcommands = variants.iter().map(|(_variant, item)| {
Expand Down Expand Up @@ -464,7 +464,7 @@ fn gen_from_arg_matches(variants: &[(&Variant, Item)]) -> TokenStream {
})
.partition(|(_, item)| {
let kind = item.kind();
matches!(&*kind, Kind::Flatten)
matches!(&*kind, Kind::Flatten(_))
});

let subcommands = variants.iter().map(|(variant, item)| {
Expand Down Expand Up @@ -571,7 +571,7 @@ fn gen_update_from_arg_matches(variants: &[(&Variant, Item)]) -> TokenStream {
})
.partition(|(_, item)| {
let kind = item.kind();
matches!(&*kind, Kind::Flatten)
matches!(&*kind, Kind::Flatten(_))
});

let subcommands = variants.iter().map(|(variant, item)| {
Expand Down
31 changes: 19 additions & 12 deletions clap_derive/src/item.rs
View file
Open in desktop
Expand Up @@ -148,7 +148,7 @@ impl Item {
}

match &*res.kind {
Kind::Flatten => {
Kind::Flatten(_) => {
if res.has_explicit_methods() {
abort!(
res.kind.span(),
Expand Down Expand Up @@ -224,7 +224,7 @@ impl Item {
}

match &*res.kind {
Kind::Flatten => {
Kind::Flatten(_) => {
if res.has_explicit_methods() {
abort!(
res.kind.span(),
Expand Down Expand Up @@ -383,7 +383,12 @@ impl Item {
let expr = attr.value_or_abort();
abort!(expr, "attribute `{}` does not accept a value", attr.name);
}
let kind = Sp::new(Kind::Flatten, attr.name.clone().span());
let ty = self
.kind()
.ty()
.cloned()
.unwrap_or_else(|| Sp::new(Ty::Other, self.kind.span()));
let kind = Sp::new(Kind::Flatten(ty), attr.name.clone().span());
Some(kind)
}
Some(MagicAttrName::Skip) if actual_attr_kind != AttrKind::Group => {
Expand Down Expand Up @@ -902,10 +907,10 @@ impl Item {
match (self.kind.get(), kind.get()) {
(Kind::Arg(_), Kind::FromGlobal(_))
| (Kind::Arg(_), Kind::Subcommand(_))
| (Kind::Arg(_), Kind::Flatten)
| (Kind::Arg(_), Kind::Flatten(_))
| (Kind::Arg(_), Kind::Skip(_, _))
| (Kind::Command(_), Kind::Subcommand(_))
| (Kind::Command(_), Kind::Flatten)
| (Kind::Command(_), Kind::Flatten(_))
| (Kind::Command(_), Kind::Skip(_, _))
| (Kind::Command(_), Kind::ExternalSubcommand)
| (Kind::Value, Kind::Skip(_, _)) => {
Expand Down Expand Up @@ -1142,7 +1147,7 @@ pub enum Kind {
Value,
FromGlobal(Sp<Ty>),
Subcommand(Sp<Ty>),
Flatten,
Flatten(Sp<Ty>),
Skip(Option<AttrValue>, AttrKind),
ExternalSubcommand,
}
Expand All @@ -1155,7 +1160,7 @@ impl Kind {
Self::Value => "value",
Self::FromGlobal(_) => "from_global",
Self::Subcommand(_) => "subcommand",
Self::Flatten => "flatten",
Self::Flatten(_) => "flatten",
Self::Skip(_, _) => "skip",
Self::ExternalSubcommand => "external_subcommand",
}
Expand All @@ -1168,18 +1173,20 @@ impl Kind {
Self::Value => AttrKind::Value,
Self::FromGlobal(_) => AttrKind::Arg,
Self::Subcommand(_) => AttrKind::Command,
Self::Flatten => AttrKind::Command,
Self::Flatten(_) => AttrKind::Command,
Self::Skip(_, kind) => *kind,
Self::ExternalSubcommand => AttrKind::Command,
}
}

pub fn ty(&self) -> Option<&Sp<Ty>> {
match self {
Self::Arg(ty) | Self::Command(ty) | Self::FromGlobal(ty) | Self::Subcommand(ty) => {
Some(ty)
}
Self::Value | Self::Flatten | Self::Skip(_, _) | Self::ExternalSubcommand => None,
Self::Arg(ty)
| Self::Command(ty)
| Self::Flatten(ty)
| Self::FromGlobal(ty)
| Self::Subcommand(ty) => Some(ty),
Self::Value | Self::Skip(_, _) | Self::ExternalSubcommand => None,
}
}
}
Expand Down
43 changes: 43 additions & 0 deletions tests/derive/flatten.rs
View file
Open in desktop
Expand Up @@ -255,3 +255,46 @@ fn docstrings_ordering_with_multiple_clap_partial() {

assert!(short_help.contains("This is the docstring for Flattened"));
}

#[test]
fn optional_flatten() {
#[derive(Parser, Debug, PartialEq, Eq)]
struct Opt {
#[command(flatten)]
source: Option<Source>,
}

#[derive(clap::Args, Debug, PartialEq, Eq)]
struct Source {
crates: Vec<String>,
#[arg(long)]
path: Option<std::path::PathBuf>,
#[arg(long)]
git: Option<String>,
}

assert_eq!(
Opt { source: None },
Opt::try_parse_from(&["test"]).unwrap()
);
assert_eq!(
Opt {
source: Some(Source {
crates: vec!["serde".to_owned()],
path: None,
git: None,
}),
},
Opt::try_parse_from(&["test", "serde"]).unwrap()
);
assert_eq!(
Opt {
source: Some(Source {
crates: Vec::new(),
path: Some("./".into()),
git: None,
}),
},
Opt::try_parse_from(&["test", "--path=./"]).unwrap()
);
}