diff --git a/javascript/packages/rewriter/test/action-view-tag-helper-to-html.test.ts b/javascript/packages/rewriter/test/action-view-tag-helper-to-html.test.ts index 8a9921bdd..294446923 100644 --- a/javascript/packages/rewriter/test/action-view-tag-helper-to-html.test.ts +++ b/javascript/packages/rewriter/test/action-view-tag-helper-to-html.test.ts @@ -829,9 +829,9 @@ describe("ActionViewTagHelperToHTMLRewriter", () => { ) }) - test("image_tag with ruby expression source wraps in image_path", () => { + test("image_tag with ruby expression source dispatches on the value", () => { expect(transform('<%= image_tag user.avatar %>')).toBe( - '' + '' ) }) @@ -859,9 +859,9 @@ describe("ActionViewTagHelperToHTMLRewriter", () => { ) }) - test("image_tag with instance variable method wraps in image_path", () => { + test("image_tag with instance variable method dispatches on the value", () => { expect(transform('<%= image_tag @post.cover_image %>')).toBe( - '' + '' ) }) diff --git a/src/analyze/action_view/image_tag.c b/src/analyze/action_view/image_tag.c index b83844ccf..aac5720a1 100644 --- a/src/analyze/action_view/image_tag.c +++ b/src/analyze/action_view/image_tag.c @@ -34,6 +34,33 @@ bool image_tag_source_is_url(const char* source, size_t length) { return false; } +char* wrap_in_image_source_dispatch( + const char* source, + size_t source_length, + const char* path_options, + hb_allocator_T* allocator +) { + hb_buffer_T buffer; + hb_buffer_init(&buffer, source_length + 160, allocator); + + hb_buffer_append(&buffer, "("); + hb_buffer_append_with_length(&buffer, source, source_length); + hb_buffer_append(&buffer, ").then { |value| value.is_a?(String) || value.is_a?(Symbol)"); + hb_buffer_append(&buffer, " ? image_path(value"); + + if (path_options && strlen(path_options) > 0) { + hb_buffer_append(&buffer, ", "); + hb_buffer_append(&buffer, path_options); + } + + hb_buffer_append(&buffer, ") : polymorphic_url(value) }"); + + char* result = hb_allocator_strdup(allocator, hb_buffer_value(&buffer)); + hb_buffer_free(&buffer); + + return result; +} + char* wrap_in_image_path( const char* source, size_t source_length, diff --git a/src/analyze/action_view/tag_helpers.c b/src/analyze/action_view/tag_helpers.c index 183d9bc43..cc4d4704a 100644 --- a/src/analyze/action_view/tag_helpers.c +++ b/src/analyze/action_view/tag_helpers.c @@ -29,6 +29,7 @@ extern char* wrap_in_url_for(const char*, size_t, hb_allocator_T*); extern char* extract_link_to_href(pm_call_node_t*, pm_parser_t*, hb_allocator_T*); extern bool detect_turbo_frame_tag(pm_call_node_t*, pm_parser_t*); extern char* extract_turbo_frame_tag_id(pm_call_node_t*, pm_parser_t*, hb_allocator_T*); +extern bool turbo_frame_tag_id_is_static(pm_call_node_t*); extern bool detect_javascript_include_tag(pm_call_node_t*, pm_parser_t*); extern char* extract_javascript_include_tag_src(pm_call_node_t*, pm_parser_t*, hb_allocator_T*); extern char* wrap_in_javascript_path(const char*, size_t, const char*, hb_allocator_T*); @@ -36,6 +37,7 @@ extern bool javascript_include_tag_source_is_url(const char*, size_t); extern bool detect_image_tag(pm_call_node_t*, pm_parser_t*); extern char* extract_image_tag_src(pm_call_node_t*, pm_parser_t*, hb_allocator_T*); extern char* wrap_in_image_path(const char*, size_t, const char*, hb_allocator_T*); +extern char* wrap_in_image_source_dispatch(const char*, size_t, const char*, hb_allocator_T*); extern bool image_tag_source_is_url(const char*, size_t); extern bool detect_stylesheet_link_tag(pm_call_node_t*, pm_parser_t*); extern char* wrap_in_stylesheet_path(const char*, size_t, const char*, hb_allocator_T*); @@ -760,7 +762,7 @@ static AST_NODE_T* transform_tag_helper_with_attributes( pm_node_t* first_argument = parse_context->info->call_node->arguments->arguments.nodes[0]; position_T id_start, id_end; prism_node_location_to_positions(&first_argument->location, parse_context, &id_start, &id_end); - bool id_is_ruby_expression = (first_argument->type != PM_STRING_NODE && first_argument->type != PM_SYMBOL_NODE); + bool id_is_ruby_expression = !turbo_frame_tag_id_is_static(parse_context->info->call_node); AST_HTML_ATTRIBUTE_NODE_T* id_attribute = id_is_ruby_expression ? create_html_attribute_with_ruby_literal("id", id_value, id_start, id_end, allocator) @@ -850,7 +852,7 @@ static AST_NODE_T* transform_tag_helper_with_attributes( source_attribute_value = wrap_in_image_path(quoted_source, quoted_length, path_options, allocator); hb_allocator_dealloc(allocator, quoted_source); } else if (!source_is_string && !is_url && !source_is_path_helper) { - source_attribute_value = wrap_in_image_path(source_value, source_length, path_options, allocator); + source_attribute_value = wrap_in_image_source_dispatch(source_value, source_length, path_options, allocator); } AST_HTML_ATTRIBUTE_NODE_T* source_attribute = @@ -1415,7 +1417,7 @@ static AST_NODE_T* transform_erb_block_to_tag_helper( pm_node_t* first_argument = parse_context->info->call_node->arguments->arguments.nodes[0]; position_T id_start, id_end; prism_node_location_to_positions(&first_argument->location, parse_context, &id_start, &id_end); - bool id_is_ruby_expression = (first_argument->type != PM_STRING_NODE && first_argument->type != PM_SYMBOL_NODE); + bool id_is_ruby_expression = !turbo_frame_tag_id_is_static(parse_context->info->call_node); AST_HTML_ATTRIBUTE_NODE_T* id_attribute = id_is_ruby_expression ? create_html_attribute_with_ruby_literal("id", id_value, id_start, id_end, allocator) diff --git a/src/analyze/action_view/turbo_frame_tag.c b/src/analyze/action_view/turbo_frame_tag.c index 7236d31a2..aaedf57d6 100644 --- a/src/analyze/action_view/turbo_frame_tag.c +++ b/src/analyze/action_view/turbo_frame_tag.c @@ -1,15 +1,98 @@ #include "../../include/lib/hb_allocator.h" +#include "../../include/lib/hb_buffer.h" #include #include #include +static bool turbo_frame_tag_literal_id(pm_node_t* node, const char** source, size_t* length) { + if (node->type == PM_STRING_NODE) { + pm_string_node_t* string_node = (pm_string_node_t*) node; + *source = (const char*) pm_string_source(&string_node->unescaped); + *length = pm_string_length(&string_node->unescaped); + + return true; + } + + if (node->type == PM_SYMBOL_NODE) { + pm_symbol_node_t* symbol_node = (pm_symbol_node_t*) node; + *source = (const char*) pm_string_source(&symbol_node->unescaped); + *length = pm_string_length(&symbol_node->unescaped); + + return true; + } + + return false; +} + +// `turbo_frame_tag *ids` is `dom_id(*ids)` when the first id is a record or Class, `ids.join("_")` otherwise. +static char* extract_turbo_frame_tag_ids( + pm_arguments_node_t* arguments, + size_t count, + pm_parser_t* parser, + hb_allocator_T* allocator +) { + (void) parser; + + hb_buffer_T buffer; + hb_buffer_init(&buffer, 128, allocator); + + bool all_literal = true; + + for (size_t index = 0; index < count; index++) { + const char* literal = NULL; + size_t literal_length = 0; + + if (!turbo_frame_tag_literal_id(arguments->arguments.nodes[index], &literal, &literal_length)) { + all_literal = false; + break; + } + + if (index > 0) { hb_buffer_append(&buffer, "_"); } + hb_buffer_append_with_length(&buffer, literal, literal_length); + } + + if (!all_literal) { + hb_buffer_clear(&buffer); + hb_buffer_append(&buffer, "["); + + for (size_t index = 0; index < count; index++) { + pm_node_t* node = arguments->arguments.nodes[index]; + + if (index > 0) { hb_buffer_append(&buffer, ", "); } + hb_buffer_append_with_length(&buffer, (const char*) node->location.start, node->location.end - node->location.start); + } + + hb_buffer_append(&buffer, "].then { |ids| ids.first.respond_to?(:to_key) || ids.first.is_a?(Class)"); + hb_buffer_append(&buffer, " ? dom_id(*ids) : ids.join(\"_\") }"); + } + + char* result = hb_allocator_strdup(allocator, hb_buffer_value(&buffer)); + hb_buffer_free(&buffer); + + return result; +} + char* extract_turbo_frame_tag_id(pm_call_node_t* call_node, pm_parser_t* parser, hb_allocator_T* allocator) { if (!call_node || !call_node->arguments) { return NULL; } pm_arguments_node_t* arguments = call_node->arguments; if (!arguments->arguments.size) { return NULL; } + size_t positional_count = 0; + + for (size_t index = 0; index < arguments->arguments.size; index++) { + pm_node_type_t type = arguments->arguments.nodes[index]->type; + + if (type == PM_KEYWORD_HASH_NODE || type == PM_SPLAT_NODE) { break; } + + positional_count++; + } + + if (positional_count == 0) { return NULL; } + + if (positional_count > 1) { return extract_turbo_frame_tag_ids(arguments, positional_count, parser, allocator); } + pm_node_t* first_argument = arguments->arguments.nodes[0]; if (first_argument->type == PM_STRING_NODE) { @@ -36,18 +119,31 @@ char* extract_turbo_frame_tag_id(pm_call_node_t* call_node, pm_parser_t* parser, } } - const char* prefix = "dom_id("; - const char* suffix = ")"; + hb_buffer_T buffer; + hb_buffer_init(&buffer, source_length + 128, allocator); - size_t prefix_length = strlen(prefix); - size_t suffix_length = strlen(suffix); - size_t total_length = prefix_length + source_length + suffix_length; - char* result = hb_allocator_alloc(allocator, total_length + 1); + hb_buffer_append(&buffer, "("); + hb_buffer_append_with_length(&buffer, source, source_length); + hb_buffer_append(&buffer, ").then { |value| value.respond_to?(:to_key) || value.is_a?(Class)"); + hb_buffer_append(&buffer, " ? dom_id(value) : value.to_s }"); - memcpy(result, prefix, prefix_length); - memcpy(result + prefix_length, source, source_length); - memcpy(result + prefix_length + source_length, suffix, suffix_length); - result[total_length] = '\0'; + char* result = hb_allocator_strdup(allocator, hb_buffer_value(&buffer)); + hb_buffer_free(&buffer); return result; } + +bool turbo_frame_tag_id_is_static(pm_call_node_t* call_node) { + if (!call_node || !call_node->arguments) { return false; } + + pm_arguments_node_t* arguments = call_node->arguments; + + for (size_t index = 0; index < arguments->arguments.size; index++) { + pm_node_t* node = arguments->arguments.nodes[index]; + + if (node->type == PM_KEYWORD_HASH_NODE || node->type == PM_SPLAT_NODE) { break; } + if (node->type != PM_STRING_NODE && node->type != PM_SYMBOL_NODE) { return false; } + } + + return true; +} diff --git a/test/engine/action_view/turbo_frame_tag_test.rb b/test/engine/action_view/turbo_frame_tag_test.rb index 6cc0db74b..86270d250 100644 --- a/test/engine/action_view/turbo_frame_tag_test.rb +++ b/test/engine/action_view/turbo_frame_tag_test.rb @@ -49,6 +49,24 @@ class TurboFrameTagTest < Minitest::Spec test "turbo_frame_tag with data attributes" do assert_optimized_snapshot('<%= turbo_frame_tag "tray", data: { controller: "frame" } do %>Content<% end %>') end + + test "turbo_frame_tag with a string id held in a local" do + assert_optimized_snapshot( + "<%= turbo_frame_tag frame_id do %>Content<% end %>", + { frame_id: "post_1" } + ) + end + + test "turbo_frame_tag with a string prefix and a string id held in a local" do + assert_optimized_snapshot( + '<%= turbo_frame_tag "notes", frame_id do %>Content<% end %>', + { frame_id: "post_1" } + ) + end + + test "turbo_frame_tag with literal ids joins them" do + assert_optimized_snapshot('<%= turbo_frame_tag "notes", :list %>') + end end end end diff --git a/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_model_wraps_in_dom_id_6668f957e9b3270e6903c501660fc9af-ef4af315cb33925c38d24ea3c2e8a1cd.txt b/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_model_wraps_in_dom_id_6668f957e9b3270e6903c501660fc9af-ef4af315cb33925c38d24ea3c2e8a1cd.txt index 5c416a147..4cb4b6395 100644 --- a/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_model_wraps_in_dom_id_6668f957e9b3270e6903c501660fc9af-ef4af315cb33925c38d24ea3c2e8a1cd.txt +++ b/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_model_wraps_in_dom_id_6668f957e9b3270e6903c501660fc9af-ef4af315cb33925c38d24ea3c2e8a1cd.txt @@ -30,7 +30,7 @@ options: {action_view_helpers: true} │ │ ├── open_quote: ∅ │ │ ├── children: (1 item) │ │ │ └── @ RubyLiteralNode (location: (1:20)-(1:24)) - │ │ │ └── content: "dom_id(post)" + │ │ │ └── content: "(post).then { |value| value.respond_to?(:to_key) || value.is_a?(Class) ? dom_id(value) : value.to_s }" │ │ │ │ │ ├── close_quote: ∅ │ │ └── quoted: true diff --git a/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0012_turbo_frame_tag_with_instance_variable_wraps_in_dom_id_8c2535a5f80133daa8817d2bdfcdd4c8-ef4af315cb33925c38d24ea3c2e8a1cd.txt b/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0012_turbo_frame_tag_with_instance_variable_wraps_in_dom_id_8c2535a5f80133daa8817d2bdfcdd4c8-ef4af315cb33925c38d24ea3c2e8a1cd.txt index 8bd62a098..b82d3dc44 100644 --- a/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0012_turbo_frame_tag_with_instance_variable_wraps_in_dom_id_8c2535a5f80133daa8817d2bdfcdd4c8-ef4af315cb33925c38d24ea3c2e8a1cd.txt +++ b/test/snapshots/analyze/action_view/tag_helper/turbo_frame_tag_test/test_0012_turbo_frame_tag_with_instance_variable_wraps_in_dom_id_8c2535a5f80133daa8817d2bdfcdd4c8-ef4af315cb33925c38d24ea3c2e8a1cd.txt @@ -28,7 +28,7 @@ options: {action_view_helpers: true} │ │ ├── open_quote: ∅ │ │ ├── children: (1 item) │ │ │ └── @ RubyLiteralNode (location: (1:20)-(1:25)) - │ │ │ └── content: "dom_id(@post)" + │ │ │ └── content: "(@post).then { |value| value.respond_to?(:to_key) || value.is_a?(Class) ? dom_id(value) : value.to_s }" │ │ │ │ │ ├── close_quote: ∅ │ │ └── quoted: true diff --git a/test/snapshots/engine/action_view/image_tag_test/test_0006_image_tag_with_ruby_expression_source_f25e3c1ce03e4a2b7484fed0fe7a28f2.txt b/test/snapshots/engine/action_view/image_tag_test/test_0006_image_tag_with_ruby_expression_source_f25e3c1ce03e4a2b7484fed0fe7a28f2.txt index d831338f1..2e108055e 100644 --- a/test/snapshots/engine/action_view/image_tag_test/test_0006_image_tag_with_ruby_expression_source_f25e3c1ce03e4a2b7484fed0fe7a28f2.txt +++ b/test/snapshots/engine/action_view/image_tag_test/test_0006_image_tag_with_ruby_expression_source_f25e3c1ce03e4a2b7484fed0fe7a28f2.txt @@ -2,5 +2,5 @@ source: "Engine::ActionView::ImageTagTest#test_0006_image_tag with ruby expression source" input: "{source: \"<%= image_tag user_avatar %>\", options: {visitors: [#], locals: {user_avatar: \"http://example.com/avatar.png\"}}}" --- -_buf = ::String.new; _buf << ''.freeze; +_buf = ::String.new; _buf << ''.freeze; _buf.to_s diff --git a/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0009_turbo_frame_tag_with_a_string_id_held_in_a_local_b752f5c855f26434c7a0ebf09268b407.txt b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0009_turbo_frame_tag_with_a_string_id_held_in_a_local_b752f5c855f26434c7a0ebf09268b407.txt new file mode 100644 index 000000000..4889922f2 --- /dev/null +++ b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0009_turbo_frame_tag_with_a_string_id_held_in_a_local_b752f5c855f26434c7a0ebf09268b407.txt @@ -0,0 +1,6 @@ +--- +source: "Engine::ActionView::TurboFrameTagTest#test_0009_turbo_frame_tag with a string id held in a local" +input: "{source: \"<%= turbo_frame_tag frame_id do %>Content<% end %>\", options: {visitors: [#], locals: {frame_id: \"post_1\"}}}" +--- +_buf = ::String.new; _buf << 'Content'.freeze; +_buf.to_s diff --git a/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0009_turbo_frame_tag_with_a_string_id_held_in_a_local_ee5293d1659c999ab179369491fae970.txt b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0009_turbo_frame_tag_with_a_string_id_held_in_a_local_ee5293d1659c999ab179369491fae970.txt new file mode 100644 index 000000000..45615c19e --- /dev/null +++ b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0009_turbo_frame_tag_with_a_string_id_held_in_a_local_ee5293d1659c999ab179369491fae970.txt @@ -0,0 +1,5 @@ +--- +source: "Engine::ActionView::TurboFrameTagTest#test_0009_turbo_frame_tag with a string id held in a local" +input: "{source: \"<%= turbo_frame_tag frame_id do %>Content<% end %>\", locals: {frame_id: \"post_1\"}, options: {action_view_helpers: true}}" +--- +Content \ No newline at end of file diff --git a/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0010_turbo_frame_tag_with_a_string_prefix_and_a_string_id_held_in_a_local_f99c1ef9dfa45cb9bb888bdeb9a0fecc.txt b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0010_turbo_frame_tag_with_a_string_prefix_and_a_string_id_held_in_a_local_f99c1ef9dfa45cb9bb888bdeb9a0fecc.txt new file mode 100644 index 000000000..3b85f6a2f --- /dev/null +++ b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0010_turbo_frame_tag_with_a_string_prefix_and_a_string_id_held_in_a_local_f99c1ef9dfa45cb9bb888bdeb9a0fecc.txt @@ -0,0 +1,5 @@ +--- +source: "Engine::ActionView::TurboFrameTagTest#test_0010_turbo_frame_tag with a string prefix and a string id held in a local" +input: "{source: \"<%= turbo_frame_tag \\\"notes\\\", frame_id do %>Content<% end %>\", locals: {frame_id: \"post_1\"}, options: {action_view_helpers: true}}" +--- +Content \ No newline at end of file diff --git a/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0010_turbo_frame_tag_with_a_string_prefix_and_a_string_id_held_in_a_local_fb07e8823d47b7f4618f7f2f56c6fec8.txt b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0010_turbo_frame_tag_with_a_string_prefix_and_a_string_id_held_in_a_local_fb07e8823d47b7f4618f7f2f56c6fec8.txt new file mode 100644 index 000000000..b03b4632b --- /dev/null +++ b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0010_turbo_frame_tag_with_a_string_prefix_and_a_string_id_held_in_a_local_fb07e8823d47b7f4618f7f2f56c6fec8.txt @@ -0,0 +1,6 @@ +--- +source: "Engine::ActionView::TurboFrameTagTest#test_0010_turbo_frame_tag with a string prefix and a string id held in a local" +input: "{source: \"<%= turbo_frame_tag \\\"notes\\\", frame_id do %>Content<% end %>\", options: {visitors: [#], locals: {frame_id: \"post_1\"}}}" +--- +_buf = ::String.new; _buf << 'Content'.freeze; +_buf.to_s diff --git a/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_literal_ids_joins_them_13d15ed6e517443019e4bac66bf642f0.txt b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_literal_ids_joins_them_13d15ed6e517443019e4bac66bf642f0.txt new file mode 100644 index 000000000..a7f299001 --- /dev/null +++ b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_literal_ids_joins_them_13d15ed6e517443019e4bac66bf642f0.txt @@ -0,0 +1,5 @@ +--- +source: "Engine::ActionView::TurboFrameTagTest#test_0011_turbo_frame_tag with literal ids joins them" +input: "{source: \"<%= turbo_frame_tag \\\"notes\\\", :list %>\", options: {visitors: [#]}}" +--- +''.freeze \ No newline at end of file diff --git a/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_literal_ids_joins_them_c05c5d47b392973f974757e1d15aa2ab.txt b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_literal_ids_joins_them_c05c5d47b392973f974757e1d15aa2ab.txt new file mode 100644 index 000000000..a822dba23 --- /dev/null +++ b/test/snapshots/engine/action_view/turbo_frame_tag_test/test_0011_turbo_frame_tag_with_literal_ids_joins_them_c05c5d47b392973f974757e1d15aa2ab.txt @@ -0,0 +1,5 @@ +--- +source: "Engine::ActionView::TurboFrameTagTest#test_0011_turbo_frame_tag with literal ids joins them" +input: "{source: \"<%= turbo_frame_tag \\\"notes\\\", :list %>\", locals: {}, options: {action_view_helpers: true}}" +--- + \ No newline at end of file