Merge pull request #638 from lovasoa/better_error_messages

improve error messages on parse errors
sunng87 · Mar 24, 2024 · e1c9ec2 · e1c9ec2
2 parents dea0ed9 + 231a3f5
commit e1c9ec2
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 62 deletions.
diff --git a/src/error.rs b/src/error.rs
@@ -192,8 +192,8 @@ pub enum TemplateErrorReason {
     MismatchingClosedHelper(String, String),
     #[error("decorator {0:?} was opened, but {1:?} is closing")]
     MismatchingClosedDecorator(String, String),
-    #[error("invalid handlebars syntax.")]
-    InvalidSyntax,
+    #[error("invalid handlebars syntax: {0}")]
+    InvalidSyntax(String),
     #[error("invalid parameter {0:?}")]
     InvalidParam(String),
     #[error("nested subexpression is not supported")]

diff --git a/src/grammar.pest b/src/grammar.pest
@@ -43,66 +43,66 @@ reference = ${ path_inline }
 
 name = _{ subexpression | reference }
 
-param = { !(keywords ~ !symbol_char) ~ (literal | reference | subexpression) }
-hash = { identifier ~ "=" ~ param }
+helper_parameter = { !(keywords ~ !symbol_char) ~ (literal | reference | subexpression) }
+hash = { identifier ~ "=" ~ helper_parameter }
 block_param = { "as" ~ "|" ~ identifier ~ identifier? ~ "|"}
-exp_line = _{ identifier ~ (hash|param)* ~ block_param?}
-partial_exp_line = _{ ((partial_identifier|name) ~ (hash|param)*) }
-
-subexpression = { "(" ~ ((identifier ~ (hash|param)+) | reference)  ~ ")" }
-
-pre_whitespace_omitter = { "~" }
-pro_whitespace_omitter = { "~" }
-
-expression = { !(invert_tag|invert_chain_tag) ~ "{{" ~ pre_whitespace_omitter? ~
-              ((identifier ~ (hash|param)+) | name )
-              ~ pro_whitespace_omitter? ~ "}}" }
-html_expression_triple_bracket_legacy = _{ "{{{" ~ pre_whitespace_omitter? ~
-                                           ((identifier ~ (hash|param)+) | name ) ~
-                                           pro_whitespace_omitter? ~ "}}}" }
-html_expression_triple_bracket = _{ "{{" ~ pre_whitespace_omitter? ~ "{" ~
-                                              ((identifier ~ (hash|param)+) | name ) ~
-                                              "}" ~ pro_whitespace_omitter? ~ "}}" }
-
-amp_expression = _{ "{{" ~ pre_whitespace_omitter? ~ "&" ~ name ~
-                       pro_whitespace_omitter? ~ "}}" }
+exp_line = _{ identifier ~ (hash|helper_parameter)* ~ block_param?}
+partial_exp_line = _{ ((partial_identifier|name) ~ (hash|helper_parameter)*) }
+
+subexpression = { "(" ~ ((identifier ~ (hash|helper_parameter)+) | reference)  ~ ")" }
+
+leading_tilde_to_omit_whitespace = { "~" }
+trailing_tilde_to_omit_whitespace = { "~" }
+
+expression = { !(invert_tag|invert_chain_tag) ~ "{{" ~ leading_tilde_to_omit_whitespace? ~
+              ((identifier ~ (hash|helper_parameter)+) | name )
+              ~ trailing_tilde_to_omit_whitespace? ~ "}}" }
+html_expression_triple_bracket_legacy = _{ "{{{" ~ leading_tilde_to_omit_whitespace? ~
+                                           ((identifier ~ (hash|helper_parameter)+) | name ) ~
+                                           trailing_tilde_to_omit_whitespace? ~ "}}}" }
+html_expression_triple_bracket = _{ "{{" ~ leading_tilde_to_omit_whitespace? ~ "{" ~
+                                              ((identifier ~ (hash|helper_parameter)+) | name ) ~
+                                              "}" ~ trailing_tilde_to_omit_whitespace? ~ "}}" }
+
+amp_expression = _{ "{{" ~ leading_tilde_to_omit_whitespace? ~ "&" ~ name ~
+                       trailing_tilde_to_omit_whitespace? ~ "}}" }
 html_expression = { (html_expression_triple_bracket_legacy | html_expression_triple_bracket)
                    | amp_expression }
 
-decorator_expression = { "{{" ~ pre_whitespace_omitter? ~ "*" ~ exp_line ~
-pro_whitespace_omitter? ~ "}}" }
-partial_expression = { "{{" ~ pre_whitespace_omitter? ~ ">" ~ partial_exp_line
-                     ~ pro_whitespace_omitter? ~ "}}" }
+decorator_expression = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "*" ~ exp_line ~
+trailing_tilde_to_omit_whitespace? ~ "}}" }
+partial_expression = { "{{" ~ leading_tilde_to_omit_whitespace? ~ ">" ~ partial_exp_line
+                     ~ trailing_tilde_to_omit_whitespace? ~ "}}" }
 
 invert_tag_item = { "else"|"^" }
-invert_tag = { !escape ~ "{{" ~ pre_whitespace_omitter? ~ invert_tag_item
-             ~ pro_whitespace_omitter? ~ "}}" }
-invert_chain_tag = { !escape ~ "{{" ~ pre_whitespace_omitter? ~ invert_tag_item
-                     ~ exp_line ~ pro_whitespace_omitter? ~ "}}" }
-helper_block_start = { "{{" ~ pre_whitespace_omitter? ~ "#" ~ exp_line ~
-                     pro_whitespace_omitter? ~ "}}" }
-helper_block_end = { "{{" ~ pre_whitespace_omitter? ~ "/" ~ identifier ~
-                   pro_whitespace_omitter? ~ "}}" }
+invert_tag = { !escape ~ "{{" ~ leading_tilde_to_omit_whitespace? ~ invert_tag_item
+             ~ trailing_tilde_to_omit_whitespace? ~ "}}" }
+invert_chain_tag = { !escape ~ "{{" ~ leading_tilde_to_omit_whitespace? ~ invert_tag_item
+                     ~ exp_line ~ trailing_tilde_to_omit_whitespace? ~ "}}" }
+helper_block_start = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "#" ~ exp_line ~
+                     trailing_tilde_to_omit_whitespace? ~ "}}" }
+helper_block_end = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ identifier ~
+                   trailing_tilde_to_omit_whitespace? ~ "}}" }
 helper_block = _{ helper_block_start ~ template ~
                   (invert_chain_tag ~ template)* ~ (invert_tag ~ template)? ~ helper_block_end }
 
-decorator_block_start = { "{{" ~ pre_whitespace_omitter? ~ "#" ~ "*"
-                        ~ exp_line ~ pro_whitespace_omitter? ~ "}}" }
-decorator_block_end = { "{{" ~ pre_whitespace_omitter? ~ "/" ~ identifier ~
-                        pro_whitespace_omitter? ~ "}}" }
+decorator_block_start = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "#" ~ "*"
+                        ~ exp_line ~ trailing_tilde_to_omit_whitespace? ~ "}}" }
+decorator_block_end = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ identifier ~
+                        trailing_tilde_to_omit_whitespace? ~ "}}" }
 decorator_block = _{ decorator_block_start ~ template ~
                      decorator_block_end }
 
-partial_block_start = { "{{" ~ pre_whitespace_omitter? ~ "#" ~ ">"
-                        ~ partial_exp_line ~ pro_whitespace_omitter? ~ "}}" }
-partial_block_end = { "{{" ~ pre_whitespace_omitter? ~ "/" ~ partial_identifier ~
-                      pro_whitespace_omitter? ~ "}}" }
+partial_block_start = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "#" ~ ">"
+                        ~ partial_exp_line ~ trailing_tilde_to_omit_whitespace? ~ "}}" }
+partial_block_end = { "{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ partial_identifier ~
+                      trailing_tilde_to_omit_whitespace? ~ "}}" }
 partial_block = _{ partial_block_start ~ template ~ partial_block_end }
 
-raw_block_start = { "{{{{" ~ pre_whitespace_omitter? ~ exp_line ~
-                    pro_whitespace_omitter? ~ "}}}}" }
-raw_block_end = { "{{{{" ~ pre_whitespace_omitter? ~ "/" ~ identifier ~
-                  pro_whitespace_omitter? ~ "}}}}" }
+raw_block_start = { "{{{{" ~ leading_tilde_to_omit_whitespace? ~ exp_line ~
+                    trailing_tilde_to_omit_whitespace? ~ "}}}}" }
+raw_block_end = { "{{{{" ~ leading_tilde_to_omit_whitespace? ~ "/" ~ identifier ~
+                  trailing_tilde_to_omit_whitespace? ~ "}}}}" }
 raw_block = _{ raw_block_start ~ raw_block_text ~ raw_block_end }
 
 hbs_comment = { "{{!" ~ "--" ~ (!"--}}" ~ ANY)* ~ "--" ~ "}}" }
@@ -121,7 +121,7 @@ template = { (
             partial_expression |
             partial_block )* }
 
-parameter = _{ param ~ EOI }
+parameter = _{ helper_parameter ~ EOI }
 handlebars = _{ template ~ EOI }
 
 /// json path visitor

diff --git a/src/grammar.rs b/src/grammar.rs
@@ -103,7 +103,7 @@ mod test {
     fn test_param() {
         let s = vec!["hello", "\"json literal\"", "nullable", "truestory"];
         for i in s.iter() {
-            assert_rule!(Rule::param, i);
+            assert_rule!(Rule::helper_parameter, i);
         }
     }
 

diff --git a/src/template.rs b/src/template.rs
@@ -368,7 +368,7 @@ impl Template {
         I: Iterator<Item = Pair<'a, Rule>>,
     {
         let mut param = it.next().unwrap();
-        if param.as_rule() == Rule::param {
+        if param.as_rule() == Rule::helper_parameter {
             param = it.next().unwrap();
         }
         let param_rule = param.as_rule();
@@ -482,7 +482,7 @@ impl Template {
         let mut omit_pro_ws = false;
         let mut block_param = None;
 
-        if it.peek().unwrap().as_rule() == Rule::pre_whitespace_omitter {
+        if it.peek().unwrap().as_rule() == Rule::leading_tilde_to_omit_whitespace {
             omit_pre_ws = true;
             it.next();
         }
@@ -507,7 +507,7 @@ impl Template {
             it.next();
 
             match rule {
-                Rule::param => {
+                Rule::helper_parameter => {
                     params.push(Template::parse_param(source, it.by_ref(), end)?);
                 }
                 Rule::hash => {
@@ -517,7 +517,7 @@ impl Template {
                 Rule::block_param => {
                     block_param = Some(Template::parse_block_param(source, it.by_ref(), end));
                 }
-                Rule::pro_whitespace_omitter => {
+                Rule::trailing_tilde_to_omit_whitespace => {
                     omit_pro_ws = true;
                 }
                 _ => {}
@@ -640,9 +640,11 @@ impl Template {
                 LineColLocation::Pos(line_col) => line_col,
                 LineColLocation::Span(line_col, _) => line_col,
             };
-            TemplateError::of(TemplateErrorReason::InvalidSyntax)
-                .at(source, line_no, col_no)
-                .in_template(options.name())
+            TemplateError::of(TemplateErrorReason::InvalidSyntax(
+                e.variant.message().to_string(),
+            ))
+            .at(source, line_no, col_no)
+            .in_template(options.name())
         })?;
 
         // dbg!(parser_queue.clone().flatten());
@@ -1133,7 +1135,10 @@ mod test {
 
         let terr = Template::compile(source).unwrap_err();
 
-        assert!(matches!(terr.reason(), TemplateErrorReason::InvalidSyntax));
+        assert!(matches!(
+            terr.reason(),
+            TemplateErrorReason::InvalidSyntax(_)
+        ));
         assert_eq!(terr.pos(), Some((4, 5)));
     }
 
@@ -1248,10 +1253,16 @@ mod test {
         let sources = ["{{invalid", "{{{invalid", "{{invalid}", "{{!hello"];
         for s in sources.iter() {
             let result = Template::compile(s.to_owned());
-            assert!(matches!(
-                *result.unwrap_err().reason(),
-                TemplateErrorReason::InvalidSyntax
-            ));
+            let err = result.expect_err("expected a syntax error");
+            let syntax_error_msg = match err.reason() {
+                TemplateErrorReason::InvalidSyntax(s) => s,
+                _ => panic!("InvalidSyntax expected"),
+            };
+            assert!(
+                syntax_error_msg.contains("expected identifier"),
+                "{}",
+                syntax_error_msg
+            );
         }
     }