Skip to content

Commit 765c61b

Browse files
committed
Linter: Implement herb-formatter-well-formatted rule
1 parent 3fb47ca commit 765c61b

File tree

9 files changed

+491
-3
lines changed

9 files changed

+491
-3
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ This page contains documentation for all Herb Linter rules.
3838
- [`herb-disable-comment-no-redundant-all`](./herb-disable-comment-no-redundant-all.md) - Disallow redundant use of `all` in `herb:disable` comments.
3939
- [`herb-disable-comment-unnecessary`](./herb-disable-comment-unnecessary.md) - Detect unnecessary `herb:disable` comments.
4040
- [`herb-disable-comment-valid-rule-name`](./herb-disable-comment-valid-rule-name.md) - Validate rule names in `herb:disable` comments.
41+
- [`herb-formatter-well-formatted`](./herb-formatter-well-formatted.md) - Check source is well-formatted.
4142
- [`html-allowed-script-type`](./html-allowed-script-type.md) - Restrict allowed `type` attributes for `<script>` tags
4243
- [`html-anchor-require-href`](./html-anchor-require-href.md) - Requires an href attribute on anchor tags
4344
- [`html-aria-attribute-must-be-valid`](./html-aria-attribute-must-be-valid.md) - Disallow invalid or unknown `aria-*` attributes.
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# Linter Rule: Check source is well-formatted
2+
3+
**Rule:** `herb-formatter-well-formatted`
4+
5+
## Description
6+
7+
This rule checks if the source code matches the output of the Herb Formatter. It shows formatting issues as diagnostics (squiggly lines in editors), allowing you to see what would change when formatting is applied.
8+
9+
## Configuration
10+
11+
This rule is **disabled by default**. To enable it, you must:
12+
13+
1. Enable the formatter in your `.herb.yml`
14+
2. Enable this rule
15+
16+
```yaml
17+
formatter:
18+
enabled: true
19+
20+
linter:
21+
rules:
22+
herb-formatter-well-formatted:
23+
enabled: true
24+
```
25+
26+
The rule will only run when both conditions are met. It also respects the formatter's file exclusion patterns.
27+
28+
## Rationale
29+
30+
This rule bridges the gap between linting and formatting by surfacing formatting issues as lint warnings. This is useful when:
31+
32+
* You want to see formatting issues inline in your editor before running the formatter
33+
* You want CI to fail on unformatted code without actually modifying files
34+
* You prefer reviewing formatting changes before applying them
35+
36+
To fix the issues, run the Herb formatter CLI: `herb format`.
37+
38+
## Messages
39+
40+
The rule generates contextual messages based on the type of formatting issue:
41+
42+
* **Incorrect indentation: expected N spaces, found M** - When indentation differs
43+
* **Unexpected whitespace** - When extra whitespace should be removed
44+
* **Missing whitespace** - When whitespace should be added
45+
* **Incorrect line breaks** - When line break count differs
46+
* **Formatting differs from expected** - Generic message for other differences
47+
48+
## Examples
49+
50+
### ✅ Good
51+
52+
```erb
53+
<div>
54+
<p>Hello</p>
55+
</div>
56+
```
57+
58+
```erb
59+
<%= render partial: "header" %>
60+
```
61+
62+
### 🚫 Bad
63+
64+
```erb
65+
<div>
66+
<p>Hello</p>
67+
</div>
68+
```
69+
70+
```erb
71+
<div> </div>
72+
```
73+
74+
```erb
75+
<div>
76+
<p>Too much indentation</p>
77+
</div>
78+
```
79+
80+
## References
81+
82+
- [GitHub Issue #916](https://github.com/marcoroth/herb/issues/916)

javascript/packages/linter/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"dependencies": {
4949
"@herb-tools/config": "0.8.10",
5050
"@herb-tools/core": "0.8.10",
51+
"@herb-tools/formatter": "0.8.10",
5152
"@herb-tools/highlighter": "0.8.10",
5253
"@herb-tools/node-wasm": "0.8.10",
5354
"@herb-tools/printer": "0.8.10",

javascript/packages/linter/project.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"@herb-tools/config:build",
1414
"@herb-tools/core:build",
1515
"@herb-tools/browser:build",
16+
"@herb-tools/formatter:build",
1617
"@herb-tools/highlighter:build",
1718
"@herb-tools/printer:build",
1819
"@herb-tools/rewriter:build"

javascript/packages/linter/src/linter.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,8 @@ export class Linter {
368368
context = {
369369
...context,
370370
validRuleNames: this.getAvailableRules().map(ruleClass => ruleClass.ruleName),
371-
ignoredOffensesByLine
371+
ignoredOffensesByLine,
372+
config: this.config
372373
}
373374

374375
const regularRules = this.rules.filter(ruleClass => ruleClass.ruleName !== "herb-disable-comment-unnecessary")

javascript/packages/linter/src/rules.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import { HerbDisableCommentNoDuplicateRulesRule } from "./rules/herb-disable-com
3838
import { HerbDisableCommentNoRedundantAllRule } from "./rules/herb-disable-comment-no-redundant-all.js"
3939
import { HerbDisableCommentUnnecessaryRule } from "./rules/herb-disable-comment-unnecessary.js"
4040
import { HerbDisableCommentValidRuleNameRule } from "./rules/herb-disable-comment-valid-rule-name.js"
41+
import { HerbFormatterWellFormattedRule } from "./rules/herb-formatter-well-formatted.js"
4142

4243
import { HTMLAllowedScriptTypeRule } from "./rules/html-allowed-script-type.js"
4344
import { HTMLAnchorRequireHrefRule } from "./rules/html-anchor-require-href.js"
@@ -120,6 +121,7 @@ export const rules: RuleClass[] = [
120121
HerbDisableCommentNoRedundantAllRule,
121122
HerbDisableCommentUnnecessaryRule,
122123
HerbDisableCommentValidRuleNameRule,
124+
HerbFormatterWellFormattedRule,
123125

124126
HTMLAllowedScriptTypeRule,
125127
HTMLAnchorRequireHrefRule,
Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
import { Herb } from "@herb-tools/node-wasm"
2+
import { Location } from "@herb-tools/core"
3+
import { Formatter } from "@herb-tools/formatter"
4+
5+
import { SourceRule } from "../types.js"
6+
import { positionFromOffset } from "./rule-utils.js"
7+
8+
import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
9+
10+
interface DiffRange {
11+
startOffset: number
12+
endOffset: number
13+
originalContent: string
14+
formattedContent: string
15+
}
16+
17+
export class HerbFormatterWellFormattedRule extends SourceRule {
18+
static autocorrectable = false
19+
static ruleName = "herb-formatter-well-formatted"
20+
21+
get defaultConfig(): FullRuleConfig {
22+
return {
23+
enabled: false,
24+
severity: "warning"
25+
}
26+
}
27+
28+
isEnabled(_source: string, context?: Partial<LintContext>): boolean {
29+
const config = context?.config
30+
31+
if (!config) return true
32+
if (!config.isFormatterEnabled) return false
33+
if (context?.fileName && !config.isFormatterEnabledForPath(context.fileName)) return false
34+
35+
return true
36+
}
37+
38+
check(source: string, context?: Partial<LintContext>): UnboundLintOffense[] {
39+
const offenses: UnboundLintOffense[] = []
40+
41+
if (source.length === 0) return offenses
42+
43+
try {
44+
const config = context?.config
45+
const filePath = context?.fileName
46+
const formatter = config ? Formatter.from(Herb, config) : new Formatter(Herb)
47+
const formatted = formatter.format(source, {}, filePath)
48+
49+
if (source === formatted) return offenses
50+
51+
const diffs = this.computeDiffs(source, formatted)
52+
53+
for (const diff of diffs) {
54+
const startPosition = positionFromOffset(source, diff.startOffset)
55+
const endPosition = positionFromOffset(source, diff.endOffset)
56+
const location = Location.from(startPosition.line, startPosition.column, endPosition.line, endPosition.column)
57+
const message = this.generateMessage(diff)
58+
59+
offenses.push({
60+
rule: this.ruleName,
61+
code: this.ruleName,
62+
source: "Herb Linter",
63+
message,
64+
location
65+
})
66+
}
67+
} catch {
68+
// Skip silently on formatting errors
69+
}
70+
71+
return offenses
72+
}
73+
74+
private computeDiffs(original: string, formatted: string): DiffRange[] {
75+
const diffs: DiffRange[] = []
76+
const originalLength = original.length
77+
const formattedLength = formatted.length
78+
79+
let originalIndex = 0
80+
let formattedIndex = 0
81+
82+
while (originalIndex < originalLength || formattedIndex < formattedLength) {
83+
if (originalIndex < originalLength && formattedIndex < formattedLength && original[originalIndex] === formatted[formattedIndex]) {
84+
originalIndex++
85+
formattedIndex++
86+
continue
87+
}
88+
89+
const diffStartOriginal = originalIndex
90+
const diffStartFormatted = formattedIndex
91+
const syncPoint = this.findSyncPoint(original, formatted, originalIndex, formattedIndex)
92+
93+
if (syncPoint) {
94+
const originalEnd = syncPoint.originalIndex
95+
const formattedEnd = syncPoint.formattedIndex
96+
97+
if (originalEnd > diffStartOriginal || formattedEnd > diffStartFormatted) {
98+
diffs.push({
99+
startOffset: diffStartOriginal,
100+
endOffset: Math.max(diffStartOriginal + 1, originalEnd),
101+
originalContent: original.slice(diffStartOriginal, originalEnd),
102+
formattedContent: formatted.slice(diffStartFormatted, formattedEnd)
103+
})
104+
}
105+
106+
originalIndex = originalEnd
107+
formattedIndex = formattedEnd
108+
} else {
109+
diffs.push({
110+
startOffset: diffStartOriginal,
111+
endOffset: originalLength,
112+
originalContent: original.slice(diffStartOriginal),
113+
formattedContent: formatted.slice(diffStartFormatted)
114+
})
115+
116+
break
117+
}
118+
}
119+
120+
return this.coalesceDiffs(diffs)
121+
}
122+
123+
private findSyncPoint(original: string, formatted: string, originalStart: number, formattedStart: number): { originalIndex: number; formattedIndex: number } | null {
124+
const originalLength = original.length
125+
const formattedLength = formatted.length
126+
const maxLookahead = 100
127+
const minMatchLength = 3
128+
129+
for (let originalOffset = 1; originalOffset <= maxLookahead && originalStart + originalOffset <= originalLength; originalOffset++) {
130+
for (let formattedOffset = 1; formattedOffset <= maxLookahead && formattedStart + formattedOffset <= formattedLength; formattedOffset++) {
131+
const originalPosition = originalStart + originalOffset
132+
const formattedPosition = formattedStart + formattedOffset
133+
134+
let matchLength = 0
135+
136+
while (
137+
originalPosition + matchLength < originalLength &&
138+
formattedPosition + matchLength < formattedLength &&
139+
original[originalPosition + matchLength] === formatted[formattedPosition + matchLength]
140+
) {
141+
matchLength++
142+
if (matchLength >= minMatchLength) {
143+
return { originalIndex: originalPosition, formattedIndex: formattedPosition }
144+
}
145+
}
146+
147+
if (originalPosition === originalLength && formattedPosition === formattedLength) {
148+
return { originalIndex: originalPosition, formattedIndex: formattedPosition }
149+
}
150+
}
151+
}
152+
153+
if (originalStart >= originalLength && formattedStart < formattedLength) {
154+
return { originalIndex: originalLength, formattedIndex: formattedLength }
155+
}
156+
157+
if (formattedStart >= formattedLength && originalStart < originalLength) {
158+
return { originalIndex: originalLength, formattedIndex: formattedLength }
159+
}
160+
161+
return null
162+
}
163+
164+
private coalesceDiffs(diffs: DiffRange[]): DiffRange[] {
165+
if (diffs.length <= 1) return diffs
166+
167+
const coalesced: DiffRange[] = []
168+
let current = diffs[0]
169+
170+
for (let i = 1; i < diffs.length; i++) {
171+
const next = diffs[i]
172+
173+
if (next.startOffset <= current.endOffset + 1) {
174+
current = {
175+
startOffset: current.startOffset,
176+
endOffset: Math.max(current.endOffset, next.endOffset),
177+
originalContent: current.originalContent + next.originalContent,
178+
formattedContent: current.formattedContent + next.formattedContent
179+
}
180+
} else {
181+
coalesced.push(current)
182+
current = next
183+
}
184+
}
185+
186+
coalesced.push(current)
187+
188+
return coalesced
189+
}
190+
191+
private generateMessage(diff: DiffRange): string {
192+
const { originalContent, formattedContent } = diff
193+
194+
const originalIndentMatch = originalContent.match(/^[ \t]+/)
195+
const formattedIndentMatch = formattedContent.match(/^[ \t]+/)
196+
197+
if (originalIndentMatch || formattedIndentMatch) {
198+
const originalIndent = originalIndentMatch?.[0] || ""
199+
const formattedIndent = formattedIndentMatch?.[0] || ""
200+
201+
if (originalIndent !== formattedIndent) {
202+
const originalSpaces = this.countIndentation(originalIndent)
203+
const formattedSpaces = this.countIndentation(formattedIndent)
204+
205+
if (originalSpaces !== formattedSpaces) {
206+
return `Incorrect indentation: expected ${formattedSpaces} spaces, found ${originalSpaces}`
207+
}
208+
}
209+
}
210+
211+
const originalIsWhitespace = /^[\s]+$/.test(originalContent)
212+
const formattedIsWhitespace = /^[\s]+$/.test(formattedContent)
213+
214+
if (originalIsWhitespace && formattedContent === "") {
215+
return "Unexpected whitespace"
216+
}
217+
218+
if (originalContent === "" && formattedIsWhitespace) {
219+
return "Missing whitespace"
220+
}
221+
222+
const originalNewlines = (originalContent.match(/\n/g) || []).length
223+
const formattedNewlines = (formattedContent.match(/\n/g) || []).length
224+
225+
if (originalNewlines !== formattedNewlines) {
226+
return "Incorrect line breaks"
227+
}
228+
229+
return "Formatting differs from expected"
230+
}
231+
232+
private countIndentation(indent: string): number {
233+
let count = 0
234+
235+
for (const char of indent) {
236+
if (char === "\t") {
237+
count += 2
238+
} else {
239+
count++
240+
}
241+
}
242+
243+
return count
244+
}
245+
}

0 commit comments

Comments
 (0)