Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 }[] = []
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.")

Expand All @@ -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 %>
<h1 class="block font-bold text-lg">
Editing a <%= model.class.name %>
</h1>
<span class="block size-12">&nbsp;</span>
<% end %>
`)
})

test("output tag with helpers inside nested each loop with conditional is allowed", () => {
expectNoOffenses(dedent`
<tbody>
<% projects.each do |project| %>
<tr>
<td><a href="<%= project.github_url %>"><%= project.label %></a></td>

<td>
<a href="<%= project.version_badge_url %>">
<%= image_tag project.version_badge_image_url, alt: "#{project.label} on rubygems" %>
</a>
</td>

<td>
<% if project.ci? %>
<a href="<%= project.ci_badge_url %>">
<%= image_tag project.ci_badge_image_url, alt: "#{project.label} CI status" %>
</a>
<% end %>
</td>
</tr>
<% end %>
</tbody>
`)
})

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 %>
`)
})
})
Loading