Skip to content

Commit 2b70a66

Browse files
fix: keep leading block comment of clause item on its own line
A block comment placed before the first item of a clause (e.g. `SELECT /* c */ foo FROM tbl`) was formatted inline on the first pass, but moved onto its own line when the result was formatted again, making formatting non-idempotent. The inline-vs-standalone decision for a block comment was based solely on its `precedingWhitespace`. The formatter always emits a newline before the first item of a clause body, so on a second pass that structural newline becomes the comment's preceding whitespace and flips the decision. Treat a block comment as standalone whenever it is already at the start of a line in the output being built, in addition to the existing checks. This makes the first pass place such comments on their own line directly, so the output is stable under repeated formatting. Inline block comments that follow other tokens on the same line (trailing comments, comments between a function name and its parentheses, or around the `.` of a qualified name) are unaffected.
1 parent a3bfeeb commit 2b70a66

3 files changed

Lines changed: 41 additions & 3 deletions

File tree

src/formatter/ExpressionFormatter.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ export default class ExpressionFormatter {
378378
}
379379

380380
private formatBlockComment(node: BlockCommentNode | DisableCommentNode) {
381-
if (node.type === NodeType.block_comment && this.isMultilineBlockComment(node)) {
381+
if (node.type === NodeType.block_comment && this.isStandaloneBlockComment(node)) {
382382
this.splitBlockComment(node.text).forEach(line => {
383383
this.layout.add(WS.NEWLINE, WS.INDENT, line);
384384
});
@@ -388,8 +388,17 @@ export default class ExpressionFormatter {
388388
}
389389
}
390390

391-
private isMultilineBlockComment(node: BlockCommentNode): boolean {
392-
return isMultiline(node.text) || isMultiline(node.precedingWhitespace || '');
391+
private isStandaloneBlockComment(node: BlockCommentNode): boolean {
392+
return (
393+
isMultiline(node.text) ||
394+
isMultiline(node.precedingWhitespace || '') ||
395+
// The comment is the first thing on its line in the output being built
396+
// (e.g. a comment leading the first item of a clause body, which the
397+
// formatter always places on a fresh line). Keeping it inline here would
398+
// make formatting non-idempotent, since reformatting would then see a
399+
// newline before the comment and move it onto its own line.
400+
this.layout.isAtStartOfLine()
401+
);
393402
}
394403

395404
private isDocComment(comment: string): boolean {

src/formatter/Layout.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,22 @@ export default class Layout {
111111
return this.items;
112112
}
113113

114+
/**
115+
* True when some content has already been added and nothing but indentation
116+
* has been emitted since the last newline, meaning the next token would be
117+
* placed at the start of a fresh (non-first) line.
118+
*/
119+
public isAtStartOfLine(): boolean {
120+
for (let i = this.items.length - 1; i >= 0; i--) {
121+
const item = this.items[i];
122+
if (item === WS.SINGLE_INDENT) {
123+
continue;
124+
}
125+
return item === WS.NEWLINE || item === WS.MANDATORY_NEWLINE;
126+
}
127+
return false;
128+
}
129+
114130
private itemToString(item: LayoutItem): string {
115131
switch (item) {
116132
case WS.SPACE:

test/features/comments.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,19 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
6161
expect(format(sql)).toBe(sql);
6262
});
6363

64+
it('moves a leading block comment of a clause item onto its own line', () => {
65+
const result = format('SELECT /* comment */ foo FROM tbl');
66+
expect(result).toBe(dedent`
67+
SELECT
68+
/* comment */
69+
foo
70+
FROM
71+
tbl
72+
`);
73+
// Formatting must be idempotent: re-formatting the result must not change it.
74+
expect(format(result)).toBe(result);
75+
});
76+
6477
it('formats tricky line comments', () => {
6578
expect(format('SELECT a--comment, here\nFROM b--comment')).toBe(dedent`
6679
SELECT

0 commit comments

Comments
 (0)