Skip to content

Commit

Permalink
fix(derive): Allow subcommands to directly nest in subcommands
Browse files Browse the repository at this point in the history 
`structopt` originally allowed
```
pub enum Opt {
  Daemon(DaemonCommand),
}

pub enum DaemonCommand {
  Start,
  Stop,
}
```

This was partially broken in clap-rs#1681 where `$ cmd daemon start` works but `cmd daemon`,
panics.  Originally, `structopt` relied on exposing the implementation
details of a derived type by providing a `is_subcommand` option, so we'd
know whether to provide `SubcommandRequiredElseHelp` or not.  This was
removed in clap-rs#1681

Fixes clap-rs#2005
  • Loading branch information
@epage
epage committed Jul 14, 2021
1 parent 281469f commit 61f34a1
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 63 deletions.
109 changes: 107 additions & 2 deletions clap_derive/src/attrs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use proc_macro_error::abort;
use quote::{quote, quote_spanned, ToTokens};
use syn::{
self, ext::IdentExt, spanned::Spanned, Attribute, Expr, Field, Ident, LitStr, MetaNameValue,
Type,
Type, Variant,
};

/// Default casing style for generated arguments.
Expand Down Expand Up @@ -452,10 +452,115 @@ impl Attrs {
match &*res.kind {
Kind::Subcommand(_) => abort!(res.kind.span(), "subcommand is only allowed on fields"),
Kind::Skip(_) => abort!(res.kind.span(), "skip is only allowed on fields"),
Kind::Arg(_) | Kind::FromGlobal(_) | Kind::Flatten | Kind::ExternalSubcommand => res,
Kind::Arg(_) => res,
Kind::FromGlobal(_) => abort!(res.kind.span(), "from_global is only allowed on fields"),
Kind::Flatten => abort!(res.kind.span(), "flatten is only allowed on fields"),
Kind::ExternalSubcommand => abort!(
res.kind.span(),
"external_subcommand is only allowed on fields"
),
}
}

pub fn from_variant(
variant: &Variant,
struct_casing: Sp<CasingStyle>,
env_casing: Sp<CasingStyle>,
) -> Self {
let name = variant.ident.clone();
let mut res = Self::new(
variant.span(),
Name::Derived(name),
None,
struct_casing,
env_casing,
);
res.push_attrs(&variant.attrs);
res.push_doc_comment(&variant.attrs, "about");

match &*res.kind {
Kind::Flatten => {
if res.has_custom_parser {
abort!(
res.parser.span(),
"parse attribute is not allowed for flattened entry"
);
}
if res.has_explicit_methods() {
abort!(
res.kind.span(),
"methods are not allowed for flattened entry"
);
}

// ignore doc comments
res.doc_comment = vec![];
}

Kind::ExternalSubcommand => (),

Kind::Subcommand(_) => {
if res.has_custom_parser {
abort!(
res.parser.span(),
"parse attribute is not allowed for subcommand"
);
}
if res.has_explicit_methods() {
abort!(
res.kind.span(),
"methods in attributes are not allowed for subcommand"
);
}
use syn::Fields::*;
use syn::FieldsUnnamed;

let field_ty = match variant.fields {
Named(_) => {
abort!(variant.span(), "structs are not allowed for subcommand");
}
Unit => abort!(variant.span(), "unit-type is not allowed for subcommand"),
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
&unnamed[0].ty
}
Unnamed(..) => {
abort!(
variant,
"non single-typed tuple is not allowed for subcommand"
)
}
};
let ty = Ty::from_syn_ty(field_ty);
match *ty {
Ty::OptionOption => {
abort!(
field_ty,
"Option<Option<T>> type is not allowed for subcommand"
);
}
Ty::OptionVec => {
abort!(
field_ty,
"Option<Vec<T>> type is not allowed for subcommand"
);
}
_ => (),
}

res.kind = Sp::new(Kind::Subcommand(ty), res.kind.span());
}
Kind::Skip(_) => {
abort!(res.kind.span(), "skip is not supported on variants");
}
Kind::FromGlobal(_) => {
abort!(res.kind.span(), "from_global is not supported on variants");
}
Kind::Arg(_) => (),
}

res
}

pub fn from_field(
field: &Field,
struct_casing: Sp<CasingStyle>,
Expand Down
51 changes: 39 additions & 12 deletions clap_derive/src/derives/subcommand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,8 @@ fn gen_augment(
let subcommands: Vec<_> = variants
.iter()
.map(|variant| {
let attrs = Attrs::from_struct(
variant.span(),
&variant.attrs,
Name::Derived(variant.ident.clone()),
let attrs = Attrs::from_variant(
variant,
parent_attribute.casing(),
parent_attribute.env_casing(),
);
Expand All @@ -134,6 +132,39 @@ fn gen_augment(
),
},

Kind::Subcommand(_) => {
let app_var = Ident::new("subcommand", Span::call_site());
let arg_block = match variant.fields {
Named(_) => {
abort!(variant, "non single-typed tuple enums are not supported")
}
Unit => quote!( #app_var ),
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
quote_spanned! { ty.span()=>
{
<#ty as clap::Subcommand>::augment_subcommands(#app_var)
}
}
}
Unnamed(..) => {
abort!(variant, "non single-typed tuple enums are not supported")
}
};

let name = attrs.cased_name();
let from_attrs = attrs.top_level_methods();
let version = attrs.version();
quote! {
let app = app.subcommand({
let #app_var = clap::App::new(#name);
let #app_var = #arg_block;
let #app_var = #app_var.setting(::clap::AppSettings::SubcommandRequiredElseHelp);
#app_var#from_attrs#version
});
}
}

_ => {
let app_var = Ident::new("subcommand", Span::call_site());
let arg_block = match variant.fields {
Expand Down Expand Up @@ -190,10 +221,8 @@ fn gen_from_arg_matches(
let (flatten_variants, variants): (Vec<_>, Vec<_>) = variants
.iter()
.filter_map(|variant| {
let attrs = Attrs::from_struct(
variant.span(),
&variant.attrs,
Name::Derived(variant.ident.clone()),
let attrs = Attrs::from_variant(
variant,
parent_attribute.casing(),
parent_attribute.env_casing(),
);
Expand Down Expand Up @@ -335,10 +364,8 @@ fn gen_update_from_args(
let (flatten_variants, variants): (Vec<_>, Vec<_>) = variants
.iter()
.filter_map(|variant| {
let attrs = Attrs::from_struct(
variant.span(),
&variant.attrs,
Name::Derived(variant.ident.clone()),
let attrs = Attrs::from_variant(
variant,
parent_attribute.casing(),
parent_attribute.env_casing(),
);
Expand Down
72 changes: 23 additions & 49 deletions clap_derive/tests/subcommands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,52 +290,26 @@ fn external_subcommand_optional() {
assert_eq!(Opt::try_parse_from(&["test"]).unwrap(), Opt { sub: None });
}

// #[test]
// #[ignore] // FIXME (@CreepySkeleton)
// fn enum_in_enum_subsubcommand() {
// #[derive(Clap, Debug, PartialEq)]
// pub enum Opt {
// Daemon(DaemonCommand),
// }

// #[derive(Clap, Debug, PartialEq)]
// pub enum DaemonCommand {
// Start,
// Stop,
// }

// let result = Opt::try_parse_from(&["test"]);
// assert!(result.is_err());

// let result = Opt::try_parse_from(&["test", "daemon"]);
// assert!(result.is_err());

// let result = Opt::parse_from(&["test", "daemon", "start"]);
// assert_eq!(Opt::Daemon(DaemonCommand::Start), result);
// }

// WTF??? It tests that `flatten` is essentially a synonym
// for `subcommand`. It's not documented and I doubt it was supposed to
// work this way.
// #[test]
// #[ignore]
// fn flatten_enum() {
// #[derive(Clap, Debug, PartialEq)]
// struct Opt {
// #[clap(flatten)]
// sub_cmd: SubCmd,
// }
// #[derive(Clap, Debug, PartialEq)]
// enum SubCmd {
// Foo,
// Bar,
// }

// assert!(Opt::try_parse_from(&["test"]).is_err());
// assert_eq!(
// Opt::parse_from(&["test", "foo"]),
// Opt {
// sub_cmd: SubCmd::Foo
// }
// );
// }
#[test]
fn enum_in_enum_subsubcommand() {
#[derive(Clap, Debug, PartialEq)]
pub enum Opt {
#[clap(subcommand)]
Daemon(DaemonCommand),
}

#[derive(Clap, Debug, PartialEq)]
pub enum DaemonCommand {
Start,
Stop,
}

let result = Opt::try_parse_from(&["test"]);
assert!(result.is_err());

let result = Opt::try_parse_from(&["test", "daemon"]);
assert!(result.is_err());

let result = Opt::parse_from(&["test", "daemon", "start"]);
assert_eq!(Opt::Daemon(DaemonCommand::Start), result);
}

0 comments on commit 61f34a1

Please sign in to comment.