fix(validate): Give precedence to ArgRequiredElseHelp

epage · Feb 11, 2022 · e8e4691 · e8e4691
1 parent c0fd675
commit e8e4691
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 13 deletions.
diff --git a/src/parse/parser.rs b/src/parse/parser.rs
@@ -483,17 +483,6 @@ impl<'help, 'app> Parser<'help, 'app> {
                 .name
                 .clone();
             self.parse_subcommand(&sc_name, matcher, it, keep_state)?;
-        } else if self.app.is_subcommand_required_set() {
-            let bn = self.app.bin_name.as_ref().unwrap_or(&self.app.name);
-            return Err(ClapError::missing_subcommand(
-                self.app,
-                bn.to_string(),
-                Usage::new(self.app, &self.required).create_usage_with_title(&[]),
-            ));
-        } else if self.is_set(AS::SubcommandRequiredElseHelp) {
-            debug!("Parser::get_matches_with: SubcommandRequiredElseHelp=true");
-            let message = self.write_help_err()?;
-            return Err(ClapError::display_help_error(self.app, message));
         }
 
         Validator::new(self).validate(parse_state, matcher, trailing_values)

diff --git a/src/parse/validator.rs b/src/parse/validator.rs
@@ -1,5 +1,5 @@
 // Internal
-use crate::build::{App, Arg, ArgPredicate, PossibleValue};
+use crate::build::{App, AppSettings, Arg, ArgPredicate, PossibleValue};
 use crate::error::{Error, Result as ClapResult};
 use crate::output::Usage;
 use crate::parse::{ArgMatcher, MatchedArg, ParseState, Parser};
@@ -61,6 +61,19 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                 return Err(Error::display_help_error(self.p.app, message));
             }
         }
+        if !has_subcmd && self.p.app.is_subcommand_required_set() {
+            let bn = self.p.app.bin_name.as_ref().unwrap_or(&self.p.app.name);
+            return Err(Error::missing_subcommand(
+                self.p.app,
+                bn.to_string(),
+                Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
+            ));
+        } else if !has_subcmd && self.p.app.is_set(AppSettings::SubcommandRequiredElseHelp) {
+            debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true");
+            let message = self.p.write_help_err()?;
+            return Err(Error::display_help_error(self.p.app, message));
+        }
+
         self.validate_conflicts(matcher)?;
         if !(self.p.app.is_subcommand_negates_reqs_set() && has_subcmd) {
             self.validate_required(matcher)?;

diff --git a/tests/builder/app_settings.rs b/tests/builder/app_settings.rs
@@ -194,7 +194,7 @@ fn arg_required_else_help() {
 }
 
 #[test]
-fn arg_required_else_help_over_reqs() {
+fn arg_required_else_help_over_req_arg() {
     let result = App::new("arg_required")
         .arg_required_else_help(true)
         .arg(Arg::new("test").index(1).required(true))
@@ -208,6 +208,22 @@ fn arg_required_else_help_over_reqs() {
     );
 }
 
+#[test]
+fn arg_required_else_help_over_req_subcommand() {
+    let result = App::new("sub_required")
+        .arg_required_else_help(true)
+        .subcommand_required(true)
+        .subcommand(App::new("sub1"))
+        .try_get_matches_from(vec![""]);
+
+    assert!(result.is_err());
+    let err = result.err().unwrap();
+    assert_eq!(
+        err.kind(),
+        ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand
+    );
+}
+
 #[test]
 fn arg_required_else_help_with_default() {
     let result = App::new("arg_required")