Parser: Properly detect locals that shadow Action View helpers

javascript/packages/linter/test/rules/html-no-unknown-tag.test.ts

@@ -500,5 +500,30 @@ describe("html-no-unknown-tag", () => {
     test("tag.send for custom element passes", () => {
       expectNoOffenses(`<%= tag.send("turbo-frame", id: "frame") do %>content<% end %>`)
     })
+
+    test("tag shadowed by a block argument passes", () => {
+      expectNoOffenses(`
+        <% @tags.each do |tag| %>
+          <%= tag.name %>
+        <% end %>
+      `)
+    })
+
+    test("tag shadowed by a local variable assignment passes", () => {
+      expectNoOffenses(`
+        <% tag = Tag.new(name: "Name") %>
+
+        <li><%= tag.name %></li>
+      `)
+    })
+
+    test("tag.hello is still flagged after an unrelated local variable assignment", () => {
+      expectWarning('Unknown HTML tag `<hello>`. This is not a standard HTML element.')
+
+      assertOffenses(`
+        <% label = "Name" %>
+        <%= tag.hello do %><%= label %><% end %>
+      `)
+    })
   })
 })

src/analyze/action_view/tag_helpers.c

@@ -48,29 +48,86 @@ typedef struct {
   size_t erb_content_offset;
 } tag_helper_parse_context_T;
 
-static bool build_scope_options_from_context(analyze_ruby_context_T* context, pm_options_t* options) {
-  size_t locals_count = context && context->tag_helper_locals ? hb_array_size(context->tag_helper_locals) : 0;
+typedef struct {
+  tag_helper_scope_T* scope;
+  hb_array_T* constants;
+  size_t from;
+  size_t to;
+} local_read_search_T;
 
-  if (locals_count == 0) { return false; }
-  if (!pm_options_scopes_init(options, 1)) { return false; }
+static void append_local_read_constant(local_read_search_T* search, pm_constant_id_t constant_id) {
+  pm_constant_t* constant = pm_constant_pool_id_to_constant(&search->scope->parser.constant_pool, constant_id);
 
-  pm_options_scope_t* scope = &options->scopes[0];
+  if (!constant || constant->length == 0) { return; }
 
-  if (!pm_options_scope_init(scope, locals_count)) {
-    pm_options_free(options);
+  for (size_t index = 0; index < hb_array_size(search->constants); index++) {
+    pm_constant_t* existing = hb_array_get(search->constants, index);
 
-    return false;
+    if (existing && existing->length == constant->length
+        && memcmp(existing->start, constant->start, constant->length) == 0) {
+      return;
+    }
   }
 
-  for (size_t index = 0; index < locals_count; index++) {
-    const char* local_name = hb_array_get(context->tag_helper_locals, index);
+  hb_array_append(search->constants, constant);
+}
+
+static bool search_local_variable_reads(const pm_node_t* node, void* data) {
+  local_read_search_T* search = (local_read_search_T*) data;
+  const uint8_t* base = (const uint8_t*) search->scope->buffer.value;
+
+  size_t start = (size_t) (node->location.start - base);
+  size_t end = (size_t) (node->location.end - base);
+
+  if (end <= search->from || start >= search->to) { return false; }
 
-    pm_string_constant_init(&scope->locals[index], local_name, strlen(local_name));
+  if (PM_NODE_TYPE(node) == PM_LOCAL_VARIABLE_READ_NODE && start >= search->from && end <= search->to) {
+    append_local_read_constant(search, ((pm_local_variable_read_node_t*) node)->name);
   }
 
   return true;
 }
 
+static bool build_scope_options_from_context(
+  analyze_ruby_context_T* context,
+  pm_options_t* options,
+  size_t from,
+  size_t to
+) {
+  if (!context || !context->tag_helper_scope || !context->tag_helper_scope->root) { return false; }
+  if (to <= from) { return false; }
+
+  local_read_search_T search = { .scope = context->tag_helper_scope,
+                                 .constants = hb_array_init(4, context->allocator),
+                                 .from = from,
+                                 .to = to };
+
+  pm_visit_node(context->tag_helper_scope->root, search_local_variable_reads, &search);
+
+  size_t locals_count = hb_array_size(search.constants);
+  bool built = false;
+
+  if (locals_count > 0 && pm_options_scopes_init(options, 1)) {
+    pm_options_scope_t* scope = &options->scopes[0];
+
+    if (pm_options_scope_init(scope, locals_count)) {
+      for (size_t index = 0; index < locals_count; index++) {
+        pm_constant_t* constant = hb_array_get(search.constants, index);
+
+        pm_string_constant_init(&scope->locals[index], (const char*) constant->start, constant->length);
+      }
+
+      built = true;
+    } else {
+      pm_options_free(options);
+    }
+  }
+
+  hb_array_free(&search.constants);
+
+  return built;
+}
+
 static tag_helper_parse_context_T* parse_tag_helper_content(
   const char* content_string,
   const char* original_source,
@@ -88,13 +145,16 @@ static tag_helper_parse_context_T* parse_tag_helper_content(
   parse_context->original_source = original_source;
   parse_context->erb_content_offset = erb_content_offset;
 
+  size_t content_length = strlen(parse_context->content_string);
+
   pm_options_t options = { 0 };
-  bool has_scope_options = build_scope_options_from_context(context, &options);
+  bool has_scope_options =
+    build_scope_options_from_context(context, &options, erb_content_offset, erb_content_offset + content_length);
 
   pm_parser_init(
     &parse_context->parser,
     parse_context->prism_source,
-    strlen(parse_context->content_string),
+    content_length,
     has_scope_options ? &options : NULL
   );
   parse_context->root = pm_parse(&parse_context->parser);
@@ -1774,67 +1834,12 @@ void transform_tag_helper_blocks(const AST_NODE_T* node, analyze_ruby_context_T*
   }
 }
 
-static size_t push_tag_helper_local_scope(const AST_NODE_T* node, analyze_ruby_context_T* context) {
-  if (!node || !context || !context->tag_helper_locals) { return 0; }
-
-  hb_array_T* block_arguments = NULL;
-
-  if (node->type == AST_ERB_BLOCK_NODE) {
-    block_arguments = ((AST_ERB_BLOCK_NODE_T*) node)->block_arguments;
-  } else if (node->type == AST_ERB_RENDER_NODE) {
-    block_arguments = ((AST_ERB_RENDER_NODE_T*) node)->block_arguments;
-  }
-
-  if (!block_arguments) { return 0; }
-
-  size_t pushed = 0;
-
-  for (size_t index = 0; index < hb_array_size(block_arguments); index++) {
-    AST_NODE_T* argument = hb_array_get(block_arguments, index);
-
-    if (!argument || argument->type != AST_RUBY_PARAMETER_NODE) { continue; }
-
-    AST_RUBY_PARAMETER_NODE_T* parameter = (AST_RUBY_PARAMETER_NODE_T*) argument;
-
-    if (!parameter->name || hb_string_is_empty(parameter->name->value)) { continue; }
-
-    char* name = hb_allocator_strndup(context->allocator, parameter->name->value.data, parameter->name->value.length);
-
-    if (name && hb_array_append(context->tag_helper_locals, name)) {
-      pushed++;
-    } else if (name) {
-      hb_allocator_dealloc(context->allocator, name);
-    }
-  }
-
-  return pushed;
-}
-
-static void pop_tag_helper_local_scope(analyze_ruby_context_T* context, size_t count) {
-  if (!context || !context->tag_helper_locals) { return; }
-
-  for (size_t index = 0; index < count; index++) {
-    size_t size = hb_array_size(context->tag_helper_locals);
-
-    if (size == 0) { break; }
-
-    char* name = hb_array_get(context->tag_helper_locals, size - 1);
-    hb_array_remove(context->tag_helper_locals, size - 1);
-
-    if (name) { hb_allocator_dealloc(context->allocator, name); }
-  }
-}
-
 bool transform_tag_helper_nodes(const AST_NODE_T* node, void* data) {
   analyze_ruby_context_T* context = (analyze_ruby_context_T*) data;
 
-  size_t pushed_locals = push_tag_helper_local_scope(node, context);
-
   transform_tag_helper_blocks(node, context);
 
   herb_visit_child_nodes(node, transform_tag_helper_nodes, data);
 
-  pop_tag_helper_local_scope(context, pushed_locals);
-
   return false;
 }

src/analyze/analyze.c

@@ -16,8 +16,10 @@
 #include "../include/ast/ast_node.h"
 #include "../include/ast/ast_nodes.h"
 #include "../include/errors.h"
+#include "../include/extract.h"
 #include "../include/lexer/token_struct.h"
 #include "../include/lib/hb_array.h"
+#include "../include/lib/hb_buffer.h"
 #include "../include/lib/hb_string.h"
 #include "../include/lib/string.h"
 #include "../include/location/location.h"
@@ -1016,6 +1018,63 @@ hb_array_T* rewrite_node_array(AST_NODE_T* node, hb_array_T* array, analyze_ruby
   return new_array;
 }
 
+static tag_helper_scope_T* tag_helper_scope_init(const char* source, hb_allocator_T* allocator) {
+  if (!source) { return NULL; }
+
+  tag_helper_scope_T* scope = hb_allocator_alloc(allocator, sizeof(tag_helper_scope_T));
+
+  if (!scope) { return NULL; }
+
+  memset(scope, 0, sizeof(tag_helper_scope_T));
+
+  if (!hb_buffer_init(&scope->buffer, strlen(source), allocator)) {
+    hb_allocator_dealloc(allocator, scope);
+
+    return NULL;
+  }
+
+  herb_extract_ruby_options_T extract_options = {
+    .semicolons = true,
+    .comments = false,
+    .preserve_positions = true,
+  };
+
+  herb_extract_ruby_to_buffer_with_options(source, &scope->buffer, &extract_options, allocator);
+
+  if (!scope->buffer.value || scope->buffer.length == 0) {
+    hb_buffer_free(&scope->buffer);
+    hb_allocator_dealloc(allocator, scope);
+
+    return NULL;
+  }
+
+  pm_options_partial_script_set(&scope->options, true);
+  pm_parser_init(&scope->parser, (const uint8_t*) scope->buffer.value, scope->buffer.length, &scope->options);
+
+  scope->root = pm_parse(&scope->parser);
+
+  if (!scope->root) {
+    pm_parser_free(&scope->parser);
+    pm_options_free(&scope->options);
+    hb_buffer_free(&scope->buffer);
+    hb_allocator_dealloc(allocator, scope);
+
+    return NULL;
+  }
+
+  return scope;
+}
+
+static void tag_helper_scope_free(tag_helper_scope_T* scope, hb_allocator_T* allocator) {
+  if (!scope) { return; }
+
+  pm_node_destroy(&scope->parser, scope->root);
+  pm_parser_free(&scope->parser);
+  pm_options_free(&scope->options);
+  hb_buffer_free(&scope->buffer);
+  hb_allocator_dealloc(allocator, scope);
+}
+
 void herb_analyze_parse_tree(
   AST_DOCUMENT_NODE_T* document,
   const char* source,
@@ -1029,7 +1088,7 @@ void herb_analyze_parse_tree(
     .document = document,
     .parent = NULL,
     .ruby_context_stack = hb_array_init(8, allocator),
-    .tag_helper_locals = hb_array_init(8, allocator),
+    .tag_helper_scope = NULL,
     .allocator = allocator,
     .source = source,
   };
@@ -1051,7 +1110,12 @@ void herb_analyze_parse_tree(
   }
 
   if (options && options->action_view_helpers) {
+    context.tag_helper_scope = tag_helper_scope_init(source, allocator);
+
     herb_visit_node((AST_NODE_T*) document, transform_tag_helper_nodes, &context);
+
+    tag_helper_scope_free(context.tag_helper_scope, allocator);
+    context.tag_helper_scope = NULL;
   }
 
   herb_transform_conditional_elements(document, allocator);
@@ -1069,11 +1133,5 @@ void herb_analyze_parse_tree(
 
   herb_parser_match_html_tags_post_analyze(document, options, allocator);
 
-  for (size_t index = hb_array_size(context.tag_helper_locals); index > 0; index--) {
-    char* local_name = hb_array_get(context.tag_helper_locals, index - 1);
-    if (local_name) { hb_allocator_dealloc(allocator, local_name); }
-  }
-
-  hb_array_free(&context.tag_helper_locals);
   hb_array_free(&context.ruby_context_stack);
 }

src/include/analyze/analyze.h

@@ -4,14 +4,22 @@
 #include "../ast/ast_nodes.h"
 #include "../lib/hb_allocator.h"
 #include "../lib/hb_array.h"
+#include "../lib/hb_buffer.h"
 #include "../parser/parser.h"
 #include "analyzed_ruby.h"
 
+typedef struct TAG_HELPER_SCOPE_STRUCT {
+  hb_buffer_T buffer;
+  pm_options_t options;
+  pm_parser_t parser;
+  pm_node_t* root;
+} tag_helper_scope_T;
+
 typedef struct ANALYZE_RUBY_CONTEXT_STRUCT {
   AST_DOCUMENT_NODE_T* document;
   AST_NODE_T* parent;
   hb_array_T* ruby_context_stack;
-  hb_array_T* tag_helper_locals;
+  tag_helper_scope_T* tag_helper_scope;
   hb_allocator_T* allocator;
   const char* source;
   bool found_strict_locals;