Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/formatter/ExpressionFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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 {
Expand Down
16 changes: 16 additions & 0 deletions src/formatter/Layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
27 changes: 27 additions & 0 deletions test/features/comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down