Merge pull request #5813 from epage/ignore

fix(parser): Fill in defaults on ignored error

Author: epage

clap_builder/src/parser/parser.rs

@@ -50,9 +50,36 @@ impl<'cmd> Parser<'cmd> {
         &mut self,
         matcher: &mut ArgMatcher,
         raw_args: &mut clap_lex::RawArgs,
-        mut args_cursor: clap_lex::ArgCursor,
+        args_cursor: clap_lex::ArgCursor,
     ) -> ClapResult<()> {
         debug!("Parser::get_matches_with");
+
+        ok!(self.parse(matcher, raw_args, args_cursor).map_err(|err| {
+            if self.cmd.is_ignore_errors_set() {
+                #[cfg(feature = "env")]
+                let _ = self.add_env(matcher);
+                let _ = self.add_defaults(matcher);
+            }
+            err
+        }));
+        ok!(self.resolve_pending(matcher));
+
+        #[cfg(feature = "env")]
+        ok!(self.add_env(matcher));
+        ok!(self.add_defaults(matcher));
+
+        Validator::new(self.cmd).validate(matcher)
+    }
+
+    // The actual parsing function
+    #[allow(clippy::cognitive_complexity)]
+    pub(crate) fn parse(
+        &mut self,
+        matcher: &mut ArgMatcher,
+        raw_args: &mut clap_lex::RawArgs,
+        mut args_cursor: clap_lex::ArgCursor,
+    ) -> ClapResult<()> {
+        debug!("Parser::parse");
         // Verify all positional assertions pass
 
         let mut subcmd_name: Option<String> = None;
@@ -436,11 +463,7 @@ impl<'cmd> Parser<'cmd> {
                     matches: sc_m.into_inner(),
                 });
 
-                ok!(self.resolve_pending(matcher));
-                #[cfg(feature = "env")]
-                ok!(self.add_env(matcher));
-                ok!(self.add_defaults(matcher));
-                return Validator::new(self.cmd).validate(matcher);
+                return Ok(());
             } else {
                 // Start error processing
                 let _ = self.resolve_pending(matcher);
@@ -474,11 +497,7 @@ impl<'cmd> Parser<'cmd> {
             ok!(self.parse_subcommand(&sc_name, matcher, raw_args, args_cursor, keep_state));
         }
 
-        ok!(self.resolve_pending(matcher));
-        #[cfg(feature = "env")]
-        ok!(self.add_env(matcher));
-        ok!(self.add_defaults(matcher));
-        Validator::new(self.cmd).validate(matcher)
+        Ok(())
     }
 
     fn match_arg_error(

tests/builder/ignore_errors.rs

@@ -4,41 +4,52 @@ use super::utils;
 
 #[test]
 fn single_short_arg_without_value() {
-    let cmd = Command::new("cmd").ignore_errors(true).arg(arg!(
-        -c --config [FILE] "Sets a custom config file"
-    ));
+    let cmd = Command::new("cmd")
+        .ignore_errors(true)
+        .arg(arg!(
+            -c --config <FILE> "Sets a custom config file"
+        ))
+        .arg(arg!(--"unset-flag"));
 
     let r = cmd.try_get_matches_from(vec!["cmd", "-c" /* missing: , "config file" */]);
 
     assert!(r.is_ok(), "unexpected error: {r:?}");
     let m = r.unwrap();
     assert!(m.contains_id("config"));
+    assert_eq!(m.get_one::<String>("config").cloned(), None);
+    assert_eq!(m.get_one::<bool>("unset-flag").copied(), Some(false));
 }
 
 #[test]
 fn single_long_arg_without_value() {
-    let cmd = Command::new("cmd").ignore_errors(true).arg(arg!(
-        -c --config [FILE] "Sets a custom config file"
-    ));
+    let cmd = Command::new("cmd")
+        .ignore_errors(true)
+        .arg(arg!(
+            -c --config <FILE> "Sets a custom config file"
+        ))
+        .arg(arg!(--"unset-flag"));
 
     let r = cmd.try_get_matches_from(vec!["cmd", "--config" /* missing: , "config file" */]);
 
     assert!(r.is_ok(), "unexpected error: {r:?}");
     let m = r.unwrap();
     assert!(m.contains_id("config"));
+    assert_eq!(m.get_one::<String>("config").cloned(), None);
+    assert_eq!(m.get_one::<bool>("unset-flag").copied(), Some(false));
 }
 
 #[test]
 fn multiple_args_and_final_arg_without_value() {
     let cmd = Command::new("cmd")
         .ignore_errors(true)
         .arg(arg!(
-            -c --config [FILE] "Sets a custom config file"
+            -c --config <FILE> "Sets a custom config file"
         ))
         .arg(arg!(
-            -x --stuff [FILE] "Sets a custom stuff file"
+            -x --stuff <FILE> "Sets a custom stuff file"
         ))
-        .arg(arg!(f: -f "Flag").action(ArgAction::SetTrue));
+        .arg(arg!(f: -f "Flag").action(ArgAction::SetTrue))
+        .arg(arg!(--"unset-flag"));
 
     let r = cmd.try_get_matches_from(vec![
         "cmd", "-c", "file", "-f", "-x", /* missing: , "some stuff" */
@@ -50,21 +61,23 @@ fn multiple_args_and_final_arg_without_value() {
         m.get_one::<String>("config").map(|v| v.as_str()),
         Some("file")
     );
-    assert!(*m.get_one::<bool>("f").expect("defaulted by clap"));
+    assert_eq!(m.get_one::<bool>("f").copied(), Some(true));
     assert_eq!(m.get_one::<String>("stuff").map(|v| v.as_str()), None);
+    assert_eq!(m.get_one::<bool>("unset-flag").copied(), Some(false));
 }
 
 #[test]
 fn multiple_args_and_intermittent_arg_without_value() {
     let cmd = Command::new("cmd")
         .ignore_errors(true)
         .arg(arg!(
-            -c --config[FILE] "Sets a custom config file"
+            -c --config <FILE> "Sets a custom config file"
         ))
         .arg(arg!(
-            -x --stuff[FILE] "Sets a custom stuff file"
+            -x --stuff <FILE> "Sets a custom stuff file"
         ))
-        .arg(arg!(f: -f "Flag").action(ArgAction::SetTrue));
+        .arg(arg!(f: -f "Flag").action(ArgAction::SetTrue))
+        .arg(arg!(--"unset-flag"));
 
     let r = cmd.try_get_matches_from(vec![
         "cmd", "-x", /* missing: ,"some stuff" */
@@ -77,8 +90,30 @@ fn multiple_args_and_intermittent_arg_without_value() {
         m.get_one::<String>("config").map(|v| v.as_str()),
         Some("file")
     );
-    assert!(*m.get_one::<bool>("f").expect("defaulted by clap"));
+    assert_eq!(m.get_one::<bool>("f").copied(), Some(true));
     assert_eq!(m.get_one::<String>("stuff").map(|v| v.as_str()), None);
+    assert_eq!(m.get_one::<bool>("unset-flag").copied(), Some(false));
+}
+
+#[test]
+fn unexpected_argument() {
+    let cmd = Command::new("cmd")
+        .ignore_errors(true)
+        .arg(arg!(
+            -c --config [FILE] "Sets a custom config file"
+        ))
+        .arg(arg!(--"unset-flag"));
+
+    let r = cmd.try_get_matches_from(vec!["cmd", "-c", "config file", "unexpected"]);
+
+    assert!(r.is_ok(), "unexpected error: {r:?}");
+    let m = r.unwrap();
+    assert!(m.contains_id("config"));
+    assert_eq!(
+        m.get_one::<String>("config").cloned(),
+        Some("config file".to_owned())
+    );
+    assert_eq!(m.get_one::<bool>("unset-flag").copied(), Some(false));
 }
 
 #[test]
@@ -100,9 +135,11 @@ fn subcommand() {
                         .long("stuff")
                         .action(ArgAction::Set)
                         .help("stuf value"),
-                ),
+                )
+                .arg(arg!(--"unset-flag")),
         )
-        .arg(Arg::new("other").long("other"));
+        .arg(Arg::new("other").long("other"))
+        .arg(arg!(--"unset-flag"));
 
     let m = cmd
         .try_get_matches_from(vec![
@@ -125,6 +162,9 @@ fn subcommand() {
         sub_m.get_one::<String>("stuff").map(|v| v.as_str()),
         Some("some other val")
     );
+    assert_eq!(sub_m.get_one::<bool>("unset-flag").copied(), Some(false));
+
+    assert_eq!(m.get_one::<bool>("unset-flag").copied(), Some(false));
 }
 
 #[test]