Linter: Make html-no-duplicate-attributes Action View Helpers aware (#1449)

Now that we have support for detecting Action View Tag helpers we can
update the `html-no-duplicate-attributes` linter rule to make it Action
View Tag helpers aware.

Since the parser already transforms the nodes we only need to add
`action_view_helpers: true` to the parser options and handle
`ERBOpenTagNode` in the visitor to reset attribute tracking for tag
helper elements.

With this pull request, the following snippets are now also flagged by
the `html-no-duplicate-attributes` linter rule:

```erb
<%= tag.div data: { value: "value-one" }, data_value: "value-two" %>

<%= tag.div aria: { label: "Label one" }, aria_label: "Label two" %>

<%= image_tag image_path("image.png"), src: "image-2.png" %>

<%= turbo_frame_tag "my-frame", id: "other-frame" %>

<%= link_to "Click here", "/path", href: "/other-path" %>

<%= javascript_include_tag "application", src: "other.js" %>
```

For example, the first example now reports as:

```
Duplicate attribute `data-value`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.
```

Author: marcoroth

javascript/packages/linter/src/rules/html-no-duplicate-attributes.ts

@@ -3,7 +3,7 @@ import { ControlFlowTrackingVisitor, ControlFlowType } from "./rule-utils.js"
 import { getAttributeName } from "@herb-tools/core"
 
 import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
-import type { HTMLOpenTagNode, HTMLAttributeNode, ParseResult, Location } from "@herb-tools/core"
+import type { HTMLOpenTagNode, HTMLAttributeNode, ParseResult, Location, ERBOpenTagNode, ParserOptions } from "@herb-tools/core"
 
 interface ControlFlowState {
   previousBranchAttributes: Set<string>
@@ -24,10 +24,19 @@ class NoDuplicateAttributesVisitor extends ControlFlowTrackingVisitor<
   private controlFlowAttributes = new Set<string>()
 
   visitHTMLOpenTagNode(node: HTMLOpenTagNode): void {
+    this.resetAttributeSets()
+    super.visitHTMLOpenTagNode(node)
+  }
+
+  visitERBOpenTagNode(node: ERBOpenTagNode): void {
+    this.resetAttributeSets()
+    super.visitERBOpenTagNode(node)
+  }
+
+  private resetAttributeSets(): void {
     this.tagAttributes = new Set()
     this.currentBranchAttributes = new Set()
     this.controlFlowAttributes = new Set()
-    super.visitHTMLOpenTagNode(node)
   }
 
   visitHTMLAttributeNode(node: HTMLAttributeNode): void {
@@ -172,6 +181,10 @@ export class HTMLNoDuplicateAttributesRule extends ParserRule {
     }
   }
 
+  get parserOptions(): Partial<ParserOptions> {
+    return { action_view_helpers: true }
+  }
+
   check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
     const visitor = new NoDuplicateAttributesVisitor(this.ruleName, context)
 

javascript/packages/linter/test/rules/html-no-duplicate-attributes.test.ts

@@ -291,4 +291,83 @@ describe("html-no-duplicate-attributes", () => {
       ></div>
     `)
   })
+
+  // Action View tag helper tests
+
+  test("passes for tag helper with unique attributes", () => {
+    expectNoOffenses('<%= tag.div class: "container", id: "main" %>')
+  })
+
+  test("fails for tag helper with duplicate data attributes via hash and underscore style", () => {
+    expectError('Duplicate attribute `data-value`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= tag.div data: { value: "value-one" }, data_value: "value-two" %>')
+  })
+
+  test("fails for tag helper with duplicate aria attributes via hash and underscore style", () => {
+    expectError('Duplicate attribute `aria-label`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= tag.div aria: { label: "Label one" }, aria_label: "Label two" %>')
+  })
+
+  test("fails for image_tag helper with duplicate src from positional argument and keyword", () => {
+    expectError('Duplicate attribute `src`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= image_tag image_path("image.png"), src: "image-2.png" %>')
+  })
+
+  test("passes for tag helper with non-overlapping data hash and underscore attributes", () => {
+    expectNoOffenses('<%= tag.div data: { controller: "content" }, data_action: "click" %>')
+  })
+
+  test("fails for tag helper with duplicate class attribute", () => {
+    expectError('Duplicate attribute `class`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= tag.div class: "one", class: "two" %>')
+  })
+
+  test("fails for javascript_include_tag with duplicate src from positional argument and keyword", () => {
+    expectError('Duplicate attribute `src`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= javascript_include_tag "application", src: "other.js" %>')
+  })
+
+  test("fails for link_to with duplicate href from positional argument and keyword", () => {
+    expectError('Duplicate attribute `href`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= link_to "Click here", "/path", href: "/other-path" %>')
+  })
+
+  test("fails for turbo_frame_tag with duplicate id from positional argument and keyword", () => {
+    expectError('Duplicate attribute `id`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= turbo_frame_tag "my-frame", id: "other-frame" %>')
+  })
+
+  test("passes for image_tag with unique attributes", () => {
+    expectNoOffenses('<%= image_tag "logo.png", alt: "Logo", class: "img-fluid" %>')
+  })
+
+  test("passes for link_to with unique attributes", () => {
+    expectNoOffenses('<%= link_to "Home", root_path, class: "nav-link" %>')
+  })
+
+  test("passes for turbo_frame_tag with unique attributes", () => {
+    expectNoOffenses('<%= turbo_frame_tag "main", src: "/path", loading: "lazy" %>')
+  })
+
+  test("fails for content_tag with duplicate data attributes via hash and underscore style", () => {
+    expectError('Duplicate attribute `data-controller`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= content_tag :div, "content", data: { controller: "one" }, data_controller: "two" %>')
+  })
+
+  test("passes for id in mutually exclusive branches with HTML and Action View tag helper", () => {
+    expectNoOffenses(dedent`
+      <% if use_tag_helper? %>
+        <%= tag.div id: "my-id" do %>
+          content
+        <% end %>
+      <% else %>
+        <div id="my-id">content</div>
+      <% end %>
+    `)
+  })
+
+  test("fails for tag helper with duplicate data attributes from multiple nested hash keys", () => {
+    expectError('Duplicate attribute `data-action`. Browsers only use the first occurrence and ignore duplicate attributes. Remove the duplicate or merge the values.')
+    assertOffenses('<%= tag.div data: { controller: "content", action: "click" }, data_action: "hover" %>')
+  })
 })