Parser: Treat javascript_tag content as foreign content (#1434)

This pull request fixes how the parser handles `javascript_tag do`
blocks when the body contains HTML-like syntax such as `<` operators in
JavaScript expressions (e.g. `n <o.length, for (let i = 0;
i<items.length; i++)`).

Previously, the outer HTML parser would interpret `<` inside a
`javascript_tag do` body as an HTML tag opening, creating a broken
`HTMLOpenTagNode` that swallowed the `<% end %>` closing tag. This
caused a null dereference in the analyzer when it tried to access
`block_node->end_node`.

The fix has two parts:

1. `html` parser option and foreign content re-parsing

A new `html` parser option (defaults to `true`) controls whether the
parser runs in HTML mode or foreign content mode. When `html: false`,
the parser treats everything as literal text and ERB tags, no HTML tag
parsing. The analyzer now re-parses `javascript_tag do` body content
with `html: false` to produce clean `LiteralNode`s, regardless of what
the outer HTML parse produced.

New `start_line` and `start_column` options allow the re-parsed body to
carry correct source locations from the original template.

2. Swallowed `<% end %>` recovery

When the outer HTML parse swallows `<% end %>` inside a broken
`HTMLOpenTagNode`, the analyzer walks the AST to find the swallowed
`ERBContentNode` with `end` content. It creates a proper `ERBEndNode`
from it and uses it as the `close_tag`, with stale
`ERBControlFlowScopeError` cleared.

For example, the following template with `action_view_helpers: true`:

```erb
<%= javascript_tag do %>
  n <o.length
<% end %>
```

Now correctly produces:
```js
@ DocumentNode (location: (1:0)-(3:9))
└── children: (1 item)
    └── @ HTMLElementNode (location: (1:0)-(3:9))
        ├── open_tag:
        │   └── @ ERBOpenTagNode (location: (1:0)-(1:24))
        │       ├── tag_opening: "<%=" (location: (1:0)-(1:3))
        │       ├── content: " javascript_tag do " (location: (1:3)-(1:22))
        │       ├── tag_closing: "%>" (location: (1:22)-(1:24))
        │       ├── tag_name: "script" (location: (1:4)-(1:18))
        │       └── children: []
        │
        ├── tag_name: "script" (location: (1:4)-(1:18))
        ├── body: (1 item)
        │   └── @ LiteralNode (location: (1:24)-(3:0))
        │       └── content: "\n  n <o.length\n"
        │
        ├── close_tag:
        │   └── @ ERBEndNode (location: (3:0)-(3:9))
        │       ├── tag_opening: "<%" (location: (3:0)-(3:2))
        │       ├── content: " end " (location: (3:2)-(3:7))
        │       └── tag_closing: "%>" (location: (3:7)-(3:9))
        │
        ├── is_void: false
        └── element_source: "ActionView::Helpers::JavaScriptHelper#javascript_tag"
```


Resolves #1426

Author: marcoroth

ext/herb/extension.c

@@ -156,6 +156,10 @@ static VALUE Herb_parse(int argc, VALUE* argv, VALUE self) {
     if (NIL_P(prism_program)) { prism_program = rb_hash_lookup(options, ID2SYM(rb_intern("prism_program"))); }
     if (!NIL_P(prism_program) && RTEST(prism_program)) { parser_options.prism_program = true; }
 
+    VALUE html = rb_hash_lookup(options, rb_utf8_str_new_cstr("html"));
+    if (NIL_P(html)) { html = rb_hash_lookup(options, ID2SYM(rb_intern("html"))); }
+    if (!NIL_P(html) && !RTEST(html)) { parser_options.html = false; }
+
     VALUE arena_stats = rb_hash_lookup(options, rb_utf8_str_new_cstr("arena_stats"));
     if (NIL_P(arena_stats)) { arena_stats = rb_hash_lookup(options, ID2SYM(rb_intern("arena_stats"))); }
     if (!NIL_P(arena_stats) && RTEST(arena_stats)) { print_arena_stats = true; }

java/herb_jni.c

@@ -108,6 +108,14 @@ Java_org_herb_Herb_parse(JNIEnv* env, jclass clazz, jstring source, jobject opti
       jboolean prismProgram = (*env)->CallBooleanMethod(env, options, getPrismProgram);
       parser_options.prism_program = (prismProgram == JNI_TRUE);
     }
+
+    jmethodID getHtml =
+        (*env)->GetMethodID(env, optionsClass, "isHtml", "()Z");
+
+    if (getHtml != NULL) {
+      jboolean html = (*env)->CallBooleanMethod(env, options, getHtml);
+      parser_options.html = (html == JNI_TRUE);
+    }
   }
 
   hb_allocator_T allocator;

java/org/herb/ParserOptions.java

@@ -10,6 +10,7 @@ public class ParserOptions {
   private boolean prismNodes = false;
   private boolean prismNodesDeep = false;
   private boolean prismProgram = false;
+  private boolean html = true;
 
   public ParserOptions() {}
 
@@ -94,6 +95,15 @@ public boolean isPrismProgram() {
     return prismProgram;
   }
 
+  public ParserOptions html(boolean value) {
+    this.html = value;
+    return this;
+  }
+
+  public boolean isHtml() {
+    return html;
+  }
+
   public static ParserOptions create() {
     return new ParserOptions();
   }

javascript/packages/core/src/parser-options.ts

@@ -8,6 +8,7 @@ export interface ParseOptions {
   prism_nodes?: boolean
   prism_nodes_deep?: boolean
   prism_program?: boolean
+  html?: boolean
 }
 
 export type SerializedParserOptions = Required<ParseOptions>
@@ -22,6 +23,7 @@ export const DEFAULT_PARSER_OPTIONS: SerializedParserOptions = {
   prism_nodes: false,
   prism_nodes_deep: false,
   prism_program: false,
+  html: true,
 }
 
 /**
@@ -55,6 +57,9 @@ export class ParserOptions {
   /** Whether the full Prism ProgramNode was serialized on the DocumentNode. */
   readonly prism_program: boolean
 
+  /** Whether HTML tag parsing is enabled during parsing. When false, HTML-like content is treated as literal text. */
+  readonly html: boolean
+
   static from(options: SerializedParserOptions): ParserOptions {
     return new ParserOptions(options)
   }
@@ -69,5 +74,6 @@ export class ParserOptions {
     this.prism_nodes = options.prism_nodes ?? DEFAULT_PARSER_OPTIONS.prism_nodes
     this.prism_nodes_deep = options.prism_nodes_deep ?? DEFAULT_PARSER_OPTIONS.prism_nodes_deep
     this.prism_program = options.prism_program ?? DEFAULT_PARSER_OPTIONS.prism_program
+    this.html = options.html ?? DEFAULT_PARSER_OPTIONS.html
   }
 }

javascript/packages/linter/test/parse-cache.test.ts

@@ -75,6 +75,7 @@ describe("ParseCache", () => {
         strict: true,
         strict_locals: false,
         action_view_helpers: false,
+        html: true,
       })
     })
 
@@ -92,6 +93,7 @@ describe("ParseCache", () => {
         strict: false,
         strict_locals: false,
         action_view_helpers: false,
+        html: true,
       })
     })
 
@@ -109,6 +111,7 @@ describe("ParseCache", () => {
         strict: true,
         strict_locals: false,
         action_view_helpers: false,
+        html: true,
       })
     })
 
@@ -126,6 +129,7 @@ describe("ParseCache", () => {
         strict: false,
         strict_locals: false,
         action_view_helpers: false,
+        html: true,
       })
     })
   })

playground/index.html

@@ -490,6 +490,17 @@
                     />
                     <span class="select-none">Prism nodes (deep)</span>
                   </label>
+
+                  <label class="flex items-center gap-1.5 text-gray-300 text-sm" title="Enable HTML tag parsing — uncheck to treat all HTML-like content as literal text (ERB is still parsed)">
+                    <input
+                      type="checkbox"
+                      data-option="html"
+                      data-action="change->playground#onOptionChange"
+                      class="rounded border-gray-600 text-green-600 focus:ring-green-500 bg-gray-700"
+                      checked
+                    />
+                    <span class="select-none">HTML</span>
+                  </label>
                 </div>
 
                 <pre

playground/src/controllers/playground_controller.js

@@ -1271,6 +1271,7 @@ export default class extends Controller {
       prism_program: false,
       prism_nodes: false,
       prism_nodes_deep: false,
+      html: true,
     }
 
     const nonDefaultOptions = {}

rust/src/herb.rs

@@ -14,6 +14,7 @@ pub struct ParserOptions {
   pub prism_nodes: bool,
   pub prism_nodes_deep: bool,
   pub prism_program: bool,
+  pub html: bool,
 }
 
 impl Default for ParserOptions {
@@ -28,6 +29,7 @@ impl Default for ParserOptions {
       prism_nodes: false,
       prism_nodes_deep: false,
       prism_program: false,
+      html: true,
     }
   }
 }
@@ -109,6 +111,9 @@ pub fn parse_with_options(source: &str, options: &ParserOptions) -> Result<Parse
       prism_program: options.prism_program,
       prism_nodes: options.prism_nodes,
       prism_nodes_deep: options.prism_nodes_deep,
+      html: options.html,
+      start_line: 0,
+      start_column: 0,
     };
 
     let ast = crate::ffi::herb_parse(c_source.as_ptr(), &c_parser_options, &mut allocator);

src/analyze/action_view/tag_helper_node_builders.c

@@ -26,7 +26,7 @@ token_T* create_synthetic_token(
   if (value) {
     size_t length = strlen(value);
     char* copied = hb_allocator_strndup(allocator, value, length);
-    token->value = (hb_string_T) { .data = copied, .length = (uint32_t) length };
+    token->value = hb_string_from_data(copied, length);
   } else {
     token->value = HB_STRING_EMPTY;
   }

src/analyze/action_view/tag_helpers.c

@@ -4,9 +4,11 @@
 #include "../../include/analyze/action_view/tag_helper_node_builders.h"
 #include "../../include/analyze/analyze.h"
 #include "../../include/ast_nodes.h"
+#include "../../include/herb.h"
 #include "../../include/html_util.h"
 #include "../../include/parser_helpers.h"
 #include "../../include/position.h"
+#include "../../include/util.h"
 #include "../../include/util/hb_allocator.h"
 #include "../../include/util/hb_array.h"
 #include "../../include/util/hb_string.h"
@@ -580,6 +582,42 @@ static hb_array_T* transform_javascript_include_tag_multi_source(
   return elements;
 }
 
+static bool erb_content_is_end_keyword(hb_string_T content) {
+  const char* start = content.data;
+  const char* end = content.data + content.length;
+
+  while (start < end && is_whitespace(*start)) {
+    start++;
+  }
+
+  while (end > start && is_whitespace(*(end - 1))) {
+    end--;
+  }
+
+  return (size_t) (end - start) == 3 && start[0] == 'e' && start[1] == 'n' && start[2] == 'd';
+}
+
+static AST_ERB_CONTENT_NODE_T* find_swallowed_erb_end_node(hb_array_T* nodes) {
+  if (!nodes) { return NULL; }
+
+  for (size_t i = 0; i < hb_array_size(nodes); i++) {
+    AST_NODE_T* node = (AST_NODE_T*) hb_array_get(nodes, i);
+    if (!node) { continue; }
+
+    if (node->type == AST_ERB_CONTENT_NODE) {
+      AST_ERB_CONTENT_NODE_T* erb = (AST_ERB_CONTENT_NODE_T*) node;
+      if (erb->content && erb_content_is_end_keyword(erb->content->value)) { return erb; }
+    }
+
+    if (node->type == AST_HTML_OPEN_TAG_NODE) {
+      AST_ERB_CONTENT_NODE_T* found = find_swallowed_erb_end_node(((AST_HTML_OPEN_TAG_NODE_T*) node)->children);
+      if (found) { return found; }
+    }
+  }
+
+  return NULL;
+}
+
 static AST_NODE_T* transform_erb_block_to_tag_helper(
   AST_ERB_BLOCK_NODE_T* block_node,
   analyze_ruby_context_T* context,
@@ -600,6 +638,7 @@ static AST_NODE_T* transform_erb_block_to_tag_helper(
   );
 
   hb_array_T* attributes = NULL;
+
   if (parse_context->info->call_node) {
     attributes = extract_html_attributes_from_call_node(
       parse_context->info->call_node,
@@ -685,25 +724,51 @@ static AST_NODE_T* transform_erb_block_to_tag_helper(
 
   hb_array_T* body = block_node->body ? block_node->body : hb_array_init(0, allocator);
   AST_NODE_T* close_tag = (AST_NODE_T*) block_node->end_node;
+  position_T element_end = block_node->base.location.end;
 
   if (tag_name && parser_is_foreign_content_tag(hb_string_from_c_string(tag_name)) && context->source
       && block_node->body && hb_array_size(block_node->body) > 0) {
     size_t start_offset = block_node->tag_closing->range.to;
-    size_t end_offset = block_node->end_node->tag_opening->range.from;
+    size_t end_offset = 0;
+
+    if (block_node->end_node && block_node->end_node->tag_opening) {
+      end_offset = block_node->end_node->tag_opening->range.from;
+    } else {
+      AST_ERB_CONTENT_NODE_T* swallowed_end = find_swallowed_erb_end_node(block_node->body);
+
+      if (swallowed_end && swallowed_end->tag_opening) {
+        end_offset = swallowed_end->tag_opening->range.from;
+
+        AST_ERB_END_NODE_T* end_node = ast_erb_end_node_init(
+          swallowed_end->tag_opening,
+          swallowed_end->content,
+          swallowed_end->tag_closing,
+          swallowed_end->base.location.start,
+          swallowed_end->base.location.end,
+          hb_array_init(0, allocator),
+          allocator
+        );
+
+        close_tag = (AST_NODE_T*) end_node;
+        element_end = close_tag->location.end;
+      }
+    }
 
     if (end_offset > start_offset) {
       position_T body_start = block_node->tag_closing->location.end;
-      position_T body_end = block_node->end_node->tag_opening->location.start;
 
       size_t content_length = end_offset - start_offset;
       char* raw_copy = hb_allocator_strndup(allocator, context->source + start_offset, content_length);
-      hb_string_T raw_content = { .data = raw_copy, .length = (uint32_t) content_length };
 
-      AST_LITERAL_NODE_T* literal_node =
-        ast_literal_node_init(raw_content, body_start, body_end, hb_array_init(0, allocator), allocator);
+      parser_options_T body_options = HERB_DEFAULT_PARSER_OPTIONS;
+      body_options.html = false;
+      body_options.analyze = false;
+      body_options.strict = false;
+      body_options.start_line = body_start.line;
+      body_options.start_column = body_start.column;
 
-      body = hb_array_init(1, allocator);
-      hb_array_append(body, literal_node);
+      AST_DOCUMENT_NODE_T* body_document = herb_parse(raw_copy, &body_options, allocator);
+      body = body_document->children;
     }
   }
 
@@ -715,7 +780,7 @@ static AST_NODE_T* transform_erb_block_to_tag_helper(
     false,
     parse_context->matched_handler->source,
     block_node->base.location.start,
-    block_node->base.location.end,
+    element_end,
     hb_array_init(0, allocator),
     allocator
   );
@@ -1055,7 +1120,65 @@ void transform_tag_helper_blocks(const AST_NODE_T* node, analyze_ruby_context_T*
       }
     }
 
-    if (replacement) { hb_array_set(array, i, replacement); }
+    if (replacement) {
+      position_T replacement_end = replacement->location.end;
+      position_T original_end = child->location.end;
+      bool has_trailing = replacement_end.line != original_end.line || replacement_end.column != original_end.column;
+
+      if (has_trailing && context->source && child->type == AST_ERB_BLOCK_NODE) {
+        AST_HTML_ELEMENT_NODE_T* element = (AST_HTML_ELEMENT_NODE_T*) replacement;
+
+        if (element->close_tag && element->close_tag->type == AST_ERB_END_NODE) {
+          AST_ERB_END_NODE_T* close_erb = (AST_ERB_END_NODE_T*) element->close_tag;
+          size_t trailing_start = close_erb->tag_closing->range.to;
+          size_t source_length = strlen(context->source);
+          size_t trailing_end = trailing_start;
+
+          while (trailing_end < source_length) {
+            position_T position = position_from_source_with_offset(context->source, trailing_end);
+
+            if (position.line > original_end.line
+                || (position.line == original_end.line && position.column >= original_end.column)) {
+              break;
+            }
+
+            trailing_end++;
+          }
+
+          if (trailing_end > trailing_start) {
+            hb_string_T trailing_content =
+              hb_string_from_data(context->source + trailing_start, trailing_end - trailing_start);
+            AST_HTML_TEXT_NODE_T* trailing_text = ast_html_text_node_init(
+              trailing_content,
+              replacement_end,
+              original_end,
+              hb_array_init(0, context->allocator),
+              context->allocator
+            );
+
+            size_t old_size = hb_array_size(array);
+            hb_array_T* new_array = hb_array_init(old_size + 1, context->allocator);
+
+            for (size_t j = 0; j < old_size; j++) {
+              if (j == i) {
+                hb_array_append(new_array, replacement);
+                hb_array_append(new_array, trailing_text);
+              } else {
+                hb_array_append(new_array, hb_array_get(array, j));
+              }
+            }
+
+            array->items = new_array->items;
+            array->size = new_array->size;
+            array->capacity = new_array->capacity;
+            i++;
+            continue;
+          }
+        }
+      }
+
+      hb_array_set(array, i, replacement);
+    }
   }
 }