From 9a180c1511bc23c7a0fe6f132a6dc87a5f2a2804 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 28 Jul 2021 08:53:32 -0500 Subject: [PATCH] feat(derive): Specify defaults by native expressions Right now - `default_value="something"` is a raw method - `default_value` uses native types This commit splits the meanings - `default_value="something"` is a raw method - `default_value_t` uses `T::default()` - `default_value_t=expr` uses an expression that evaluates to `T` This is meant to mirror the `value_of` / `value_of_t` API. At the moment, this is limited to `T: Display` to work with clap's default system. Something we can look at in the future is a way to loosen that restriction. One quick win is to specialize when `arg_enum` is set. The main downside is complicating the processing of attributes because it then means we need some processed before others. Since this builds on `clap`s existing default system, this also means users do not get any performance gains out of using `default_value_t`, since we still need to parse it but we also need to convert it to a string. Fixes #1694 --- clap_derive/src/attrs.rs | 34 ++++++++++--------- clap_derive/src/parse.rs | 23 +++++++------ clap_derive/tests/arg_enum.rs | 2 +- clap_derive/tests/default_value.rs | 32 +++++++++++++++-- .../ui/default_value_wo_value_removed.rs | 21 ++++++++++++ .../ui/default_value_wo_value_removed.stderr | 8 +++++ 6 files changed, 91 insertions(+), 29 deletions(-) create mode 100644 clap_derive/tests/ui/default_value_wo_value_removed.rs create mode 100644 clap_derive/tests/ui/default_value_wo_value_removed.stderr diff --git a/clap_derive/src/attrs.rs b/clap_derive/src/attrs.rs index 971795ec299..0d465f817ca 100644 --- a/clap_derive/src/attrs.rs +++ b/clap_derive/src/attrs.rs @@ -339,35 +339,37 @@ impl Attrs { VerbatimDocComment(ident) => self.verbatim_doc_comment = Some(ident), - DefaultValue(ident, lit) => { - let val = if let Some(lit) = lit { - quote!(#lit) + DefaultValueT(ident, expr) => { + let val = if let Some(expr) = expr { + quote!(#expr) } else { let ty = if let Some(ty) = self.ty.as_ref() { ty } else { abort!( ident, - "#[clap(default_value)] (without an argument) can be used \ + "#[clap(default_value_t)] (without an argument) can be used \ only on field level"; note = "see \ https://docs.rs/structopt/0.3.5/structopt/#magical-methods") }; - - quote_spanned!(ident.span()=> { - clap::lazy_static::lazy_static! { - static ref DEFAULT_VALUE: &'static str = { - let val = <#ty as ::std::default::Default>::default(); - let s = ::std::string::ToString::to_string(&val); - ::std::boxed::Box::leak(s.into_boxed_str()) - }; - } - *DEFAULT_VALUE - }) + quote!(<#ty as ::std::default::Default>::default()) }; - self.methods.push(Method::new(ident, val)); + let val = quote_spanned!(ident.span()=> { + clap::lazy_static::lazy_static! { + static ref DEFAULT_VALUE: &'static str = { + let val = #val; + let s = ::std::string::ToString::to_string(&val); + ::std::boxed::Box::leak(s.into_boxed_str()) + }; + } + *DEFAULT_VALUE + }); + + let raw_ident = Ident::new("default_value", ident.span()); + self.methods.push(Method::new(raw_ident, val)); } About(ident, about) => { diff --git a/clap_derive/src/parse.rs b/clap_derive/src/parse.rs index b408f33bb81..e563bc72cdd 100644 --- a/clap_derive/src/parse.rs +++ b/clap_derive/src/parse.rs @@ -26,7 +26,6 @@ pub enum ClapAttr { About(Ident, Option), Author(Ident, Option), Version(Ident, Option), - DefaultValue(Ident, Option), // ident = "string literal" RenameAllEnv(Ident, LitStr), @@ -41,6 +40,7 @@ pub enum ClapAttr { // ident = arbitrary_expr NameExpr(Ident, Expr), + DefaultValueT(Ident, Option), // ident(arbitrary_expr,*) MethodCall(Ident, Vec), @@ -75,7 +75,6 @@ impl Parse for ClapAttr { match &*name_str { "rename_all" => Ok(RenameAll(name, lit)), "rename_all_env" => Ok(RenameAllEnv(name, lit)), - "default_value" => Ok(DefaultValue(name, Some(lit))), "version" => { check_empty_lit("version"); @@ -105,13 +104,11 @@ impl Parse for ClapAttr { } } else { match input.parse::() { - Ok(expr) => { - if name_str == "skip" { - Ok(Skip(name, Some(expr))) - } else { - Ok(NameExpr(name, expr)) - } - } + Ok(expr) => match &*name_str { + "skip" => Ok(Skip(name, Some(expr))), + "default_value_t" => Ok(DefaultValueT(name, Some(expr))), + _ => Ok(NameExpr(name, expr)), + }, Err(_) => abort! { assign_token, @@ -176,7 +173,13 @@ impl Parse for ClapAttr { "external_subcommand" => Ok(ExternalSubcommand(name)), "verbatim_doc_comment" => Ok(VerbatimDocComment(name)), - "default_value" => Ok(DefaultValue(name, None)), + "default_value" => { + abort!(name, + "`#[clap(default_value)` attribute (without a value) has been replaced by `#[clap(default_value_t)]`."; + help = "Change the attribute to `#[clap(default_value_t)]`"; + ) + } + "default_value_t" => Ok(DefaultValueT(name, None)), "about" => (Ok(About(name, None))), "author" => (Ok(Author(name, None))), "version" => Ok(Version(name, None)), diff --git a/clap_derive/tests/arg_enum.rs b/clap_derive/tests/arg_enum.rs index fd927fd85ad..cbffaa41684 100644 --- a/clap_derive/tests/arg_enum.rs +++ b/clap_derive/tests/arg_enum.rs @@ -60,7 +60,7 @@ fn default_value() { #[derive(Clap, PartialEq, Debug)] struct Opt { - #[clap(arg_enum, default_value)] + #[clap(arg_enum, default_value_t)] arg: ArgChoice, } diff --git a/clap_derive/tests/default_value.rs b/clap_derive/tests/default_value.rs index 6666ac589bd..f6de63a1836 100644 --- a/clap_derive/tests/default_value.rs +++ b/clap_derive/tests/default_value.rs @@ -5,10 +5,38 @@ mod utils; use utils::*; #[test] -fn auto_default_value() { +fn default_value() { #[derive(Clap, PartialEq, Debug)] struct Opt { - #[clap(default_value)] + #[clap(default_value = "3")] + arg: i32, + } + assert_eq!(Opt { arg: 3 }, Opt::parse_from(&["test"])); + assert_eq!(Opt { arg: 1 }, Opt::parse_from(&["test", "1"])); + + let help = get_long_help::(); + assert!(help.contains("[default: 3]")); +} + +#[test] +fn default_value_t() { + #[derive(Clap, PartialEq, Debug)] + struct Opt { + #[clap(default_value_t = 3)] + arg: i32, + } + assert_eq!(Opt { arg: 3 }, Opt::parse_from(&["test"])); + assert_eq!(Opt { arg: 1 }, Opt::parse_from(&["test", "1"])); + + let help = get_long_help::(); + assert!(help.contains("[default: 3]")); +} + +#[test] +fn auto_default_value_t() { + #[derive(Clap, PartialEq, Debug)] + struct Opt { + #[clap(default_value_t)] arg: i32, } assert_eq!(Opt { arg: 0 }, Opt::parse_from(&["test"])); diff --git a/clap_derive/tests/ui/default_value_wo_value_removed.rs b/clap_derive/tests/ui/default_value_wo_value_removed.rs new file mode 100644 index 00000000000..71885e8d7ac --- /dev/null +++ b/clap_derive/tests/ui/default_value_wo_value_removed.rs @@ -0,0 +1,21 @@ +// Copyright 2018 Guillaume Pinot (@TeXitoi) +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use clap::Clap; + +#[derive(Clap, Debug)] +#[clap(name = "basic")] +struct Opt { + #[clap(default_value)] + value: i32, +} + +fn main() { + let opt = Opt::parse(); + println!("{:?}", opt); +} diff --git a/clap_derive/tests/ui/default_value_wo_value_removed.stderr b/clap_derive/tests/ui/default_value_wo_value_removed.stderr new file mode 100644 index 00000000000..821ebc53d12 --- /dev/null +++ b/clap_derive/tests/ui/default_value_wo_value_removed.stderr @@ -0,0 +1,8 @@ +error: `#[clap(default_value)` attribute (without a value) has been replaced by `#[clap(default_value_t)]`. + + = help: Change the attribute to `#[clap(default_value_t)]` + + --> $DIR/default_value_wo_value_removed.rs:14:12 + | +14 | #[clap(default_value)] + | ^^^^^^^^^^^^^