From 9febd9db92f55bcb9e57c8e1265812e29680a04b Mon Sep 17 00:00:00 2001 From: Marko Kajzer Date: Sat, 18 Apr 2026 22:08:01 +0200 Subject: [PATCH 1/3] fix(actionview-no-silent-helper): ignore helper calls in branch logic --- .../src/rules/actionview-no-silent-helper.ts | 16 ++++++++-------- .../rules/actionview-no-silent-helper.test.ts | 17 ++++++++++++++++- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts b/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts index e0c11b868..83b8724db 100644 --- a/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts +++ b/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts @@ -1,10 +1,10 @@ -import { ParserRule } from "../types.js" -import { BaseRuleVisitor } from "./rule-utils.js" import { isERBOutputNode, PrismVisitor } from "@herb-tools/core" -import { isActionViewHelperCall } from "./action-view-utils.js" +import type { ParseResult, ERBContentNode, ParserOptions, PrismNode } from "@herb-tools/core" +import { isActionViewHelperCall } from "./action-view-utils.js" +import { BaseRuleVisitor } from "./rule-utils.js" +import { ParserRule } from "../types.js" import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" -import type { ParseResult, ERBNode, ParserOptions, PrismNode } from "@herb-tools/core" class ActionViewHelperCallCollector extends PrismVisitor { public readonly matches: { helperName: string }[] = [] @@ -21,19 +21,19 @@ class ActionViewHelperCallCollector extends PrismVisitor { } class ActionViewNoSilentHelperVisitor extends BaseRuleVisitor { - visitERBNode(node: ERBNode): void { + visitERBContentNode(node: ERBContentNode): void { this.checkSilentHelper(node) - super.visitERBNode(node) + super.visitERBContentNode(node) } - private checkSilentHelper(node: ERBNode): void { + private checkSilentHelper(node: ERBContentNode): void { if (isERBOutputNode(node)) return const tagOpening = node.tag_opening?.value if (!tagOpening) return if (tagOpening.startsWith("<%%")) return - const prismNode = (node as any).prismNode + const prismNode = node.prismNode if (!prismNode) return const collector = new ActionViewHelperCallCollector() diff --git a/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts b/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts index 73121633e..d6a7ae639 100644 --- a/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts +++ b/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts @@ -1,5 +1,4 @@ import dedent from "dedent" - import { describe, test } from "vitest" import { ActionViewNoSilentHelperRule } from "../../src/rules/actionview-no-silent-helper.js" @@ -14,6 +13,22 @@ describe("ActionViewNoSilentHelperRule", () => { `) }) + test("output tag with link_to in conditional is allowed", () => { + expectNoOffenses(dedent` + <% if admin? %> + <%= link_to "Admin", admin_path %> + <% end %> + `) + }) + + test("output tag with link_to in loop is allowed", () => { + expectNoOffenses(dedent` + <% ["Herb", "Home"].each do %> + <%= link_to it, home_path %> + <% end %> + `) + }) + test("output tag with form_with is allowed", () => { expectNoOffenses(dedent` <%= form_with model: @user do |f| %> From 6bc0a4303050480fc29e90965b807e7d161ca7f7 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sun, 19 Apr 2026 07:08:10 +0900 Subject: [PATCH 2/3] Linter: Fix `actionview-no-silent-helper` inside blocks Co-Authored-By: Marko Kajzer --- .../src/rules/actionview-no-silent-helper.ts | 10 +-- .../rules/actionview-no-silent-helper.test.ts | 85 +++++++++++++++++++ 2 files changed, 90 insertions(+), 5 deletions(-) diff --git a/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts b/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts index e0c11b868..5ae1f5702 100644 --- a/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts +++ b/javascript/packages/linter/src/rules/actionview-no-silent-helper.ts @@ -4,7 +4,7 @@ import { isERBOutputNode, PrismVisitor } from "@herb-tools/core" import { isActionViewHelperCall } from "./action-view-utils.js" import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" -import type { ParseResult, ERBNode, ParserOptions, PrismNode } from "@herb-tools/core" +import type { ParseResult, ERBContentNode, ParserOptions, PrismNode } from "@herb-tools/core" class ActionViewHelperCallCollector extends PrismVisitor { public readonly matches: { helperName: string }[] = [] @@ -21,19 +21,19 @@ class ActionViewHelperCallCollector extends PrismVisitor { } class ActionViewNoSilentHelperVisitor extends BaseRuleVisitor { - visitERBNode(node: ERBNode): void { + visitERBContentNode(node: ERBContentNode): void { this.checkSilentHelper(node) - super.visitERBNode(node) + super.visitERBContentNode(node) } - private checkSilentHelper(node: ERBNode): void { + private checkSilentHelper(node: ERBContentNode): void { if (isERBOutputNode(node)) return const tagOpening = node.tag_opening?.value if (!tagOpening) return if (tagOpening.startsWith("<%%")) return - const prismNode = (node as any).prismNode + const prismNode = node.prismNode if (!prismNode) return const collector = new ActionViewHelperCallCollector() diff --git a/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts b/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts index 73121633e..4903ae30f 100644 --- a/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts +++ b/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts @@ -116,6 +116,16 @@ describe("ActionViewNoSilentHelperRule", () => { `) }) + test("silent tag with tag.div inside conditional is not allowed", () => { + expectError("Avoid using `<% %>` with `tag`. Use `<%= %>` to ensure the helper's output is rendered.") + + assertOffenses(dedent` + <% if true %> + <% tag.div "Hello" %> + <% end %> + `) + }) + test("silent tag with postfix conditional is not allowed", () => { expectError("Avoid using `<% %>` with `link_to`. Use `<%= %>` to ensure the helper's output is rendered.") @@ -129,4 +139,79 @@ describe("ActionViewNoSilentHelperRule", () => { <%= link_to "Home", root_path if show_link %> `) }) + + test("output tag with link_to inside conditional is allowed", () => { + expectNoOffenses(dedent` + <% if true %> + <%= link_to "Foo", foo_path %> + <% end %> + `) + }) + + test("output tag with link_to inside loop is allowed", () => { + expectNoOffenses(dedent` + <% ["Home", "About"].each do |label| %> + <%= link_to label, root_path %> + <% end %> + `) + }) + + test("output tag with hidden_field_tag inside each loop is allowed", () => { + expectNoOffenses(dedent` + <% data.each do |key, value| %> + <%= hidden_field_tag "form[#{key}]" %> + <% end %> + `) + }) + + test("output tag with helpers inside content_for block is allowed", () => { + expectNoOffenses(dedent` + <% content_for :mobile_header do %> + <%= link_to :back, { class: "size-12 block mt-6" } do %> + <%= image_tag "icons/back.svg", alt: "Back icon", class: "size-6" %> + <% end %> +

+ Editing a <%= model.class.name %> +

+   + <% end %> + `) + }) + + test("output tag with helpers inside nested each loop with conditional is allowed", () => { + expectNoOffenses(dedent` + + <% projects.each do |project| %> + + <%= project.label %> + + + + <%= image_tag project.version_badge_image_url, alt: "#{project.label} on rubygems" %> + + + + + <% if project.ci? %> + + <%= image_tag project.ci_badge_image_url, alt: "#{project.label} CI status" %> + + <% end %> + + + <% end %> + + `) + }) + + test("output tag with helpers inside nested conditional with block is allowed", () => { + expectNoOffenses(dedent` + <% if @topic.talks_count.to_i > 8 %> + <%= link_to talks_gem_path(gem_name: @gem.gem_name), class: "link text-primary flex items-center gap-1" do %> + View all <%= @topic.talks_count %> talks + <%= fa("arrow-right", size: :xs) %> + <% end %> + <% end %> + `) + }) }) From 04555e6c2c264f61c7f6103498612eed973e5382 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sun, 19 Apr 2026 07:13:16 +0900 Subject: [PATCH 3/3] More tests --- .../rules/actionview-no-silent-helper.test.ts | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts b/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts index 4903ae30f..a3b72ed7d 100644 --- a/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts +++ b/javascript/packages/linter/test/rules/actionview-no-silent-helper.test.ts @@ -214,4 +214,63 @@ describe("ActionViewNoSilentHelperRule", () => { <% end %> `) }) + + test("output tag with link_to in conditional is allowed", () => { + expectNoOffenses(dedent` + <% if admin? %> + <%= link_to "Admin", admin_path %> + <% end %> + `) + }) + + test("output tag with link_to in loop is allowed", () => { + expectNoOffenses(dedent` + <% ["Herb", "Home"].each do %> + <%= link_to it, home_path %> + <% end %> + `) + }) + + test("output tag with link_to inside times loop is allowed", () => { + expectNoOffenses(dedent` + <% 3.times do %> + <%= link_to("Foo", foo_path) %> + <% end %> + `) + }) + + test("output tag with link_to inside each loop in list is allowed", () => { + expectNoOffenses(dedent` +
    + <% @stickers.each do |sticker| %> +
  • + <%= link_to sticker %> +
    • + <% end %> +
+ `) + }) + + test("output tag with helpers inside each_with_index loop with nested blocks and conditional is allowed", () => { + expectNoOffenses(dedent` +
+ <% @page.records.each_with_index do |byte, index| %> + <%= link_to byte_path(byte), class: "block group py-3" do %> +
+ <%= image_tag byte.episode.podcast.artwork_url, alt: "", class: "size-11 rounded-lg" %> +
+

<%= byte.episode.podcast.name %>

+

<%= byte.title %>

+

<%= byte.subtitle %>

+
+
+ <% end %> + + <% unless index == @page.records.size - 1 %> +
+ <% end %> + <% end %> +
+ `) + }) })