From 35c3815bdd5116f53ee2abf507b896ef792d428c Mon Sep 17 00:00:00 2001 From: Marko Kajzer Date: Mon, 30 Mar 2026 02:20:25 +0200 Subject: [PATCH 1/2] Linter: Implement `erb-disallow-inline-styles` --- .../docs/rules/erb-disallow-inline-styles.md | 66 ++++++++++++++ javascript/packages/linter/src/rules.ts | 4 +- .../src/rules/erb-disallow-inline-styles.ts | 72 ++++++++++++++++ javascript/packages/linter/src/rules/index.ts | 3 +- .../test/__snapshots__/cli.test.ts.snap | 42 ++++----- .../rules/erb-disallow-inline-styles.test.ts | 85 +++++++++++++++++++ 6 files changed, 249 insertions(+), 23 deletions(-) create mode 100644 javascript/packages/linter/docs/rules/erb-disallow-inline-styles.md create mode 100644 javascript/packages/linter/src/rules/erb-disallow-inline-styles.ts create mode 100644 javascript/packages/linter/test/rules/erb-disallow-inline-styles.test.ts diff --git a/javascript/packages/linter/docs/rules/erb-disallow-inline-styles.md b/javascript/packages/linter/docs/rules/erb-disallow-inline-styles.md new file mode 100644 index 000000000..bf02f3522 --- /dev/null +++ b/javascript/packages/linter/docs/rules/erb-disallow-inline-styles.md @@ -0,0 +1,66 @@ +# Linter Rule: Disallow inline ` + +``` + +```erb +<%= content_tag :style do %> + .danger { color: red; } +<% end %> +``` + +```erb +<%= tag.style do %> + .danger { color: red; } +<% end %> +``` + +```erb + +``` + +```erb +
Content
+``` + +## References + +- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o) +- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index 7a4f935b3..8d15ab293 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -12,10 +12,11 @@ import { ActionViewStrictLocalsFirstLineRule } from "./rules/actionview-strict-l import { ActionViewStrictLocalsPartialOnlyRule } from "./rules/actionview-strict-locals-partial-only.js" import { ERBCommentSyntax } from "./rules/erb-comment-syntax.js"; +import { ERBDisallowInlineStylesRule } from "./rules/erb-disallow-inline-styles.js" import { ERBNoCaseNodeChildrenRule } from "./rules/erb-no-case-node-children.js" -import { ERBNoDebugOutputRule } from "./rules/erb-no-debug-output.js" import { ERBNoConditionalHTMLElementRule } from "./rules/erb-no-conditional-html-element.js" import { ERBNoConditionalOpenTagRule } from "./rules/erb-no-conditional-open-tag.js" +import { ERBNoDebugOutputRule } from "./rules/erb-no-debug-output.js" import { ERBNoDuplicateBranchElementsRule } from "./rules/erb-no-duplicate-branch-elements.js" import { ERBNoEmptyControlFlowRule } from "./rules/erb-no-empty-control-flow.js" import { ERBNoEmptyTagsRule } from "./rules/erb-no-empty-tags.js" @@ -115,6 +116,7 @@ export const rules: RuleClass[] = [ ActionViewStrictLocalsPartialOnlyRule, ERBCommentSyntax, + ERBDisallowInlineStylesRule, ERBNoCaseNodeChildrenRule, ERBNoDebugOutputRule, ERBNoEmptyControlFlowRule, diff --git a/javascript/packages/linter/src/rules/erb-disallow-inline-styles.ts b/javascript/packages/linter/src/rules/erb-disallow-inline-styles.ts new file mode 100644 index 000000000..c4bd382ed --- /dev/null +++ b/javascript/packages/linter/src/rules/erb-disallow-inline-styles.ts @@ -0,0 +1,72 @@ +import { getTagLocalName, getAttributeName, isERBOpenTagNode } from "@herb-tools/core" +import type { ParseResult, ParserOptions, HTMLElementNode, HTMLAttributeNode } from "@herb-tools/core" + +import { BaseRuleVisitor } from "./rule-utils.js" +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" +import { ParserRule } from "../types.js" + +const STYLESHEET_LINK_TAG_ELEMENT_SOURCE = "ActionView::Helpers::AssetTagHelper#stylesheet_link_tag" + +class ERBDisallowInlineStylesVisitor extends BaseRuleVisitor { + visitHTMLElementNode(node: HTMLElementNode): void { + if (getTagLocalName(node) === "style") { + this.checkInlineStyle(node) + } + + super.visitHTMLElementNode(node) + } + + visitHTMLAttributeNode(node: HTMLAttributeNode): void { + const attributeName = getAttributeName(node) + + if (attributeName && attributeName.toLowerCase() === "style") { + this.addOffense( + "Avoid inline `style` attribute. Use an external stylesheet or CSS classes instead.", + node.location, + ) + } + + super.visitHTMLAttributeNode(node) + } + + private checkInlineStyle(node: HTMLElementNode): void { + if (this.isStylesheetLinkTagElement(node)) return + + this.addOffense( + "Avoid inline `") + }) + + test("fails with style tag", () => { + expectWarning("Avoid inline ` + `) + }) + }) + + describe("action view helpers", () => { + test("passes with stylesheet_link_tag", () => { + expectNoOffenses(dedent` + <%= stylesheet_link_tag "application" %> + `) + }) + + test("fails with content_tag helper", () => { + expectWarning("Avoid inline ` - -``` - -```erb -<%= content_tag :style do %> - .danger { color: red; } -<% end %> -``` - -```erb -<%= tag.style do %> - .danger { color: red; } -<% end %> -``` - -```erb - -``` - -```erb -
Content
-``` - -## References - -- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o) -- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) diff --git a/javascript/packages/linter/docs/rules/html-no-style-attributes.md b/javascript/packages/linter/docs/rules/html-no-style-attributes.md new file mode 100644 index 000000000..8cb59eec2 --- /dev/null +++ b/javascript/packages/linter/docs/rules/html-no-style-attributes.md @@ -0,0 +1,38 @@ +# Linter Rule: Disallow inline `style` attributes + +**Rule:** `html-no-style-attributes` + +## Description + +Disallow the use of inline `style` attributes on HTML elements. All styling should be applied via external stylesheets or class-based styling. + +## Rationale + +Inline `style` attributes make templates harder to maintain, promote duplication, and prevent adoption of strict CSP policies (`style-src 'self'`). Using CSS classes and external stylesheets improves maintainability and security. + +## Examples + +### ✅ Good + +```erb + +``` + +```erb +
Content
+``` + +### 🚫 Bad + +```erb + +``` + +```erb +
Content
+``` + +## References + +- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o) +- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) diff --git a/javascript/packages/linter/docs/rules/html-no-style-elements.md b/javascript/packages/linter/docs/rules/html-no-style-elements.md new file mode 100644 index 000000000..4faffb6a9 --- /dev/null +++ b/javascript/packages/linter/docs/rules/html-no-style-elements.md @@ -0,0 +1,53 @@ +# Linter Rule: Disallow inline ` + +``` + +```erb +<%= content_tag :style do %> + .danger { color: red; } +<% end %> +``` + +```erb +<%= tag.style do %> + .danger { color: red; } +<% end %> +``` + +## References + +- Inspired by [@pushcx](https://bsky.app/profile/push.cx/post/3lsfddauapk2o) +- [MDN: Content Security Policy (CSP)](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index 8d15ab293..6e36f3e23 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -12,7 +12,6 @@ import { ActionViewStrictLocalsFirstLineRule } from "./rules/actionview-strict-l import { ActionViewStrictLocalsPartialOnlyRule } from "./rules/actionview-strict-locals-partial-only.js" import { ERBCommentSyntax } from "./rules/erb-comment-syntax.js"; -import { ERBDisallowInlineStylesRule } from "./rules/erb-disallow-inline-styles.js" import { ERBNoCaseNodeChildrenRule } from "./rules/erb-no-case-node-children.js" import { ERBNoConditionalHTMLElementRule } from "./rules/erb-no-conditional-html-element.js" import { ERBNoConditionalOpenTagRule } from "./rules/erb-no-conditional-open-tag.js" @@ -86,11 +85,13 @@ import { HTMLNoEmptyHeadingsRule } from "./rules/html-no-empty-headings.js" import { HTMLNoNestedLinksRule } from "./rules/html-no-nested-links.js" import { HTMLNoPositiveTabIndexRule } from "./rules/html-no-positive-tab-index.js" import { HTMLNoSelfClosingRule } from "./rules/html-no-self-closing.js" -import { HTMLNoUnescapedEntitiesRule } from "./rules/html-no-unescaped-entities.js" -import { HTMLNoUnknownTagRule } from "./rules/html-no-unknown-tag.js" import { HTMLNoSpaceInTagRule } from "./rules/html-no-space-in-tag.js" +import { HTMLNoStyleAttributesRule } from "./rules/html-no-style-attributes.js" +import { HTMLNoStyleElementsRule } from "./rules/html-no-style-elements.js" import { HTMLNoTitleAttributeRule } from "./rules/html-no-title-attribute.js" import { HTMLNoUnderscoresInAttributeNamesRule } from "./rules/html-no-underscores-in-attribute-names.js" +import { HTMLNoUnescapedEntitiesRule } from "./rules/html-no-unescaped-entities.js" +import { HTMLNoUnknownTagRule } from "./rules/html-no-unknown-tag.js" import { HTMLRequireClosingTagsRule } from "./rules/html-require-closing-tags.js" import { HTMLRequireScriptNonceRule } from "./rules/html-require-script-nonce.js" import { HTMLTagNameLowercaseRule } from "./rules/html-tag-name-lowercase.js" @@ -116,13 +117,12 @@ export const rules: RuleClass[] = [ ActionViewStrictLocalsPartialOnlyRule, ERBCommentSyntax, - ERBDisallowInlineStylesRule, ERBNoCaseNodeChildrenRule, - ERBNoDebugOutputRule, - ERBNoEmptyControlFlowRule, ERBNoConditionalHTMLElementRule, ERBNoConditionalOpenTagRule, + ERBNoDebugOutputRule, ERBNoDuplicateBranchElementsRule, + ERBNoEmptyControlFlowRule, ERBNoEmptyTagsRule, ERBNoExtraNewLineRule, ERBNoExtraWhitespaceRule, @@ -135,8 +135,6 @@ export const rules: RuleClass[] = [ ERBNoOutputInAttributePositionRule, ERBNoRawOutputInAttributeValueRule, ERBNoSilentStatementRule, - ERBNoUnusedExpressionsRule, - ERBNoUnusedLiteralsRule, ERBNoSilentTagInAttributeNameRule, ERBNoStatementInScriptRule, ERBNoThenInControlFlowRule, @@ -144,6 +142,8 @@ export const rules: RuleClass[] = [ ERBNoUnsafeJSAttributeRule, ERBNoUnsafeRawRule, ERBNoUnsafeScriptInterpolationRule, + ERBNoUnusedExpressionsRule, + ERBNoUnusedLiteralsRule, ERBPreferDirectOutputRule, ERBPreferImageTagHelperRule, ERBRequireTrailingNewlineRule, @@ -190,11 +190,13 @@ export const rules: RuleClass[] = [ HTMLNoNestedLinksRule, HTMLNoPositiveTabIndexRule, HTMLNoSelfClosingRule, - HTMLNoUnescapedEntitiesRule, - HTMLNoUnknownTagRule, HTMLNoSpaceInTagRule, + HTMLNoStyleAttributesRule, + HTMLNoStyleElementsRule, HTMLNoTitleAttributeRule, HTMLNoUnderscoresInAttributeNamesRule, + HTMLNoUnescapedEntitiesRule, + HTMLNoUnknownTagRule, HTMLRequireClosingTagsRule, HTMLRequireScriptNonceRule, HTMLTagNameLowercaseRule, diff --git a/javascript/packages/linter/src/rules/html-no-style-attributes.ts b/javascript/packages/linter/src/rules/html-no-style-attributes.ts new file mode 100644 index 000000000..cbb4790ba --- /dev/null +++ b/javascript/packages/linter/src/rules/html-no-style-attributes.ts @@ -0,0 +1,47 @@ +import { getAttributeName } from "@herb-tools/core" +import type { ParseResult, ParserOptions, HTMLAttributeNode } from "@herb-tools/core" + +import { BaseRuleVisitor } from "./rule-utils.js" +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" +import { ParserRule } from "../types.js" + +class HTMLNoStyleAttributesVisitor extends BaseRuleVisitor { + visitHTMLAttributeNode(node: HTMLAttributeNode): void { + const attributeName = getAttributeName(node) + + if (attributeName && attributeName.toLowerCase() === "style") { + this.addOffense( + "Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.", + node.location, + ) + } + + super.visitHTMLAttributeNode(node) + } +} + +export class HTMLNoStyleAttributesRule extends ParserRule { + static ruleName = "html-no-style-attributes" + static introducedIn = this.version("unreleased") + + get defaultConfig(): FullRuleConfig { + return { + enabled: false, + severity: "warning" + } + } + + get parserOptions(): Partial { + return { + action_view_helpers: true, + } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const visitor = new HTMLNoStyleAttributesVisitor(this.ruleName, context) + + visitor.visit(result.value) + + return visitor.offenses + } +} diff --git a/javascript/packages/linter/src/rules/erb-disallow-inline-styles.ts b/javascript/packages/linter/src/rules/html-no-style-elements.ts similarity index 62% rename from javascript/packages/linter/src/rules/erb-disallow-inline-styles.ts rename to javascript/packages/linter/src/rules/html-no-style-elements.ts index c4bd382ed..feb17551f 100644 --- a/javascript/packages/linter/src/rules/erb-disallow-inline-styles.ts +++ b/javascript/packages/linter/src/rules/html-no-style-elements.ts @@ -1,5 +1,5 @@ -import { getTagLocalName, getAttributeName, isERBOpenTagNode } from "@herb-tools/core" -import type { ParseResult, ParserOptions, HTMLElementNode, HTMLAttributeNode } from "@herb-tools/core" +import { getTagLocalName, isERBOpenTagNode } from "@herb-tools/core" +import type { ParseResult, ParserOptions, HTMLElementNode } from "@herb-tools/core" import { BaseRuleVisitor } from "./rule-utils.js" import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" @@ -7,7 +7,7 @@ import { ParserRule } from "../types.js" const STYLESHEET_LINK_TAG_ELEMENT_SOURCE = "ActionView::Helpers::AssetTagHelper#stylesheet_link_tag" -class ERBDisallowInlineStylesVisitor extends BaseRuleVisitor { +class HTMLNoStyleElementsVisitor extends BaseRuleVisitor { visitHTMLElementNode(node: HTMLElementNode): void { if (getTagLocalName(node) === "style") { this.checkInlineStyle(node) @@ -16,19 +16,6 @@ class ERBDisallowInlineStylesVisitor extends BaseRuleVisitor { super.visitHTMLElementNode(node) } - visitHTMLAttributeNode(node: HTMLAttributeNode): void { - const attributeName = getAttributeName(node) - - if (attributeName && attributeName.toLowerCase() === "style") { - this.addOffense( - "Avoid inline `style` attribute. Use an external stylesheet or CSS classes instead.", - node.location, - ) - } - - super.visitHTMLAttributeNode(node) - } - private checkInlineStyle(node: HTMLElementNode): void { if (this.isStylesheetLinkTagElement(node)) return @@ -45,8 +32,8 @@ class ERBDisallowInlineStylesVisitor extends BaseRuleVisitor { } } -export class ERBDisallowInlineStylesRule extends ParserRule { - static ruleName = "erb-disallow-inline-styles" +export class HTMLNoStyleElementsRule extends ParserRule { + static ruleName = "html-no-style-elements" static introducedIn = this.version("unreleased") get defaultConfig(): FullRuleConfig { @@ -63,7 +50,7 @@ export class ERBDisallowInlineStylesRule extends ParserRule { } check(result: ParseResult, context?: Partial): UnboundLintOffense[] { - const visitor = new ERBDisallowInlineStylesVisitor(this.ruleName, context) + const visitor = new HTMLNoStyleElementsVisitor(this.ruleName, context) visitor.visit(result.value) diff --git a/javascript/packages/linter/src/rules/index.ts b/javascript/packages/linter/src/rules/index.ts index 9166d6014..0ef31033c 100644 --- a/javascript/packages/linter/src/rules/index.ts +++ b/javascript/packages/linter/src/rules/index.ts @@ -17,7 +17,6 @@ export * from "./actionview-strict-locals-first-line.js" export * from "./actionview-strict-locals-partial-only.js" export * from "./erb-comment-syntax.js" -export * from "./erb-disallow-inline-styles.js" export * from "./erb-no-case-node-children.js" export * from "./erb-no-conditional-open-tag.js" export * from "./erb-no-debug-output.js" @@ -88,11 +87,13 @@ export * from "./html-no-empty-headings.js" export * from "./html-no-nested-links.js" export * from "./html-no-positive-tab-index.js" export * from "./html-no-self-closing.js" -export * from "./html-no-unescaped-entities.js" -export * from "./html-no-unknown-tag.js" export * from "./html-no-space-in-tag.js" +export * from "./html-no-style-attributes.js" +export * from "./html-no-style-elements.js" export * from "./html-no-title-attribute.js" export * from "./html-no-underscores-in-attribute-names.js" +export * from "./html-no-unescaped-entities.js" +export * from "./html-no-unknown-tag.js" export * from "./html-require-closing-tags.js" export * from "./html-require-script-nonce.js" export * from "./html-tag-name-lowercase.js" diff --git a/javascript/packages/linter/test/rules/html-no-style-attributes.test.ts b/javascript/packages/linter/test/rules/html-no-style-attributes.test.ts new file mode 100644 index 000000000..462593fdd --- /dev/null +++ b/javascript/packages/linter/test/rules/html-no-style-attributes.test.ts @@ -0,0 +1,95 @@ +import dedent from "dedent" +import { describe, test } from "vitest" + +import { HTMLNoStyleAttributesRule } from "../../src/rules/html-no-style-attributes.js" +import { createLinterTest } from "../helpers/linter-test-helper.js" + +const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(HTMLNoStyleAttributesRule) + +describe("html-no-style-attributes", () => { + test("passes with class attribute", () => { + expectNoOffenses(dedent` + + `) + }) + + test("passes with data attributes", () => { + expectNoOffenses(dedent` +
Content
+ `) + }) + + test("fails with inline style attribute", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses(dedent` + + `) + }) + + test("fails with ERB tag in style attribute", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses(dedent` +
Content
+ `) + }) + + test("fails with ERB interpolation within style value", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses(dedent` +
;">Content
+ `) + }) + + test("fails with url() containing ERB in style attribute", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses(dedent` +
Content
+ `) + }) + + describe("Action View helpers", () => { + test("passes for tag.div without style", () => { + expectNoOffenses('<%= tag.div class: "container" %>') + }) + + test("fails for tag.div with style", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses('<%= tag.div style: "color: red" %>') + }) + + test("passes for content_tag without style", () => { + expectNoOffenses('<%= content_tag :div, "content", class: "main" %>') + }) + + test("fails for content_tag with style", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses('<%= content_tag :div, "content", style: "color: red" %>') + }) + + test("passes for link_to without style", () => { + expectNoOffenses('<%= link_to "About", "/about", class: "link" %>') + }) + + test("fails for link_to with style", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses('<%= link_to "About", "/about", style: "color: red" %>') + }) + + test("fails for link_to block with style", () => { + expectWarning("Avoid inline `style` attribute. Use CSS classes or an external stylesheet instead.") + + assertOffenses(dedent` + <%= link_to "https://example.com", style: "display: inline-block;" do %> + Click here + <% end %> + `) + }) + }) +}) diff --git a/javascript/packages/linter/test/rules/erb-disallow-inline-styles.test.ts b/javascript/packages/linter/test/rules/html-no-style-elements.test.ts similarity index 60% rename from javascript/packages/linter/test/rules/erb-disallow-inline-styles.test.ts rename to javascript/packages/linter/test/rules/html-no-style-elements.test.ts index 97d36ba2d..82b98b530 100644 --- a/javascript/packages/linter/test/rules/erb-disallow-inline-styles.test.ts +++ b/javascript/packages/linter/test/rules/html-no-style-elements.test.ts @@ -1,12 +1,12 @@ import dedent from "dedent" import { describe, test } from "vitest" -import { ERBDisallowInlineStylesRule } from "../../src/rules/erb-disallow-inline-styles.js" +import { HTMLNoStyleElementsRule } from "../../src/rules/html-no-style-elements.js" import { createLinterTest } from "../helpers/linter-test-helper.js" -const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(ERBDisallowInlineStylesRule) +const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(HTMLNoStyleElementsRule) -describe("erb-disallow-inline-styles", () => { +describe("html-no-style-elements", () => { describe("inline style tags", () => { test("fails with empty style tag", () => { expectWarning("Avoid inline ` `) }) + + test("fails with style tag containing ERB comment", () => { + expectWarning("Avoid inline ` + `) + }) }) describe("action view helpers", () => { @@ -52,34 +62,4 @@ describe("erb-disallow-inline-styles", () => { `) }) }) - - describe("style attributes", () => { - test("passes with class attribute", () => { - expectNoOffenses(dedent` - - `) - }) - - test("passes with data attributes", () => { - expectNoOffenses(dedent` -
Content
- `) - }) - - test("fails with inline style attribute", () => { - expectWarning("Avoid inline `style` attribute. Use an external stylesheet or CSS classes instead.") - - assertOffenses(dedent` - - `) - }) - - test("fails with ERB tag in style attribute", () => { - expectWarning("Avoid inline `style` attribute. Use an external stylesheet or CSS classes instead.") - - assertOffenses(dedent` -
Content
- `) - }) - }) })