Parser: Handle more Action View helpers transformations cases (#1465)

This pull request improves how the parser handles several Action View
helper patterns to more closely match Rails' runtime behavior.

The attribute extraction now handles static string and symbol arrays in
class attributes by joining them into space-separated values, matching
how Rails renders `class: ["kitties", "puppies"]` as `class="kitties
puppies"`.

Mixed arrays containing variables or conditional hashes are wrapped in
`token_list(...)` instead, so the output evaluates correctly at runtime.
For example, `class: { active: true, hidden: false }` now produces
`class="<%= token_list({ active: true, hidden: false }) %>"` rather than
leaving the hash as a raw Ruby literal.

For `data:` and `aria:` hash values, non-string types are now wrapped in
`.to_json` to match Rails' `prefix_tag_option` behavior, while integers
and symbols are inlined directly as literal values.

Additionally, Ruby introspection methods like `tag.send`, `tag.class`,
`tag.method`, and methods ending in `?` or `!` are no longer treated as
tag names. These are recognized as dynamic calls with `tag_name: null`,
following the same pattern used by `content_tag` with a variable tag
name.

The `escape` keyword argument is now filtered from extracted attributes,
matching Rails which consumes it internally and does not render it as an
HTML attribute. The `search_tag_helper_node` function was also fixed to
correctly find the outermost matching call when tag helpers are nested
as arguments, such as `content_tag(:div, content_tag(:p, "Hello
world!"))`.

Author: marcoroth

bin/actionview-render

@@ -67,6 +67,7 @@ class ActionViewRenderer
   end
 
   def render_template
+    require "bigdecimal"
     require "action_view"
 
     lookup_context = ActionView::LookupContext.new([])

javascript/packages/language-service/test/parse-html-document.test.ts

@@ -34,6 +34,32 @@ describe("parseHTMLDocument", () => {
     expect((root.herbNode as any).element_source).toBe("ActionView::Helpers::TagHelper#tag")
   })
 
+  test("parses ActionView tag.img with content argument as void element", () => {
+    const service = createService()
+    const document = createDocument('<%= tag.img "/image.png" %>')
+    const html = service.parseHTMLDocument(document)
+
+    expect(html.roots).toHaveLength(1)
+
+    const root = html.roots[0] as HerbHTMLNode
+    expect(root.tag).toBe("img")
+    expect(root.herbNode).toBeDefined()
+    expect((root.herbNode as any).element_source).toBe("ActionView::Helpers::TagHelper#tag")
+    expect((root.herbNode as any).is_void).toBe(true)
+  })
+
+  test("parses ActionView tag.img with content argument and data attributes", () => {
+    const service = createService()
+    const document = createDocument('<%= tag.img "/image.png", data: { controller: "image" } %>')
+    const html = service.parseHTMLDocument(document)
+
+    expect(html.roots).toHaveLength(1)
+
+    const root = html.roots[0] as HerbHTMLNode
+    expect(root.tag).toBe("img")
+    expect(root.attributes?.["data-controller"]).toBe('"image"')
+  })
+
   test("parses ActionView nested data hash into data-* attributes", () => {
     const service = createService()
     const document = createDocument(`<%= tag.div data: { controller: "scroll", action: "click->scroll#go" } %>`)

javascript/packages/node/binding.gyp

@@ -64,6 +64,7 @@
         "./extension/libherb/prism/ruby_parser.c",
         "./extension/libherb/util/html_util.c",
         "./extension/libherb/util/io.c",
+        "./extension/libherb/util/ruby_util.c",
         "./extension/libherb/util/utf8.c",
         "./extension/libherb/util/util.c",
         "./extension/libherb/visitor.c",

javascript/packages/rewriter/test/action-view-tag-helper-to-html.test.ts

@@ -143,7 +143,7 @@ describe("ActionViewTagHelperToHTMLRewriter", () => {
       `
 
       const expected = dedent`
-        <div data-controller="content" data-user-id="<%= 123 %>">
+        <div data-controller="content" data-user-id="123">
           Content
         </div>
       `
@@ -167,6 +167,18 @@ describe("ActionViewTagHelperToHTMLRewriter", () => {
       )
     })
 
+    test("tag.img with content argument reports parser error", () => {
+      expect(() => transform('<%= tag.img "/image.png" %>')).toThrow(
+        "Void element `img` cannot have content"
+      )
+    })
+
+    test("tag.img with content argument and data attributes reports parser error", () => {
+      expect(() => transform('<%= tag.img "/image.png", data: { controller: "image" } %>')).toThrow(
+        "Void element `img` cannot have content"
+      )
+    })
+
     test("tag.details with inline block", () => {
       expect(transform('<%= tag.details { "Some content" } %>')).toBe(
         "<details>Some content</details>"
@@ -684,6 +696,64 @@ describe("ActionViewTagHelperToHTMLRewriter", () => {
     })
   })
 
+  describe("class attribute handling", () => {
+    test("tag.div with conditional class hash wraps in token_list", () => {
+      expect(transform('<%= tag.div class: { active: true, hidden: false } %>')).toBe(
+        '<div class="<%= token_list({ active: true, hidden: false }) %>"></div>'
+      )
+    })
+
+    test("tag.div with dynamic conditional class hash wraps in token_list", () => {
+      expect(transform('<%= tag.div class: { active: @is_active } %>')).toBe(
+        '<div class="<%= token_list({ active: @is_active }) %>"></div>'
+      )
+    })
+
+    test("tag.div with mixed array class wraps in token_list", () => {
+      expect(transform('<%= tag.div class: ["a", variable] %>')).toBe(
+        '<div class="<%= token_list(["a", variable]) %>"></div>'
+      )
+    })
+
+    test("tag.div with static string array class joins with spaces", () => {
+      expect(transform('<%= tag.div class: ["kitties", "puppies"] %>')).toBe(
+        '<div class="kitties puppies"></div>'
+      )
+    })
+
+    test("tag.div with %w() class joins with spaces", () => {
+      expect(transform('<%= tag.div class: %w( kitties puppies ) %>')).toBe(
+        '<div class="kitties puppies"></div>'
+      )
+    })
+
+    test("content_tag with mixed array and conditional hash class wraps in token_list", () => {
+      expect(transform('<%= content_tag(:div, "Hello world!", class: ["strong", { highlight: current_user.admin? }]) %>')).toBe(
+        '<div class="<%= token_list(["strong", { highlight: current_user.admin? }]) %>">Hello world!</div>'
+      )
+    })
+  })
+
+  describe("data attribute handling", () => {
+    test("tag.div with integer data attribute inlines directly", () => {
+      expect(transform('<%= tag.div data: { count: 42 } %>')).toBe(
+        '<div data-count="42"></div>'
+      )
+    })
+
+    test("tag.div with array data attribute wraps in .to_json", () => {
+      expect(transform('<%= tag.div data: { items: ["a", "b"] } %>')).toBe(
+        '<div data-items="<%= ["a", "b"].to_json %>"></div>'
+      )
+    })
+
+    test("tag.div with nested hash data attribute wraps in .to_json", () => {
+      expect(transform('<%= tag.div data: { config: { nested: "hash" } } %>')).toBe(
+        '<div data-config="<%= { nested: "hash" }.to_json %>"></div>'
+      )
+    })
+  })
+
   describe("tag.attributes", () => {
     test("tag.attributes extracts attributes into parent element", () => {
       expect(transform('<input <%= tag.attributes(type: :text, aria: { label: "Search" }) %>>')).toBe(

src/analyze/action_view/attribute_extraction_helpers.c

@@ -2,6 +2,7 @@
 #include "../../include/analyze/action_view/tag_helper_node_builders.h"
 #include "../../include/lib/hb_allocator.h"
 #include "../../include/lib/hb_array.h"
+#include "../../include/lib/hb_buffer.h"
 #include "../../include/lib/hb_string.h"
 #include "../../include/util/html_util.h"
 #include "../../include/util/util.h"
@@ -208,12 +209,56 @@ static char* build_prefixed_key(const char* prefix, const char* raw_key, hb_allo
   return result;
 }
 
+static bool is_static_string_array(pm_array_node_t* array) {
+  if (!array || array->elements.size == 0) { return false; }
+
+  for (size_t i = 0; i < array->elements.size; i++) {
+    pm_node_t* element = array->elements.nodes[i];
+
+    if (element->type != PM_STRING_NODE && element->type != PM_SYMBOL_NODE) { return false; }
+  }
+
+  return true;
+}
+
+static char* join_static_string_array(pm_array_node_t* array, hb_allocator_T* allocator) {
+  hb_buffer_T buffer;
+  hb_buffer_init(&buffer, 64, allocator);
+
+  for (size_t i = 0; i < array->elements.size; i++) {
+    if (i > 0) { hb_buffer_append(&buffer, " "); }
+
+    char* value = extract_string_from_prism_node(array->elements.nodes[i], allocator);
+
+    if (value) {
+      hb_buffer_append(&buffer, value);
+      hb_allocator_dealloc(allocator, value);
+    }
+  }
+
+  char* result = hb_allocator_strdup(allocator, hb_buffer_value(&buffer));
+
+  return result;
+}
+
 static AST_HTML_ATTRIBUTE_NODE_T* create_attribute_from_value(
   const char* name_string,
   pm_node_t* value_node,
   attribute_positions_T* positions,
-  hb_allocator_T* allocator
+  hb_allocator_T* allocator,
+  bool is_nested
 ) {
+  if (value_node->type == PM_ARRAY_NODE && !is_nested && is_static_string_array((pm_array_node_t*) value_node)) {
+    char* joined = join_static_string_array((pm_array_node_t*) value_node, allocator);
+    if (!joined) { return NULL; }
+
+    AST_HTML_ATTRIBUTE_NODE_T* attribute =
+      create_html_attribute_node_precise(name_string, joined, positions, allocator);
+    hb_allocator_dealloc(allocator, joined);
+
+    return attribute;
+  }
+
   if (value_node->type == PM_SYMBOL_NODE || value_node->type == PM_STRING_NODE) {
     char* value_string = extract_string_from_prism_node(value_node, allocator);
     if (!value_string) { return NULL; }
@@ -227,10 +272,22 @@ static AST_HTML_ATTRIBUTE_NODE_T* create_attribute_from_value(
     if (is_boolean_attribute(hb_string((char*) name_string))) {
       return create_html_attribute_node_precise(name_string, NULL, positions, allocator);
     }
+
     return create_html_attribute_node_precise(name_string, "true", positions, allocator);
   } else if (value_node->type == PM_FALSE_NODE) {
     if (is_boolean_attribute(hb_string((char*) name_string))) { return NULL; }
+
     return create_html_attribute_node_precise(name_string, "false", positions, allocator);
+  } else if (value_node->type == PM_INTEGER_NODE) {
+    size_t value_length = value_node->location.end - value_node->location.start;
+    char* value_string = hb_allocator_strndup(allocator, (const char*) value_node->location.start, value_length);
+    if (!value_string) { return NULL; }
+
+    AST_HTML_ATTRIBUTE_NODE_T* attribute =
+      create_html_attribute_node_precise(name_string, value_string, positions, allocator);
+    hb_allocator_dealloc(allocator, value_string);
+
+    return attribute;
   } else if (value_node->type == PM_INTERPOLATED_STRING_NODE) {
     return create_html_attribute_with_interpolated_value(
       name_string,
@@ -241,12 +298,36 @@ static AST_HTML_ATTRIBUTE_NODE_T* create_attribute_from_value(
     );
   } else {
     size_t value_length = value_node->location.end - value_node->location.start;
-    char* ruby_content = hb_allocator_strndup(allocator, (const char*) value_node->location.start, value_length);
+    char* raw_content = hb_allocator_strndup(allocator, (const char*) value_node->location.start, value_length);
+
+    if (raw_content && value_node->location.start) {
+      char* ruby_content = raw_content;
+
+      if (!is_nested && strcmp(name_string, "class") == 0
+          && (value_node->type == PM_HASH_NODE || value_node->type == PM_ARRAY_NODE)) {
+        hb_buffer_T class_buffer;
+        hb_buffer_init(&class_buffer, value_length + 16, allocator);
+        hb_buffer_append(&class_buffer, "token_list(");
+        hb_buffer_append(&class_buffer, raw_content);
+        hb_buffer_append(&class_buffer, ")");
+
+        ruby_content = hb_buffer_value(&class_buffer);
+      }
+
+      // Rails calls .to_json on non-string/symbol values inside data:/aria: hashes
+      if (is_nested) {
+        hb_buffer_T json_buffer;
+        hb_buffer_init(&json_buffer, value_length + 16, allocator);
+        hb_buffer_append(&json_buffer, raw_content);
+        hb_buffer_append(&json_buffer, ".to_json");
+
+        ruby_content = hb_buffer_value(&json_buffer);
+      }
 
-    if (ruby_content && value_node->location.start) {
       AST_HTML_ATTRIBUTE_NODE_T* attribute =
         create_html_attribute_with_ruby_literal_precise(name_string, ruby_content, positions, allocator);
-      hb_allocator_dealloc(allocator, ruby_content);
+      hb_allocator_dealloc(allocator, raw_content);
+
       return attribute;
     }
 
@@ -318,6 +399,11 @@ AST_HTML_ATTRIBUTE_NODE_T* extract_html_attribute_from_assoc(
   char* name_string = extract_string_from_prism_node(assoc->key, allocator);
   if (!name_string) { return NULL; }
 
+  if (strcmp(name_string, "escape") == 0) {
+    hb_allocator_dealloc(allocator, name_string);
+    return NULL;
+  }
+
   if (!assoc->value) {
     hb_allocator_dealloc(allocator, name_string);
     return NULL;
@@ -365,7 +451,7 @@ AST_HTML_ATTRIBUTE_NODE_T* extract_html_attribute_from_assoc(
 
   char* dashed_name = convert_underscores_to_dashes(name_string);
   AST_HTML_ATTRIBUTE_NODE_T* attribute_node =
-    create_attribute_from_value(dashed_name ? dashed_name : name_string, assoc->value, &positions, allocator);
+    create_attribute_from_value(dashed_name ? dashed_name : name_string, assoc->value, &positions, allocator, false);
 
   if (dashed_name) { free(dashed_name); }
   hb_allocator_dealloc(allocator, name_string);
@@ -467,7 +553,7 @@ hb_array_T* extract_html_attributes_from_keyword_hash(
             fill_attribute_positions(hash_assoc, source, original_source, erb_content_offset, &hash_positions);
 
             AST_HTML_ATTRIBUTE_NODE_T* attribute =
-              create_attribute_from_value(attribute_key_string, hash_assoc->value, &hash_positions, allocator);
+              create_attribute_from_value(attribute_key_string, hash_assoc->value, &hash_positions, allocator, true);
 
             if (attribute) { hb_array_append(attributes, attribute); }
             hb_allocator_dealloc(allocator, attribute_key_string);

src/analyze/action_view/tag.c

@@ -1,4 +1,5 @@
 #include "../../include/analyze/action_view/tag_helper_handler.h"
+#include "../../include/util/ruby_util.h"
 
 #include <prism.h>
 #include <stdbool.h>
@@ -23,6 +24,10 @@ char* extract_tag_dot_name(pm_call_node_t* call_node, pm_parser_t* parser, hb_al
   pm_constant_t* constant = pm_constant_pool_id_to_constant(&parser->constant_pool, call_node->name);
   if (!constant) { return NULL; }
 
+  if (is_ruby_introspection_method(hb_string_from_data((const char*) constant->start, constant->length))) {
+    return NULL;
+  }
+
   char* name = hb_allocator_strndup(allocator, (const char*) constant->start, constant->length);
 
   for (size_t i = 0; i < constant->length && name[i] != '\0'; i++) {

src/analyze/action_view/tag_helpers.c

@@ -132,7 +132,7 @@ bool search_tag_helper_node(const pm_node_t* node, void* data) {
           search_data->info->has_block = handlers[i].supports_block();
         }
 
-        return true;
+        return false;
       }
     }
 

src/include/util/ruby_util.h

@@ -0,0 +1,9 @@
+#ifndef HERB_RUBY_UTIL_H
+#define HERB_RUBY_UTIL_H
+
+#include "../lib/hb_string.h"
+#include <stdbool.h>
+
+bool is_ruby_introspection_method(hb_string_T method_name);
+
+#endif

src/util/ruby_util.c

@@ -0,0 +1,42 @@
+#include "../include/util/ruby_util.h"
+#include "../include/lib/hb_string.h"
+
+#include <stdbool.h>
+#include <stddef.h>
+
+static hb_string_T ruby_introspection_methods[] = HB_STRING_LIST(
+  "send",
+  "public_send",
+  "__send__",
+  "try",
+  "try!",
+  "method",
+  "class",
+  "inspect",
+  "to_s",
+  "object_id",
+  "__id__",
+  "dup",
+  "clone",
+  "freeze",
+  "frozen",
+  "tap",
+  "then",
+  "yield_self"
+);
+
+bool is_ruby_introspection_method(hb_string_T method_name) {
+  size_t count = sizeof(ruby_introspection_methods) / sizeof(ruby_introspection_methods[0]);
+
+  for (size_t index = 0; index < count; index++) {
+    if (hb_string_equals(method_name, ruby_introspection_methods[index])) { return true; }
+  }
+
+  if (method_name.length > 0) {
+    char last_char = method_name.data[method_name.length - 1];
+
+    if (last_char == '?' || last_char == '!') { return true; }
+  }
+
+  return false;
+}