Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(parser): Prefer invalid subcommands over invalid args #4219

Merged
merged 1 commit into from Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions examples/derive_ref/interop_tests.md
Expand Up @@ -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]

Expand Down Expand Up @@ -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>

Expand Down
6 changes: 3 additions & 3 deletions src/builder/command.rs
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 0 additions & 5 deletions src/mkeymap.rs
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion src/parser/parser.rs
Expand Up @@ -492,6 +492,7 @@ impl<'cmd> Parser<'cmd> {
);
}
}

let candidates = suggestions::did_you_mean(
&arg_os.display().to_string(),
self.cmd.all_subcommand_names(),
Expand All @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion tests/builder/app_settings.rs
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion tests/derive/subcommands.rs
Expand Up @@ -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,
);
}

Expand Down