Merge branch 'fix-block-comment-idempotency'

Author: nene

AUTHORS

@@ -61,6 +61,7 @@ Sean Song <mail@seansong.dev>
 Sebastian Lyng Johansen <seblyng98@gmail.com>
 Sergei Egorov <sergei.egorov@zeroturnaround.com>
 Sai Asish Y <say.apm35@gmail.com>
+Sarath Francis <sarathfrancis90@gmail.com>
 Shub <shub1493biswas@gmail.com>
 Stanislav Germanovskii <s.germanovskiy@tinkoff.ru>
 Steven Yung <stevenyung@fastmail.com>

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,20 @@ export default class ExpressionFormatter {
     }
   }
 
-  private isMultilineBlockComment(node: BlockCommentNode): boolean {
-    return isMultiline(node.text) || isMultiline(node.precedingWhitespace || '');
+  // True when:
+  //
+  // - the source text had a newline before the comment
+  // - the comment itself is multi-line
+  // - we have already added a newline to output text (right before this to-be added comment)
+  //
+  // The last one will ensure the comment position stays idempotent - so that
+  // re-formatting the same SQL won't result in comment position changing.
+  private isStandaloneBlockComment(node: BlockCommentNode): boolean {
+    return (
+      isMultiline(node.text) ||
+      isMultiline(node.precedingWhitespace || '') ||
+      this.layout.isAtStartOfLine()
+    );
   }
 
   private isDocComment(comment: string): boolean {

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:

test/features/comments.ts

@@ -61,6 +61,51 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
     expect(format(sql)).toBe(sql);
   });
 
+  it('moves a leading block comment of SELECT clause 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 SELECT statement positions idempotent', () => {
+    const sql = 'SELECT foo /* c */, /* c */ bar FROM /* c */ tbl1 JOIN /* c */ tbl2';
+    const result = dedent`
+      SELECT
+        foo /* c */,
+        /* c */
+        bar
+      FROM
+        /* c */
+        tbl1
+        JOIN /* c */ tbl2
+    `;
+    expect(format(sql)).toBe(result);
+    expect(format(result)).toBe(result);
+  });
+
+  it('keeps block comments in various CREATE TABLE statement positions idempotent', () => {
+    const sql =
+      'CREATE TABLE /* c */ tbl (/* c */ id INT, /* c */ first_name TEXT, last_name TEXT)';
+    const result = dedent`
+      CREATE TABLE /* c */ tbl (
+        /* c */
+        id INT,
+        /* c */
+        first_name TEXT,
+        last_name TEXT
+      )
+    `;
+    expect(format(sql)).toBe(result);
+    expect(format(result)).toBe(result);
+  });
+
   it('formats tricky line comments', () => {
     expect(format('SELECT a--comment, here\nFROM b--comment')).toBe(dedent`
       SELECT