Allow empty value by default, introduce NoEquals Error

This commit is contained in:
ldm0 2021-03-10 17:39:50 +00:00
parent 30d784ffd1
commit a1d4bd8488
9 changed files with 188 additions and 41 deletions

View file

@ -67,7 +67,7 @@ fn assert_app_flags(arg: &Arg) {
}
}
checker!(AllowEmptyValues requires TakesValue);
checker!(NoEmptyValues requires TakesValue);
checker!(RequireDelimiter requires TakesValue | UseValueDelimiter);
checker!(HidePossibleValues requires TakesValue);
checker!(AllowHyphenValues requires TakesValue);

View file

@ -4160,10 +4160,11 @@ impl<'help> Arg<'help> {
self.multiple_occurrences(multi).multiple_values(multi)
}
/// Allows an argument to accept explicitly empty values. An empty value must be specified at
/// the command line with an explicit `""`, `''`, or `--option=`
/// Don't allow an argument to accept explicitly empty values. An empty value
/// must be specified at the command line with an explicit `""`, `''`, or
/// `--option=`
///
/// **NOTE:** By default empty values are *not* allowed
/// **NOTE:** By default empty values are allowed.
///
/// **NOTE:** Setting this requires [`ArgSettings::TakesValue`].
///
@ -4173,11 +4174,11 @@ impl<'help> Arg<'help> {
/// # use clap::{App, Arg, ArgSettings};
/// Arg::new("file")
/// .long("file")
/// .setting(ArgSettings::TakesValue)
/// .setting(ArgSettings::AllowEmptyValues)
/// .forbid_empty_values(true)
/// # ;
/// ```
/// The default is to *not* allow empty values.
///
/// The default is allowing empty values.
///
/// ```rust
/// # use clap::{App, Arg, ErrorKind, ArgSettings};
@ -4185,24 +4186,7 @@ impl<'help> Arg<'help> {
/// .arg(Arg::new("cfg")
/// .long("config")
/// .short('v')
/// .setting(ArgSettings::TakesValue))
/// .try_get_matches_from(vec![
/// "prog", "--config="
/// ]);
///
/// assert!(res.is_err());
/// assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
/// ```
/// By adding this setting, we can allow empty values
///
/// ```rust
/// # use clap::{App, Arg, ArgSettings};
/// let res = App::new("prog")
/// .arg(Arg::new("cfg")
/// .long("config")
/// .short('v')
/// .setting(ArgSettings::TakesValue)
/// .setting(ArgSettings::AllowEmptyValues))
/// .takes_value(true))
/// .try_get_matches_from(vec![
/// "prog", "--config="
/// ]);
@ -4210,8 +4194,36 @@ impl<'help> Arg<'help> {
/// assert!(res.is_ok());
/// assert_eq!(res.unwrap().value_of("config"), None);
/// ```
///
/// By adding this setting, we can forbid empty values.
///
/// ```rust
/// # use clap::{App, Arg, ErrorKind, ArgSettings};
/// let res = App::new("prog")
/// .arg(Arg::new("cfg")
/// .long("config")
/// .short('v')
/// .takes_value(true)
/// .forbid_empty_values(true))
/// .try_get_matches_from(vec![
/// "prog", "--config="
/// ]);
///
/// assert!(res.is_err());
/// assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
/// ```
/// [`ArgSettings::TakesValue`]: ./enum.ArgSettings.html#variant.TakesValue
#[inline]
pub fn forbid_empty_values(self, multi: bool) -> Self {
if multi {
self.setting(ArgSettings::NoEmptyValues)
} else {
self.unset_setting(ArgSettings::NoEmptyValues)
}
}
/// Placeholder documentation
#[inline]
pub fn multiple_values(self, multi: bool) -> Self {
if multi {
self.setting(ArgSettings::MultipleValues)

View file

@ -8,7 +8,7 @@ bitflags! {
struct Flags: u32 {
const REQUIRED = 1;
const MULTIPLE_OCC = 1 << 1;
const EMPTY_VALS = 1 << 2;
const NO_EMPTY_VALS = 1 << 2;
const GLOBAL = 1 << 3;
const HIDDEN = 1 << 4;
const TAKES_VAL = 1 << 5;
@ -39,7 +39,7 @@ impl_settings! { ArgSettings, ArgFlags,
Required("required") => Flags::REQUIRED,
MultipleOccurrences("multipleoccurrences") => Flags::MULTIPLE_OCC,
MultipleValues("multiplevalues") => Flags::MULTIPLE_VALS,
AllowEmptyValues("allowemptyvalues") => Flags::EMPTY_VALS,
NoEmptyValues("noemptyvalues") => Flags::NO_EMPTY_VALS,
Hidden("hidden") => Flags::HIDDEN,
TakesValue("takesvalue") => Flags::TAKES_VAL,
UseValueDelimiter("usevaluedelimiter") => Flags::USE_DELIM,
@ -80,8 +80,8 @@ pub enum ArgSettings {
MultipleValues,
/// Allows an arg to appear multiple times
MultipleOccurrences,
/// Allows an arg accept empty values such as `""`
AllowEmptyValues,
/// Disallows an arg from accepting empty values such as `""`
NoEmptyValues,
/// Hides an arg from the help message
Hidden,
/// Allows an argument to take a value (such as `--option value`)
@ -130,8 +130,8 @@ mod test {
ArgSettings::AllowHyphenValues
);
assert_eq!(
"allowemptyvalues".parse::<ArgSettings>().unwrap(),
ArgSettings::AllowEmptyValues
"noemptyvalues".parse::<ArgSettings>().unwrap(),
ArgSettings::NoEmptyValues
);
assert_eq!(
"hidepossiblevalues".parse::<ArgSettings>().unwrap(),

View file

@ -116,6 +116,7 @@ pub enum ErrorKind {
/// let res = App::new("prog")
/// .arg(Arg::new("color")
/// .setting(ArgSettings::TakesValue)
/// .setting(ArgSettings::NoEmptyValues)
/// .long("color"))
/// .try_get_matches_from(vec!["prog", "--color="]);
/// assert!(res.is_err());

View file

@ -1153,7 +1153,7 @@ impl<'help, 'app> Parser<'help, 'app> {
// has_eq: --flag=value
let mut has_eq = false;
let no_val = val.is_none();
let empty_vals = opt.is_set(ArgSettings::AllowEmptyValues);
let no_empty_vals = opt.is_set(ArgSettings::NoEmptyValues);
let min_vals_zero = opt.min_vals.unwrap_or(1) == 0;
let require_equals = opt.is_set(ArgSettings::RequireEquals);
@ -1161,7 +1161,7 @@ impl<'help, 'app> Parser<'help, 'app> {
if let Some(fv) = val {
has_eq = fv.starts_with("=");
let v = fv.trim_start_n_matches(1, b'=');
if !empty_vals && (v.is_empty() || (require_equals && !has_eq)) {
if no_empty_vals && (v.is_empty() || (require_equals && !has_eq)) {
debug!("Found Empty - Error");
return Err(ClapError::empty_value(
opt,

View file

@ -125,9 +125,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
));
}
}
if !arg.is_set(ArgSettings::AllowEmptyValues)
&& val.is_empty()
&& matcher.contains(&arg.id)
if arg.is_set(ArgSettings::NoEmptyValues) && val.is_empty() && matcher.contains(&arg.id)
{
debug!("Validator::validate_arg_values: illegal empty val found");
return Err(Error::empty_value(

View file

@ -14,7 +14,11 @@ fn opts() {
#[test]
fn opt_without_value_fail() {
let r = App::new("df")
.arg(Arg::from("-o [opt] 'some opt'").default_value("default"))
.arg(
Arg::from("-o [opt] 'some opt'")
.default_value("default")
.forbid_empty_values(true),
)
.try_get_matches_from(vec!["", "-o"]);
assert!(r.is_err());
let err = r.unwrap_err();

129
tests/empty_values.rs Normal file
View file

@ -0,0 +1,129 @@
mod utils;
use clap::{App, Arg, ArgSettings, ErrorKind};
#[test]
fn empty_values() {
let m = App::new("config")
.arg(Arg::new("config").long("config").takes_value(true))
.get_matches_from(&["config", "--config", ""]);
assert_eq!(m.value_of("config"), Some(""));
}
#[test]
fn empty_values_with_equals() {
let m = App::new("config")
.arg(Arg::new("config").long("config").takes_value(true))
.get_matches_from(&["config", "--config="]);
assert_eq!(m.value_of("config"), Some(""));
let m = App::new("config")
.arg(Arg::new("config").short('c').takes_value(true))
.get_matches_from(&["config", "-c="]);
assert_eq!(m.value_of("config"), Some(""))
}
#[test]
fn no_empty_values() {
let m = App::new("config")
.arg(
Arg::new("config")
.long("config")
.takes_value(true)
.forbid_empty_values(true),
)
.try_get_matches_from(&["config", "--config", ""]);
assert!(m.is_err());
assert_eq!(m.unwrap_err().kind, ErrorKind::EmptyValue);
let m = App::new("config")
.arg(
Arg::new("config")
.short('c')
.takes_value(true)
.forbid_empty_values(true),
)
.try_get_matches_from(&["config", "-c", ""]);
assert!(m.is_err());
assert_eq!(m.unwrap_err().kind, ErrorKind::EmptyValue)
}
#[test]
fn no_empty_values_with_equals() {
let m = App::new("config")
.arg(
Arg::new("config")
.long("config")
.takes_value(true)
.setting(ArgSettings::NoEmptyValues),
)
.try_get_matches_from(&["config", "--config="]);
assert!(m.is_err());
assert_eq!(m.unwrap_err().kind, ErrorKind::EmptyValue);
let m = App::new("config")
.arg(
Arg::new("config")
.short('c')
.takes_value(true)
.setting(ArgSettings::NoEmptyValues),
)
.try_get_matches_from(&["config", "-c="]);
assert!(m.is_err());
assert_eq!(m.unwrap_err().kind, ErrorKind::EmptyValue);
}
#[test]
fn no_empty_values_without_equals() {
let m = App::new("config")
.arg(
Arg::new("config")
.long("config")
.takes_value(true)
.setting(ArgSettings::NoEmptyValues),
)
.try_get_matches_from(&["config", "--config"]);
assert!(m.is_err());
assert_eq!(m.unwrap_err().kind, ErrorKind::EmptyValue);
let m = App::new("config")
.arg(
Arg::new("config")
.short('c')
.takes_value(true)
.setting(ArgSettings::NoEmptyValues),
)
.try_get_matches_from(&["config", "-c"]);
assert!(m.is_err());
assert_eq!(m.unwrap_err().kind, ErrorKind::EmptyValue)
}
#[test]
fn no_empty_values_without_equals_but_requires_equals() {
let app = App::new("config").arg(
Arg::new("config")
.long("config")
.takes_value(true)
.setting(ArgSettings::NoEmptyValues)
.setting(ArgSettings::RequireEquals),
);
let m = app.clone().try_get_matches_from(&["config", "--config"]);
// Should error on no equals rather than empty value.
assert!(m.is_err());
assert_eq!(m.unwrap_err().kind, ErrorKind::NoEquals);
static NO_EUQALS_ERROR: &str =
"error: Equal sign is needed when assigning values to '--config=<config>'.
USAGE:
config [OPTIONS]
For more information try --help";
assert!(utils::compare_output(
app,
"config --config",
NO_EUQALS_ERROR,
true
));
}

View file

@ -34,6 +34,7 @@ fn require_equals_fail() {
.arg(
Arg::new("cfg")
.setting(ArgSettings::RequireEquals)
.setting(ArgSettings::NoEmptyValues)
.setting(ArgSettings::TakesValue)
.long("config"),
)
@ -104,6 +105,7 @@ fn require_equals_no_empty_values_fail() {
Arg::new("cfg")
.setting(ArgSettings::TakesValue)
.setting(ArgSettings::RequireEquals)
.setting(ArgSettings::NoEmptyValues)
.long("config"),
)
.arg(Arg::new("some"))
@ -119,7 +121,6 @@ fn require_equals_empty_vals_pass() {
Arg::new("cfg")
.setting(ArgSettings::TakesValue)
.setting(ArgSettings::RequireEquals)
.setting(ArgSettings::AllowEmptyValues)
.long("config"),
)
.try_get_matches_from(vec!["prog", "--config="]);
@ -422,8 +423,10 @@ fn did_you_mean() {
fn issue_665() {
let res = App::new("tester")
.arg("-v, --reroll-count=[N] 'Mark the patch series as PATCH vN'")
.arg(Arg::from(
"--subject-prefix [Subject-Prefix] 'Use [Subject-Prefix] instead of the standard [PATCH] prefix'") )
.arg(
Arg::from("--subject-prefix [Subject-Prefix] 'Use [Subject-Prefix] instead of the standard [PATCH] prefix'")
.setting(ArgSettings::NoEmptyValues)
)
.try_get_matches_from(vec!["test", "--subject-prefix", "-v", "2"]);
assert!(res.is_err());
@ -449,7 +452,7 @@ fn issue_1047_min_zero_vals_default_val() {
fn issue_1105_setup(argv: Vec<&'static str>) -> Result<ArgMatches, clap::Error> {
App::new("opts")
.arg(Arg::from("-o, --option [opt] 'some option'").setting(ArgSettings::AllowEmptyValues))
.arg(Arg::from("-o, --option [opt] 'some option'"))
.arg(Arg::from("--flag 'some flag'"))
.try_get_matches_from(argv)
}