Scan for positional items instead of trying to take the next available

Changelog.md

@@ -3,6 +3,7 @@
 ## bpaf [0.9.6], bpaf_derive [0.5.6] - Unreleased
 - make sure env-only arguments and flags are working
 - support raw identifiers in derive macro (#282)
+- better error messages for unexpected values that prevent positional parses
 
 ## bpaf [0.9.5], bpaf_derive [0.5.5] - 2023-08-24
 - fancier squashing: parse `-abfoo` as `-a -b=foo` if b is a short argument

src/args.rs

@@ -676,33 +676,30 @@ impl State {
     pub(crate) fn take_positional_word(
         &mut self,
         metavar: Metavar,
-    ) -> Result<Option<(usize, bool, OsString)>, Error> {
-        #[allow(clippy::range_plus_one)] // gives wrong type otherwise
-        match self.items_iter().next() {
-            Some((ix, Arg::PosWord(w))) => {
+    ) -> Result<(usize, bool, OsString), Error> {
+        match self.items_iter().find_map(|(ix, arg)| match arg {
+            Arg::Word(w) => Some((ix, false, w)),
+            Arg::PosWord(w) => Some((ix, true, w)),
+            _ => None,
+        }) {
+            Some((ix, strict, w)) => {
                 let w = w.clone();
                 self.current = Some(ix);
                 self.remove(ix);
-                Ok(Some((ix, true, w)))
+                Ok((ix, strict, w))
             }
-            Some((ix, Arg::Word(w))) => {
-                let w = w.clone();
-                self.current = Some(ix);
-                self.remove(ix);
-                Ok(Some((ix, false, w)))
-            }
-            Some((position, _arg)) => {
+            None => {
+                let scope = self.scope();
                 let missing = MissingItem {
                     item: Item::Positional {
                         help: None,
                         metavar,
                     },
-                    position,
-                    scope: position..position + 1,
+                    position: scope.start,
+                    scope,
                 };
                 Err(Error(Message::Missing(vec![missing])))
             }
-            None => Ok(None),
         }
     }
 
@@ -760,7 +757,7 @@ mod tests {
         let flag = a.take_flag(&long("speed"));
         assert!(flag);
         assert!(!a.is_empty());
-        let s = a.take_positional_word(M).unwrap().unwrap();
+        let s = a.take_positional_word(M).unwrap();
         assert_eq!(s.2, "12");
         assert!(a.is_empty());
     }
@@ -856,7 +853,7 @@ mod tests {
     fn command_and_positional() {
         let mut a = State::from(&["cmd", "pos"]);
         assert!(a.take_cmd("cmd"));
-        let w = a.take_positional_word(M).unwrap().unwrap();
+        let w = a.take_positional_word(M).unwrap();
         assert_eq!(w.2, "pos");
         assert!(a.is_empty());
     }
@@ -865,7 +862,7 @@ mod tests {
     fn positionals_after_double_dash1() {
         let mut a = State::from(&["-v", "--", "-x"]);
         assert!(a.take_flag(&short('v')));
-        let w = a.take_positional_word(M).unwrap().unwrap();
+        let w = a.take_positional_word(M).unwrap();
         assert_eq!(w.2, "-x");
         assert!(a.is_empty());
     }
@@ -874,7 +871,7 @@ mod tests {
     fn positionals_after_double_dash2() {
         let mut a = State::from(&["-v", "--", "-x"]);
         assert!(a.take_flag(&short('v')));
-        let w = a.take_positional_word(M).unwrap().unwrap();
+        let w = a.take_positional_word(M).unwrap();
         assert_eq!(w.2, "-x");
         assert!(a.is_empty());
     }
@@ -884,7 +881,7 @@ mod tests {
         let mut a = State::from(&["-v", "12", "--", "-x"]);
         let w = a.take_arg(&short('v'), false, M).unwrap().unwrap();
         assert_eq!(w, "12");
-        let w = a.take_positional_word(M).unwrap().unwrap();
+        let w = a.take_positional_word(M).unwrap();
         assert_eq!(w.2, "-x");
         assert!(a.is_empty());
     }

src/docs2/adjacent_struct_0.md

@@ -139,7 +139,7 @@ Options { point: [Point { point: (), x: 10, y: 20, z: 3.1415 }, Point { point: (
 
 <div class='bpaf-doc'>
 $ app --point 10 20 --rotate 3.1415<br>
-<b>Error:</b> expected <tt><i>Z</i></tt>, got <b>--rotate</b>. Pass <tt><b>--help</b></tt> for usage information
+<b>Error:</b> expected <tt><i>Z</i></tt>, pass <tt><b>--help</b></tt> for usage information
 <style>
 div.bpaf-doc {
     padding: 14px;

src/docs2/compose_basic_to_options.md

@@ -57,7 +57,7 @@ The other one is `--version` - passing a string literal or something like
 
 <div class='bpaf-doc'>
 $ app --version<br>
-Version: 3.1415
+<p>Version: 3.1415</p>
 <style>
 div.bpaf-doc {
     padding: 14px;

src/docs2/custom_help_version.md

@@ -115,7 +115,7 @@ both working
 
 <div class='bpaf-doc'>
 $ app --version<br>
-Version: 0.42
+<p>Version: 0.42</p>
 <style>
 div.bpaf-doc {
     padding: 14px;
@@ -134,7 +134,7 @@ div.bpaf-doc  { padding-left: 1em; }
 
 <div class='bpaf-doc'>
 $ app -v<br>
-Version: 0.42
+<p>Version: 0.42</p>
 <style>
 div.bpaf-doc {
     padding: 14px;

src/docs2/derive_basic_custom_consumer.md

@@ -64,7 +64,7 @@ long name is missing
 
 <div class='bpaf-doc'>
 $ app --switch 42<br>
-<b>Error:</b> expected <tt><i>NUM</i></tt>, got <b>--switch</b>. Pass <tt><b>--help</b></tt> for usage information
+<b>Error:</b> <b>--switch</b> is not expected in this context
 <style>
 div.bpaf-doc {
     padding: 14px;

src/docs2/to_options.md

@@ -82,7 +82,7 @@ The other one is `--version` - passing a string literal or something like
 
 <div class='bpaf-doc'>
 $ app --version<br>
-Version: 3.1415
+<p>Version: 3.1415</p>
 <style>
 div.bpaf-doc {
     padding: 14px;

src/params.rs

@@ -794,36 +794,29 @@ fn parse_pos_word(
     help: &Option<Doc>,
 ) -> Result<OsString, Error> {
     let metavar = Metavar(metavar);
-    if let Some((ix, is_strict, word)) = args.take_positional_word(metavar)? {
-        if strict && !is_strict {
-            #[cfg(feature = "autocomplete")]
-            args.push_pos_sep();
+    match args.take_positional_word(metavar) {
+        Ok((ix, is_strict, word)) => {
+            if strict && !is_strict {
+                #[cfg(feature = "autocomplete")]
+                args.push_pos_sep();
 
-            return Err(Error(Message::StrictPos(ix, metavar)));
-        }
-        #[cfg(feature = "autocomplete")]
-        if args.touching_last_remove() && !args.check_no_pos_ahead() {
-            args.push_metavar(metavar.0, help, false);
-            args.set_no_pos_ahead();
+                return Err(Error(Message::StrictPos(ix, metavar)));
+            }
+            #[cfg(feature = "autocomplete")]
+            if args.touching_last_remove() && !args.check_no_pos_ahead() {
+                args.push_metavar(metavar.0, help, false);
+                args.set_no_pos_ahead();
+            }
+            Ok(word)
         }
-        Ok(word)
-    } else {
-        #[cfg(feature = "autocomplete")]
-        if !args.check_no_pos_ahead() {
-            args.push_metavar(metavar.0, help, false);
-            args.set_no_pos_ahead();
+        Err(err) => {
+            #[cfg(feature = "autocomplete")]
+            if !args.check_no_pos_ahead() {
+                args.push_metavar(metavar.0, help, false);
+                args.set_no_pos_ahead();
+            }
+            Err(err)
         }
-
-        let position = args.items_iter().next().map_or(args.scope().end, |x| x.0);
-        let missing = MissingItem {
-            item: Item::Positional {
-                metavar,
-                help: help.clone(),
-            },
-            position,
-            scope: args.scope(),
-        };
-        Err(Error(Message::Missing(vec![missing])))
     }
 }
 

tests/errors.rs

@@ -199,3 +199,35 @@ fn adjacent_option_complains_to() {
     // TODO - this should point to the whole "-ayam" thing
     assert_eq!(r, "couldn't parse `yam`: invalid digit found in string");
 }
+
+#[test]
+fn some_pos_with_invalid_flag() {
+    let a = short('a').switch();
+    let b = positional::<usize>("B").some("Want B");
+    let parser = construct!(a, b).to_options();
+
+    let r = parser.run_inner(&["-c", "12"]).unwrap_err().unwrap_stderr();
+    assert_eq!(r, "`-c` is not expected in this context");
+
+    let r = parser.run_inner(&["12", "-c"]).unwrap_err().unwrap_stderr();
+    assert_eq!(r, "`-c` is not expected in this context");
+}
+
+#[test]
+fn pos_with_invalid_arg() {
+    let a = short('a').argument::<usize>("A").optional();
+    let b = positional::<usize>("B");
+    let parser = construct!(a, b).to_options();
+
+    let r = parser.run_inner(&["-c", "12"]).unwrap_err().unwrap_stderr();
+    assert_eq!(r, "`-c` is not expected in this context");
+
+    let r = parser.run_inner(&["12", "-c"]).unwrap_err().unwrap_stderr();
+    assert_eq!(r, "`-c` is not expected in this context");
+
+    let r = parser.run_inner(&["-c", "t"]).unwrap_err().unwrap_stderr();
+    assert_eq!(r, "couldn't parse `t`: invalid digit found in string");
+
+    let r = parser.run_inner(&["t", "-c"]).unwrap_err().unwrap_stderr();
+    assert_eq!(r, "couldn't parse `t`: invalid digit found in string");
+}