Skip to content
Merged
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
@@ -1,5 +1,4 @@
import dedent from "dedent"

import { describe, test } from "vitest"

import { ActionViewNoSilentHelperRule } from "../../src/rules/actionview-no-silent-helper.js"
Expand All @@ -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| %>
Expand Down Expand Up @@ -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.")

Expand All @@ -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 %>
<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 %>
`)
})

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`
<ul>
<% @stickers.each do |sticker| %>
<li>
<%= link_to sticker %>
</li>
<% end %>
</ul>
`)
})

test("output tag with helpers inside each_with_index loop with nested blocks and conditional is allowed", () => {
expectNoOffenses(dedent`
<div class="w-full max-w-2xl mx-auto px-5">
<% @page.records.each_with_index do |byte, index| %>
<%= link_to byte_path(byte), class: "block group py-3" do %>
<div class="flex gap-3">
<%= image_tag byte.episode.podcast.artwork_url, alt: "", class: "size-11 rounded-lg" %>
<div class="flex-1 min-w-0">
<p class="text-base text-neutral-500 truncate"><%= byte.episode.podcast.name %></p>
<h2 class="mt-0.5 font-semibold"><%= byte.title %></h2>
<p class="mt-1 text-base text-neutral-600"><%= byte.subtitle %></p>
</div>
</div>
<% end %>

<% unless index == @page.records.size - 1 %>
<hr class="border-neutral-950/5">
<% end %>
<% end %>
</div>
`)
})
})
Loading