Parser: Use location-based detection for ERB control types

src/analyze.c

@@ -90,6 +90,138 @@ static size_t process_subsequent_block(
   control_type_t parent_type
 );
 
+typedef struct {
+  control_type_t type;
+  uint32_t offset;
+  bool found;
+} earliest_control_keyword_t;
+
+typedef struct {
+  earliest_control_keyword_t* result;
+  const uint8_t* source_start;
+} location_walker_context_t;
+
+static bool find_earliest_control_keyword_walker(const pm_node_t* node, void* data) {
+  if (!node) { return true; }
+
+  location_walker_context_t* context = (location_walker_context_t*) data;
+  earliest_control_keyword_t* result = context->result;
+
+  control_type_t current_type = CONTROL_TYPE_UNKNOWN;
+  uint32_t keyword_offset = UINT32_MAX;
+
+  switch (node->type) {
+    case PM_IF_NODE: {
+      pm_if_node_t* if_node = (pm_if_node_t*) node;
+      current_type = CONTROL_TYPE_IF;
+      keyword_offset = (uint32_t) (if_node->if_keyword_loc.start - context->source_start);
+      break;
+    }
+
+    case PM_UNLESS_NODE: {
+      pm_unless_node_t* unless_node = (pm_unless_node_t*) node;
+      current_type = CONTROL_TYPE_UNLESS;
+      keyword_offset = (uint32_t) (unless_node->keyword_loc.start - context->source_start);
+      break;
+    }
+
+    case PM_CASE_NODE: {
+      pm_case_node_t* case_node = (pm_case_node_t*) node;
+      current_type = CONTROL_TYPE_CASE;
+      keyword_offset = (uint32_t) (case_node->case_keyword_loc.start - context->source_start);
+      break;
+    }
+
+    case PM_CASE_MATCH_NODE: {
+      pm_case_match_node_t* case_match_node = (pm_case_match_node_t*) node;
+      current_type = CONTROL_TYPE_CASE_MATCH;
+      keyword_offset = (uint32_t) (case_match_node->case_keyword_loc.start - context->source_start);
+      break;
+    }
+
+    case PM_WHILE_NODE: {
+      pm_while_node_t* while_node = (pm_while_node_t*) node;
+      current_type = CONTROL_TYPE_WHILE;
+      keyword_offset = (uint32_t) (while_node->keyword_loc.start - context->source_start);
+      break;
+    }
+
+    case PM_UNTIL_NODE: {
+      pm_until_node_t* until_node = (pm_until_node_t*) node;
+      current_type = CONTROL_TYPE_UNTIL;
+      keyword_offset = (uint32_t) (until_node->keyword_loc.start - context->source_start);
+      break;
+    }
+
+    case PM_FOR_NODE: {
+      pm_for_node_t* for_node = (pm_for_node_t*) node;
+      current_type = CONTROL_TYPE_FOR;
+      keyword_offset = (uint32_t) (for_node->for_keyword_loc.start - context->source_start);
+      break;
+    }
+
+    case PM_BEGIN_NODE: {
+      pm_begin_node_t* begin_node = (pm_begin_node_t*) node;
+      current_type = CONTROL_TYPE_BEGIN;
+
+      if (begin_node->begin_keyword_loc.start != NULL) {
+        keyword_offset = (uint32_t) (begin_node->begin_keyword_loc.start - context->source_start);
+      } else {
+        keyword_offset = (uint32_t) (node->location.start - context->source_start);
+      }
+      break;
+    }
+
+    case PM_YIELD_NODE: {
+      current_type = CONTROL_TYPE_YIELD;
+      keyword_offset = (uint32_t) (node->location.start - context->source_start);
+      break;
+    }
+
+    case PM_CALL_NODE: {
+      pm_call_node_t* call = (pm_call_node_t*) node;
+
+      if (call->block != NULL) {
+        current_type = CONTROL_TYPE_BLOCK;
+        keyword_offset = (uint32_t) (node->location.start - context->source_start);
+      }
+      break;
+    }
+
+    case PM_NEXT_NODE:
+    case PM_BREAK_NODE:
+    case PM_RETURN_NODE: {
+      current_type = CONTROL_TYPE_UNKNOWN;
+      keyword_offset = (uint32_t) (node->location.start - context->source_start);
+      break;
+    }
+
+    default: break;
+  }
+
+  if (keyword_offset != UINT32_MAX) {
+    if (!result->found || keyword_offset < result->offset) {
+      result->type = current_type;
+      result->offset = keyword_offset;
+      result->found = true;
+    }
+  }
+
+  return true;
+}
+
+static control_type_t find_earliest_control_keyword(pm_node_t* root, const uint8_t* source_start) {
+  if (!root) { return CONTROL_TYPE_UNKNOWN; }
+
+  earliest_control_keyword_t result = { .type = CONTROL_TYPE_UNKNOWN, .offset = UINT32_MAX, .found = false };
+
+  location_walker_context_t context = { .result = &result, .source_start = source_start };
+
+  pm_visit_node(root, find_earliest_control_keyword_walker, &context);
+
+  return result.found ? result.type : CONTROL_TYPE_UNKNOWN;
+}
+
 static control_type_t detect_control_type(AST_ERB_CONTENT_NODE_T* erb_node) {
   if (!erb_node || erb_node->base.type != AST_ERB_CONTENT_NODE) { return CONTROL_TYPE_UNKNOWN; }
 
@@ -99,26 +231,18 @@ static control_type_t detect_control_type(AST_ERB_CONTENT_NODE_T* erb_node) {
 
   if (ruby->valid) { return CONTROL_TYPE_UNKNOWN; }
 
-  if (has_block_node(ruby)) { return CONTROL_TYPE_BLOCK; }
-  if (has_if_node(ruby)) { return CONTROL_TYPE_IF; }
+  pm_node_t* root = ruby->root;
+
   if (has_elsif_node(ruby)) { return CONTROL_TYPE_ELSIF; }
   if (has_else_node(ruby)) { return CONTROL_TYPE_ELSE; }
   if (has_end(ruby)) { return CONTROL_TYPE_END; }
-  if (has_case_node(ruby)) { return CONTROL_TYPE_CASE; }
-  if (has_case_match_node(ruby)) { return CONTROL_TYPE_CASE_MATCH; }
   if (has_when_node(ruby)) { return CONTROL_TYPE_WHEN; }
   if (has_in_node(ruby)) { return CONTROL_TYPE_IN; }
-  if (has_begin_node(ruby)) { return CONTROL_TYPE_BEGIN; }
   if (has_rescue_node(ruby)) { return CONTROL_TYPE_RESCUE; }
   if (has_ensure_node(ruby)) { return CONTROL_TYPE_ENSURE; }
-  if (has_unless_node(ruby)) { return CONTROL_TYPE_UNLESS; }
-  if (has_while_node(ruby)) { return CONTROL_TYPE_WHILE; }
-  if (has_until_node(ruby)) { return CONTROL_TYPE_UNTIL; }
-  if (has_for_node(ruby)) { return CONTROL_TYPE_FOR; }
   if (has_block_closing(ruby)) { return CONTROL_TYPE_BLOCK_CLOSE; }
-  if (has_yield_node(ruby)) { return CONTROL_TYPE_YIELD; }
 
-  return CONTROL_TYPE_UNKNOWN;
+  return find_earliest_control_keyword(root, ruby->parser.start);
 }
 
 static bool is_subsequent_type(control_type_t parent_type, control_type_t child_type) {

src/analyze_helpers.c

@@ -110,12 +110,20 @@ bool search_block_nodes(const pm_node_t* node, void* data) {
   analyzed_ruby_T* analyzed = (analyzed_ruby_T*) data;
 
   if (node->type == PM_BLOCK_NODE) {
-    analyzed->has_block_node = true;
-    return true;
-  } else {
-    pm_visit_child_nodes(node, search_block_nodes, analyzed);
+    pm_block_node_t* block_node = (pm_block_node_t*) node;
+
+    size_t opening_length = block_node->opening_loc.end - block_node->opening_loc.start;
+
+    if ((opening_length == 2 && block_node->opening_loc.start[0] == 'd' && block_node->opening_loc.start[1] == 'o')
+        || (opening_length == 1 && block_node->opening_loc.start[0] == '{')) {
+      analyzed->has_block_node = true;
+
+      return true;
+    }
   }
 
+  pm_visit_child_nodes(node, search_block_nodes, analyzed);
+
   return false;
 }
 

test/analyze/block_test.rb

@@ -164,5 +164,35 @@ class BlockTest < Minitest::Spec
         <%= yield(:header) if content_for?(:header) %>
       HTML
     end
+
+    test "unclosed brace block should error" do
+      assert_parsed_snapshot(<<~HTML)
+        <% items.each { |item| %>
+          <%= item %>
+        <% } %>
+      HTML
+    end
+
+    test "unclosed brace block with end should error" do
+      assert_parsed_snapshot(<<~HTML)
+        <% items.each { |item| %>
+          <%= item %>
+        <% end %>
+      HTML
+    end
+
+    test "closed brace block in single tag is not a block" do
+      assert_parsed_snapshot(<<~HTML)
+        <% items.map { |item| item.name } %>
+      HTML
+    end
+
+    test "do/end block works as expected" do
+      assert_parsed_snapshot(<<~HTML)
+        <% items.each do |item| %>
+          <%= item %>
+        <% end %>
+      HTML
+    end
   end
 end

test/analyze/if_test.rb

@@ -134,6 +134,14 @@ class IfTest < Minitest::Spec
       HTML
     end
 
+    test "if with yield in condition" do
+      assert_parsed_snapshot(<<~HTML)
+        <% if yield(:a) %>
+          content
+        <% end %>
+      HTML
+    end
+
     test "conditional attribute value" do
       skip
 

test/parser/erb_test.rb

@@ -258,5 +258,56 @@ class ERBTest < Minitest::Spec
         more %> <% code %>
       HTML
     end
+
+    test "if/then/else with trimming and nested output tags (real-world RDoc example)" do
+      assert_parsed_snapshot(<<~'HTML')
+        <%- if @options.main_page and main_page = @files.find { |f| f.full_name == @options.main_page } then %>
+          <meta name="description" content="<%= h "#{@title}: #{excerpt(main_page.comment)}" %>">
+        <%- else %>
+          <meta name="description" content="Documentation for <%= h @title %>">
+        <%- end %>
+      HTML
+    end
+
+    test "if/elsif with block syntax in condition" do
+      assert_parsed_snapshot(<<~HTML)
+        <% if value %>
+
+        <% elsif items.any? { |item| item.true? } %>
+
+        <% end %>
+      HTML
+    end
+
+    test "if/elsif with symbol to proc in condition" do
+      assert_parsed_snapshot(<<~HTML)
+        <% if value %>
+
+        <% elsif items.any?(&:true?) %>
+
+        <% end %>
+      HTML
+    end
+
+    test "if/elsif/else with multiple block conditions and output (real-world form errors)" do
+      assert_parsed_snapshot(<<~HTML)
+        <% if f.object.errors.any? { |e| e.type == :blank } %>
+          Name is required.
+        <% elsif f.object.errors.any? { |e| e.type == :taken } %>
+          Coffee bag with this name and roast date already exists on this roaster.
+        <% else %>
+          <%= f.object.errors.first.message %>
+        <% end %>
+      HTML
+    end
+
+    test "if/elsif/else with assignment and block in condition" do
+      assert_parsed_snapshot(<<~HTML)
+        <% if something = @some.find { |t| t.id == 1 } %>
+        <% elsif other_condition %>
+        <% else %>
+        <% end %>
+      HTML
+    end
   end
 end