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..24d94a7c3 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| %> @@ -116,6 +131,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 +154,138 @@ 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 %> + `) + }) + + 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` + + `) + }) + + 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 %> +
+ `) + }) })