diff --git a/javascript/packages/linter/docs/rules/README.md b/javascript/packages/linter/docs/rules/README.md index a72f548b9..b65cb7a14 100644 --- a/javascript/packages/linter/docs/rules/README.md +++ b/javascript/packages/linter/docs/rules/README.md @@ -6,6 +6,7 @@ This page contains documentation for all Herb Linter rules. - [`erb-comment-syntax`](./erb-comment-syntax.md) - Disallow Ruby comments immediately after ERB tags - [`erb-no-case-node-children`](./erb-no-case-node-children.md) - Don't use `children` for `case/when` and `case/in` nodes +- [`erb-no-inline-case-conditions`](./erb-no-inline-case-conditions.md) - Disallow inline `case`/`when` and `case`/`in` conditions in a single ERB tag - [`erb-no-conditional-html-element`](./erb-no-conditional-html-element.md) - Disallow conditional HTML elements - [`erb-no-empty-tags`](./erb-no-empty-tags.md) - Disallow empty ERB tags - [`erb-no-extra-newline`](./erb-no-extra-newline.md) - Disallow extra newlines. diff --git a/javascript/packages/linter/docs/rules/erb-no-inline-case-conditions.md b/javascript/packages/linter/docs/rules/erb-no-inline-case-conditions.md new file mode 100644 index 000000000..d5b537b10 --- /dev/null +++ b/javascript/packages/linter/docs/rules/erb-no-inline-case-conditions.md @@ -0,0 +1,85 @@ +# Linter Rule: Disallow inline case conditions + +**Rule:** `erb-no-inline-case-conditions` + +## Description + +Disallow placing `case` and its first `when`/`in` condition in the same ERB tag. When a `case` statement and its condition appear in a single ERB tag (e.g., `<% case x when y %>`), the parser cannot reliably process, compile, or format the template. This rule flags such patterns and guides users toward separate ERB tags. + +## Rationale + +ERB templates that combine `case` with a `when` or `in` condition in a single tag create parsing ambiguity. The parser creates synthetic condition nodes to handle this pattern in non-strict mode, but the resulting AST cannot be reliably formatted or compiled. + +Using separate ERB tags for `case` and its conditions: + +- Makes the template structure unambiguous for the parser +- Enables proper formatting and compilation +- Improves readability by clearly separating the case expression from its branches +- Follows the conventional ERB style used across the Ruby on Rails ecosystem + +## Examples + +### ✅ Good + +`case`/`when` in separate ERB tags: + +```erb +<% case variable %> +<% when "a" %> + A +<% when "b" %> + B +<% else %> + Other +<% end %> +``` + +`case`/`in` (pattern matching) in separate ERB tags: + +```erb +<% case value %> +<% in 1 %> + One +<% in 2 %> + Two +<% else %> + Other +<% end %> +``` + +### 🚫 Bad + +Inline `case`/`when` in a single ERB tag: + +```erb +<% case variable when "a" %> + A +<% when "b" %> + B +<% end %> +``` + +Inline `case`/`in` in a single ERB tag: + +```erb +<% case value in 1 %> + One +<% in 2 %> + Two +<% end %> +``` + +`case`/`when` on separate lines but still in the same ERB tag: + +```erb +<% case variable + when "a" %> + A +<% when "b" %> + B +<% end %> +``` + +## References + +\- diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index f9e982a19..fdf11727f 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -2,6 +2,7 @@ import type { RuleClass } from "./types.js" import { ERBCommentSyntax } from "./rules/erb-comment-syntax.js"; import { ERBNoCaseNodeChildrenRule } from "./rules/erb-no-case-node-children.js" +import { ERBNoInlineCaseConditionsRule } from "./rules/erb-no-inline-case-conditions.js" import { ERBNoConditionalHTMLElementRule } from "./rules/erb-no-conditional-html-element.js" import { ERBNoConditionalOpenTagRule } from "./rules/erb-no-conditional-open-tag.js" import { ERBNoEmptyTagsRule } from "./rules/erb-no-empty-tags.js" @@ -68,6 +69,7 @@ import { ParserNoErrorsRule } from "./rules/parser-no-errors.js" export const rules: RuleClass[] = [ ERBCommentSyntax, ERBNoCaseNodeChildrenRule, + ERBNoInlineCaseConditionsRule, ERBNoConditionalHTMLElementRule, ERBNoConditionalOpenTagRule, ERBNoEmptyTagsRule, diff --git a/javascript/packages/linter/src/rules/erb-no-inline-case-conditions.ts b/javascript/packages/linter/src/rules/erb-no-inline-case-conditions.ts new file mode 100644 index 000000000..8c478ecba --- /dev/null +++ b/javascript/packages/linter/src/rules/erb-no-inline-case-conditions.ts @@ -0,0 +1,54 @@ +import { ParserRule } from "../types.js" +import { BaseRuleVisitor } from "./rule-utils.js" + +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" +import type { ERBCaseNode, ERBCaseMatchNode, ERBWhenNode, ERBInNode, ParseResult, ParserOptions } from "@herb-tools/core" + +class ERBNoInlineCaseConditionsVisitor extends BaseRuleVisitor { + visitERBCaseNode(node: ERBCaseNode): void { + this.checkConditions(node, "when") + this.visitChildNodes(node) + } + + visitERBCaseMatchNode(node: ERBCaseMatchNode): void { + this.checkConditions(node, "in") + this.visitChildNodes(node) + } + + private checkConditions(node: ERBCaseNode | ERBCaseMatchNode, type: string): void { + if (!node.conditions || node.conditions.length === 0) return + + for (const condition of node.conditions as (ERBWhenNode | ERBInNode)[]) { + if (condition.tag_opening === null) { + this.addOffense( + `A \`case\` statement with \`${type}\` conditions in a single ERB tag cannot be reliably parsed, compiled, and formatted. Use separate ERB tags for \`case\` and its conditions (e.g., \`<% case x %>\` followed by \`<% ${type} y %>\`).`, + node.location, + ) + break + } + } + } +} + +export class ERBNoInlineCaseConditionsRule extends ParserRule { + name = "erb-no-inline-case-conditions" + + get defaultConfig(): FullRuleConfig { + return { + enabled: true, + severity: "warning", + } + } + + get parserOptions(): Partial { + return { strict: false } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const visitor = new ERBNoInlineCaseConditionsVisitor(this.name, context) + + visitor.visit(result.value) + + return visitor.offenses + } +} diff --git a/javascript/packages/linter/src/rules/index.ts b/javascript/packages/linter/src/rules/index.ts index 03da67f93..f3239ed68 100644 --- a/javascript/packages/linter/src/rules/index.ts +++ b/javascript/packages/linter/src/rules/index.ts @@ -5,6 +5,7 @@ export * from "./herb-disable-comment-base.js" export * from "./erb-comment-syntax.js" export * from "./erb-no-case-node-children.js" +export * from "./erb-no-inline-case-conditions.js" export * from "./erb-no-conditional-open-tag.js" export * from "./erb-no-empty-tags.js" export * from "./erb-no-extra-newline.js" diff --git a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap index 7c716f53b..c480978ab 100644 --- a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap +++ b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap @@ -787,7 +787,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for bad file 1`] "summary": { "filesChecked": 1, "filesWithOffenses": 1, - "ruleCount": 55, + "ruleCount": 56, "totalErrors": 2, "totalHints": 0, "totalIgnored": 0, @@ -808,7 +808,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for clean file 1` "summary": { "filesChecked": 1, "filesWithOffenses": 0, - "ruleCount": 55, + "ruleCount": 56, "totalErrors": 0, "totalHints": 0, "totalIgnored": 0, @@ -881,7 +881,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for file with err "summary": { "filesChecked": 1, "filesWithOffenses": 1, - "ruleCount": 55, + "ruleCount": 56, "totalErrors": 3, "totalHints": 0, "totalIgnored": 0, diff --git a/javascript/packages/linter/test/rules/erb-no-inline-case-conditions.test.ts b/javascript/packages/linter/test/rules/erb-no-inline-case-conditions.test.ts new file mode 100644 index 000000000..54c302a74 --- /dev/null +++ b/javascript/packages/linter/test/rules/erb-no-inline-case-conditions.test.ts @@ -0,0 +1,76 @@ +import dedent from "dedent" + +import { describe, test } from "vitest" + +import { ERBNoInlineCaseConditionsRule } from "../../src/rules/erb-no-inline-case-conditions.js" +import { createLinterTest } from "../helpers/linter-test-helper.js" + +const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(ERBNoInlineCaseConditionsRule) + +describe("ERBNoInlineCaseConditionsRule", () => { + describe("case/when", () => { + test("valid case/when in separate ERB tags", () => { + expectNoOffenses(dedent` + <% case variable %> + <% when "a" %> + A + <% when "b" %> + B + <% else %> + Other + <% end %> + `) + }) + + test("invalid inline case/when", () => { + expectWarning('A `case` statement with `when` conditions in a single ERB tag cannot be reliably parsed, compiled, and formatted. Use separate ERB tags for `case` and its conditions (e.g., `<% case x %>` followed by `<% when y %>`).') + + assertOffenses(dedent` + <% case variable when "a" %> + A + <% when "b" %> + B + <% end %> + `) + }) + + test("invalid case/when on newline in same tag", () => { + expectWarning('A `case` statement with `when` conditions in a single ERB tag cannot be reliably parsed, compiled, and formatted. Use separate ERB tags for `case` and its conditions (e.g., `<% case x %>` followed by `<% when y %>`).') + + assertOffenses(`<% case variable\n when "a" %>\n A\n<% when "b" %>\n B\n<% end %>`) + }) + }) + + describe("case/in", () => { + test("valid case/in in separate ERB tags", () => { + expectNoOffenses(dedent` + <% case value %> + <% in 1 %> + One + <% in 2 %> + Two + <% else %> + Other + <% end %> + `) + }) + + test("invalid inline case/in", () => { + expectWarning('A `case` statement with `in` conditions in a single ERB tag cannot be reliably parsed, compiled, and formatted. Use separate ERB tags for `case` and its conditions (e.g., `<% case x %>` followed by `<% in y %>`).') + + assertOffenses(dedent` + <% case value in 1 %> + One + <% in 2 %> + Two + <% end %> + `) + }) + + test("invalid case/in on newline in same tag", () => { + expectWarning('A `case` statement with `in` conditions in a single ERB tag cannot be reliably parsed, compiled, and formatted. Use separate ERB tags for `case` and its conditions (e.g., `<% case x %>` followed by `<% in y %>`).') + + assertOffenses(`<% case value\n in 1 %>\n One\n<% in 2 %>\n Two\n<% end %>`) + }) + }) +})