Parser: Re-parse on unexpected ";" error for more accurate errors (marcoroth#902)

This pull request changes the way the parser handles the `unexpected
";"` errors.

In marcoroth#824, we changed the
`herb_extract_ruby_to_buffer_with_semicolons` method to only add
semicolons when needed. This pull request reverts that. But instead of
just outputting the `unexpected ";"` errors (as it was before marcoroth#824), we
check if the `;` was actually part of the source file.

If it wasn't part of the source file we re-parse this individual
`ERBContentNode` to check for syntax errors in the raw source of that
`ERBContentNode` instead, which shouldn't have the semicolon and thus
shouldn't append the `unexpected ";"` errors.

Resolves marcoroth#854
Resolves marcoroth#865
Resolves marcoroth#899

Author: marcoroth

java/snapshots/test.ruby.txt

@@ -34,7 +34,7 @@ describe("@herb-tools/browser", () => {
     const simpleHtml = '<div><%= "Hello World" %></div>'
     const ruby = Herb.extractRuby(simpleHtml)
     expect(ruby).toBeDefined()
-    expect(ruby).toBe('         "Hello World"         ')
+    expect(ruby).toBe('         "Hello World"  ;      ')
   })
 
   test("extractHTML() extracts HTML content", async () => {

javascript/packages/browser/test/browser.test.ts

@@ -57,7 +57,6 @@ describe("ParserNoErrorsRule", () => {
 
   test("should report Ruby parse errors in ERB tags", () => {
     expectError("expect_expression_after_operator: unexpected end-of-input; expected an expression after the operator (`RUBY_PARSE_ERROR`)")
-    expectError("unexpected_token_close_context: unexpected end-of-input, assuming it is closing the parent top level context (`RUBY_PARSE_ERROR`)")
     assertOffenses(`<%= 1 + %>`)
   })
 

javascript/packages/linter/test/rules/parser-no-errors.test.ts

@@ -34,7 +34,7 @@ describe("@herb-tools/node-wasm", () => {
     const simpleHtml = '<div><%= "Hello World" %></div>'
     const ruby = Herb.extractRuby(simpleHtml)
     expect(ruby).toBeDefined()
-    expect(ruby).toBe('         "Hello World"         ')
+    expect(ruby).toBe('         "Hello World"  ;      ')
   })
 
   test("extractHTML() extracts HTML content", async () => {

javascript/packages/node-wasm/test/node-wasm.test.ts

@@ -30,6 +30,7 @@
         "./extension/libherb/parser_helpers.c",
         "./extension/libherb/parser_match_tags.c",
         "./extension/libherb/parser.c",
+        "./extension/libherb/position.c",
         "./extension/libherb/pretty_print.c",
         "./extension/libherb/prism_helpers.c",
         "./extension/libherb/range.c",

javascript/packages/node/binding.gyp

@@ -34,7 +34,7 @@ describe("@herb-tools/node", () => {
     const simpleHtml = '<div><%= "Hello World" %></div>'
     const ruby = Herb.extractRuby(simpleHtml)
     expect(ruby).toBeDefined()
-    expect(ruby).toBe('         "Hello World"         ')
+    expect(ruby).toBe('         "Hello World"  ;      ')
   })
 
   test("extractHTML() extracts HTML content", async () => {

javascript/packages/node/test/node.test.ts

@@ -12,7 +12,7 @@ fn test_version_functions() {
 fn test_extract_ruby() {
   let source = "<div><%= name %></div>";
   let ruby = extract_ruby(source).unwrap();
-  assert_eq!(ruby, "         name         ");
+  assert_eq!(ruby, "         name  ;      ");
 }
 
 #[test]
@@ -32,7 +32,7 @@ fn test_extract_ruby_complex() {
   let ruby = extract_ruby(source).unwrap();
   assert_eq!(
     ruby,
-    "     \n     users.each do |user|   \n           user.name       \n     end   \n      "
+    "     \n     users.each do |user|  ;\n           user.name  ;    \n     end  ;\n      "
   );
 }
 

rust/tests/cli_commands_test.rs

@@ -1409,6 +1409,35 @@ void herb_analyze_parse_tree(AST_DOCUMENT_NODE_T* document, const char* source)
   free(invalid_context);
 }
 
+static void parse_erb_content_errors(AST_NODE_T* erb_node, const char* source) {
+  if (!erb_node || erb_node->type != AST_ERB_CONTENT_NODE) { return; }
+  AST_ERB_CONTENT_NODE_T* content_node = (AST_ERB_CONTENT_NODE_T*) erb_node;
+
+  if (!content_node->content || !content_node->content->value) { return; }
+
+  const char* content = content_node->content->value;
+  if (strlen(content) == 0) { return; }
+
+  pm_parser_t parser;
+  pm_options_t options = { 0, .partial_script = true };
+  pm_parser_init(&parser, (const uint8_t*) content, strlen(content), &options);
+
+  pm_node_t* root = pm_parse(&parser);
+
+  const pm_diagnostic_t* error = (const pm_diagnostic_t*) parser.error_list.head;
+
+  if (error != NULL) {
+    RUBY_PARSE_ERROR_T* parse_error =
+      ruby_parse_error_from_prism_error_with_positions(error, erb_node->location.start, erb_node->location.end);
+
+    hb_array_append(erb_node->errors, parse_error);
+  }
+
+  pm_node_destroy(&parser, root);
+  pm_parser_free(&parser);
+  pm_options_free(&options);
+}
+
 void herb_analyze_parse_errors(AST_DOCUMENT_NODE_T* document, const char* source) {
   char* extracted_ruby = herb_extract_ruby_with_semicolons(source);
 
@@ -1422,6 +1451,19 @@ void herb_analyze_parse_errors(AST_DOCUMENT_NODE_T* document, const char* source
 
   for (const pm_diagnostic_t* error = (const pm_diagnostic_t*) parser.error_list.head; error != NULL;
        error = (const pm_diagnostic_t*) error->node.next) {
+    size_t error_offset = (size_t) (error->location.start - parser.start);
+
+    if (strstr(error->message, "unexpected ';'") != NULL) {
+      if (error_offset < strlen(extracted_ruby) && extracted_ruby[error_offset] == ';') {
+        if (error_offset >= strlen(source) || source[error_offset] != ';') {
+          AST_NODE_T* erb_node = find_erb_content_at_offset(document, source, error_offset);
+
+          if (erb_node) { parse_erb_content_errors(erb_node, source); }
+
+          continue;
+        }
+      }
+    }
 
     RUBY_PARSE_ERROR_T* parse_error = ruby_parse_error_from_prism_error(error, (AST_NODE_T*) document, source, &parser);
     hb_array_append(document->base.errors, parse_error);

rust/tests/snapshots/snapshot_test__extract_ruby_output.snap

@@ -1,8 +1,10 @@
 #include "include/ast_node.h"
 #include "include/ast_nodes.h"
 #include "include/errors.h"
+#include "include/position.h"
 #include "include/token.h"
 #include "include/util.h"
+#include "include/visitor.h"
 
 #include <prism.h>
 #include <stdio.h>
@@ -76,3 +78,30 @@ void ast_node_set_positions_from_token(AST_NODE_T* node, const token_T* token) {
 bool ast_node_is(const AST_NODE_T* node, const ast_node_type_T type) {
   return node->type == type;
 }
+
+typedef struct {
+  position_T position;
+  AST_NODE_T* found_node;
+} find_erb_at_position_context_T;
+
+static bool find_erb_at_position_visitor(const AST_NODE_T* node, void* data) {
+  find_erb_at_position_context_T* context = (find_erb_at_position_context_T*) data;
+
+  if (node->type == AST_ERB_CONTENT_NODE) {
+    if (position_is_within_range(context->position, node->location.start, node->location.end)) {
+      context->found_node = (AST_NODE_T*) node;
+      return false;
+    }
+  }
+
+  return true;
+}
+
+AST_NODE_T* find_erb_content_at_offset(AST_DOCUMENT_NODE_T* document, const char* source, size_t offset) {
+  position_T position = position_from_source_with_offset(source, offset);
+  find_erb_at_position_context_T context = { .position = position, .found_node = NULL };
+
+  herb_visit_node((AST_NODE_T*) document, find_erb_at_position_visitor, &context);
+
+  return context.found_node;
+}