fix!: num_args controls user args rather than parsed values

This mostly impacts use of delimiters.

CHANGELOG.md

@@ -20,7 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - `arg!` now sets `ArgAction::SetTrue`, `ArgAction::Count`, `ArgAction::Set`, or `ArgAction::Append` as appropriate (#3795)
 - Default actions are now `Set` and `SetTrue`
 - Removed `PartialEq` and `Eq` from `Command`
-- Replace `Arg::number_of_values` (average across occurrences) with `Arg::num_args` (per occurrence)
+- Replace `Arg::number_of_values` (average across occurrences) with `Arg::num_args` (per occurrence, raw CLI args, not parsed values)
   - `num_args(0)` no longer implies `takes_value(true).multiple_values(true)`
   - `num_args(1)` no longer implies `multiple_values(true)`
 - Replace `Arg::min_values` (across all occurrences) with `Arg::num_args(N..)` (per occurrence)

src/builder/arg.rs

@@ -908,13 +908,13 @@ impl<'help> Arg<'help> {
         }
     }
 
-    /// Specifies the number of values allowed per occurrence of this argument
+    /// Specifies the number of arguments parsed per occurrence
     ///
     /// For example, if you had a `-f <file>` argument where you wanted exactly 3 'files' you would
     /// set `.num_args(3)`, and this argument wouldn't be satisfied unless the user
     /// provided 3 and only 3 values.
     ///
-    /// **NOTE:** Users may specify values for arguments in any of the following methods
+    /// Users may specify values for arguments in any of the following methods
     ///
     /// - Using a space such as `-o value` or `--option value`
     /// - Using an equals and no space such as `-o=value` or `--option=value`
@@ -934,7 +934,7 @@ impl<'help> Arg<'help> {
     /// - It reaches the [`Arg::value_terminator`] if set
     ///
     /// Alternatively,
-    /// - Require a delimiter between values with [Arg::require_value_delimiter]
+    /// - Use a delimiter between values with [Arg::value_delimiter]
     /// - Require a flag occurrence per value with [`ArgAction::Append`]
     /// - Require positional arguments to appear after `--` with [`Arg::last`]
     ///

src/parser/arg_matcher.rs

@@ -122,10 +122,6 @@ impl ArgMatcher {
         self.matches.subcommand_name()
     }
 
-    pub(crate) fn iter(&self) -> indexmap::map::Iter<Id, MatchedArg> {
-        self.matches.args.iter()
-    }
-
     pub(crate) fn check_explicit<'a>(&self, arg: &Id, predicate: ArgPredicate<'a>) -> bool {
         self.get(arg).map_or(false, |a| a.check_explicit(predicate))
     }

src/parser/matches/matched_arg.rs

@@ -75,6 +75,7 @@ impl MatchedArg {
         self.indices.push(index)
     }
 
+    #[cfg(feature = "unstable-grouped")]
     pub(crate) fn vals(&self) -> Iter<Vec<AnyValue>> {
         self.vals.iter()
     }

src/parser/mod.rs

@@ -16,6 +16,7 @@ pub(crate) use self::matches::{MatchedArg, SubCommand};
 pub(crate) use self::parser::Identifier;
 pub(crate) use self::parser::PendingArg;
 pub(crate) use self::parser::{ParseState, Parser};
+pub(crate) use self::validator::get_possible_values_cli;
 pub(crate) use self::validator::Validator;
 
 pub use self::matches::RawValues;

src/parser/parser.rs

@@ -8,6 +8,7 @@ use std::{
 use clap_lex::RawOsStr;
 
 // Internal
+use crate::builder::PossibleValue;
 use crate::builder::{Arg, Command};
 use crate::error::Error as ClapError;
 use crate::error::Result as ClapResult;
@@ -1120,6 +1121,12 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
     ) -> ClapResult<ParseResult> {
         self.resolve_pending(matcher)?;
 
+        debug!(
+            "Parser::react action={:?}, identifier={:?}, source={:?}",
+            arg.get_action(),
+            ident,
+            source
+        );
         if raw_vals.is_empty() {
             // We assume this case is valid: require equals, but min_vals == 0.
             if !arg.default_missing_vals.is_empty() {
@@ -1141,12 +1148,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
             };
         }
 
-        debug!(
-            "Parser::react action={:?}, identifier={:?}, source={:?}",
-            arg.get_action(),
-            ident,
-            source
-        );
+        self.verify_num_args(arg, &raw_vals)?;
+
         match arg.get_action() {
             ArgAction::Set => {
                 if source == ValueSource::CommandLine
@@ -1248,6 +1251,65 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
         }
     }
 
+    fn verify_num_args(&self, arg: &Arg<'help>, raw_vals: &[OsString]) -> ClapResult<()> {
+        if self.cmd.is_ignore_errors_set() {
+            return Ok(());
+        }
+
+        let actual = raw_vals.len();
+
+        let min_vals = arg.get_min_vals().unwrap_or(1);
+        if arg.is_takes_value_set() && 0 < min_vals && actual == 0 {
+            // Issue 665 (https://github.com/clap-rs/clap/issues/665)
+            // Issue 1105 (https://github.com/clap-rs/clap/issues/1105)
+            return Err(ClapError::empty_value(
+                self.cmd,
+                &super::get_possible_values_cli(arg)
+                    .iter()
+                    .filter(|pv| !pv.is_hide_set())
+                    .map(PossibleValue::get_name)
+                    .collect::<Vec<_>>(),
+                arg.to_string(),
+            ));
+        }
+
+        if let Some(expected) = arg.get_num_args() {
+            if let Some(expected) = expected.num_values() {
+                if expected != actual {
+                    debug!("Validator::validate_arg_num_vals: Sending error WrongNumberOfValues");
+                    return Err(ClapError::wrong_number_of_values(
+                        self.cmd,
+                        arg.to_string(),
+                        expected,
+                        actual,
+                        Usage::new(self.cmd).create_usage_with_title(&[]),
+                    ));
+                }
+            } else if actual < expected.min_values() {
+                return Err(ClapError::too_few_values(
+                    self.cmd,
+                    arg.to_string(),
+                    expected.min_values(),
+                    actual,
+                    Usage::new(self.cmd).create_usage_with_title(&[]),
+                ));
+            } else if expected.max_values() < actual {
+                debug!("Validator::validate_arg_num_vals: Sending error TooManyValues");
+                return Err(ClapError::too_many_values(
+                    self.cmd,
+                    raw_vals
+                        .last()
+                        .expect(INTERNAL_ERROR_MSG)
+                        .to_string_lossy()
+                        .into_owned(),
+                    arg.to_string(),
+                    Usage::new(self.cmd).create_usage_with_title(&[]),
+                ));
+            }
+        }
+        Ok(())
+    }
+
     fn remove_overrides(&self, arg: &Arg<'help>, matcher: &mut ArgMatcher) {
         debug!("Parser::remove_overrides: id={:?}", arg.id);
         for override_id in &arg.overrides {

src/parser/validator.rs

@@ -3,10 +3,10 @@ use crate::builder::{Arg, ArgPredicate, Command, PossibleValue};
 use crate::error::{Error, Result as ClapResult};
 use crate::output::fmt::Stream;
 use crate::output::Usage;
-use crate::parser::{ArgMatcher, MatchedArg, ParseState};
+use crate::parser::{ArgMatcher, ParseState};
 use crate::util::ChildGraph;
 use crate::util::Id;
-use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
+use crate::INTERNAL_ERROR_MSG;
 
 pub(crate) struct Validator<'help, 'cmd> {
     cmd: &'cmd Command<'help>,
@@ -40,7 +40,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
             if should_err {
                 return Err(Error::empty_value(
                     self.cmd,
-                    &get_possible_values(o)
+                    &get_possible_values_cli(o)
                         .iter()
                         .filter(|pv| !pv.is_hide_set())
                         .map(PossibleValue::get_name)
@@ -78,7 +78,6 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
         if !(self.cmd.is_subcommand_negates_reqs_set() && has_subcmd) {
             self.validate_required(matcher, &mut conflicts)?;
         }
-        self.validate_matched_args(matcher)?;
 
         Ok(())
     }
@@ -231,92 +230,6 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
         }
     }
 
-    fn validate_matched_args(&self, matcher: &ArgMatcher) -> ClapResult<()> {
-        debug!("Validator::validate_matched_args");
-        matcher.iter().try_for_each(|(name, ma)| {
-            debug!(
-                "Validator::validate_matched_args:iter:{:?}: vals={:#?}",
-                name,
-                ma.vals_flatten()
-            );
-            if let Some(arg) = self.cmd.find(name) {
-                self.validate_arg_num_vals(arg, ma)?;
-            }
-            Ok(())
-        })
-    }
-
-    fn validate_arg_num_vals(&self, a: &Arg, ma: &MatchedArg) -> ClapResult<()> {
-        debug!("Validator::validate_arg_num_vals");
-        for (_i, group) in ma.vals().enumerate() {
-            let actual = group.len();
-            debug!(
-                "Validator::validate_arg_num_vals: group={}, actual={}",
-                _i, actual
-            );
-
-            let min_vals = a.get_min_vals().unwrap_or(1);
-            if a.is_takes_value_set() && 0 < min_vals && actual == 0 {
-                // Issue 665 (https://github.com/clap-rs/clap/issues/665)
-                // Issue 1105 (https://github.com/clap-rs/clap/issues/1105)
-                return Err(Error::empty_value(
-                    self.cmd,
-                    &get_possible_values(a)
-                        .iter()
-                        .filter(|pv| !pv.is_hide_set())
-                        .map(PossibleValue::get_name)
-                        .collect::<Vec<_>>(),
-                    a.to_string(),
-                ));
-            }
-
-            if let Some(expected) = a.get_num_args() {
-                if let Some(expected) = expected.num_values() {
-                    if expected != actual {
-                        debug!(
-                            "Validator::validate_arg_num_vals: Sending error WrongNumberOfValues"
-                        );
-                        return Err(Error::wrong_number_of_values(
-                            self.cmd,
-                            a.to_string(),
-                            expected,
-                            actual,
-                            Usage::new(self.cmd)
-                                .required(&self.required)
-                                .create_usage_with_title(&[]),
-                        ));
-                    }
-                } else if actual < expected.min_values() {
-                    return Err(Error::too_few_values(
-                        self.cmd,
-                        a.to_string(),
-                        expected.min_values(),
-                        actual,
-                        Usage::new(self.cmd)
-                            .required(&self.required)
-                            .create_usage_with_title(&[]),
-                    ));
-                } else if expected.max_values() < actual {
-                    debug!("Validator::validate_arg_num_vals: Sending error TooManyValues");
-                    return Err(Error::too_many_values(
-                        self.cmd,
-                        ma.raw_vals_flatten()
-                            .last()
-                            .expect(INTERNAL_ERROR_MSG)
-                            .to_str()
-                            .expect(INVALID_UTF8)
-                            .to_string(),
-                        a.to_string(),
-                        Usage::new(self.cmd)
-                            .required(&self.required)
-                            .create_usage_with_title(&[]),
-                    ));
-                }
-            }
-        }
-        Ok(())
-    }
-
     fn validate_required(
         &mut self,
         matcher: &ArgMatcher,
@@ -540,7 +453,7 @@ impl Conflicts {
     }
 }
 
-fn get_possible_values<'help>(a: &Arg<'help>) -> Vec<PossibleValue<'help>> {
+pub(crate) fn get_possible_values_cli<'help>(a: &Arg<'help>) -> Vec<PossibleValue<'help>> {
     if !a.is_takes_value_set() {
         vec![]
     } else {