fix: try_map loses some of what is expected in error when used sequentially (#827)

src/combinator.rs

@@ -655,23 +655,35 @@ where
         // Remove the pre-inner alt, to be reinserted later so we always preserve it
         let old_alt = inp.errors.alt.take();
 
-        let out = self.parser.go::<Emit>(inp)?;
+        let res = self.parser.go::<Emit>(inp);
         let span = inp.span_since(&before);
         let new_alt = inp.errors.alt.take();
 
-        match (self.mapper)(out, span) {
+        // Reinsert the original alt
+        inp.errors.alt = old_alt;
+        match res {
             Ok(out) => {
-                // If successful, reinsert the original alt and then apply the new alt on top of it, since both are valid
-                inp.errors.alt = old_alt;
-                if let Some(new_alt) = new_alt {
-                    inp.add_alt_err(&before.inner, new_alt.err);
+                match (self.mapper)(out, span) {
+                    Ok(out) => {
+                        // If successful apply the new alt on top of the original alt, since both are valid
+                        if let Some(new_alt) = new_alt {
+                            inp.add_alt_err(&new_alt.pos, new_alt.err);
+                        }
+                        Ok(M::bind(|| out))
+                    }
+
+                    Err(err) => {
+                        // If unsuccessful replace the new alt with the mapper error (since it overrides it)
+                        inp.add_alt_err(&before.inner, err);
+                        Err(())
+                    }
                 }
-                Ok(M::bind(|| out))
             }
-            Err(err) => {
-                // If unsuccessful, reinsert the original alt but replace the new alt with the mapper error (since it overrides it)
-                inp.errors.alt = old_alt;
-                inp.add_alt_err(&before.inner, err);
+
+            Err(_) => {
+                // Can't fail!
+                let new_alt = new_alt.unwrap();
+                inp.add_alt_err(&new_alt.pos, new_alt.err);
                 Err(())
             }
         }
@@ -709,11 +721,37 @@ where
     #[inline(always)]
     fn go<M: Mode>(&self, inp: &mut InputRef<'src, '_, I, E>) -> PResult<M, O> {
         let before = inp.cursor();
-        let out = self.parser.go::<Emit>(inp)?;
-        match (self.mapper)(out, &mut MapExtra::new(&before, inp)) {
-            Ok(out) => Ok(M::bind(|| out)),
-            Err(err) => {
-                inp.add_alt_err(&inp.cursor().inner, err);
+        // Remove the pre-inner alt, to be reinserted later so we always preserve it
+        let old_alt = inp.errors.alt.take();
+
+        let res = self.parser.go::<Emit>(inp);
+        let new_alt = inp.errors.alt.take();
+
+        // Reinsert the original alt
+        inp.errors.alt = old_alt;
+        match res {
+            Ok(out) => {
+                match (self.mapper)(out, &mut MapExtra::new(&before, inp)) {
+                    Ok(out) => {
+                        // If successful apply the new alt on top of the original alt, since both are valid
+                        if let Some(new_alt) = new_alt {
+                            inp.add_alt_err(&new_alt.pos, new_alt.err);
+                        }
+                        Ok(M::bind(|| out))
+                    }
+
+                    Err(err) => {
+                        // If unsuccessful replace the new alt with the mapper error (since it overrides it)
+                        inp.add_alt_err(&before.inner, err);
+                        Err(())
+                    }
+                }
+            }
+
+            Err(_) => {
+                // Can't fail!
+                let new_alt = new_alt.unwrap();
+                inp.add_alt_err(&new_alt.pos, new_alt.err);
                 Err(())
             }
         }

src/lib.rs

@@ -3836,6 +3836,62 @@ mod tests {
         );
     }
 
+    #[test]
+    fn try_map() {
+        use crate::{DefaultExpected, LabelError};
+
+        let parser = group((
+            just("a").or_not(),
+            just("b").try_map(|_, _| Ok(())).or_not(),
+            just::<_, &str, extra::Err<Rich<_>>>("c"),
+        ))
+        .ignored();
+
+        assert_eq!(
+            parser.parse("").into_output_errors(),
+            (
+                None,
+                vec![LabelError::<&str, _>::expected_found(
+                    vec![
+                        DefaultExpected::Token('a'.into()),
+                        DefaultExpected::Token('b'.into()),
+                        DefaultExpected::Token('c'.into()),
+                    ],
+                    None,
+                    SimpleSpan::new((), 0..0)
+                )]
+            )
+        );
+    }
+
+    #[test]
+    fn try_map_with() {
+        use crate::{DefaultExpected, LabelError};
+
+        let parser = group((
+            just("a").or_not(),
+            just("b").try_map_with(|_, _| Ok(())).or_not(),
+            just::<_, &str, extra::Err<Rich<_>>>("c"),
+        ))
+        .ignored();
+
+        assert_eq!(
+            parser.parse("").into_output_errors(),
+            (
+                None,
+                vec![LabelError::<&str, _>::expected_found(
+                    vec![
+                        DefaultExpected::Token('a'.into()),
+                        DefaultExpected::Token('b'.into()),
+                        DefaultExpected::Token('c'.into()),
+                    ],
+                    None,
+                    SimpleSpan::new((), 0..0)
+                )]
+            )
+        );
+    }
+
     #[test]
     fn filter() {
         use crate::{DefaultExpected, LabelError};