From f632424e65060121f08531aa84483ca9edb36e18 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Dec 2022 20:16:15 -0600 Subject: [PATCH 1/3] test(parser): Consolidate args_conflicts_with tests --- tests/builder/app_settings.rs | 46 ----------------------------------- tests/builder/conflicts.rs | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/tests/builder/app_settings.rs b/tests/builder/app_settings.rs index 7cfe3443696..c2fc61c4033 100644 --- a/tests/builder/app_settings.rs +++ b/tests/builder/app_settings.rs @@ -654,52 +654,6 @@ Options: utils::assert_output(cmd, "clap-test --help", REQUIRE_EQUALS, false); } -#[test] -fn args_negate_subcommands_one_level() { - let res = Command::new("disablehelp") - .args_conflicts_with_subcommands(true) - .subcommand_negates_reqs(true) - .arg(arg!( "some arg")) - .arg(arg!( "some arg")) - .subcommand( - Command::new("sub1").subcommand(Command::new("sub2").subcommand(Command::new("sub3"))), - ) - .try_get_matches_from(vec!["", "pickles", "sub1"]); - assert!(res.is_ok(), "error: {:?}", res.unwrap_err().kind()); - let m = res.unwrap(); - assert_eq!( - m.get_one::("arg2").map(|v| v.as_str()), - Some("sub1") - ); -} - -#[test] -fn args_negate_subcommands_two_levels() { - let res = Command::new("disablehelp") - .args_conflicts_with_subcommands(true) - .subcommand_negates_reqs(true) - .arg(arg!( "some arg")) - .arg(arg!( "some arg")) - .subcommand( - Command::new("sub1") - .args_conflicts_with_subcommands(true) - .subcommand_negates_reqs(true) - .arg(arg!( "some")) - .arg(arg!( "some")) - .subcommand(Command::new("sub2").subcommand(Command::new("sub3"))), - ) - .try_get_matches_from(vec!["", "sub1", "arg", "sub2"]); - assert!(res.is_ok(), "error: {:?}", res.unwrap_err().kind()); - let m = res.unwrap(); - assert_eq!( - m.subcommand_matches("sub1") - .unwrap() - .get_one::("arg2") - .map(|v| v.as_str()), - Some("sub2") - ); -} - #[test] fn propagate_vals_down() { let m = Command::new("myprog") diff --git a/tests/builder/conflicts.rs b/tests/builder/conflicts.rs index 32a3342aa22..e2a15f17fc9 100644 --- a/tests/builder/conflicts.rs +++ b/tests/builder/conflicts.rs @@ -655,6 +655,52 @@ fn exclusive_with_required() { cmd.clone().try_get_matches_from(["bug", "--test"]).unwrap(); } +#[test] +fn args_negate_subcommands_one_level() { + let res = Command::new("disablehelp") + .args_conflicts_with_subcommands(true) + .subcommand_negates_reqs(true) + .arg(arg!( "some arg")) + .arg(arg!( "some arg")) + .subcommand( + Command::new("sub1").subcommand(Command::new("sub2").subcommand(Command::new("sub3"))), + ) + .try_get_matches_from(vec!["", "pickles", "sub1"]); + assert!(res.is_ok(), "error: {:?}", res.unwrap_err().kind()); + let m = res.unwrap(); + assert_eq!( + m.get_one::("arg2").map(|v| v.as_str()), + Some("sub1") + ); +} + +#[test] +fn args_negate_subcommands_two_levels() { + let res = Command::new("disablehelp") + .args_conflicts_with_subcommands(true) + .subcommand_negates_reqs(true) + .arg(arg!( "some arg")) + .arg(arg!( "some arg")) + .subcommand( + Command::new("sub1") + .args_conflicts_with_subcommands(true) + .subcommand_negates_reqs(true) + .arg(arg!( "some")) + .arg(arg!( "some")) + .subcommand(Command::new("sub2").subcommand(Command::new("sub3"))), + ) + .try_get_matches_from(vec!["", "sub1", "arg", "sub2"]); + assert!(res.is_ok(), "error: {:?}", res.unwrap_err().kind()); + let m = res.unwrap(); + assert_eq!( + m.subcommand_matches("sub1") + .unwrap() + .get_one::("arg2") + .map(|v| v.as_str()), + Some("sub2") + ); +} + #[test] fn subcommand_conflict_negates_required() { let cmd = Command::new("test") From 2a374db6391bbd28b25ac1c56ea6fa1342aea164 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Dec 2022 20:23:35 -0600 Subject: [PATCH 2/3] test(parser): Show bad behavior --- tests/builder/conflicts.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/builder/conflicts.rs b/tests/builder/conflicts.rs index e2a15f17fc9..b94137eab37 100644 --- a/tests/builder/conflicts.rs +++ b/tests/builder/conflicts.rs @@ -701,6 +701,32 @@ fn args_negate_subcommands_two_levels() { ); } +#[test] +#[cfg(feature = "error-context")] +fn subcommand_conflict_error_message() { + static CONFLICT_ERR: &str = "\ +error: The subcommand 'sub1' wasn't recognized + + Did you mean 'sub1'? + + If you believe you received this message in error, try re-running with 'test -- sub1' + +Usage: test [OPTIONS] + test + +For more information try '--help' +"; + + let cmd = Command::new("test") + .args_conflicts_with_subcommands(true) + .arg(arg!(-p --place <"place id"> "Place ID to open")) + .subcommand( + Command::new("sub1").subcommand(Command::new("sub2").subcommand(Command::new("sub3"))), + ); + + utils::assert_output(cmd, "test --place id sub1", CONFLICT_ERR, true); +} + #[test] fn subcommand_conflict_negates_required() { let cmd = Command::new("test") From 453ac0bfb9311e666fab159ec251248ae8bc11aa Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 20 Dec 2022 20:27:55 -0600 Subject: [PATCH 3/3] fix(parser): Be less confusing with args/subcommand conflicts The new error message still isn't great but its better than the old one. Reported at https://hachyderm.io/@eminence/109548978776785113 --- src/parser/parser.rs | 52 ++++++++++++++++++++------------------ tests/builder/conflicts.rs | 6 +---- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 9b7f9a9fa55..dc99e32ece6 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -502,33 +502,35 @@ impl<'cmd> Parser<'cmd> { } } - let candidates = suggestions::did_you_mean( - &arg_os.display().to_string(), - self.cmd.all_subcommand_names(), - ); - // If the argument looks like a subcommand. - if !candidates.is_empty() { - return ClapError::invalid_subcommand( - self.cmd, - arg_os.display().to_string(), - candidates, - self.cmd - .get_bin_name() - .unwrap_or_else(|| self.cmd.get_name()) - .to_owned(), - Usage::new(self.cmd).create_usage_with_title(&[]), + if !(self.cmd.is_args_conflicts_with_subcommands_set() && valid_arg_found) { + let candidates = suggestions::did_you_mean( + &arg_os.display().to_string(), + self.cmd.all_subcommand_names(), ); - } + // If the argument looks like a subcommand. + if !candidates.is_empty() { + return ClapError::invalid_subcommand( + self.cmd, + arg_os.display().to_string(), + candidates, + self.cmd + .get_bin_name() + .unwrap_or_else(|| self.cmd.get_name()) + .to_owned(), + Usage::new(self.cmd).create_usage_with_title(&[]), + ); + } - // If the argument must be a subcommand. - if self.cmd.has_subcommands() - && (!self.cmd.has_positionals() || self.cmd.is_infer_subcommands_set()) - { - return ClapError::unrecognized_subcommand( - self.cmd, - arg_os.display().to_string(), - Usage::new(self.cmd).create_usage_with_title(&[]), - ); + // If the argument must be a subcommand. + if self.cmd.has_subcommands() + && (!self.cmd.has_positionals() || self.cmd.is_infer_subcommands_set()) + { + return ClapError::unrecognized_subcommand( + self.cmd, + arg_os.display().to_string(), + Usage::new(self.cmd).create_usage_with_title(&[]), + ); + } } let suggested_trailing_arg = !trailing_values diff --git a/tests/builder/conflicts.rs b/tests/builder/conflicts.rs index b94137eab37..1644b86f37f 100644 --- a/tests/builder/conflicts.rs +++ b/tests/builder/conflicts.rs @@ -705,11 +705,7 @@ fn args_negate_subcommands_two_levels() { #[cfg(feature = "error-context")] fn subcommand_conflict_error_message() { static CONFLICT_ERR: &str = "\ -error: The subcommand 'sub1' wasn't recognized - - Did you mean 'sub1'? - - If you believe you received this message in error, try re-running with 'test -- sub1' +error: Found argument 'sub1' which wasn't expected, or isn't valid in this context Usage: test [OPTIONS] test