From fc65d69224d6f845948628ab0cb92a383794bb95 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 10 Aug 2022 17:03:00 -0500 Subject: [PATCH] fix(derive)!: Infer action for help/version This will make it easier for people to override these Fixes #4057 --- clap_derive/src/attrs.rs | 49 +++++++++++++++++++++++++++------ clap_derive/src/derives/args.rs | 12 ++++++++ clap_derive/src/utils/mod.rs | 2 +- clap_derive/src/utils/ty.rs | 13 ++++++++- src/_derive/mod.rs | 19 +++++++------ tests/derive/help.rs | 28 +++++++++++++++++++ 6 files changed, 104 insertions(+), 19 deletions(-) diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs index 0c6824f1873..ed0de5ce29f 100644 --- a/clap_derive/src/attrs.rs +++ b/clap_derive/src/attrs.rs @@ -889,14 +889,14 @@ impl Attrs { .unwrap_or_else(|| { if let Some(value_parser) = self.value_parser.as_ref() { let span = value_parser.span(); - default_action(field_type, span) + default_action(self.name.raw_literal().as_deref(), field_type, span) } else { let span = self .value_parser .as_ref() .map(|a| a.span()) .unwrap_or_else(|| self.kind.span()); - default_action(field_type, span) + default_action(self.name.raw_literal().as_deref(), field_type, span) } }) } @@ -949,14 +949,27 @@ impl ValueParser { } } -fn default_value_parser(inner_type: &Type, span: Span) -> Method { +fn default_value_parser(name: Option<&str>, inner_type: &Type, span: Span) -> Method { let func = Ident::new("value_parser", span); - Method::new( - func, + let call = if crate::utils::is_unit_type(inner_type) { + match name { + Some("help") | Some("version") => { + quote_spanned! { span=> + clap::value_parser!(bool) + } + } + _ => { + quote_spanned! { span=> + clap::value_parser!(String) + } + } + } + } else { quote_spanned! { span=> clap::value_parser!(#inner_type) - }, - ) + } + }; + Method::new(func, call) } #[derive(Clone)] @@ -971,7 +984,7 @@ impl Action { match self { Self::Explicit(method) => method, #[cfg(not(feature = "unstable-v5"))] - Self::Implicit(ident) => default_action(_field_type, ident.span()), + Self::Implicit(ident) => default_action(None, _field_type, ident.span()), } } @@ -984,9 +997,20 @@ impl Action { } } -fn default_action(field_type: &Type, span: Span) -> Method { +fn default_action(name: Option<&str>, field_type: &Type, span: Span) -> Method { let ty = Ty::from_syn_ty(field_type); let args = match *ty { + Ty::Unit => match name { + Some("help") => quote_spanned! { span=> + clap::ArgAction::Help + }, + Some("version") => quote_spanned! { span=> + clap::ArgAction::Version + }, + _ => quote_spanned! { span=> + clap::ArgAction::Set + }, + }, Ty::Vec | Ty::OptionVec => { quote_spanned! { span=> clap::ArgAction::Append @@ -1160,6 +1184,13 @@ impl Name { } } + pub fn raw_literal(&self) -> Option { + match self { + Name::Assigned(_) => None, + Name::Derived(ident) => Some(ident.unraw().to_string()), + } + } + pub fn translate(self, style: CasingStyle) -> TokenStream { use CasingStyle::*; diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs index d5943e61c0d..40c0f3d8d47 100644 --- a/clap_derive/src/derives/args.rs +++ b/clap_derive/src/derives/args.rs @@ -248,6 +248,14 @@ pub fn gen_augment( let value_name = attrs.value_name(); let implicit_methods = match **ty { + Ty::Unit => { + quote_spanned! { ty.span()=> + .value_name(#value_name) + #value_parser + #action + } + } + Ty::Option => { quote_spanned! { ty.span()=> .value_name(#value_name) @@ -504,6 +512,10 @@ fn gen_parsers( let arg_matches = format_ident!("__clap_arg_matches"); let field_value = match **ty { + Ty::Unit => { + quote_spanned! { ty.span()=> () } + } + Ty::Option => { quote_spanned! { ty.span()=> #arg_matches.#get_one(#id) diff --git a/clap_derive/src/utils/mod.rs b/clap_derive/src/utils/mod.rs index 77a467c754e..24c2d491112 100644 --- a/clap_derive/src/utils/mod.rs +++ b/clap_derive/src/utils/mod.rs @@ -5,5 +5,5 @@ mod ty; pub use self::{ doc_comments::process_doc_comment, spanned::Sp, - ty::{inner_type, is_simple_ty, sub_type, subty_if_name, Ty}, + ty::{inner_type, is_simple_ty, is_unit_type, sub_type, subty_if_name, Ty}, }; diff --git a/clap_derive/src/utils/ty.rs b/clap_derive/src/utils/ty.rs index 0bcb59f276b..e6814ad5662 100644 --- a/clap_derive/src/utils/ty.rs +++ b/clap_derive/src/utils/ty.rs @@ -13,6 +13,7 @@ pub enum Ty { Option, OptionOption, OptionVec, + Unit, Other, } @@ -21,7 +22,9 @@ impl Ty { use self::Ty::*; let t = |kind| Sp::new(kind, ty.span()); - if is_generic_ty(ty, "Vec") { + if is_unit_type(ty) { + t(Unit) + } else if is_generic_ty(ty, "Vec") { t(Vec) } else if let Some(subty) = subty_if_name(ty, "Option") { if is_generic_ty(subty, "Option") { @@ -52,6 +55,14 @@ pub fn sub_type(ty: &syn::Type) -> Option<&syn::Type> { subty_if(ty, |_| true) } +pub fn is_unit_type(ty: &syn::Type) -> bool { + if let syn::Type::Tuple(tuple) = ty { + tuple.elems.is_empty() + } else { + false + } +} + fn only_last_segment(mut ty: &syn::Type) -> Option<&PathSegment> { while let syn::Type::Group(syn::TypeGroup { elem, .. }) = ty { ty = elem; diff --git a/src/_derive/mod.rs b/src/_derive/mod.rs index f271e01fe0a..869a18b0482 100644 --- a/src/_derive/mod.rs +++ b/src/_derive/mod.rs @@ -254,14 +254,17 @@ //! //! `clap` assumes some intent based on the type used: //! -//! | Type | Effect | Implies | -//! |---------------------|--------------------------------------|--------------------------------------------------------------------------| -//! | `bool` | flag | `.action(ArgAction::SetTrue) | -//! | `Option` | optional argument | `.action(ArgAction::Set).required(false)` | -//! | `Option>` | optional value for optional argument | `.action(ArgAction::Set).required(false).min_values(0).max_values(1)` | -//! | `T` | required argument | `.action(ArgAction::Set).required(!has_default)` | -//! | `Vec` | `0..` occurrences of argument | `.action(ArgAction::Append).required(false).multiple_occurrences(true)` | -//! | `Option>` | `0..` occurrences of argument | `.action(ArgAction::Append).required(false).multiple_occurrences(true)` | +//! | Type | Effect | Implies | +//! |-------------------------|--------------------------------------|--------------------------------------------------------------------------| +//! | `()` and `id="help"` | print help | `.action(ArgAction::Help) | +//! | `()` and `id="version"` | print version | `.action(ArgAction::Version) | +//! | `()` | caller dependent | | +//! | `bool` | flag | `.action(ArgAction::SetTrue) | +//! | `Option` | optional argument | `.action(ArgAction::Set).required(false)` | +//! | `Option>` | optional value for optional argument | `.action(ArgAction::Set).required(false).min_values(0).max_values(1)` | +//! | `T` | required argument | `.action(ArgAction::Set).required(!has_default)` | +//! | `Vec` | `0..` occurrences of argument | `.action(ArgAction::Append).required(false).multiple_occurrences(true)` | +//! | `Option>` | `0..` occurrences of argument | `.action(ArgAction::Append).required(false).multiple_occurrences(true)` | //! //! Notes: //! - For custom type behavior, you can override the implied attributes/settings and/or set additional ones diff --git a/tests/derive/help.rs b/tests/derive/help.rs index e0518e00ec2..ac08287ae88 100644 --- a/tests/derive/help.rs +++ b/tests/derive/help.rs @@ -441,3 +441,31 @@ OPTIONS: let help = String::from_utf8(buffer).unwrap(); snapbox::assert_eq(HELP, help); } + +#[test] +fn custom_help_flag() { + #[derive(Debug, Clone, Parser)] + #[clap(disable_help_flag = true)] + struct CliOptions { + #[clap(short = 'h', long = "verbose-help")] + help: (), + } + + let result = CliOptions::try_parse_from(["cmd", "--verbose-help"]); + let err = result.unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp); +} + +#[test] +fn custom_version_flag() { + #[derive(Debug, Clone, Parser)] + #[clap(disable_version_flag = true, version = "2.0.0")] + struct CliOptions { + #[clap(short = 'V', long = "verbose-version")] + version: (), + } + + let result = CliOptions::try_parse_from(["cmd", "--verbose-version"]); + let err = result.unwrap_err(); + assert_eq!(err.kind(), clap::error::ErrorKind::DisplayVersion); +}