From 5c47ad253bf5dea4ff9d8af89a11b201c5744886 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 7 Nov 2025 04:14:46 +0100 Subject: [PATCH 1/2] Linter: Fix `html-no-empty-attributes` when there's only an `if` --- .../src/rules/html-no-empty-attributes.ts | 48 ++++++++++++++++++- .../rules/html-no-empty-attributes.test.ts | 27 +++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/javascript/packages/linter/src/rules/html-no-empty-attributes.ts b/javascript/packages/linter/src/rules/html-no-empty-attributes.ts index 66e22f67e..48d3752f5 100644 --- a/javascript/packages/linter/src/rules/html-no-empty-attributes.ts +++ b/javascript/packages/linter/src/rules/html-no-empty-attributes.ts @@ -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', @@ -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) @@ -50,6 +95,7 @@ class NoEmptyAttributesVisitor extends AttributeVisitorMixin { private checkEmptyAttribute(attributeName: string, attributeValue: string, attributeNode: HTMLAttributeNode): void { if (!isRestrictedAttribute(attributeName)) return if (attributeValue.trim() !== "") return + if (containsOutputContent(attributeNode)) return const hasExplicitValue = attributeNode.value !== null diff --git a/javascript/packages/linter/test/rules/html-no-empty-attributes.test.ts b/javascript/packages/linter/test/rules/html-no-empty-attributes.test.ts index 7a4d17d21..a0a92a6da 100644 --- a/javascript/packages/linter/test/rules/html-no-empty-attributes.test.ts +++ b/javascript/packages/linter/test/rules/html-no-empty-attributes.test.ts @@ -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" @@ -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(`
Content
`) }) + + test("passes for class with ERB control flow containing static text", () => { + expectNoOffenses(dedent` +

+ Content +

+ `) + }) + + test("passes for class with ERB control flow containing output tags", () => { + expectNoOffenses(dedent` +

+ Content +

+ `) + }) + + 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` +

+ Content +

+ `) + }) }) From c269786beebf23b10bb3e8ed3bc6ef78e33972de Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 7 Nov 2025 04:43:54 +0100 Subject: [PATCH 2/2] Only check attribute value --- .../packages/linter/src/rules/html-no-empty-attributes.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/javascript/packages/linter/src/rules/html-no-empty-attributes.ts b/javascript/packages/linter/src/rules/html-no-empty-attributes.ts index 48d3752f5..13088c4ef 100644 --- a/javascript/packages/linter/src/rules/html-no-empty-attributes.ts +++ b/javascript/packages/linter/src/rules/html-no-empty-attributes.ts @@ -95,7 +95,9 @@ class NoEmptyAttributesVisitor extends AttributeVisitorMixin { private checkEmptyAttribute(attributeName: string, attributeValue: string, attributeNode: HTMLAttributeNode): void { if (!isRestrictedAttribute(attributeName)) return if (attributeValue.trim() !== "") return - if (containsOutputContent(attributeNode)) return + + if (!attributeNode?.value) return + if (containsOutputContent(attributeNode.value)) return const hasExplicitValue = attributeNode.value !== null