Parser: Fix Action View Helpers with positional arguments and block (#1406)

This pull request fixes how the parser handles Action View tag helpers
when both positional arguments and an inline block are present.

Previously, `content_tag` and `tag.*` would use the positional content
argument even when a block was given. Rails always prefers the block
content, so now the parser matches that behavior:

```html+erb
<%= content_tag(:div, "ignored") { "Block wins" } %>
<!-- Rails renders: <div>Block wins</div> -->
```

This also fixes attribute extraction for `link_to` when the second
argument is an explicit hash literal or a variable:

```html+erb
<%= link_to("/about", { class: "btn" }) { "About" } %>
<!-- Now correctly extracts: href="/about", class="btn", body="About" -->
```

```html+erb
<%= link_to("/about", html_opts) { "About" } %>
<!--  Variable options are now represented as RubyHTMLAttributesSplatNode -->
```

Follow up on #1404

Author: marcoroth

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

@@ -179,6 +179,12 @@ describe("ActionViewTagHelperToHTMLRewriter", () => {
       )
     })
 
+    test("tag.div with content argument and block prefers block content", () => {
+      expect(transform('<%= tag.div("argument") { "Block" } %>')).toBe(
+        "<div>Block</div>"
+      )
+    })
+
     test("tag.div with splat attributes", () => {
       const input = dedent`
         <%= tag.div class: "content", **attributes do %>
@@ -214,6 +220,12 @@ describe("ActionViewTagHelperToHTMLRewriter", () => {
         "<p><%= @user.name %></p>"
       )
     })
+
+    test("content_tag with content argument and block prefers block content", () => {
+      expect(transform('<%= content_tag(:div, "argument") { "Block" } %>')).toBe(
+        "<div>Block</div>"
+      )
+    })
   })
 
   describe("nested helpers", () => {
@@ -270,6 +282,12 @@ describe("ActionViewTagHelperToHTMLRewriter", () => {
         '<a href="#"><%= @user.name %></a>'
       )
     })
+
+    test("link_to with content argument and block prefers block content", () => {
+      expect(transform('<%= link_to("#", "argument") { "Block" } %>')).toBe(
+        `<a href="#">Block</a>`
+      )
+    })
   })
 
   describe("turbo_frame_tag helpers", () => {

src/analyze/action_view/attribute_extraction_helpers.c

@@ -265,7 +265,7 @@ bool has_html_attributes_in_call(pm_call_node_t* call_node) {
 
   pm_node_t* last_argument = arguments->arguments.nodes[arguments->arguments.size - 1];
 
-  return last_argument && last_argument->type == PM_KEYWORD_HASH_NODE;
+  return last_argument && (last_argument->type == PM_KEYWORD_HASH_NODE || last_argument->type == PM_HASH_NODE);
 }
 
 hb_array_T* extract_html_attributes_from_call_node(
@@ -280,6 +280,19 @@ hb_array_T* extract_html_attributes_from_call_node(
   pm_arguments_node_t* arguments = call_node->arguments;
   pm_node_t* last_argument = arguments->arguments.nodes[arguments->arguments.size - 1];
 
+  if (last_argument->type == PM_HASH_NODE) {
+    pm_hash_node_t* hash_node = (pm_hash_node_t*) last_argument;
+    pm_keyword_hash_node_t synthetic = { .base = hash_node->base, .elements = hash_node->elements };
+
+    return extract_html_attributes_from_keyword_hash(
+      &synthetic,
+      source,
+      original_source,
+      erb_content_offset,
+      allocator
+    );
+  }
+
   return extract_html_attributes_from_keyword_hash(
     (pm_keyword_hash_node_t*) last_argument,
     source,

src/analyze/action_view/content_tag.c

@@ -40,6 +40,9 @@ char* extract_content_tag_content(pm_call_node_t* call_node, pm_parser_t* parser
 
   if (!call_node) { return NULL; }
 
+  char* block_content = extract_inline_block_content(call_node, allocator);
+  if (block_content) { return block_content; }
+
   if (call_node->arguments) {
     pm_arguments_node_t* arguments = call_node->arguments;
 
@@ -59,7 +62,7 @@ char* extract_content_tag_content(pm_call_node_t* call_node, pm_parser_t* parser
     }
   }
 
-  return extract_inline_block_content(call_node, allocator);
+  return NULL;
 }
 
 bool content_tag_supports_block(void) {

src/analyze/action_view/tag.c

@@ -38,6 +38,9 @@ char* extract_tag_dot_content(pm_call_node_t* call_node, pm_parser_t* parser, hb
 
   if (!call_node) { return NULL; }
 
+  char* block_content = extract_inline_block_content(call_node, allocator);
+  if (block_content) { return block_content; }
+
   if (call_node->arguments) {
     pm_arguments_node_t* arguments = call_node->arguments;
 
@@ -52,7 +55,7 @@ char* extract_tag_dot_content(pm_call_node_t* call_node, pm_parser_t* parser, hb
     }
   }
 
-  return extract_inline_block_content(call_node, allocator);
+  return NULL;
 }
 
 bool tag_dot_supports_block(void) {

src/analyze/action_view/tag_helpers.c

@@ -510,8 +510,10 @@ static AST_NODE_T* transform_link_to_helper(
   pm_arguments_node_t* link_arguments = info->call_node->arguments;
   bool has_inline_block = info->call_node->block && info->call_node->block->type == PM_BLOCK_NODE;
 
-  bool keyword_hash_is_url = !has_inline_block && link_arguments && link_arguments->arguments.size == 2
-                          && link_arguments->arguments.nodes[1]->type == PM_KEYWORD_HASH_NODE;
+  bool second_arg_is_hash = link_arguments && link_arguments->arguments.size == 2
+                         && (link_arguments->arguments.nodes[1]->type == PM_KEYWORD_HASH_NODE
+                             || link_arguments->arguments.nodes[1]->type == PM_HASH_NODE);
+  bool keyword_hash_is_url = !has_inline_block && second_arg_is_hash;
 
   if (!keyword_hash_is_url) {
     attributes = extract_html_attributes_from_call_node(
@@ -525,6 +527,38 @@ static AST_NODE_T* transform_link_to_helper(
 
   if (!attributes) { attributes = hb_array_init(4, allocator); }
 
+  if (has_inline_block && link_arguments && link_arguments->arguments.size >= 2) {
+    pm_node_t* second_arg = link_arguments->arguments.nodes[1];
+
+    if (second_arg->type != PM_KEYWORD_HASH_NODE && second_arg->type != PM_HASH_NODE
+        && second_arg->type != PM_STRING_NODE && second_arg->type != PM_SYMBOL_NODE) {
+      size_t source_length = second_arg->location.end - second_arg->location.start;
+      char* content = hb_allocator_strndup(allocator, (const char*) second_arg->location.start, source_length);
+
+      if (content) {
+        position_T position = prism_location_to_position_with_offset(
+          &second_arg->location,
+          parse_context->original_source,
+          parse_context->erb_content_offset,
+          parse_context->prism_source
+        );
+
+        AST_RUBY_HTML_ATTRIBUTES_SPLAT_NODE_T* splat_node = ast_ruby_html_attributes_splat_node_init(
+          hb_string_from_c_string(content),
+          HB_STRING_EMPTY,
+          position,
+          position,
+          hb_array_init(0, allocator),
+          allocator
+        );
+
+        if (splat_node) { hb_array_append(attributes, (AST_NODE_T*) splat_node); }
+
+        hb_allocator_dealloc(allocator, content);
+      }
+    }
+  }
+
   // `method:` implies `rel="nofollow"`
   bool has_data_method = false;
   hb_string_T data_method_string = hb_string("data-method");

test/analyze/action_view/tag_helper/content_tag_test.rb

@@ -173,5 +173,29 @@ class ContentTagTest < Minitest::Spec
         <%= content_tag(:span) { "Text" } %>
       HTML
     end
+
+    test "content_tag with content argument and block prefers block content" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= content_tag(:div, "argument") { "Block" } %>
+      HTML
+    end
+
+    test "content_tag with inline block and data attributes" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= content_tag(:div, data: { controller: "example" }) { "Hello" } %>
+      HTML
+    end
+
+    test "content_tag with content argument and attributes and block prefers block" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= content_tag(:div, "Content", class: "box") { "Block" } %>
+      HTML
+    end
+
+    test "content_tag with content argument and multiple attributes and block prefers block" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= content_tag(:div, "Content", id: "main", class: "box") { "Block" } %>
+      HTML
+    end
   end
 end

test/analyze/action_view/tag_helper/tag_test.rb

@@ -175,5 +175,23 @@ class TagTest < Minitest::Spec
         <%= tag.span { "Text" } %>
       HTML
     end
+
+    test "tag.div with content argument and block prefers block content" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= tag.div("argument") { "Block" } %>
+      HTML
+    end
+
+    test "tag.div with inline block and data attributes" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= tag.div(data: { controller: "example" }) { "Hello" } %>
+      HTML
+    end
+
+    test "tag.div with content argument and attributes and block prefers block" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= tag.div("Content", class: "box") { "Block" } %>
+      HTML
+    end
   end
 end

test/analyze/action_view/url_helper/link_to_test.rb

@@ -251,5 +251,47 @@ class LinkToTest < Minitest::Spec
         <%= link_to("#") { @user.name } %>
       HTML
     end
+
+    test "link_to with inline block and multiple data attributes" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= link_to("/profile", data: { turbo_method: "delete", turbo_confirm: "Sure?" }) { "Delete" } %>
+      HTML
+    end
+
+    test "link_to with inline block and string url" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= link_to("/about") { "About Us" } %>
+      HTML
+    end
+
+    test "link_to with inline block and path helper and attributes" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= link_to(root_path, class: "btn", data: { turbo_method: "delete" }) { "Home" } %>
+      HTML
+    end
+
+    test "link_to with inline block and explicit hash options" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= link_to("/about", { class: "btn" }) { "x" } %>
+      HTML
+    end
+
+    test "link_to with inline block and variable options" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= link_to("/about", html_opts) { "About" } %>
+      HTML
+    end
+
+    test "link_to with inline block and path helper with variable options" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= link_to(root_path, options) { "Home" } %>
+      HTML
+    end
+
+    test "link_to with inline block and string as second argument" do
+      assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
+        <%= link_to("#", "argument") { "Block" } %>
+      HTML
+    end
   end
 end