From 6e4d39d2d0453ee06e5027e6d7d2a8308a213cda Mon Sep 17 00:00:00 2001 From: Zane Duffield Date: Thu, 19 Dec 2024 15:49:47 +0800 Subject: [PATCH 1/3] test(path): Add tests for path parser error messages --- src/path/parser.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/path/parser.rs b/src/path/parser.rs index 6666e21f..44a0586e 100644 --- a/src/path/parser.rs +++ b/src/path/parser.rs @@ -117,4 +117,34 @@ mod test { assert_eq!(parsed, expected); } + + #[test] + fn test_invalid_identifier() { + let err = from_str("!").unwrap_err(); + assert_eq!("", err.to_string()); + } + + #[test] + fn test_invalid_child() { + let err = from_str("a..").unwrap_err(); + assert_eq!("", err.to_string()); + } + + #[test] + fn test_invalid_subscript() { + let err = from_str("a[b]").unwrap_err(); + assert_eq!("", err.to_string()); + } + + #[test] + fn test_incomplete_subscript() { + let err = from_str("a[0").unwrap_err(); + assert_eq!("", err.to_string()); + } + + #[test] + fn test_invalid_postfix() { + let err = from_str("a!b").unwrap_err(); + assert_eq!("", err.to_string()); + } } From 586306f6971c800c4af9aac76bc8b56bb3d7c168 Mon Sep 17 00:00:00 2001 From: Zane Duffield Date: Thu, 19 Dec 2024 15:52:31 +0800 Subject: [PATCH 2/3] feat(path): Improve error messages for path parser --- src/path/parser.rs | 50 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/src/path/parser.rs b/src/path/parser.rs index 44a0586e..5bbd266f 100644 --- a/src/path/parser.rs +++ b/src/path/parser.rs @@ -2,13 +2,15 @@ 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::StrContext; +use winnow::error::StrContextValue; use winnow::prelude::*; use winnow::token::any; use winnow::token::take_while; @@ -17,7 +19,7 @@ use crate::path::Expression; pub(crate) fn from_str(mut input: &str) -> Result { let input = &mut input; - path(input).map_err(|e| e.into_inner().unwrap()) + path.parse(input).map_err(|e| e.into_inner()) } fn path(i: &mut &str) -> PResult { @@ -31,7 +33,6 @@ fn path(i: &mut &str) -> PResult { }, ) .parse_next(i)?; - eof.parse_next(i)?; Ok(expr) } @@ -41,9 +42,21 @@ fn ident(i: &mut &str) -> PResult { fn postfix(i: &mut &str) -> PResult { 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 { 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,6 +84,9 @@ fn integer(i: &mut &str) -> PResult { (opt('-'), digit1).take().try_map(FromStr::from_str), _: space0 ) + .context(StrContext::Expected(StrContextValue::Description( + "integer", + ))) .map(|(i,)| i) .parse_next(i) } @@ -121,30 +143,36 @@ mod test { #[test] fn test_invalid_identifier() { let err = from_str("!").unwrap_err(); - assert_eq!("", err.to_string()); + assert_eq!( + "invalid identifier\nexpected ASCII alphanumeric, `_`, `-`", + err.to_string() + ); } #[test] fn test_invalid_child() { let err = from_str("a..").unwrap_err(); - assert_eq!("", err.to_string()); + assert_eq!( + "invalid identifier\nexpected ASCII alphanumeric, `_`, `-`", + err.to_string() + ); } #[test] fn test_invalid_subscript() { let err = from_str("a[b]").unwrap_err(); - assert_eq!("", err.to_string()); + assert_eq!("invalid subscript\nexpected integer", err.to_string()); } #[test] fn test_incomplete_subscript() { let err = from_str("a[0").unwrap_err(); - assert_eq!("", err.to_string()); + assert_eq!("invalid subscript\nexpected `]`", err.to_string()); } #[test] fn test_invalid_postfix() { let err = from_str("a!b").unwrap_err(); - assert_eq!("", err.to_string()); + assert_eq!("invalid postfix\nexpected `[`, `.`", err.to_string()); } } From 7cf53ec23c4ebaa0af00b7ee58b846105aaf8e12 Mon Sep 17 00:00:00 2001 From: Zane Duffield Date: Thu, 19 Dec 2024 16:21:43 +0800 Subject: [PATCH 3/3] feat(path): Show location in parse error messages --- src/path/mod.rs | 2 +- src/path/parser.rs | 60 ++++++++++++++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/path/mod.rs b/src/path/mod.rs index 3c0b4213..aaa55b12 100644 --- 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 index 5bbd266f..dcd0f284 100644 --- a/src/path/parser.rs +++ b/src/path/parser.rs @@ -9,6 +9,7 @@ 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::*; @@ -17,9 +18,8 @@ use winnow::token::take_while; use crate::path::Expression; -pub(crate) fn from_str(mut input: &str) -> Result { - let input = &mut input; - path.parse(input).map_err(|e| e.into_inner()) +pub(crate) fn from_str(input: &str) -> Result> { + path.parse(input) } fn path(i: &mut &str) -> PResult { @@ -93,6 +93,8 @@ fn integer(i: &mut &str) -> PResult { #[cfg(test)] mod test { + use snapbox::{assert_data_eq, str}; + use super::Expression::*; use super::*; @@ -143,36 +145,70 @@ mod test { #[test] fn test_invalid_identifier() { let err = from_str("!").unwrap_err(); - assert_eq!( - "invalid identifier\nexpected ASCII alphanumeric, `_`, `-`", - err.to_string() + 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_eq!( - "invalid identifier\nexpected ASCII alphanumeric, `_`, `-`", - err.to_string() + 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_eq!("invalid subscript\nexpected integer", err.to_string()); + 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_eq!("invalid subscript\nexpected `]`", err.to_string()); + 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_eq!("invalid postfix\nexpected `[`, `.`", err.to_string()); + assert_data_eq!( + err.to_string(), + str![[r#" +a!b + ^ +invalid postfix +expected `[`, `.` +"#]] + ); } }