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..b0e7a6f02a 100644 --- a/test/features/comments.ts +++ b/test/features/comments.ts @@ -61,6 +61,33 @@ 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('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