diff --git a/javascript/packages/linter/docs/rules/README.md b/javascript/packages/linter/docs/rules/README.md index 4ac203b84..5c88667e4 100644 --- a/javascript/packages/linter/docs/rules/README.md +++ b/javascript/packages/linter/docs/rules/README.md @@ -135,6 +135,7 @@ This page contains documentation for all Herb Linter rules. #### Turbo +- [`turbo-permanent-no-misleading-value`](./turbo-permanent-no-misleading-value.md) - Disallow misleading values on `data-turbo-permanent` - [`turbo-permanent-require-id`](./turbo-permanent-require-id.md) - Require `id` attribute on elements with `data-turbo-permanent` diff --git a/javascript/packages/linter/docs/rules/turbo-permanent-no-misleading-value.md b/javascript/packages/linter/docs/rules/turbo-permanent-no-misleading-value.md new file mode 100644 index 000000000..b5d64eb3f --- /dev/null +++ b/javascript/packages/linter/docs/rules/turbo-permanent-no-misleading-value.md @@ -0,0 +1,32 @@ +# Linter Rule: Disallow misleading values on `data-turbo-permanent` + +**Rule:** `turbo-permanent-no-misleading-value` + +## Description + +Ensure that `data-turbo-permanent` is used correctly on HTML elements. The `data-turbo-permanent` attribute marks elements that should persist across page navigations in Turbo Drive. + +## Rationale + +`data-turbo-permanent` is active whenever the attribute is present, so `data-turbo-permanent="false"` behaves the same as `data-turbo-permanent` or `data-turbo-permanent="true"`. Allowing values is misleading and should be disallowed to avoid confusion. + +## Examples + +### ✅ Good + +```html +
1 item
+``` + +### 🚫 Bad + +```html +
1 item
+
1 item
+
1 item
+``` + +## References + +- [Turbo: `data-turbo-permanent` attribute](https://turbo.hotwired.dev/handbook/drive#permanent-elements) +- [HTML: Boolean Attributes](https://developer.mozilla.org/en-US/docs/Glossary/Boolean/HTML) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index b960ce2f2..f7bef916b 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -106,6 +106,7 @@ import { SourceIndentationRule } from "./rules/source-indentation.js" import { SVGTagNameCapitalizationRule } from "./rules/svg-tag-name-capitalization.js" +import { TurboPermanentNoMisleadingValueRule } from "./rules/turbo-permanent-no-misleading-value.js" import { TurboPermanentRequireIdRule } from "./rules/turbo-permanent-require-id.js" export const rules: RuleClass[] = [ @@ -215,5 +216,6 @@ export const rules: RuleClass[] = [ SVGTagNameCapitalizationRule, + TurboPermanentNoMisleadingValueRule, TurboPermanentRequireIdRule, ] diff --git a/javascript/packages/linter/src/rules/turbo-permanent-no-misleading-value.ts b/javascript/packages/linter/src/rules/turbo-permanent-no-misleading-value.ts new file mode 100644 index 000000000..0d52d6a76 --- /dev/null +++ b/javascript/packages/linter/src/rules/turbo-permanent-no-misleading-value.ts @@ -0,0 +1,64 @@ +import { BaseRuleVisitor } from "./rule-utils.js" +import { getAttribute, hasAttributeValue } from "@herb-tools/core" + +import { ParserRule, BaseAutofixContext, Mutable } from "../types.js" +import type { UnboundLintOffense, LintOffense, LintContext, FullRuleConfig } from "../types.js" +import type { HTMLAttributeNode, HTMLOpenTagNode, ParseResult } from "@herb-tools/core" + +interface TurboPermanentAutofixContext extends BaseAutofixContext { + node: Mutable +} + +class TurboPermanentNoMisleadingValueVisitor extends BaseRuleVisitor { + visitHTMLOpenTagNode(node: HTMLOpenTagNode): void { + this.checkTurboPermanentAttribute(node) + super.visitHTMLOpenTagNode(node) + } + + private checkTurboPermanentAttribute(node: HTMLOpenTagNode): void { + const attribute = getAttribute(node, "data-turbo-permanent") + + if (!attribute) return + if (!hasAttributeValue(attribute)) return + + this.addOffense( + "Attribute `data-turbo-permanent` should not contain any value. Its presence alone enables the behavior, so values like `\"true\"` or `\"false\"` are misleading.", + attribute.value!.location, + { + node: attribute + } + ) + } +} + +export class TurboPermanentNoMisleadingValueRule extends ParserRule { + static autocorrectable = true + static ruleName = "turbo-permanent-no-misleading-value" + static introducedIn = this.version("0.9.0") + + get defaultConfig(): FullRuleConfig { + return { + enabled: true, + severity: "error" + } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const visitor = new TurboPermanentNoMisleadingValueVisitor(this.ruleName, context) + + visitor.visit(result.value) + + return visitor.offenses + } + + autofix(offense: LintOffense, result: ParseResult, _context?: Partial): ParseResult | null { + if (!offense.autofixContext) return null + + const { node } = offense.autofixContext + + node.equals = null + node.value = null + + return result + } +} diff --git a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap index 8888e0b32..c461d4e6f 100644 --- a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap +++ b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap @@ -111,7 +111,7 @@ test/fixtures/ignored.html.erb:8:8 Offenses 5 errors | 2 warnings (7 offenses across 1 file) Note 3 additional offenses reported (would have been ignored) Fixable 7 offenses | 4 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 10 not enabled" `; exports[`CLI Output Formatting > Excluded Files > skips excluded file in subdirectory with README.md project indicator 1`] = ` @@ -177,7 +177,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 1`] = ` @@ -202,7 +202,7 @@ test/fixtures/no-trailing-newline.html.erb:1:29 Checked 1 file Offenses 1 error | 0 warnings (1 offense across 1 file) Fixable 1 offense | 1 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 2`] = ` @@ -266,7 +266,7 @@ test/fixtures/erb-no-extra-whitespace-inside-tags.html.erb:1:4 Checked 1 file Offenses 4 errors | 0 warnings (4 offenses across 1 file) Fixable 4 offenses | 3 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > Ignores disabled rules 1`] = ` @@ -307,7 +307,7 @@ test/fixtures/ignored.html.erb:6:14 Checked 1 file Offenses 2 errors | 0 warnings | 3 ignored (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > allows tag.attributes in attribute position 1`] = ` @@ -346,7 +346,7 @@ test/fixtures/tag-attributes.html.erb:5:0 Checked 1 file Offenses 2 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > diplays only parsers errors if one is present 1`] = ` @@ -372,7 +372,7 @@ test/fixtures/parser-errors.html.erb:2:16 Checked 1 file Offenses 1 error | 0 warnings (1 offense across 1 file) Fixable 0 offenses - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > displays most violated rules with multiple offenses 1`] = ` @@ -587,7 +587,7 @@ test/fixtures/multiple-rule-offenses.html.erb:4:7 Checked 1 file Offenses 8 errors | 6 warnings (14 offenses across 1 file) Fixable 14 offenses | 4 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > displays rule offenses when showing all rules 1`] = ` @@ -684,7 +684,7 @@ test/fixtures/few-rule-offenses.html.erb:6:0 Checked 1 file Offenses 4 errors | 2 warnings (6 offenses across 1 file) Fixable 6 offenses | 3 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for bad file 1`] = ` @@ -723,7 +723,7 @@ test/fixtures/bad-file.html.erb:1:16 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for clean file 1`] = ` @@ -735,7 +735,7 @@ exports[`CLI Output Formatting > formats GitHub Actions output correctly for cle Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for file with errors 1`] = ` @@ -795,7 +795,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output with --format=github option 1`] = ` @@ -834,7 +834,7 @@ test/fixtures/test-file-simple.html.erb:2:22 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats JSON output correctly for bad file 1`] = ` @@ -881,7 +881,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for bad file 1`] "summary": { "filesChecked": 1, "filesWithOffenses": 1, - "ruleCount": 83, + "ruleCount": 84, "totalErrors": 2, "totalHints": 0, "totalIgnored": 0, @@ -902,7 +902,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for clean file 1` "summary": { "filesChecked": 1, "filesWithOffenses": 0, - "ruleCount": 83, + "ruleCount": 84, "totalErrors": 0, "totalHints": 0, "totalIgnored": 0, @@ -975,7 +975,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for file with err "summary": { "filesChecked": 1, "filesWithOffenses": 1, - "ruleCount": 83, + "ruleCount": 84, "totalErrors": 2, "totalHints": 0, "totalIgnored": 0, @@ -1038,7 +1038,7 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats simple output correctly 1`] = ` @@ -1057,7 +1057,7 @@ test/fixtures/test-file-simple.html.erb: Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats simple output for bad-file correctly 1`] = ` @@ -1076,7 +1076,7 @@ test/fixtures/bad-file.html.erb: Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > formats success output correctly 1`] = ` @@ -1088,7 +1088,7 @@ exports[`CLI Output Formatting > formats success output correctly 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > handles boolean attributes 1`] = ` @@ -1100,7 +1100,7 @@ exports[`CLI Output Formatting > handles boolean attributes 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > handles multiple errors correctly 1`] = ` @@ -1135,7 +1135,7 @@ test/fixtures/bad-file.html.erb:1:16 Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > herb:disable rules 1`] = ` @@ -1267,7 +1267,7 @@ test/fixtures/disabled-1.html.erb:14:19 Checked 1 file Offenses 2 errors | 6 warnings | 8 ignored (8 offenses across 1 file) Fixable 8 offenses | 1 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > herb:disable rules 2`] = ` @@ -1470,7 +1470,7 @@ test/fixtures/disabled-2.html.erb:2:44 Checked 1 file Offenses 6 errors | 7 warnings | 5 ignored (13 offenses across 1 file) Fixable 13 offenses | 6 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; exports[`CLI Output Formatting > uses GitHub Actions format by default when GITHUB_ACTIONS is true 1`] = ` @@ -1530,5 +1530,5 @@ test/fixtures/test-file-with-errors.html.erb:2:22 Checked 1 file Offenses 2 errors | 1 warning (3 offenses across 1 file) Fixable 3 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 16 not enabled" + Rules 84 enabled | 16 not enabled" `; diff --git a/javascript/packages/linter/test/autofix/turbo-permanent-no-misleading-value.autofix.test.ts b/javascript/packages/linter/test/autofix/turbo-permanent-no-misleading-value.autofix.test.ts new file mode 100644 index 000000000..c100daa93 --- /dev/null +++ b/javascript/packages/linter/test/autofix/turbo-permanent-no-misleading-value.autofix.test.ts @@ -0,0 +1,65 @@ +import { describe, test, expect, beforeAll } from "vitest" +import { Herb } from "@herb-tools/node-wasm" +import { Linter } from "../../src/linter.js" +import { TurboPermanentNoMisleadingValueRule } from "../../src/rules/turbo-permanent-no-misleading-value.js" + +describe("turbo-permanent-no-misleading-value autofix", () => { + beforeAll(async () => { + await Herb.load() + }) + + test("removes `\"true\"` value", () => { + const input = '
1 item
' + const expected = '
1 item
' + + const linter = new Linter(Herb, [TurboPermanentNoMisleadingValueRule]) + const result = linter.autofix(input) + + expect(result.source).toBe(expected) + expect(result.fixed).toHaveLength(1) + expect(result.unfixed).toHaveLength(0) + }) + + test("removes `\"false\"` value", () => { + const input = '
1 item
' + const expected = '
1 item
' + + const linter = new Linter(Herb, [TurboPermanentNoMisleadingValueRule]) + const result = linter.autofix(input) + + expect(result.source).toBe(expected) + expect(result.fixed).toHaveLength(1) + }) + + test("removes empty string value", () => { + const input = '
1 item
' + const expected = '
1 item
' + + const linter = new Linter(Herb, [TurboPermanentNoMisleadingValueRule]) + const result = linter.autofix(input) + + expect(result.source).toBe(expected) + expect(result.fixed).toHaveLength(1) + }) + + test("removes arbitrary value", () => { + const input = '
1 item
' + const expected = '
1 item
' + + const linter = new Linter(Herb, [TurboPermanentNoMisleadingValueRule]) + const result = linter.autofix(input) + + expect(result.source).toBe(expected) + expect(result.fixed).toHaveLength(1) + }) + + test("leaves valueless attribute untouched", () => { + const input = '
1 item
' + + const linter = new Linter(Herb, [TurboPermanentNoMisleadingValueRule]) + const result = linter.autofix(input) + + expect(result.source).toBe(input) + expect(result.fixed).toHaveLength(0) + }) +}) diff --git a/javascript/packages/linter/test/rules/turbo-permanent-no-misleading-value.test.ts b/javascript/packages/linter/test/rules/turbo-permanent-no-misleading-value.test.ts new file mode 100644 index 000000000..aaec2830e --- /dev/null +++ b/javascript/packages/linter/test/rules/turbo-permanent-no-misleading-value.test.ts @@ -0,0 +1,37 @@ +import { describe, test } from "vitest" +import { TurboPermanentNoMisleadingValueRule } from "../../src/rules/turbo-permanent-no-misleading-value.js" +import { createLinterTest } from "../helpers/linter-test-helper.js" + +const { expectNoOffenses, expectError, assertOffenses } = createLinterTest(TurboPermanentNoMisleadingValueRule) + +const MESSAGE = "Attribute `data-turbo-permanent` should not contain any value. Its presence alone enables the behavior, so values like `\"true\"` or `\"false\"` are misleading." + +describe("turbo-permanent-no-misleading-value", () => { + test("passes when no explicit value is given", () => { + expectNoOffenses('
1 item
') + }) + + test("passes when attribute is absent", () => { + expectNoOffenses('
1 item
') + }) + + test("fails with value `true`", () => { + expectError(MESSAGE) + assertOffenses('
1 item
') + }) + + test("fails with value `false`", () => { + expectError(MESSAGE) + assertOffenses('
1 item
') + }) + + test("fails with arbitrary value", () => { + expectError(MESSAGE) + assertOffenses('
1 item
') + }) + + test("fails with empty string value", () => { + expectError(MESSAGE) + assertOffenses('
1 item
') + }) +})