From e3a46754ace5604fcc960c13f98b29c717039e83 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Wed, 1 Apr 2026 00:27:03 +0200 Subject: [PATCH] Linter: Implement `erb-no-multiple-statements` rule --- .../packages/linter/docs/rules/README.md | 1 + .../docs/rules/erb-no-multiple-statements.md | 51 +++++++ javascript/packages/linter/src/rules.ts | 2 + .../src/rules/erb-no-multiple-statements.ts | 86 ++++++++++++ javascript/packages/linter/src/rules/index.ts | 1 + .../test/__snapshots__/cli.test.ts.snap | 48 +++---- .../rules/erb-no-multiple-statements.test.ts | 125 ++++++++++++++++++ 7 files changed, 290 insertions(+), 24 deletions(-) create mode 100644 javascript/packages/linter/docs/rules/erb-no-multiple-statements.md create mode 100644 javascript/packages/linter/src/rules/erb-no-multiple-statements.ts create mode 100644 javascript/packages/linter/test/rules/erb-no-multiple-statements.test.ts diff --git a/javascript/packages/linter/docs/rules/README.md b/javascript/packages/linter/docs/rules/README.md index a506bccaf..b9ab52e49 100644 --- a/javascript/packages/linter/docs/rules/README.md +++ b/javascript/packages/linter/docs/rules/README.md @@ -35,6 +35,7 @@ This page contains documentation for all Herb Linter rules. - [`erb-no-instance-variables-in-partials`](./erb-no-instance-variables-in-partials.md) - Disallow instance variables in partials - [`erb-no-interpolated-class-names`](./erb-no-interpolated-class-names.md) - Disallow ERB interpolation inside CSS class names - [`erb-no-javascript-tag-helper`](./erb-no-javascript-tag-helper.md) - Disallow `javascript_tag` helper +- [`erb-no-multiple-statements`](./erb-no-multiple-statements.md) - Disallow multiple Ruby statements in a single-line ERB tag - [`erb-no-output-control-flow`](./erb-no-output-control-flow.md) - Prevents outputting control flow blocks - [`erb-no-output-in-attribute-name`](./erb-no-output-in-attribute-name.md) - Disallow ERB output in attribute names - [`erb-no-output-in-attribute-position`](./erb-no-output-in-attribute-position.md) - Disallow ERB output in attribute position diff --git a/javascript/packages/linter/docs/rules/erb-no-multiple-statements.md b/javascript/packages/linter/docs/rules/erb-no-multiple-statements.md new file mode 100644 index 000000000..55a9d4822 --- /dev/null +++ b/javascript/packages/linter/docs/rules/erb-no-multiple-statements.md @@ -0,0 +1,51 @@ +# Linter Rule: Disallow multiple Ruby statements in a single-line ERB tag + +**Rule:** `erb-no-multiple-statements` + +## Description + +Disallow multiple Ruby statements separated by semicolons within a single-line ERB tag. Each ERB tag on a single line should contain at most one Ruby statement. + +## Rationale + +Multiple Ruby statements on a single line in an ERB tag reduce readability and make templates harder to maintain. Splitting statements into separate ERB tags makes each statement easier to understand at a glance. + +This rule only applies to single-line ERB tags. Multi-line ERB tags are not checked, as they naturally provide visual separation between statements. + +## Examples + +### ✅ Good + +```erb +<% user = User.find(1) %> +<% post = user.posts.first %> +``` + +```erb +<%= user.name %> +``` + +```erb +<% + user = User.find(1) + post = user.posts.first +%> +``` + +### 🚫 Bad + +```erb +<% user = User.find(1); post = user.posts.first %> +``` + +```erb +<%= user = User.find(1); user.name %> +``` + +```erb +<% a = 1; b = 2; c = 3 %> +``` + +## References + +- [Ruby Style Guide - Semicolons](https://rubystyle.guide/#no-semicolon) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index 7a4f935b3..987431b0c 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -25,6 +25,7 @@ import { ERBNoInlineCaseConditionsRule } from "./rules/erb-no-inline-case-condit import { ERBNoInstanceVariablesInPartialsRule } from "./rules/erb-no-instance-variables-in-partials.js" import { ERBNoInterpolatedClassNamesRule } from "./rules/erb-no-interpolated-class-names.js" import { ERBNoJavascriptTagHelperRule } from "./rules/erb-no-javascript-tag-helper.js" +import { ERBNoMultipleStatementsRule } from "./rules/erb-no-multiple-statements.js" import { ERBNoOutputControlFlowRule } from "./rules/erb-no-output-control-flow.js" import { ERBNoOutputInAttributeNameRule } from "./rules/erb-no-output-in-attribute-name.js" import { ERBNoOutputInAttributePositionRule } from "./rules/erb-no-output-in-attribute-position.js" @@ -128,6 +129,7 @@ export const rules: RuleClass[] = [ ERBNoInstanceVariablesInPartialsRule, ERBNoInterpolatedClassNamesRule, ERBNoJavascriptTagHelperRule, + ERBNoMultipleStatementsRule, ERBNoOutputControlFlowRule, ERBNoOutputInAttributeNameRule, ERBNoOutputInAttributePositionRule, diff --git a/javascript/packages/linter/src/rules/erb-no-multiple-statements.ts b/javascript/packages/linter/src/rules/erb-no-multiple-statements.ts new file mode 100644 index 000000000..213c66d08 --- /dev/null +++ b/javascript/packages/linter/src/rules/erb-no-multiple-statements.ts @@ -0,0 +1,86 @@ +import { ParserRule } from "../types.js" +import { BaseRuleVisitor } from "./rule-utils.js" + +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" +import type { ParseResult, ERBContentNode, ParserOptions, PrismNode } from "@herb-tools/core" + +function collectStatements(programNode: PrismNode): PrismNode[] { + const statements = programNode?.statements?.body + + if (!Array.isArray(statements)) return [] + + return statements +} + +class NoMultipleStatementsVisitor extends BaseRuleVisitor { + private readonly statements: PrismNode[] + + constructor(ruleName: string, context: Partial | undefined, statements: PrismNode[]) { + super(ruleName, context) + + this.statements = statements + } + + visitERBContentNode(node: ERBContentNode): void { + const startLine = node.location.start.line + const endLine = node.location.end.line + if (startLine !== endLine) return + + const tagOpening = node.tag_opening?.value + if (tagOpening === "<%#") return + + const contentRange = node.content?.range + if (!contentRange) return + + const rangeStart = contentRange.start + const rangeEnd = contentRange.end + + let statementCount = 0 + + for (const statement of this.statements) { + const statementOffset = statement.location.startOffset + + if (statementOffset >= rangeStart && statementOffset < rangeEnd) { + statementCount++ + } + } + + if (statementCount <= 1) return + + this.addOffense( + `Avoid multiple Ruby statements in a single-line ERB tag. Split each statement into its own ERB tag for better readability.`, + node.location, + ) + } +} + +export class ERBNoMultipleStatementsRule extends ParserRule { + static ruleName = "erb-no-multiple-statements" + static introducedIn = this.version("unreleased") + + get defaultConfig(): FullRuleConfig { + return { + enabled: true, + severity: "warning", + } + } + + get parserOptions(): Partial { + return { + prism_nodes: true, + prism_program: true, + } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const documentPrismNode = result.value.prismNode + if (!documentPrismNode) return [] + + const statements = collectStatements(documentPrismNode) + if (statements.length === 0) return [] + + const visitor = new NoMultipleStatementsVisitor(this.ruleName, context, statements) + 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 815093506..8094cda45 100644 --- a/javascript/packages/linter/src/rules/index.ts +++ b/javascript/packages/linter/src/rules/index.ts @@ -28,6 +28,7 @@ export * from "./erb-no-extra-whitespace-inside-tags.js" export * from "./erb-no-inline-case-conditions.js" export * from "./erb-no-instance-variables-in-partials.js" export * from "./erb-no-javascript-tag-helper.js" +export * from "./erb-no-multiple-statements.js" export * from "./erb-no-output-control-flow.js" export * from "./erb-no-output-in-attribute-name.js" export * from "./erb-no-output-in-attribute-position.js" diff --git a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap index 21a92ae51..c5888b43d 100644 --- a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap +++ b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap @@ -109,7 +109,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -170,7 +170,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 | 10 not enabled" + Rules 84 enabled | 10 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 1`] = ` @@ -193,7 +193,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 | 10 not enabled" + Rules 84 enabled | 10 not enabled" `; exports[`CLI Output Formatting > GitHub Actions format includes rule codes 2`] = ` @@ -255,7 +255,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -297,7 +297,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -324,7 +324,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -540,7 +540,7 @@ test/fixtures/multiple-rule-offenses.html.erb:4:2 Checked 1 file Offenses 8 errors | 6 warnings (14 offenses across 1 file) Fixable 14 offenses | 4 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -638,7 +638,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -678,7 +678,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 | 10 not enabled" + Rules 84 enabled | 10 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for clean file 1`] = ` @@ -689,7 +689,7 @@ exports[`CLI Output Formatting > formats GitHub Actions output correctly for cle Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled" + Rules 84 enabled | 10 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output correctly for file with errors 1`] = ` @@ -747,7 +747,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 | 10 not enabled" + Rules 84 enabled | 10 not enabled" `; exports[`CLI Output Formatting > formats GitHub Actions output with --format=github option 1`] = ` @@ -784,7 +784,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -834,7 +834,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, @@ -855,7 +855,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, @@ -928,7 +928,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, @@ -989,7 +989,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1009,7 +1009,7 @@ exports[`CLI Output Formatting > formats simple output correctly 1`] = ` Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1029,7 +1029,7 @@ exports[`CLI Output Formatting > formats simple output for bad-file correctly 1` Checked 1 file Offenses 2 errors | 0 warnings (2 offenses across 1 file) Fixable 2 offenses | 2 autocorrectable using \`--fix\` - Rules 83 enabled | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1043,7 +1043,7 @@ exports[`CLI Output Formatting > formats success output correctly 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1057,7 +1057,7 @@ exports[`CLI Output Formatting > handles boolean attributes 1`] = ` Checked 1 file Offenses 0 offenses Fixable 0 offenses - Rules 83 enabled | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1093,7 +1093,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1226,7 +1226,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1430,7 +1430,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 | 10 not enabled + Rules 84 enabled | 10 not enabled TIP: Run herb-lint --init to create a .herb.yml and lock the version. This ensures upgrading Herb won't enable new rules until you update the version in .herb.yml." @@ -1491,5 +1491,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 | 10 not enabled" + Rules 84 enabled | 10 not enabled" `; diff --git a/javascript/packages/linter/test/rules/erb-no-multiple-statements.test.ts b/javascript/packages/linter/test/rules/erb-no-multiple-statements.test.ts new file mode 100644 index 000000000..8c0eb5994 --- /dev/null +++ b/javascript/packages/linter/test/rules/erb-no-multiple-statements.test.ts @@ -0,0 +1,125 @@ +import dedent from "dedent" +import { describe, test } from "vitest" +import { ERBNoMultipleStatementsRule } from "../../src/rules/erb-no-multiple-statements.js" +import { createLinterTest } from "../helpers/linter-test-helper.js" + +const { expectNoOffenses, expectWarning, assertOffenses } = createLinterTest(ERBNoMultipleStatementsRule) + +describe("erb-no-multiple-statements", () => { + test("passes for a single statement in a silent ERB tag", () => { + expectNoOffenses('<% user = User.find(1) %>') + }) + + test("passes for a single expression in an output ERB tag", () => { + expectNoOffenses('<%= user.name %>') + }) + + test("passes for an empty ERB tag", () => { + expectNoOffenses('<% %>') + }) + + test("passes for an ERB comment", () => { + expectNoOffenses('<%# this is a comment %>') + }) + + test("passes for an ERB comment with a semicolon", () => { + expectNoOffenses('<%# this; is a; comment %>') + }) + + test("passes for multiple statements on multiple lines", () => { + expectNoOffenses(dedent` + <% + user = User.find(1) + post = user.posts.first + %> + `) + }) + + test("passes for a single method call", () => { + expectNoOffenses('<%= render partial: "header" %>') + }) + + test("passes for a single assignment", () => { + expectNoOffenses('<% @user = current_user %>') + }) + + test("passes for a single method chain", () => { + expectNoOffenses('<%= user.posts.where(published: true).count %>') + }) + + test("passes for a ternary operator", () => { + expectNoOffenses('<%= user.admin? ? "Admin" : "User" %>') + }) + + test("passes for single-line ERB tags with single statements next to each other", () => { + expectNoOffenses(dedent` + <% user = User.find(1) %> + <% post = user.posts.first %> + `) + }) + + test("passes for ERB control flow on a single line", () => { + expectNoOffenses(dedent` + <% if user.admin? %> + Admin + <% end %> + `) + }) + + test("fails for two statements separated by a semicolon", () => { + expectWarning( + "Avoid multiple Ruby statements in a single-line ERB tag. Split each statement into its own ERB tag for better readability.", + [1], + ) + + assertOffenses('<% user = User.find(1); post = user.posts.first %>') + }) + + test("fails for two statements in an output ERB tag", () => { + expectWarning( + "Avoid multiple Ruby statements in a single-line ERB tag. Split each statement into its own ERB tag for better readability.", + [1], + ) + + assertOffenses('<%= user = User.find(1); user.name %>') + }) + + test("fails for three statements separated by semicolons", () => { + expectWarning( + "Avoid multiple Ruby statements in a single-line ERB tag. Split each statement into its own ERB tag for better readability.", + [1], + ) + + assertOffenses('<% a = 1; b = 2; c = 3 %>') + }) + + test("fails for multiple statements in a template context", () => { + expectWarning( + "Avoid multiple Ruby statements in a single-line ERB tag. Split each statement into its own ERB tag for better readability.", + [2], + ) + + assertOffenses(dedent` +
+ <% user = User.find(1); post = user.posts.first %> +
+ `) + }) + + test("reports multiple offenses for multiple single-line ERB tags with multiple statements", () => { + expectWarning( + "Avoid multiple Ruby statements in a single-line ERB tag. Split each statement into its own ERB tag for better readability.", + [1], + ) + + expectWarning( + "Avoid multiple Ruby statements in a single-line ERB tag. Split each statement into its own ERB tag for better readability.", + [2], + ) + + assertOffenses(dedent` + <% a = 1; b = 2 %> + <% c = 3; d = 4 %> + `) + }) +})