diff --git a/clap_builder/src/error/format.rs b/clap_builder/src/error/format.rs index 49e617d071f..2895a39f691 100644 --- a/clap_builder/src/error/format.rs +++ b/clap_builder/src/error/format.rs @@ -146,12 +146,10 @@ fn write_dynamic_context( match error.kind() { ErrorKind::ArgumentConflict => { - let invalid_arg = error.get(ContextKind::InvalidArg); - let prior_arg = error.get(ContextKind::PriorArg); - if let (Some(ContextValue::String(invalid_arg)), Some(prior_arg)) = - (invalid_arg, prior_arg) - { - if ContextValue::String(invalid_arg.clone()) == *prior_arg { + let mut prior_arg = error.get(ContextKind::PriorArg); + if let Some(ContextValue::String(invalid_arg)) = error.get(ContextKind::InvalidArg) { + if Some(&ContextValue::String(invalid_arg.clone())) == prior_arg { + prior_arg = None; let _ = write!( styled, "the argument '{}{invalid_arg}{}' cannot be used multiple times", @@ -165,36 +163,48 @@ fn write_dynamic_context( invalid.render(), invalid.render_reset() ); + } + } else if let Some(ContextValue::String(invalid_arg)) = + error.get(ContextKind::InvalidSubcommand) + { + let _ = write!( + styled, + "the subcommand '{}{invalid_arg}{}' cannot be used with", + invalid.render(), + invalid.render_reset() + ); + } else { + styled.push_str(error.kind().as_str().unwrap()); + } - match prior_arg { - ContextValue::Strings(values) => { - styled.push_str(":"); - for v in values { - let _ = write!( - styled, - "\n{TAB}{}{v}{}", - invalid.render(), - invalid.render_reset() - ); - } - } - ContextValue::String(value) => { + if let Some(prior_arg) = prior_arg { + match prior_arg { + ContextValue::Strings(values) => { + styled.push_str(":"); + for v in values { let _ = write!( styled, - " '{}{value}{}'", + "\n{TAB}{}{v}{}", invalid.render(), invalid.render_reset() ); } - _ => { - styled.push_str(" one or more of the other specified arguments"); - } + } + ContextValue::String(value) => { + let _ = write!( + styled, + " '{}{value}{}'", + invalid.render(), + invalid.render_reset() + ); + } + _ => { + styled.push_str(" one or more of the other specified arguments"); } } - true - } else { - false } + + true } ErrorKind::NoEquals => { let invalid_arg = error.get(ContextKind::InvalidArg); diff --git a/clap_builder/src/error/mod.rs b/clap_builder/src/error/mod.rs index af90e2756f1..c0be1aa69e3 100644 --- a/clap_builder/src/error/mod.rs +++ b/clap_builder/src/error/mod.rs @@ -391,6 +391,34 @@ impl Error { err } + pub(crate) fn subcommand_conflict( + cmd: &Command, + sub: String, + mut others: Vec, + usage: Option, + ) -> Self { + let mut err = Self::new(ErrorKind::ArgumentConflict).with_cmd(cmd); + + #[cfg(feature = "error-context")] + { + let others = match others.len() { + 0 => ContextValue::None, + 1 => ContextValue::String(others.pop().unwrap()), + _ => ContextValue::Strings(others), + }; + err = err.extend_context_unchecked([ + (ContextKind::InvalidSubcommand, ContextValue::String(sub)), + (ContextKind::PriorArg, others), + ]); + if let Some(usage) = usage { + err = err + .insert_context_unchecked(ContextKind::Usage, ContextValue::StyledStr(usage)); + } + } + + err + } + pub(crate) fn empty_value(cmd: &Command, good_vals: &[String], arg: String) -> Self { Self::invalid_value(cmd, "".to_owned(), good_vals, arg) } diff --git a/clap_builder/src/parser/parser.rs b/clap_builder/src/parser/parser.rs index 93616d68a64..e840efc9566 100644 --- a/clap_builder/src/parser/parser.rs +++ b/clap_builder/src/parser/parser.rs @@ -444,11 +444,27 @@ impl<'cmd> Parser<'cmd> { } else { // Start error processing let _ = self.resolve_pending(matcher); - return Err(self.match_arg_error(&arg_os, valid_arg_found, trailing_values)); + return Err(self.match_arg_error( + &arg_os, + valid_arg_found, + trailing_values, + matcher, + )); } } if let Some(ref pos_sc_name) = subcmd_name { + if self.cmd.is_args_conflicts_with_subcommands_set() && valid_arg_found { + return Err(ClapError::subcommand_conflict( + self.cmd, + pos_sc_name.clone(), + matcher + .arg_ids() + .map(|id| self.cmd.find(id).unwrap().to_string()) + .collect(), + Usage::new(self.cmd).create_usage_with_title(&[]), + )); + } let sc_name = self .cmd .find_subcommand(pos_sc_name) @@ -470,6 +486,7 @@ impl<'cmd> Parser<'cmd> { arg_os: &clap_lex::ParsedArg<'_>, valid_arg_found: bool, trailing_values: bool, + matcher: &ArgMatcher, ) -> ClapError { // If argument follows a `--` if trailing_values { @@ -490,7 +507,19 @@ impl<'cmd> Parser<'cmd> { && self.cmd.has_positionals() && (arg_os.is_long() || arg_os.is_short()); - if !(self.cmd.is_args_conflicts_with_subcommands_set() && valid_arg_found) { + if self.cmd.has_subcommands() { + if self.cmd.is_args_conflicts_with_subcommands_set() && valid_arg_found { + return ClapError::subcommand_conflict( + self.cmd, + arg_os.display().to_string(), + matcher + .arg_ids() + .map(|id| self.cmd.find(id).unwrap().to_string()) + .collect(), + Usage::new(self.cmd).create_usage_with_title(&[]), + ); + } + let candidates = suggestions::did_you_mean( &arg_os.display().to_string(), self.cmd.all_subcommand_names(), @@ -508,9 +537,7 @@ impl<'cmd> Parser<'cmd> { } // If the argument must be a subcommand. - if self.cmd.has_subcommands() - && (!self.cmd.has_positionals() || self.cmd.is_infer_subcommands_set()) - { + if !self.cmd.has_positionals() || self.cmd.is_infer_subcommands_set() { return ClapError::unrecognized_subcommand( self.cmd, arg_os.display().to_string(), diff --git a/tests/builder/conflicts.rs b/tests/builder/conflicts.rs index 00011844136..6e3cd08ddac 100644 --- a/tests/builder/conflicts.rs +++ b/tests/builder/conflicts.rs @@ -684,6 +684,145 @@ fn exclusive_with_required() { cmd.clone().try_get_matches_from(["bug", "--test"]).unwrap(); } +#[test] +#[cfg(feature = "error-context")] +fn option_conflicts_with_subcommand() { + static CONFLICT_ERR: &str = "\ +error: the subcommand 'sub1' cannot be used with '--place ' + +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")); + + utils::assert_output(cmd, "test --place id sub1", CONFLICT_ERR, true); +} + +#[test] +#[cfg(feature = "error-context")] +fn positional_conflicts_with_subcommand() { + static CONFLICT_ERR: &str = "\ +error: the subcommand 'sub1' cannot be used with '' + +Usage: test + test + +For more information, try '--help'. +"; + + let cmd = Command::new("test") + .args_conflicts_with_subcommands(true) + .arg(arg!( "some arg")) + .subcommand(Command::new("sub1")); + + utils::assert_output(cmd, "test value1 sub1", CONFLICT_ERR, true); +} + +#[test] +#[cfg(feature = "error-context")] +fn flag_conflicts_with_subcommand_long_flag() { + static CONFLICT_ERR: &str = "\ +error: the subcommand 'sub' cannot be used with '--hello' + +Usage: test [OPTIONS] + test + +For more information, try '--help'. +"; + + let cmd = Command::new("test") + .args_conflicts_with_subcommands(true) + .arg(arg!(--hello)) + .subcommand(Command::new("sub").long_flag("sub")); + + utils::assert_output(cmd, "test --hello --sub", CONFLICT_ERR, true); +} + +#[test] +#[cfg(feature = "error-context")] +fn flag_conflicts_with_subcommand_short_flag() { + static CONFLICT_ERR: &str = "\ +error: the subcommand 'sub' cannot be used with '--hello' + +Usage: test [OPTIONS] + test + +For more information, try '--help'. +"; + + let cmd = Command::new("test") + .args_conflicts_with_subcommands(true) + .arg(arg!(--hello)) + .subcommand(Command::new("sub").short_flag('s')); + + utils::assert_output(cmd, "test --hello -s", CONFLICT_ERR, true); +} + +#[test] +#[cfg(feature = "error-context")] +fn positional_conflicts_with_subcommand_precedent() { + static CONFLICT_ERR: &str = "\ +error: the subcommand 'sub' cannot be used with '' + +Usage: test + test + +For more information, try '--help'. +"; + + let cmd = Command::new("test") + .args_conflicts_with_subcommands(true) + .subcommand_precedence_over_arg(true) + .arg(arg!( "some arg")) + .subcommand(Command::new("sub")); + + utils::assert_output(cmd, "test hello sub", CONFLICT_ERR, true); +} + +#[test] +#[cfg(feature = "error-context")] +fn flag_conflicts_with_subcommand_precedent() { + static CONFLICT_ERR: &str = "\ +error: the subcommand 'sub' cannot be used with '--hello' + +Usage: test [OPTIONS] + test + +For more information, try '--help'. +"; + + let cmd = Command::new("test") + .args_conflicts_with_subcommands(true) + .subcommand_precedence_over_arg(true) + .arg(arg!(--hello)) + .subcommand(Command::new("sub")); + + utils::assert_output(cmd, "test --hello sub", CONFLICT_ERR, true); +} + +#[test] +fn subcommand_conflict_negates_required() { + let cmd = Command::new("test") + .args_conflicts_with_subcommands(true) + .subcommand(Command::new("config")) + .arg(arg!(-p --place <"place id"> "Place ID to open").required(true)); + + let result = cmd.try_get_matches_from(["test", "config"]); + assert!( + result.is_ok(), + "args_conflicts_with_subcommands should ignore required: {}", + result.unwrap_err() + ); + let m = result.unwrap(); + assert_eq!(m.subcommand_name().unwrap(), "config"); +} + #[test] fn args_negate_subcommands_one_level() { let res = Command::new("disablehelp") @@ -729,42 +868,3 @@ fn args_negate_subcommands_two_levels() { Some("sub2") ); } - -#[test] -#[cfg(feature = "error-context")] -fn subcommand_conflict_error_message() { - static CONFLICT_ERR: &str = "\ -error: unexpected argument 'sub1' found - -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") - .args_conflicts_with_subcommands(true) - .subcommand(Command::new("config")) - .arg(arg!(-p --place <"place id"> "Place ID to open").required(true)); - - let result = cmd.try_get_matches_from(["test", "config"]); - assert!( - result.is_ok(), - "args_conflicts_with_subcommands should ignore required: {}", - result.unwrap_err() - ); - let m = result.unwrap(); - assert_eq!(m.subcommand_name().unwrap(), "config"); -}