Skip to content

Commit ae806a8

Browse files
authored
Parser: Don't search for Action View Tag Helpers recursively (#1431)
This pull request fixes a bug in the parser analysis phase with `action_view_helpers: true` enabled where `search_tag_helper_node` would recursively traverse the entire Prism AST of an ERB expression, causing tag helpers nested inside another call's arguments or block body to be incorrectly detected and transformed as the top-level tag helper. For example, the following template: ```erb <% content_for :head, tag.meta(name: "viewport", content: "width=device-width, initial-scale=1") %> ``` Would incorrectly transform the `ERBContentNode` as if `tag.meta` were the outermost call. It now correctly produces the following using `action_view_helpers: true`: ```js @ DocumentNode (location: (1:0)-(2:0)) └── children: (2 items) ├── @ ERBContentNode (location: (1:0)-(1:99)) │ ├── tag_opening: "<%" (location: (1:0)-(1:2)) │ ├── content: " content_for :head, tag.meta(name: "viewport", content: "width=device-width, initial-scale=1") " (location: (1:2)-(1:97)) │ ├── tag_closing: "%>" (location: (1:97)-(1:99)) │ ├── parsed: true │ └── valid: true │ └── @ HTMLTextNode (location: (1:99)-(2:0)) └── content: "\n" ``` The fix stops recursion at any `PM_CALL_NODE` that doesn't match a known tag helper. A tag helper must be the outermost call in the ERB expression, not nested inside another call's arguments or block body. Resolves #1423 Resolves #1429
1 parent 2236f9d commit ae806a8

7 files changed

+130
-0
lines changed

‎src/analyze/action_view/tag_helpers.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ bool search_tag_helper_node(const pm_node_t* node, void* data) {
127127
return true;
128128
}
129129
}
130+
131+
return false;
130132
}
131133

132134
pm_visit_child_nodes(node, search_tag_helper_node, search_data);

‎test/analyze/action_view/tag_helper/content_tag_test.rb‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,5 +217,11 @@ class ContentTagTest < Minitest::Spec
217217
<%= content_tag :script, "alert('Hello')", type: "application/javascript" %>
218218
HTML
219219
end
220+
221+
test "content_tag nested as argument to another method is not incorrectly detected as top-level tag helper" do
222+
assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
223+
<%= t(".terms_notice", link: content_tag(:a, t(".terms_of_service"), href: terms_path)).html_safe %>
224+
HTML
225+
end
220226
end
221227
end

‎test/analyze/action_view/tag_helper/tag_test.rb‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,5 +221,35 @@ class TagTest < Minitest::Spec
221221
<%= tag.script src: "/assets/application.js", defer: true %>
222222
HTML
223223
end
224+
225+
test "tag.div nested inside unknown helper block is not incorrectly detected as top-level tag helper" do
226+
assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
227+
<%= my_component(data: @items) do |component|
228+
component.with_slot do
229+
tag.div class: "container" do
230+
link_to("Click", url_path)
231+
end
232+
end
233+
end %>
234+
HTML
235+
end
236+
237+
test "tag helpers nested inside lambda inside unknown helper are not incorrectly detected" do
238+
assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
239+
<%= my_table(data: @things) do |table|
240+
table.with_column(value: lambda do |thing|
241+
tag.div class: "flex" do
242+
link_to(thing.name, thing_path(thing))
243+
end
244+
end)
245+
end %>
246+
HTML
247+
end
248+
249+
test "tag.meta nested as argument to content_for is not incorrectly detected as top-level tag helper" do
250+
assert_parsed_snapshot(<<~HTML, action_view_helpers: true)
251+
<% content_for :head, tag.meta(name: "viewport", content: "width=device-width, initial-scale=1") %>
252+
HTML
253+
end
224254
end
225255
end

‎test/snapshots/analyze/action_view/tag_helper/content_tag_test/test_0032_content_tag_nested_as_argument_to_another_method_is_not_incorrectly_detected_as_top-level_tag_helper_344731c2d70775305e3f7744a16f2a1e-ef4af315cb33925c38d24ea3c2e8a1cd.txt‎

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/snapshots/analyze/action_view/tag_helper/tag_test/test_0032_tag.div_nested_inside_unknown_helper_block_is_not_incorrectly_detected_as_top-level_tag_helper_ef9f1ab8c52461760dc551efdc83bca6-ef4af315cb33925c38d24ea3c2e8a1cd.txt‎

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/snapshots/analyze/action_view/tag_helper/tag_test/test_0033_tag_helpers_nested_inside_lambda_inside_unknown_helper_are_not_incorrectly_detected_b883fe75cdcaabb4d70714e419d632d9-ef4af315cb33925c38d24ea3c2e8a1cd.txt‎

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/snapshots/analyze/action_view/tag_helper/tag_test/test_0034_tag.meta_nested_as_argument_to_content_for_is_not_incorrectly_detected_as_top-level_tag_helper_44a9ce7b1c6c56530f4e7bf3b31d59d7-ef4af315cb33925c38d24ea3c2e8a1cd.txt‎

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)