Skip to content

Commit 6700bed

Browse files
authored
Linter: Make html-no-duplicate-ids Action View Helpers aware (#1448)
Now that we have support for detecting Action View Tag helpers we can update the `html-no-duplicate-ids` linter rule to make it Action View Tag helpers aware. With this pull request, the following is now also flagged by the `html-no-duplicate-ids` linter rule: ```erb <%= tag.div id: "my-id" do %> content <% end %> <div id="my-id">content</div> ``` and ```erb <%= turbo_frame_tag "my-frame" do %> content <% end %> <div id="my-frame">content</div> ``` And IDs in mutually exclusive branches with tag helpers on one side and HTML on the other are correctly recognized as non-duplicates: ```erb <% if use_tag_helper? %> <%= tag.div id: "my-id" do %> content <% end %> <% else %> <div id="my-id">content</div> <% end %> ```
1 parent 37b30b5 commit 6700bed

File tree

2 files changed

+115
-1
lines changed

2 files changed

+115
-1
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Printer, IdentityPrinter } from "@herb-tools/printer"
55

66
import { hasERBOutput, getValidatableStaticContent, isEffectivelyStatic, isNode, getStaticAttributeName, isERBOutputNode } from "@herb-tools/core"
77

8-
import type { ParseResult, HTMLAttributeNode, ERBContentNode } from "@herb-tools/core"
8+
import type { ParseResult, HTMLAttributeNode, ERBContentNode, ParserOptions } from "@herb-tools/core"
99
import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types"
1010

1111
interface ControlFlowState {
@@ -211,6 +211,10 @@ export class HTMLNoDuplicateIdsRule extends ParserRule {
211211
}
212212
}
213213

214+
get parserOptions(): Partial<ParserOptions> {
215+
return { action_view_helpers: true }
216+
}
217+
214218
check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
215219
const visitor = new NoDuplicateIdsVisitor(this.ruleName, context)
216220

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,4 +450,114 @@ describe("html-no-duplicate-ids", () => {
450450
<% end %>
451451
`)
452452
})
453+
454+
test("passes for tag helper with unique id", () => {
455+
expectNoOffenses('<%= tag.div id: "unique" %>')
456+
})
457+
458+
test("fails for duplicate IDs between tag helper and HTML element", () => {
459+
expectError('Duplicate ID `my-id` found. IDs must be unique within a document.')
460+
assertOffenses(dedent`
461+
<%= tag.div id: "my-id" do %>
462+
content
463+
<% end %>
464+
<div id="my-id">content</div>
465+
`)
466+
})
467+
468+
test("fails for duplicate IDs between two tag helpers", () => {
469+
expectError('Duplicate ID `my-id` found. IDs must be unique within a document.')
470+
assertOffenses(dedent`
471+
<%= tag.div id: "my-id" do %>
472+
content
473+
<% end %>
474+
<%= tag.span id: "my-id" %>
475+
`)
476+
})
477+
478+
test("passes for IDs in mutually exclusive branches with HTML and tag helper", () => {
479+
expectNoOffenses(dedent`
480+
<% if use_tag_helper? %>
481+
<%= tag.div id: "my-id" do %>
482+
content
483+
<% end %>
484+
<% else %>
485+
<div id="my-id">content</div>
486+
<% end %>
487+
`)
488+
})
489+
490+
test("fails for duplicate IDs in same branch with void tag helper and HTML", () => {
491+
expectError('Duplicate ID `my-id` found within the same control flow branch. IDs must be unique within the same control flow branch.')
492+
assertOffenses(dedent`
493+
<% if condition? %>
494+
<%= tag.img id: "my-id", src: "/image.png", alt: "Photo" %>
495+
<img id="my-id" src="/other.png" alt="Other">
496+
<% end %>
497+
`)
498+
})
499+
500+
test("fails for duplicate IDs in same branch with block tag helper and HTML", () => {
501+
expectError('Duplicate ID `my-id` found within the same control flow branch. IDs must be unique within the same control flow branch.')
502+
assertOffenses(dedent`
503+
<% if condition? %>
504+
<%= tag.div id: "my-id" do %>
505+
content
506+
<% end %>
507+
<div id="my-id">content</div>
508+
<% end %>
509+
`)
510+
})
511+
512+
test("passes for IDs in mutually exclusive branches with tag helpers on both sides", () => {
513+
expectNoOffenses(dedent`
514+
<% if condition? %>
515+
<%= tag.div id: "shared-id" do %>
516+
branch one
517+
<% end %>
518+
<% else %>
519+
<%= tag.span id: "shared-id" do %>
520+
branch two
521+
<% end %>
522+
<% end %>
523+
`)
524+
})
525+
526+
test("fails for turbo_frame_tag with duplicate id from positional argument and another element", () => {
527+
expectError('Duplicate ID `my-frame` found. IDs must be unique within a document.')
528+
assertOffenses(dedent`
529+
<%= turbo_frame_tag "my-frame" do %>
530+
content
531+
<% end %>
532+
<div id="my-frame">content</div>
533+
`)
534+
})
535+
536+
test("passes for turbo_frame_tag id in mutually exclusive branch with HTML id", () => {
537+
expectNoOffenses(dedent`
538+
<% if turbo? %>
539+
<%= turbo_frame_tag "my-frame" do %>
540+
content
541+
<% end %>
542+
<% else %>
543+
<div id="my-frame">content</div>
544+
<% end %>
545+
`)
546+
})
547+
548+
test("fails for image_tag with duplicate id and HTML element", () => {
549+
expectError('Duplicate ID `logo` found. IDs must be unique within a document.')
550+
assertOffenses(dedent`
551+
<%= image_tag "logo.png", id: "logo" %>
552+
<img src="logo.png" id="logo">
553+
`)
554+
})
555+
556+
test("fails for link_to with duplicate id and HTML element", () => {
557+
expectError('Duplicate ID `home-link` found. IDs must be unique within a document.')
558+
assertOffenses(dedent`
559+
<%= link_to "Home", root_path, id: "home-link" %>
560+
<a href="/" id="home-link">Home</a>
561+
`)
562+
})
453563
})

0 commit comments

Comments
 (0)