From 2b70a665025e1c6c4f1d310a540acb4b7b7616fe Mon Sep 17 00:00:00 2001 From: Sarath Francis Date: Wed, 3 Jun 2026 03:15:04 -0400 Subject: [PATCH 1/2] 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. --- src/formatter/ExpressionFormatter.ts | 15 ++++++++++++--- src/formatter/Layout.ts | 16 ++++++++++++++++ test/features/comments.ts | 13 +++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/formatter/ExpressionFormatter.ts b/src/formatter/ExpressionFormatter.ts index a306e22fd1..8f93be365f 100644 --- a/src/formatter/ExpressionFormatter.ts +++ b/src/formatter/ExpressionFormatter.ts @@ -378,7 +378,7 @@ export default class ExpressionFormatter { } private formatBlockComment(node: BlockCommentNode | DisableCommentNode) { - if (node.type === NodeType.block_comment && this.isMultilineBlockComment(node)) { + if (node.type === NodeType.block_comment && this.isStandaloneBlockComment(node)) { this.splitBlockComment(node.text).forEach(line => { this.layout.add(WS.NEWLINE, WS.INDENT, line); }); @@ -388,8 +388,17 @@ export default class ExpressionFormatter { } } - private isMultilineBlockComment(node: BlockCommentNode): boolean { - return isMultiline(node.text) || isMultiline(node.precedingWhitespace || ''); + private isStandaloneBlockComment(node: BlockCommentNode): boolean { + return ( + isMultiline(node.text) || + isMultiline(node.precedingWhitespace || '') || + // The comment is the first thing on its line in the output being built + // (e.g. a comment leading the first item of a clause body, which the + // formatter always places on a fresh line). Keeping it inline here would + // make formatting non-idempotent, since reformatting would then see a + // newline before the comment and move it onto its own line. + this.layout.isAtStartOfLine() + ); } private isDocComment(comment: string): boolean { diff --git a/src/formatter/Layout.ts b/src/formatter/Layout.ts index 37b38f6d0d..b08684865f 100644 --- a/src/formatter/Layout.ts +++ b/src/formatter/Layout.ts @@ -111,6 +111,22 @@ export default class Layout { return this.items; } + /** + * True when some content has already been added and nothing but indentation + * has been emitted since the last newline, meaning the next token would be + * placed at the start of a fresh (non-first) line. + */ + public isAtStartOfLine(): boolean { + for (let i = this.items.length - 1; i >= 0; i--) { + const item = this.items[i]; + if (item === WS.SINGLE_INDENT) { + continue; + } + return item === WS.NEWLINE || item === WS.MANDATORY_NEWLINE; + } + return false; + } + private itemToString(item: LayoutItem): string { switch (item) { case WS.SPACE: diff --git a/test/features/comments.ts b/test/features/comments.ts index 3cb57070ec..68e39c3d6e 100644 --- a/test/features/comments.ts +++ b/test/features/comments.ts @@ -61,6 +61,19 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig expect(format(sql)).toBe(sql); }); + it('moves a leading block comment of a clause item onto its own line', () => { + const result = format('SELECT /* comment */ foo FROM tbl'); + expect(result).toBe(dedent` + SELECT + /* comment */ + foo + FROM + tbl + `); + // Formatting must be idempotent: re-formatting the result must not change it. + expect(format(result)).toBe(result); + }); + it('formats tricky line comments', () => { expect(format('SELECT a--comment, here\nFROM b--comment')).toBe(dedent` SELECT From 54d4cf2bbd6a42e3b2bd1563f9d6738aee65eefa Mon Sep 17 00:00:00 2001 From: Sarath Francis Date: Wed, 3 Jun 2026 06:51:19 -0400 Subject: [PATCH 2/2] test: cover block comments in more clause positions --- test/features/comments.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/features/comments.ts b/test/features/comments.ts index 68e39c3d6e..b0e7a6f02a 100644 --- a/test/features/comments.ts +++ b/test/features/comments.ts @@ -74,6 +74,20 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig expect(format(result)).toBe(result); }); + it('keeps block comments in various clause positions idempotent', () => { + // The fix should generalize beyond a comment right after SELECT: comments + // after a keyword, between items, and in UPDATE/SET must all be stable + // under re-formatting. + for (const sql of [ + 'SELECT foo /* c */, /* c */ bar FROM /* c */ tbl1 JOIN /* c */ tbl2', + 'UPDATE /* c */ tbl SET /* c */ x = 10', + 'SELECT /* c */ a, b FROM t WHERE /* c */ x = 1', + ]) { + const once = format(sql); + expect(format(once)).toBe(once); + } + }); + it('formats tricky line comments', () => { expect(format('SELECT a--comment, here\nFROM b--comment')).toBe(dedent` SELECT