Parser: Improve error recovery for ERB tags (#1170)

Author: marcoroth

config.yml

@@ -253,6 +253,38 @@ errors:
         - name: opening_quote
           type: token
 
+    - name: UnclosedERBTagError
+      message:
+        template: "ERB tag `%s` at (%u:%u) is missing closing `%%>`."
+        arguments:
+          - opening_tag->value
+          - opening_tag->location.start.line
+          - opening_tag->location.start.column
+
+      fields:
+        - name: opening_tag
+          type: token
+
+    - name: NestedERBTagError
+      message:
+        template: "ERB tag `%s` at (%u:%u) was terminated by nested `<%%` tag at (%u:%u). Nesting `<%%` tags is not supported."
+        arguments:
+          - opening_tag->value
+          - opening_tag->location.start.line
+          - opening_tag->location.start.column
+          - nested_tag_line
+          - nested_tag_column
+
+      fields:
+        - name: opening_tag
+          type: token
+
+        - name: nested_tag_line
+          type: size_t
+
+        - name: nested_tag_column
+          type: size_t
+
 warnings:
   fields: []
   types: []

sig/herb/errors.rbs

@@ -26,6 +26,16 @@
 #include <stdlib.h>
 #include <string.h>
 
+static position_T erb_content_end_position(const AST_ERB_CONTENT_NODE_T* erb_node) {
+  if (erb_node->tag_closing != NULL) {
+    return erb_node->tag_closing->location.end;
+  } else if (erb_node->content != NULL) {
+    return erb_node->content->location.end;
+  } else {
+    return erb_node->tag_opening->location.end;
+  }
+}
+
 static analyzed_ruby_T* herb_analyze_ruby(hb_string_T source) {
   analyzed_ruby_T* analyzed = init_analyzed_ruby(source);
 
@@ -279,11 +289,11 @@ static control_type_t find_earliest_control_keyword(pm_node_t* root, const uint8
 
 static control_type_t detect_control_type(AST_ERB_CONTENT_NODE_T* erb_node) {
   if (!erb_node || erb_node->base.type != AST_ERB_CONTENT_NODE) { return CONTROL_TYPE_UNKNOWN; }
+  if (erb_node->tag_closing == NULL) { return CONTROL_TYPE_UNKNOWN; }
 
   analyzed_ruby_T* ruby = erb_node->analyzed_ruby;
 
   if (!ruby) { return CONTROL_TYPE_UNKNOWN; }
-
   if (ruby->valid) { return CONTROL_TYPE_UNKNOWN; }
 
   pm_node_t* root = ruby->root;
@@ -340,7 +350,7 @@ static AST_NODE_T* create_control_node(
   erb_node->base.errors = NULL;
 
   position_T start_position = erb_node->tag_opening->location.start;
-  position_T end_position = erb_node->tag_closing->location.end;
+  position_T end_position = erb_content_end_position(erb_node);
 
   if (end_node) {
     end_position = end_node->base.location.end;
@@ -695,14 +705,16 @@ static size_t process_control_structure(
           }
         }
 
+        position_T when_end_position = erb_content_end_position(erb_content);
+
         AST_ERB_WHEN_NODE_T* when_node = ast_erb_when_node_init(
           erb_content->tag_opening,
           erb_content->content,
           erb_content->tag_closing,
           then_keyword,
           when_statements,
           erb_content->tag_opening->location.start,
-          erb_content->tag_closing->location.end,
+          when_end_position,
           when_errors
         );
 
@@ -736,14 +748,16 @@ static size_t process_control_structure(
           }
         }
 
+        position_T in_end_position = erb_content_end_position(erb_content);
+
         AST_ERB_IN_NODE_T* in_node = ast_erb_in_node_init(
           erb_content->tag_opening,
           erb_content->content,
           erb_content->tag_closing,
           in_then_keyword,
           in_statements,
           erb_content->tag_opening->location.start,
-          erb_content->tag_closing->location.end,
+          in_end_position,
           in_errors
         );
 
@@ -785,7 +799,7 @@ static size_t process_control_structure(
             next_erb->tag_closing,
             else_children,
             next_erb->tag_opening->location.start,
-            next_erb->tag_closing->location.end,
+            erb_content_end_position(next_erb),
             else_errors
           );
 
@@ -811,7 +825,7 @@ static size_t process_control_structure(
             end_erb->content,
             end_erb->tag_closing,
             end_erb->tag_opening->location.start,
-            end_erb->tag_closing->location.end,
+            erb_content_end_position(end_erb),
             end_errors
           );
 
@@ -823,7 +837,7 @@ static size_t process_control_structure(
     }
 
     position_T start_position = erb_node->tag_opening->location.start;
-    position_T end_position = erb_node->tag_closing->location.end;
+    position_T end_position = erb_content_end_position(erb_node);
 
     if (end_node) {
       end_position = end_node->base.location.end;
@@ -933,7 +947,7 @@ static size_t process_control_structure(
             next_erb->tag_closing,
             else_children,
             next_erb->tag_opening->location.start,
-            next_erb->tag_closing->location.end,
+            erb_content_end_position(next_erb),
             else_errors
           );
 
@@ -979,7 +993,7 @@ static size_t process_control_structure(
             next_erb->tag_closing,
             ensure_children,
             next_erb->tag_opening->location.start,
-            next_erb->tag_closing->location.end,
+            erb_content_end_position(next_erb),
             ensure_errors
           );
 
@@ -1005,7 +1019,7 @@ static size_t process_control_structure(
             end_erb->content,
             end_erb->tag_closing,
             end_erb->tag_opening->location.start,
-            end_erb->tag_closing->location.end,
+            erb_content_end_position(end_erb),
             end_errors
           );
 
@@ -1017,7 +1031,7 @@ static size_t process_control_structure(
     }
 
     position_T start_position = erb_node->tag_opening->location.start;
-    position_T end_position = erb_node->tag_closing->location.end;
+    position_T end_position = erb_content_end_position(erb_node);
 
     if (end_node) {
       end_position = end_node->base.location.end;
@@ -1068,12 +1082,14 @@ static size_t process_control_structure(
           hb_array_T* end_errors = close_erb->base.errors;
           close_erb->base.errors = NULL;
 
+          position_T close_end_pos = erb_content_end_position(close_erb);
+
           end_node = ast_erb_end_node_init(
             close_erb->tag_opening,
             close_erb->content,
             close_erb->tag_closing,
             close_erb->tag_opening->location.start,
-            close_erb->tag_closing->location.end,
+            close_end_pos,
             end_errors
           );
 
@@ -1085,7 +1101,7 @@ static size_t process_control_structure(
     }
 
     position_T start_position = erb_node->tag_opening->location.start;
-    position_T end_position = erb_node->tag_closing->location.end;
+    position_T end_position = erb_content_end_position(erb_node);
 
     if (end_node) {
       end_position = end_node->base.location.end;
@@ -1142,12 +1158,14 @@ static size_t process_control_structure(
         hb_array_T* end_errors = end_erb->base.errors;
         end_erb->base.errors = NULL;
 
+        position_T end_erb_final_pos = erb_content_end_position(end_erb);
+
         end_node = ast_erb_end_node_init(
           end_erb->tag_opening,
           end_erb->content,
           end_erb->tag_closing,
           end_erb->tag_opening->location.start,
-          end_erb->tag_closing->location.end,
+          end_erb_final_pos,
           end_errors
         );
 

src/analyze.c

@@ -30,6 +30,7 @@ bool lexer_peek_for_html_comment_start(const lexer_T* lexer, uint32_t offset);
 bool lexer_peek_for_html_comment_end(const lexer_T* lexer, uint32_t offset);
 bool lexer_peek_for_html_comment_invalid_end(const lexer_T* lexer, uint32_t offset);
 
+bool lexer_peek_erb_start(const lexer_T* lexer, uint32_t offset);
 bool lexer_peek_erb_close_tag(const lexer_T* lexer, uint32_t offset);
 bool lexer_peek_erb_dash_close_tag(const lexer_T* lexer, uint32_t offset);
 bool lexer_peek_erb_percent_close_tag(const lexer_T* lexer, uint32_t offset);

src/include/lexer_peek_helpers.h

@@ -204,11 +204,17 @@ static token_T* lexer_parse_erb_content(lexer_T* lexer) {
 
   while (!lexer_peek_erb_end(lexer, 0)) {
     if (lexer_eof(lexer)) {
-      token_T* token = token_init(
-        hb_string_range(lexer->source, start_position, lexer->current_position),
-        TOKEN_ERROR,
-        lexer
-      ); // Handle unexpected EOF
+      token_T* token =
+        token_init(hb_string_range(lexer->source, start_position, lexer->current_position), TOKEN_ERB_CONTENT, lexer);
+
+      return token;
+    }
+
+    if (lexer_peek_erb_start(lexer, 0)) {
+      lexer->state = STATE_DATA;
+
+      token_T* token =
+        token_init(hb_string_range(lexer->source, start_position, lexer->current_position), TOKEN_ERB_CONTENT, lexer);
 
       return token;
     }

src/lexer.c

@@ -78,6 +78,10 @@ bool lexer_peek_erb_end(const lexer_T* lexer, uint32_t offset) {
   );
 }
 
+bool lexer_peek_erb_start(const lexer_T* lexer, uint32_t offset) {
+  return lexer_peek_for(lexer, offset, hb_string("<%"), false);
+}
+
 bool lexer_peek_for_token_type_after_whitespace(lexer_T* lexer, token_type_T token_type) {
   uint32_t saved_position = lexer->current_position;
   uint32_t saved_line = lexer->current_line;

src/lexer_peek_helpers.c

@@ -914,7 +914,7 @@ static AST_HTML_OPEN_TAG_NODE_T* parser_parse_html_open_tag(parser_T* parser) {
   token_T* tag_name = parser_consume_expected(parser, TOKEN_IDENTIFIER, errors);
 
   while (token_is_none_of(parser, TOKEN_HTML_TAG_END, TOKEN_HTML_TAG_SELF_CLOSE, TOKEN_EOF)) {
-    if (token_is(parser, TOKEN_HTML_TAG_START)) {
+    if (token_is_any_of(parser, TOKEN_HTML_TAG_START, TOKEN_HTML_TAG_START_CLOSE)) {
       append_unclosed_open_tag_error(tag_name, tag_name->location.start, parser->current_token->location.start, errors);
 
       AST_HTML_OPEN_TAG_NODE_T* open_tag_node = ast_html_open_tag_node_init(
@@ -1190,7 +1190,32 @@ static AST_ERB_CONTENT_NODE_T* parser_parse_erb_tag(parser_T* parser) {
 
   token_T* opening_tag = parser_consume_expected(parser, TOKEN_ERB_START, errors);
   token_T* content = parser_consume_expected(parser, TOKEN_ERB_CONTENT, errors);
-  token_T* closing_tag = parser_consume_expected(parser, TOKEN_ERB_END, errors);
+
+  token_T* closing_tag = NULL;
+  position_T end_position;
+
+  if (token_is(parser, TOKEN_ERB_END)) {
+    closing_tag = parser_consume_expected(parser, TOKEN_ERB_END, errors);
+    end_position = closing_tag->location.end;
+  } else if (token_is(parser, TOKEN_ERB_START)) {
+    append_nestederb_tag_error(
+      opening_tag,
+      parser->current_token->location.start.line,
+      parser->current_token->location.start.column,
+      parser->current_token->location.start,
+      parser->current_token->location.end,
+      errors
+    );
+    end_position = parser->current_token->location.start;
+  } else {
+    append_unclosederb_tag_error(
+      opening_tag,
+      opening_tag->location.start,
+      parser->current_token->location.start,
+      errors
+    );
+    end_position = parser->current_token->location.start;
+  }
 
   AST_ERB_CONTENT_NODE_T* erb_node = ast_erb_content_node_init(
     opening_tag,
@@ -1200,13 +1225,13 @@ static AST_ERB_CONTENT_NODE_T* parser_parse_erb_tag(parser_T* parser) {
     false,
     false,
     opening_tag->location.start,
-    closing_tag->location.end,
+    end_position,
     errors
   );
 
   token_free(opening_tag);
   token_free(content);
-  token_free(closing_tag);
+  if (closing_tag != NULL) { token_free(closing_tag); }
 
   return erb_node;
 }

src/parser.c

@@ -251,7 +251,7 @@ class EvaluationTest < Minitest::Spec
         <div>
           <%# This is a comment %>
           <p>Visible content</p>
-          <%# Another comment with <%= "erb" %> inside %>
+          <%# Another comment %>
           <p>More content</p>
         </div>
       ERB