fix(parser): Don't treat missing values as missing args

This commit is contained in:
Ed Page 2022-06-02 13:45:18 -05:00
parent cc2714beab
commit 50f4018dcf
5 changed files with 33 additions and 7 deletions

View file

@ -18,6 +18,11 @@ pub enum MatchesError {
UnknownArgument { UnknownArgument {
// Missing `id` but blocked on a public id type which will hopefully come with `unstable-v4` // Missing `id` but blocked on a public id type which will hopefully come with `unstable-v4`
}, },
/// Present argument must have one value
#[non_exhaustive]
ExpectedOne {
// Missing `id` but blocked on a public id type which will hopefully come with `unstable-v4`
},
} }
impl MatchesError { impl MatchesError {
@ -51,6 +56,9 @@ impl std::fmt::Display for MatchesError {
Self::UnknownArgument {} => { Self::UnknownArgument {} => {
writeln!(f, "Unknown argument or group id. Make sure you are using the argument id and not the short or long flags") writeln!(f, "Unknown argument or group id. Make sure you are using the argument id and not the short or long flags")
} }
Self::ExpectedOne {} => {
writeln!(f, "Present argument must have one value. Make sure you are using the correct lookup (`get_one` vs `get_many`)")
}
} }
} }
} }

View file

@ -1063,8 +1063,11 @@ impl ArgMatches {
) -> Result<Option<&T>, MatchesError> { ) -> Result<Option<&T>, MatchesError> {
let id = Id::from(name); let id = Id::from(name);
let arg = self.try_get_arg_t::<T>(&id)?; let arg = self.try_get_arg_t::<T>(&id)?;
let value = match arg.and_then(|a| a.first()) { let value = match arg.map(|a| a.first()) {
Some(value) => value, Some(Some(value)) => value,
Some(None) => {
return Err(MatchesError::ExpectedOne {});
}
None => { None => {
return Ok(None); return Ok(None);
} }

View file

@ -38,7 +38,10 @@ fn env_bool_literal() {
let m = r.unwrap(); let m = r.unwrap();
assert!(m.is_present("present")); assert!(m.is_present("present"));
assert_eq!(m.occurrences_of("present"), 0); assert_eq!(m.occurrences_of("present"), 0);
assert_eq!(m.get_one::<String>("present").map(|v| v.as_str()), None); assert!(matches!(
m.try_get_one::<String>("present").unwrap_err(),
clap::parser::MatchesError::ExpectedOne { .. }
));
assert!(!m.is_present("negated")); assert!(!m.is_present("negated"));
assert!(!m.is_present("absent")); assert!(!m.is_present("absent"));
} }

View file

@ -111,7 +111,10 @@ fn group_single_flag() {
let m = res.unwrap(); let m = res.unwrap();
assert!(m.is_present("grp")); assert!(m.is_present("grp"));
assert!(m.get_one::<String>("grp").map(|v| v.as_str()).is_none()); assert!(matches!(
m.try_get_one::<String>("grp").unwrap_err(),
clap::parser::MatchesError::ExpectedOne { .. }
));
} }
#[test] #[test]

View file

@ -49,7 +49,10 @@ fn multiple_args_and_final_arg_without_value() {
Some("file") Some("file")
); );
assert!(m.is_present("f")); assert!(m.is_present("f"));
assert_eq!(m.get_one::<String>("stuff").map(|v| v.as_str()), None); assert!(matches!(
m.try_get_one::<String>("stuff").unwrap_err(),
clap::parser::MatchesError::ExpectedOne { .. }
));
} }
#[test] #[test]
@ -76,7 +79,10 @@ fn multiple_args_and_intermittent_arg_without_value() {
Some("file") Some("file")
); );
assert!(m.is_present("f")); assert!(m.is_present("f"));
assert_eq!(m.get_one::<String>("stuff").map(|v| v.as_str()), None); assert!(matches!(
m.try_get_one::<String>("stuff").unwrap_err(),
clap::parser::MatchesError::ExpectedOne { .. }
));
} }
#[test] #[test]
@ -118,7 +124,10 @@ fn subcommand() {
sub_m.is_present("test"), sub_m.is_present("test"),
"expected subcommand to be present due to partial parsing" "expected subcommand to be present due to partial parsing"
); );
assert_eq!(sub_m.get_one::<String>("test").map(|v| v.as_str()), None); assert!(matches!(
sub_m.try_get_one::<String>("test").unwrap_err(),
clap::parser::MatchesError::ExpectedOne { .. }
));
assert_eq!( assert_eq!(
sub_m.get_one::<String>("stuff").map(|v| v.as_str()), sub_m.get_one::<String>("stuff").map(|v| v.as_str()),
Some("some other val") Some("some other val")