Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion javascript/packages/linter/src/rules/html-no-empty-attributes.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { ParserRule } from "../types.js"
import { AttributeVisitorMixin, StaticAttributeStaticValueParams, DynamicAttributeStaticValueParams } from "./rule-utils.js"
import { IdentityPrinter } from "@herb-tools/printer"
import { Visitor, isERBOutputNode } from "@herb-tools/core"

import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
import type { ParseResult, HTMLAttributeNode } from "@herb-tools/core"
import type { ParseResult, HTMLAttributeNode, ERBContentNode, LiteralNode, Node } from "@herb-tools/core"

const RESTRICTED_ATTRIBUTES = new Set([
'id',
Expand Down Expand Up @@ -37,6 +38,50 @@ function isDataAttribute(attributeName: string): boolean {
return attributeName.startsWith('data-')
}

/**
* Visitor that checks if a node tree contains any output content.
* Output content includes:
* - Non-whitespace literal text (LiteralNode)
* - ERB output tags (<%= %>, <%== %>)
*/
class ContainsOutputContentVisitor extends Visitor {
public hasOutputContent: boolean = false

visitLiteralNode(node: LiteralNode): void {
if (this.hasOutputContent) return

if (node.content && node.content.trim() !== "") {
this.hasOutputContent = true

return
}

this.visitChildNodes(node)
}

visitERBContentNode(node: ERBContentNode): void {
if (this.hasOutputContent) return

if (isERBOutputNode(node)) {
this.hasOutputContent = true

return
}

this.visitChildNodes(node)
}
}


function containsOutputContent(node: Node): boolean {
const visitor = new ContainsOutputContentVisitor()

visitor.visit(node)

return visitor.hasOutputContent
}


class NoEmptyAttributesVisitor extends AttributeVisitorMixin {
protected checkStaticAttributeStaticValue({ attributeName, attributeValue, attributeNode }: StaticAttributeStaticValueParams): void {
this.checkEmptyAttribute(attributeName, attributeValue, attributeNode)
Expand All @@ -51,6 +96,9 @@ class NoEmptyAttributesVisitor extends AttributeVisitorMixin {
if (!isRestrictedAttribute(attributeName)) return
if (attributeValue.trim() !== "") return

if (!attributeNode?.value) return
if (containsOutputContent(attributeNode.value)) return

const hasExplicitValue = attributeNode.value !== null

if (isDataAttribute(attributeName)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import dedent from "dedent"
import { describe, test } from "vitest"
import { HTMLNoEmptyAttributesRule } from "../../src/rules/html-no-empty-attributes.js"
import { createLinterTest } from "../helpers/linter-test-helper.js"
Expand Down Expand Up @@ -143,4 +144,30 @@ describe("html-no-empty-attributes", () => {
expectWarning('Data attribute `data-turbo-permanent` should not have an empty value. Either provide a meaningful value or use `data-turbo-permanent` instead of `data-turbo-permanent=""`.')
assertOffenses(`<div data-turbo-permanent="">Content</div>`)
})

test("passes for class with ERB control flow containing static text", () => {
expectNoOffenses(dedent`
<h1 class="<% if valid? %> valid <% else %> error <% end %>">
Content
</h1>
`)
})

test("passes for class with ERB control flow containing output tags", () => {
expectNoOffenses(dedent`
<h1 class="<% if valid? %> <%= valid %> <% else %> <%= error %> <% end %>">
Content
</h1>
`)
})

test("fails for class with ERB control flow containing only non-output tags", () => {
expectWarning('Attribute `class` must not be empty. Either provide a meaningful value or remove the attribute entirely.')

assertOffenses(dedent`
<h1 class="<% if valid? %> <% no_op %> <% else %> <% no_op %> <% end %>">
Content
</h1>
`)
})
})
Loading