Skip to content

Commit e79f27c

Browse files
committed
Linter: Implement erb-closing-tag-indent rule
1 parent a20c5ba commit e79f27c

File tree

8 files changed

+397
-26
lines changed

8 files changed

+397
-26
lines changed

javascript/packages/linter/docs/rules/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ This page contains documentation for all Herb Linter rules.
1515

1616
#### ERB
1717

18+
- [`erb-closing-tag-indent`](./erb-closing-tag-indent.md) - Enforce consistent closing ERB tag indentation
1819
- [`erb-comment-syntax`](./erb-comment-syntax.md) - Disallow Ruby comments immediately after ERB tags
1920
- [`erb-no-case-node-children`](./erb-no-case-node-children.md) - Don't use `children` for `case/when` and `case/in` nodes
2021
- [`erb-no-conditional-html-element`](./erb-no-conditional-html-element.md) - Disallow conditional HTML elements
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Linter Rule: Enforce consistent closing ERB tag indentation
2+
3+
**Rule:** `erb-closing-tag-indent`
4+
5+
## Description
6+
7+
This rule enforces that the closing ERB tag (`%>`) is consistently indented relative to its opening tag (`<%` or `<%=`). When an ERB tag spans multiple lines, the closing `%>` must be on its own line and indented to match the column position of the opening tag.
8+
9+
## Rationale
10+
11+
Inconsistent indentation of closing ERB tags makes templates harder to read and maintain. When an ERB tag spans multiple lines, the closing `%>` should visually align with the opening `<%` to clearly show the tag boundaries. Conversely, if the opening tag is on the same line as the content, the closing tag should also be on the same line.
12+
13+
## Examples
14+
15+
### ✅ Good
16+
17+
```erb
18+
<%= title %>
19+
```
20+
21+
```erb
22+
<% if admin? %>
23+
<h1>Content</h1>
24+
<% end %>
25+
```
26+
27+
```erb
28+
<%
29+
some_helper(
30+
arg1,
31+
arg2
32+
)
33+
%>
34+
```
35+
36+
```erb
37+
<%
38+
if true
39+
%>
40+
```
41+
42+
### ❌ Bad
43+
44+
```erb
45+
<% if true
46+
%>
47+
```
48+
49+
```erb
50+
<%
51+
if true %>
52+
```
53+
54+
```erb
55+
<%
56+
if true
57+
%>
58+
```
59+
60+
## References
61+
62+
- [Inspiration: ERB Lint `ClosingErbTagIndent` rule](https://github.com/Shopify/erb_lint/blob/main/lib/erb_lint/linters/closing_erb_tag_indent.rb)

javascript/packages/linter/src/rules.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@ import { ActionViewNoVoidElementContentRule } from "./rules/actionview-no-void-e
77
import { ActionViewStrictLocalsFirstLineRule } from "./rules/actionview-strict-locals-first-line.js"
88
import { ActionViewStrictLocalsPartialOnlyRule } from "./rules/actionview-strict-locals-partial-only.js"
99

10+
import { ERBClosingTagIndentRule } from "./rules/erb-closing-tag-indent.js"
1011
import { ERBCommentSyntax } from "./rules/erb-comment-syntax.js";
1112
import { ERBNoCaseNodeChildrenRule } from "./rules/erb-no-case-node-children.js"
12-
import { ERBNoEmptyControlFlowRule } from "./rules/erb-no-empty-control-flow.js"
1313
import { ERBNoConditionalHTMLElementRule } from "./rules/erb-no-conditional-html-element.js"
1414
import { ERBNoConditionalOpenTagRule } from "./rules/erb-no-conditional-open-tag.js"
1515
import { ERBNoDuplicateBranchElementsRule } from "./rules/erb-no-duplicate-branch-elements.js"
16+
import { ERBNoEmptyControlFlowRule } from "./rules/erb-no-empty-control-flow.js"
1617
import { ERBNoEmptyTagsRule } from "./rules/erb-no-empty-tags.js"
1718
import { ERBNoExtraNewLineRule } from "./rules/erb-no-extra-newline.js"
1819
import { ERBNoExtraWhitespaceRule } from "./rules/erb-no-extra-whitespace-inside-tags.js"
@@ -100,12 +101,13 @@ export const rules: RuleClass[] = [
100101
ActionViewStrictLocalsFirstLineRule,
101102
ActionViewStrictLocalsPartialOnlyRule,
102103

104+
ERBClosingTagIndentRule,
103105
ERBCommentSyntax,
104106
ERBNoCaseNodeChildrenRule,
105-
ERBNoEmptyControlFlowRule,
106107
ERBNoConditionalHTMLElementRule,
107108
ERBNoConditionalOpenTagRule,
108109
ERBNoDuplicateBranchElementsRule,
110+
ERBNoEmptyControlFlowRule,
109111
ERBNoEmptyTagsRule,
110112
ERBNoExtraNewLineRule,
111113
ERBNoExtraWhitespaceRule,
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import type { ERBNode, ParseResult } from "@herb-tools/core"
2+
3+
import { BaseRuleVisitor } from "./rule-utils.js"
4+
import { ParserRule, BaseAutofixContext, Mutable } from "../types.js"
5+
import type { UnboundLintOffense, LintOffense, LintContext, FullRuleConfig } from "../types.js"
6+
7+
interface ClosingErbTagIndentAutofixContext extends BaseAutofixContext {
8+
node: Mutable<ERBNode>
9+
fixType: "remove-newline" | "add-newline" | "fix-indent"
10+
expectedIndent: number
11+
}
12+
13+
class ClosingErbTagIndentVisitor extends BaseRuleVisitor<ClosingErbTagIndentAutofixContext> {
14+
visitERBNode(node: ERBNode): void {
15+
const openTag = node.tag_opening
16+
const closeTag = node.tag_closing
17+
const content = node.content
18+
if (!openTag || !closeTag || !content) return
19+
20+
const value = content.value
21+
if (!value.length) return
22+
23+
const startsWithNewline = value.startsWith("\n")
24+
const endsWithNewline = this.endsWithNewline(value)
25+
26+
if (!startsWithNewline && endsWithNewline) {
27+
this.addOffense(
28+
`Remove newline before \`${closeTag.value}\`. The opening \`${openTag.value}\` is not followed by a newline, so the closing tag should be on the same line.`,
29+
closeTag.location,
30+
{ node, fixType: "remove-newline", expectedIndent: 0 }
31+
)
32+
} else if (startsWithNewline && !endsWithNewline) {
33+
const expectedIndent = openTag.location.start.column
34+
35+
this.addOffense(
36+
`Add newline before \`${closeTag.value}\`. The opening \`${openTag.value}\` is followed by a newline, so the closing tag should be on its own line.`,
37+
closeTag.location,
38+
{ node, fixType: "add-newline", expectedIndent }
39+
)
40+
} else if (startsWithNewline && endsWithNewline) {
41+
const expectedIndent = openTag.location.start.column
42+
const actualIndent = this.trailingIndent(value)
43+
if (actualIndent === expectedIndent) return
44+
45+
this.addOffense(
46+
`Incorrect indentation for \`${closeTag.value}\`. Expected ${expectedIndent} ${expectedIndent === 1 ? "space" : "spaces"} but found ${actualIndent}.`,
47+
closeTag.location,
48+
{ node, fixType: "fix-indent", expectedIndent }
49+
)
50+
}
51+
}
52+
53+
private endsWithNewline(value: string): boolean {
54+
const lastNewlineIndex = value.lastIndexOf("\n")
55+
if (lastNewlineIndex === -1) return false
56+
57+
const afterLastNewline = value.substring(lastNewlineIndex + 1)
58+
return afterLastNewline.length === 0 || /^\s*$/.test(afterLastNewline)
59+
}
60+
61+
private trailingIndent(value: string): number {
62+
const lastNewlineIndex = value.lastIndexOf("\n")
63+
if (lastNewlineIndex === -1) return 0
64+
65+
return value.length - lastNewlineIndex - 1
66+
}
67+
}
68+
69+
export class ERBClosingTagIndentRule extends ParserRule<ClosingErbTagIndentAutofixContext> {
70+
static autocorrectable = true
71+
static reindentAfterAutofix = true
72+
static ruleName = "erb-closing-tag-indent"
73+
74+
get defaultConfig(): FullRuleConfig {
75+
return {
76+
enabled: true,
77+
severity: "error"
78+
}
79+
}
80+
81+
check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense<ClosingErbTagIndentAutofixContext>[] {
82+
const visitor = new ClosingErbTagIndentVisitor(this.ruleName, context)
83+
84+
visitor.visit(result.value)
85+
86+
return visitor.offenses
87+
}
88+
89+
autofix(offense: LintOffense<ClosingErbTagIndentAutofixContext>, result: ParseResult, _context?: Partial<LintContext>): ParseResult | null {
90+
if (!offense.autofixContext) return null
91+
92+
const { node, fixType, expectedIndent } = offense.autofixContext
93+
if (!node.content) return null
94+
95+
const content = node.content.value
96+
97+
switch (fixType) {
98+
case "add-newline": {
99+
const trimmed = content.trimEnd()
100+
node.content.value = trimmed + "\n" + " ".repeat(expectedIndent)
101+
102+
return result
103+
}
104+
case "remove-newline": {
105+
const lastNewlineIndex = content.lastIndexOf("\n")
106+
if (lastNewlineIndex === -1) return null
107+
108+
const beforeNewline = content.substring(0, lastNewlineIndex).trimEnd()
109+
node.content.value = beforeNewline + " "
110+
111+
return result
112+
}
113+
case "fix-indent": {
114+
const lastNewlineIndex = content.lastIndexOf("\n")
115+
if (lastNewlineIndex === -1) return null
116+
117+
node.content.value = content.substring(0, lastNewlineIndex + 1) + " ".repeat(expectedIndent)
118+
119+
return result
120+
}
121+
}
122+
}
123+
}

javascript/packages/linter/src/rules/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export * from "./actionview-no-void-element-content.js"
1111
export * from "./actionview-strict-locals-first-line.js"
1212
export * from "./actionview-strict-locals-partial-only.js"
1313

14+
export * from "./erb-closing-tag-indent.js"
1415
export * from "./erb-comment-syntax.js"
1516
export * from "./erb-no-case-node-children.js"
1617
export * from "./erb-no-conditional-open-tag.js"

0 commit comments

Comments
 (0)