Merge pull request #4219 from epage/sub

fix(parser): Prefer invalid subcommands over invalid args

CHANGELOG.md

@@ -294,6 +294,7 @@ Behavior Changes
 - *(version)* Use `Command::display_name` rather than `Command::bin_name` (#3966)
 - *(parser)* Always fill in `""` argument for external subcommands (#3263)
 - *(parser)* Short flags now have higher precedence than hyphen values with `Arg::allow_hyphen_values`, like `Command::allow_hyphen_values` (#4187)
+- *(parser)* Prefer `InvalidSubcommand` over `UnknownArgument` in more cases (#4219)
 - *(derive)* Detect escaped external subcommands that look like built-in subcommands (#3703)
 - *(derive)* Leave `Arg::id` as `verbatim` casing (#3282)
 - *(derive)* Default to `#[clap(value_parser, action)]` instead of `#[clap(parse)]` (#3827)

examples/derive_ref/interop_tests.md

@@ -85,7 +85,7 @@ For more information try --help
 ```console
 $ interop_augment_subcommands unknown
 ? failed
-error: Found argument 'unknown' which wasn't expected, or isn't valid in this context
+error: The subcommand 'unknown' wasn't recognized
 
 Usage: interop_augment_subcommands[EXE] [COMMAND]
 
@@ -189,7 +189,7 @@ Cli {
 ```console
 $ interop_hand_subcommand unknown
 ? failed
-error: Found argument 'unknown' which wasn't expected, or isn't valid in this context
+error: The subcommand 'unknown' wasn't recognized
 
 Usage: interop_hand_subcommand[EXE] [OPTIONS] <COMMAND>
 

src/builder/command.rs

@@ -1166,7 +1166,7 @@ impl Command {
     ///         "myprog", "help"
     ///     ]);
     /// assert!(res.is_err());
-    /// assert_eq!(res.unwrap_err().kind(), ErrorKind::UnknownArgument);
+    /// assert_eq!(res.unwrap_err().kind(), ErrorKind::InvalidSubcommand);
     /// ```
     ///
     /// [`subcommand`]: crate::Command::subcommand()
@@ -4313,8 +4313,8 @@ impl Command {
     }
 
     #[inline]
-    pub(crate) fn has_args(&self) -> bool {
-        !self.args.is_empty()
+    pub(crate) fn has_positionals(&self) -> bool {
+        self.get_positionals().next().is_some()
     }
 
     pub(crate) fn has_visible_subcommands(&self) -> bool {

src/mkeymap.rs

@@ -113,11 +113,6 @@ impl MKeyMap {
             .map(|k| &self.args[k.index])
     }
 
-    /// Find out if the map have no arg.
-    pub(crate) fn is_empty(&self) -> bool {
-        self.args.is_empty()
-    }
-
     /// Return iterators of all keys.
     pub(crate) fn keys(&self) -> impl Iterator<Item = &KeyType> {
         self.keys.iter().map(|x| &x.key)

src/parser/parser.rs

@@ -492,6 +492,7 @@ impl<'cmd> Parser<'cmd> {
                 );
             }
         }
+
         let candidates = suggestions::did_you_mean(
             &arg_os.display().to_string(),
             self.cmd.all_subcommand_names(),
@@ -513,15 +514,18 @@ impl<'cmd> Parser<'cmd> {
                 Usage::new(self.cmd).create_usage_with_title(&[]),
             );
         }
+
         // If the argument must be a subcommand.
-        if !self.cmd.has_args() || self.cmd.is_infer_subcommands_set() && self.cmd.has_subcommands()
+        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(&[]),
             );
         }
+
         ClapError::unknown_argument(
             self.cmd,
             arg_os.display().to_string(),

tests/builder/app_settings.rs

@@ -600,7 +600,7 @@ fn disable_help_subcommand() {
         .try_get_matches_from(vec!["", "help"]);
     assert!(result.is_err());
     let err = result.err().unwrap();
-    assert_eq!(err.kind(), ErrorKind::UnknownArgument);
+    assert_eq!(err.kind(), ErrorKind::InvalidSubcommand);
 }
 
 #[test]

tests/derive/subcommands.rs

@@ -551,7 +551,7 @@ fn skip_subcommand() {
     let res = Opt::try_parse_from(&["test", "skip"]);
     assert_eq!(
         res.unwrap_err().kind(),
-        clap::error::ErrorKind::UnknownArgument,
+        clap::error::ErrorKind::InvalidSubcommand,
     );
 }