From ed345d9c91304308b7b521470a89fafe11103f0f Mon Sep 17 00:00:00 2001 From: Joshua Barretto Date: Tue, 31 Dec 2024 11:49:03 +0000 Subject: [PATCH] Fixed phantom error generation when mapping errors --- src/combinator.rs | 28 ++++++++++++---------------- src/input.rs | 22 ++++++++-------------- src/lib.rs | 35 +++++++++++++++++++++++++++++++---- src/recovery.rs | 6 +++--- 4 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/combinator.rs b/src/combinator.rs index 200011c5..a608bb98 100644 --- a/src/combinator.rs +++ b/src/combinator.rs @@ -827,7 +827,7 @@ where if res.is_err() { let alt = inp.take_alt(); - inp.memos.insert(key, Some(alt)); + inp.memos.insert(key, alt); } else { inp.memos.remove(&key); } @@ -2596,18 +2596,9 @@ where where Self: Sized, { - let old_alt = inp.take_alt(); - let res = self.parser.go::(inp); - - if res.is_err() { - let mut new_alt = inp.take_alt(); - new_alt.err = (self.mapper)(new_alt.err); - - inp.errors.alt = Some(old_alt); - inp.add_alt_err(&new_alt.pos, new_alt.err); - } - - res + (&self.parser) + .map_err_with_state(|e, _, _| (self.mapper)(e)) + .go::(inp) } go_extra!(O); @@ -2648,6 +2639,7 @@ where // go_extra!(O); // } +// TODO: Remove combinator, replace with map_err_with /// See [`Parser::map_err_with_state`]. #[derive(Copy, Clone)] pub struct MapErrWithState { @@ -2668,13 +2660,17 @@ where Self: Sized, { let start = inp.cursor(); + let old_alt = inp.take_alt(); let res = self.parser.go::(inp); if res.is_err() { - let mut e = inp.take_alt(); + // Can't fail! + let mut new_alt = inp.take_alt().unwrap(); let span = inp.span_since(&start); - e.err = (self.mapper)(e.err, span, inp.state()); - inp.errors.alt = Some(e); + new_alt.err = (self.mapper)(new_alt.err, span, inp.state()); + + inp.errors.alt = old_alt; + inp.add_alt_err(&new_alt.pos, new_alt.err); } res diff --git a/src/input.rs b/src/input.rs index 5b86d470..9e838252 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1524,7 +1524,8 @@ impl<'src, 'parse, I: Input<'src>, E: ParserExtra<'src, I>> InputRef<'src, 'pars pub fn parse>(&mut self, parser: P) -> Result { match parser.go::(self) { Ok(out) => Ok(out), - Err(()) => Err(self.take_alt().err), + // Can't fail! + Err(()) => Err(self.take_alt().unwrap().err), } } @@ -1536,7 +1537,8 @@ impl<'src, 'parse, I: Input<'src>, E: ParserExtra<'src, I>> InputRef<'src, 'pars pub fn check>(&mut self, parser: P) -> Result<(), E::Error> { match parser.go::(self) { Ok(()) => Ok(()), - Err(()) => Err(self.take_alt().err), + // Can't fail! + Err(()) => Err(self.take_alt().unwrap().err), } } @@ -1729,9 +1731,7 @@ impl<'src, 'parse, I: Input<'src>, E: ParserExtra<'src, I>> InputRef<'src, 'pars // Prioritize errors before choosing whether to generate the alt (avoids unnecessary error creation) self.errors.alt = Some(match self.errors.alt.take() { - Some(alt) => match { - I::cursor_location(&alt.pos).cmp(&I::cursor_location(at)) - } { + Some(alt) => match { I::cursor_location(&alt.pos).cmp(&I::cursor_location(at)) } { Ordering::Equal => { Located::at(alt.pos, alt.err.merge_expected_found(expected, found, span)) } @@ -1762,15 +1762,9 @@ impl<'src, 'parse, I: Input<'src>, E: ParserExtra<'src, I>> InputRef<'src, 'pars }); } - // Take the alt error. If one doesn't exist, generate a fake one. - pub(crate) fn take_alt(&mut self) -> Located { - let fake_span = self.span_since(&self.cursor()); - self.errors.alt.take().unwrap_or_else(|| { - Located::at( - self.cursor.clone(), - E::Error::expected_found([], None, fake_span), - ) - }) + // Take the alt error, if one exists + pub(crate) fn take_alt(&mut self) -> Option> { + self.errors.alt.take() } } diff --git a/src/lib.rs b/src/lib.rs index 62d4bb95..afd188fa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -357,12 +357,15 @@ pub trait Parser<'src, I: Input<'src>, O, E: ParserExtra<'src, I> = extra::Defau let mut own = InputOwn::new_state(input, state); let mut inp = own.as_ref_start(); let res = self.then_ignore(end()).go::(&mut inp); - let alt = inp.take_alt(); + let alt = inp.take_alt().map(|alt| alt.err).unwrap_or_else(|| { + let fake_span = inp.span_since(&inp.cursor()); + E::Error::expected_found([], None, fake_span) + }); let mut errs = own.into_errs(); let out = match res { Ok(out) => Some(out), Err(()) => { - errs.push(alt.err); + errs.push(alt); None } }; @@ -402,12 +405,15 @@ pub trait Parser<'src, I: Input<'src>, O, E: ParserExtra<'src, I> = extra::Defau let mut own = InputOwn::new_state(input, state); let mut inp = own.as_ref_start(); let res = self.then_ignore(end()).go::(&mut inp); - let alt = inp.take_alt(); + let alt = inp.take_alt().map(|alt| alt.err).unwrap_or_else(|| { + let fake_span = inp.span_since(&inp.cursor()); + E::Error::expected_found([], None, fake_span) + }); let mut errs = own.into_errs(); let out = match res { Ok(()) => Some(()), Err(()) => { - errs.push(alt.err); + errs.push(alt); None } }; @@ -3625,4 +3631,25 @@ mod tests { )]), ); } + + #[test] + fn map_err() { + use crate::{error::Error, util::Maybe::Val}; + + let parser = just::>('"').map_err(move |e: Rich| { + println!("Found = {:?}", e.found()); + println!("Expected = {:?}", e.expected().collect::>()); + println!("Span = {:?}", e.span()); + Error::<&str>::expected_found([Some(Val('"'))], e.found().copied().map(Val), *e.span()) + }); + + assert_eq!( + parser.parse(r#"H"#).into_result(), + Err(vec![Error::<&str>::expected_found( + [Some(Val('"'))], + Some(Val('H')), + (0..1).into() + )]) + ); + } } diff --git a/src/recovery.rs b/src/recovery.rs index 7a45fb18..50e3e3b3 100644 --- a/src/recovery.rs +++ b/src/recovery.rs @@ -41,7 +41,7 @@ where inp: &mut InputRef<'src, '_, I, E>, _parser: &P, ) -> PResult { - let alt = inp.take_alt(); + let alt = inp.take_alt().unwrap(); // Can't fail! let out = match self.0.go::(inp) { Ok(out) => out, Err(()) => { @@ -110,7 +110,7 @@ where inp: &mut InputRef<'src, '_, I, E>, parser: &P, ) -> PResult { - let alt = inp.take_alt(); + let alt = inp.take_alt().unwrap(); // Can't fail! loop { let before = inp.save(); if let Ok(()) = self.until.go::(inp) { @@ -170,7 +170,7 @@ where inp: &mut InputRef<'src, '_, I, E>, _parser: &P, ) -> PResult { - let alt = inp.take_alt(); + let alt = inp.take_alt().unwrap(); // Can't fail! loop { let before = inp.save(); if let Ok(()) = self.until.go::(inp) {