Merge pull request #618 from zaneduffield/better-parse-errors

Improve path parse error messages
rust-cli · Dec 19, 2024 · a06d7f0 · a06d7f0
2 parents 8c10215 + 7cf53ec
commit a06d7f0
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 9 deletions.
diff --git a/src/path/mod.rs b/src/path/mod.rs
@@ -27,7 +27,7 @@ impl FromStr for Expression {
 struct ParseError(String);
 
 impl ParseError {
-    fn new(inner: winnow::error::ContextError) -> Self {
+    fn new(inner: winnow::error::ParseError<&str, winnow::error::ContextError>) -> Self {
         Self(inner.to_string())
     }
 }

diff --git a/src/path/parser.rs b/src/path/parser.rs
@@ -2,22 +2,24 @@ use std::str::FromStr;
 
 use winnow::ascii::digit1;
 use winnow::ascii::space0;
+use winnow::combinator::cut_err;
 use winnow::combinator::dispatch;
-use winnow::combinator::eof;
 use winnow::combinator::fail;
 use winnow::combinator::opt;
 use winnow::combinator::repeat;
 use winnow::combinator::seq;
 use winnow::error::ContextError;
+use winnow::error::ParseError;
+use winnow::error::StrContext;
+use winnow::error::StrContextValue;
 use winnow::prelude::*;
 use winnow::token::any;
 use winnow::token::take_while;
 
 use crate::path::Expression;
 
-pub(crate) fn from_str(mut input: &str) -> Result<Expression, ContextError> {
-    let input = &mut input;
-    path(input).map_err(|e| e.into_inner().unwrap())
+pub(crate) fn from_str(input: &str) -> Result<Expression, ParseError<&str, ContextError>> {
+    path.parse(input)
 }
 
 fn path(i: &mut &str) -> PResult<Expression> {
@@ -31,7 +33,6 @@ fn path(i: &mut &str) -> PResult<Expression> {
             },
         )
         .parse_next(i)?;
-    eof.parse_next(i)?;
     Ok(expr)
 }
 
@@ -41,9 +42,21 @@ fn ident(i: &mut &str) -> PResult<Expression> {
 
 fn postfix(i: &mut &str) -> PResult<Child> {
     dispatch! {any;
-        '[' => seq!(integer.map(Child::Index), _: ']').map(|(i,)| i),
-        '.' => raw_ident.map(Child::Key),
-        _ => fail,
+        '[' => cut_err(
+            seq!(
+                integer.map(Child::Index),
+                _: ']'.context(StrContext::Expected(StrContextValue::CharLiteral(']'))),
+            )
+                .map(|(i,)| i)
+                .context(StrContext::Label("subscript"))
+        ),
+        '.' => cut_err(raw_ident.map(Child::Key)),
+        _ => cut_err(
+            fail
+                .context(StrContext::Label("postfix"))
+                .context(StrContext::Expected(StrContextValue::CharLiteral('[')))
+                .context(StrContext::Expected(StrContextValue::CharLiteral('.')))
+        ),
     }
     .parse_next(i)
 }
@@ -56,6 +69,12 @@ enum Child {
 fn raw_ident(i: &mut &str) -> PResult<String> {
     take_while(1.., ('a'..='z', 'A'..='Z', '0'..='9', '_', '-'))
         .map(ToString::to_string)
+        .context(StrContext::Label("identifier"))
+        .context(StrContext::Expected(StrContextValue::Description(
+            "ASCII alphanumeric",
+        )))
+        .context(StrContext::Expected(StrContextValue::CharLiteral('_')))
+        .context(StrContext::Expected(StrContextValue::CharLiteral('-')))
         .parse_next(i)
 }
 
@@ -65,12 +84,17 @@ fn integer(i: &mut &str) -> PResult<isize> {
         (opt('-'), digit1).take().try_map(FromStr::from_str),
         _: space0
     )
+    .context(StrContext::Expected(StrContextValue::Description(
+        "integer",
+    )))
     .map(|(i,)| i)
     .parse_next(i)
 }
 
 #[cfg(test)]
 mod test {
+    use snapbox::{assert_data_eq, str};
+
     use super::Expression::*;
     use super::*;
 
@@ -117,4 +141,74 @@ mod test {
 
         assert_eq!(parsed, expected);
     }
+
+    #[test]
+    fn test_invalid_identifier() {
+        let err = from_str("!").unwrap_err();
+        assert_data_eq!(
+            err.to_string(),
+            str![[r#"
+!
+^
+invalid identifier
+expected ASCII alphanumeric, `_`, `-`
+"#]]
+        );
+    }
+
+    #[test]
+    fn test_invalid_child() {
+        let err = from_str("a..").unwrap_err();
+        assert_data_eq!(
+            err.to_string(),
+            str![[r#"
+a..
+  ^
+invalid identifier
+expected ASCII alphanumeric, `_`, `-`
+"#]]
+        );
+    }
+
+    #[test]
+    fn test_invalid_subscript() {
+        let err = from_str("a[b]").unwrap_err();
+        assert_data_eq!(
+            err.to_string(),
+            str![[r#"
+a[b]
+  ^
+invalid subscript
+expected integer
+"#]]
+        );
+    }
+
+    #[test]
+    fn test_incomplete_subscript() {
+        let err = from_str("a[0").unwrap_err();
+        assert_data_eq!(
+            err.to_string(),
+            str![[r#"
+a[0
+   ^
+invalid subscript
+expected `]`
+"#]]
+        );
+    }
+
+    #[test]
+    fn test_invalid_postfix() {
+        let err = from_str("a!b").unwrap_err();
+        assert_data_eq!(
+            err.to_string(),
+            str![[r#"
+a!b
+  ^
+invalid postfix
+expected `[`, `.`
+"#]]
+        );
+    }
 }