Parser: Introduce two-pass algorithm for matching HTML Elements

.clang-format-ignore

@@ -1,9 +1,12 @@
 ext/herb/error_helpers.c
 ext/herb/nodes.c
+src/analyze_missing_end.c
+src/analyze_transform.c
 src/ast_nodes.c
 src/ast_pretty_print.c
 src/errors.c
 src/include/ast_nodes.h
 src/include/ast_pretty_print.h
 src/include/errors.h
+src/parser_match_tags.c
 src/visitor.c

.gitignore

@@ -113,12 +113,15 @@ rust/src/errors.rs
 rust/src/nodes.rs
 sig/serialized_ast_errors.rbs
 sig/serialized_ast_nodes.rbs
+src/analyze_missing_end.c
+src/analyze_transform.c
 src/ast_nodes.c
 src/ast_pretty_print.c
 src/errors.c
 src/include/ast_nodes.h
 src/include/ast_pretty_print.h
 src/include/errors.h
+src/parser_match_tags.c
 src/visitor.c
 wasm/error_helpers.cpp
 wasm/error_helpers.h

config.yml

@@ -46,7 +46,7 @@ errors:
 
     - name: MissingOpeningTagError
       message:
-        template: "Found closing tag `</%s>` at (%u:%u) without a matching opening tag."
+        template: "Found closing tag `</%s>` at (%u:%u) without a matching opening tag in the same scope."
         arguments:
           - closing_tag->value
           - closing_tag->location.start.line
@@ -58,7 +58,7 @@ errors:
 
     - name: MissingClosingTagError
       message:
-        template: "Opening tag `<%s>` at (%u:%u) doesn't have a matching closing tag `</%s>`."
+        template: "Opening tag `<%s>` at (%u:%u) doesn't have a matching closing tag `</%s>` in the same scope."
         arguments:
           - opening_tag->value
           - opening_tag->location.start.line
@@ -151,6 +151,26 @@ errors:
         - name: level
           type: string
 
+    - name: ERBControlFlowScopeError
+      message:
+        template: "%s appears outside its control flow block. Keep ERB control flow statements together within the same HTML scope (tag, attribute, or content)."
+        arguments:
+          - keyword
+
+      fields:
+        - name: keyword
+          type: string
+
+    - name: MissingERBEndTagError
+      message:
+        template: "%s started here but never closed with an end tag. The end tag may be in a different scope."
+        arguments:
+          - keyword
+
+      fields:
+        - name: keyword
+          type: string
+
 warnings:
   fields: []
   types: []

ext/herb/extension.c

@@ -78,6 +78,8 @@ static VALUE Herb_parse_file(VALUE self, VALUE path) {
 
   AST_DOCUMENT_NODE_T* root = herb_parse(string, NULL);
 
+  herb_analyze_parse_tree(root, string);
+
   VALUE result = create_parse_result(root, source_value);
 
   ast_node_free((AST_NODE_T*) root);

javascript/packages/formatter/src/format-printer.ts

@@ -1056,8 +1056,8 @@ export class FormatPrinter extends Printer {
       inner = node.children.map(child => {
         if (isNode(child, HTMLTextNode) || isNode(child, LiteralNode)) {
           return child.content
-        } else if (isERBNode(child) || isNode(child, ERBContentNode)) {
-          return this.reconstructERBNode(child, false)
+        } else if (isERBNode(child)) {
+          return IdentityPrinter.print(child)
         } else {
           return ""
         }

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

@@ -17,10 +17,8 @@ describe("ParserNoErrorsRule", () => {
   })
 
   test("should report errors for mismatched tag names", () => {
-    expectError("Tag `<h3>` opened at (3:1) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Tag `<h2>` opened at (1:1) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Opening tag `<h2>` at (1:1) doesn't have a matching closing tag `</h2>`. (`MISSING_CLOSING_TAG_ERROR`)")
-    expectError("Opening tag `<h3>` at (3:1) doesn't have a matching closing tag `</h3>`. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Opening tag `<h2>` at (1:1) doesn't have a matching closing tag `</h2>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Opening tag `<h3>` at (3:1) doesn't have a matching closing tag `</h3>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
     assertOffenses(dedent`
       <h2>
         Some content
@@ -29,10 +27,7 @@ describe("ParserNoErrorsRule", () => {
   })
 
   test("should report errors for unclosed elements", () => {
-    expectError("Tag `<p>` opened at (2:3) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Tag `<div>` opened at (1:1) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Opening tag `<div>` at (1:1) doesn't have a matching closing tag `</div>`. (`MISSING_CLOSING_TAG_ERROR`)")
-    expectError("Opening tag `<p>` at (2:3) closed with `</div>` at (3:2). (`TAG_NAMES_MISMATCH_ERROR`)")
+    expectError("Opening tag `<p>` at (2:3) doesn't have a matching closing tag `</p>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
     assertOffenses(dedent`
       <div>
         <p>Some content
@@ -46,7 +41,7 @@ describe("ParserNoErrorsRule", () => {
   })
 
   test("should report errors for missing opening tags", () => {
-    expectError("Found closing tag `</div>` at (2:2) without a matching opening tag. (`MISSING_OPENING_TAG_ERROR`)")
+    expectError("Found closing tag `</div>` at (2:2) without a matching opening tag in the same scope. (`MISSING_OPENING_TAG_ERROR`)")
     assertOffenses(dedent`
       Some content
       </div>
@@ -66,10 +61,10 @@ describe("ParserNoErrorsRule", () => {
   })
 
   test("should report multiple parser errors", () => {
-    expectError("Tag `<p>` opened at (2:3) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Tag `<h2>` opened at (1:1) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Opening tag `<p>` at (2:3) closed with `</div>` at (4:2). (`TAG_NAMES_MISMATCH_ERROR`)")
-    expectError("Opening tag `<p>` at (2:3) closed with `</h3>` at (3:2). (`TAG_NAMES_MISMATCH_ERROR`)")
+    expectError("Opening tag `<h2>` at (1:1) doesn't have a matching closing tag `</h2>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Opening tag `<p>` at (2:3) doesn't have a matching closing tag `</p>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Found closing tag `</h3>` at (3:2) without a matching opening tag in the same scope. (`MISSING_OPENING_TAG_ERROR`)")
+    expectError("Found closing tag `</div>` at (4:2) without a matching opening tag in the same scope. (`MISSING_OPENING_TAG_ERROR`)")
     assertOffenses(dedent`
       <h2>
         <p>Unclosed paragraph
@@ -79,8 +74,8 @@ describe("ParserNoErrorsRule", () => {
   })
 
   test("should work alongside other linting rules", () => {
-    expectError("Tag `<h2>` opened at (1:1) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Opening tag `<h2>` at (1:1) closed with `</h3>` at (3:2). (`TAG_NAMES_MISMATCH_ERROR`)")
+    expectError("Opening tag `<h2>` at (1:1) doesn't have a matching closing tag `</h2>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Found closing tag `</h3>` at (3:2) without a matching opening tag in the same scope. (`MISSING_OPENING_TAG_ERROR`)")
     assertOffenses(dedent`
       <h2>
         <% %>
@@ -89,10 +84,8 @@ describe("ParserNoErrorsRule", () => {
   })
 
   test("should include error location information", () => {
-    expectError("Tag `<h2>` opened at (2:3) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Tag `<div>` opened at (1:1) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Opening tag `<h2>` at (2:3) closed with `</div>` at (3:2). (`TAG_NAMES_MISMATCH_ERROR`)")
-    expectError("Opening tag `<h2>` at (2:3) closed with `</h3>` at (2:15). (`TAG_NAMES_MISMATCH_ERROR`)")
+    expectError("Opening tag `<h2>` at (2:3) doesn't have a matching closing tag `</h2>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Found closing tag `</h3>` at (2:15) without a matching opening tag in the same scope. (`MISSING_OPENING_TAG_ERROR`)")
     assertOffenses(dedent`
       <div>
         <h2>Content</h3>
@@ -101,10 +94,8 @@ describe("ParserNoErrorsRule", () => {
   })
 
   test("should handle the specific case from issue #359", () => {
-    expectError("Tag `<h2>` opened at (1:25) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Tag `<h2>` opened at (1:1) was never closed before the end of document. (`UNCLOSED_ELEMENT_ERROR`)")
-    expectError("Opening tag `<h2>` at (1:1) doesn't have a matching closing tag `</h2>`. (`MISSING_CLOSING_TAG_ERROR`)")
-    expectError("Opening tag `<h2>` at (1:25) doesn't have a matching closing tag `</h2>`. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Opening tag `<h2>` at (1:1) doesn't have a matching closing tag `</h2>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
+    expectError("Opening tag `<h2>` at (1:25) doesn't have a matching closing tag `</h2>` in the same scope. (`MISSING_CLOSING_TAG_ERROR`)")
     assertOffenses(dedent`
       <h2>Some heading content<h2>
     `)

javascript/packages/node/binding.gyp

@@ -11,6 +11,8 @@
 
         # Herb main source files
         "./extension/libherb/analyze_helpers.c",
+        "./extension/libherb/analyze_missing_end.c",
+        "./extension/libherb/analyze_transform.c",
         "./extension/libherb/analyze.c",
         "./extension/libherb/analyzed_ruby.c",
         "./extension/libherb/ast_node.c",
@@ -26,6 +28,7 @@
         "./extension/libherb/lexer.c",
         "./extension/libherb/location.c",
         "./extension/libherb/parser_helpers.c",
+        "./extension/libherb/parser_match_tags.c",
         "./extension/libherb/parser.c",
         "./extension/libherb/pretty_print.c",
         "./extension/libherb/prism_helpers.c",

rust/tests/error_handling_test.rs

@@ -10,10 +10,9 @@ fn test_unclosed_element_error() {
   let result = parse(source).unwrap();
 
   let tree_inspect = result.inspect();
-  assert!(tree_inspect.contains("UnclosedElementError"));
-  assert!(tree_inspect.contains("Tag `<div>` opened at (1:1) was never closed"));
+
   assert!(tree_inspect.contains("MissingClosingTagError"));
-  assert!(tree_inspect.contains("Opening tag `<div>` at (1:1) doesn't have a matching closing tag"));
+  assert!(tree_inspect.contains("Opening tag `<div>` at (1:1) doesn't have a matching closing"));
 }
 
 #[test]
@@ -22,10 +21,19 @@ fn test_tag_names_mismatch_error() {
 
   let source = "<div></span>";
   let result = parse(source).unwrap();
-
   let tree_inspect = result.inspect();
-  assert!(tree_inspect.contains("TagNamesMismatchError"));
-  assert!(tree_inspect.contains("Opening tag `<div>` at (1:1) closed with `</span>`"));
+
+  print!("{}", tree_inspect);
+
+  assert!(tree_inspect.contains("MissingClosingTagError (location: (1:0)-(1:5))"));
+  assert!(tree_inspect.contains(
+    "Opening tag `<div>` at (1:1) doesn't have a matching closing tag `</div>` in the same scope."
+  ));
+
+  assert!(tree_inspect.contains("MissingOpeningTagError (location: (1:5)-(1:12))"));
+  assert!(tree_inspect.contains(
+    "Found closing tag `</span>` at (1:7) without a matching opening tag in the same scope."
+  ));
 }
 
 #[test]
@@ -34,8 +42,8 @@ fn test_no_errors_with_valid_html() {
 
   let source = "<div>Hello</div>";
   let result = parse(source).unwrap();
-
   let tree_inspect = result.inspect();
+
   assert!(!tree_inspect.contains("error"));
   assert!(!tree_inspect.contains("ERROR"));
 }

rust/tests/parse_result_test.rs

@@ -49,10 +49,6 @@ fn test_parse_result_failed_with_unclosed_element() {
 
   let errors = result.recursive_errors();
 
-  assert!(errors
-    .iter()
-    .any(|e| e.error_type() == "UNCLOSED_ELEMENT_ERROR"));
-
   assert!(errors
     .iter()
     .any(|e| e.error_type() == "MISSING_CLOSING_TAG_ERROR"));
@@ -81,7 +77,11 @@ fn test_parse_result_failed_with_tag_mismatch() {
 
   assert!(errors
     .iter()
-    .any(|e| e.error_type() == "TAG_NAMES_MISMATCH_ERROR"));
+    .any(|e| e.error_type() == "MISSING_OPENING_TAG_ERROR"));
+
+  assert!(errors
+    .iter()
+    .any(|e| e.error_type() == "MISSING_CLOSING_TAG_ERROR"));
 }
 
 #[test]