From a7e04a53e4a9e310663a0cdb5bfb0753d84b82bd Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Thu, 11 Jan 2024 10:20:19 -0600
Subject: [PATCH] fix(parser): Improve subcommand conflict error

---
 clap_builder/src/error/format.rs  |  9 +++++++++
 clap_builder/src/error/mod.rs     | 28 ++++++++++++++++++++++++++++
 clap_builder/src/parser/parser.rs | 16 ++++++++++++----
 tests/builder/conflicts.rs        |  8 ++++----
 4 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/clap_builder/src/error/format.rs b/clap_builder/src/error/format.rs
index 3b7dfb434a3..2895a39f691 100644
--- a/clap_builder/src/error/format.rs
+++ b/clap_builder/src/error/format.rs
@@ -164,6 +164,15 @@ fn write_dynamic_context(
                         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());
             }
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<F: ErrorFormatter> Error<F> {
         err
     }
 
+    pub(crate) fn subcommand_conflict(
+        cmd: &Command,
+        sub: String,
+        mut others: Vec<String>,
+        usage: Option<StyledStr>,
+    ) -> 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 d24d37ade63..3be08ce81ef 100644
--- a/clap_builder/src/parser/parser.rs
+++ b/clap_builder/src/parser/parser.rs
@@ -444,7 +444,12 @@ 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,
+                ));
             }
         }
 
@@ -470,6 +475,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 {
@@ -492,11 +498,13 @@ impl<'cmd> Parser<'cmd> {
 
         if self.cmd.has_subcommands() {
             if self.cmd.is_args_conflicts_with_subcommands_set() && valid_arg_found {
-                return ClapError::unknown_argument(
+                return ClapError::subcommand_conflict(
                     self.cmd,
                     arg_os.display().to_string(),
-                    None,
-                    suggested_trailing_arg,
+                    matcher
+                        .arg_ids()
+                        .map(|id| self.cmd.find(id).unwrap().to_string())
+                        .collect(),
                     Usage::new(self.cmd).create_usage_with_title(&[]),
                 );
             }
diff --git a/tests/builder/conflicts.rs b/tests/builder/conflicts.rs
index 72d9a986250..386b82bc31f 100644
--- a/tests/builder/conflicts.rs
+++ b/tests/builder/conflicts.rs
@@ -688,7 +688,7 @@ fn exclusive_with_required() {
 #[cfg(feature = "error-context")]
 fn option_conflicts_with_subcommand() {
     static CONFLICT_ERR: &str = "\
-error: unexpected argument 'sub1' found
+error: the subcommand 'sub1' cannot be used with '--place <place id>'
 
 Usage: test [OPTIONS]
        test <COMMAND>
@@ -708,7 +708,7 @@ For more information, try '--help'.
 #[cfg(feature = "error-context")]
 fn positional_conflicts_with_subcommand() {
     static CONFLICT_ERR: &str = "\
-error: unexpected argument 'sub1' found
+error: the subcommand 'sub1' cannot be used with '<arg1>'
 
 Usage: test <arg1>
        test <COMMAND>
@@ -752,7 +752,7 @@ fn flag_conflicts_with_subcommand_short_flag() {
 #[cfg(feature = "error-context")]
 fn positional_conflicts_with_subcommand_precedent() {
     static CONFLICT_ERR: &str = "\
-error: unexpected argument 'sub' found
+error: the subcommand 'sub' cannot be used with '<arg1>'
 
 Usage: test <arg1>
        test <COMMAND>
@@ -773,7 +773,7 @@ For more information, try '--help'.
 #[cfg(feature = "error-context")]
 fn flag_conflicts_with_subcommand_precedent() {
     static CONFLICT_ERR: &str = "\
-error: unexpected argument 'sub' found
+error: the subcommand 'sub' cannot be used with '--hello'
 
 Usage: test [OPTIONS]
        test <COMMAND>