Merge pull request #4 from datalakehouse/sql-formatter-newline-comma-option

fix(comments): Added a major fix to how comments are handled to make it consistent across different situations

Author: swayam-aicg

docs/commaPosition.md

@@ -39,6 +39,7 @@ WHERE
 ```
 
 ### leadingWithSpace
+
 ```sql
 SELECT
   name
@@ -53,6 +54,7 @@ WHERE
 ```
 
 ### Other examples
+
 ```sql
 -- No effect on INSERT Statements
 INSERT INTO

src/formatter/ExpressionFormatter.ts

@@ -68,6 +68,7 @@ export default class ExpressionFormatter {
   private dialectCfg: ProcessedDialectFormatOptions;
   private params: Params;
   private layout: Layout;
+  private pendingLineComment: (LineCommentNode | BlockCommentNode | DisableCommentNode)[] = []; // Keeps track if any pending Line/Comment is remaining
 
   private inline = false;
   private nodes: AstNode[] = [];
@@ -184,9 +185,38 @@ export default class ExpressionFormatter {
   }
 
   private formatPropertyAccess(node: PropertyAccessNode) {
-    this.formatNode(node.object);
+    this.formatComments(node.object.leadingComments);
+    this.formatNodeWithoutComments(node.object);
+
+    // Handle trailing comments of object BEFORE the dot (inline)
+    if (node.object.trailingComments && node.object.trailingComments.length > 0) {
+      for (const comment of node.object.trailingComments) {
+        if (comment.type === NodeType.block_comment) {
+          this.layout.add(WS.NO_SPACE, WS.SPACE, (comment as BlockCommentNode).text);
+        } else if (comment.type === NodeType.line_comment) {
+          this.layout.add(WS.NO_SPACE, WS.SPACE, (comment as LineCommentNode).text);
+        }
+      }
+    }
+
+    // Add the dot operator
     this.layout.add(WS.NO_SPACE, node.operator);
-    this.formatNode(node.property);
+
+    // Handle leading comments of property AFTER the dot (inline)
+    if (node.property.leadingComments && node.property.leadingComments.length > 0) {
+      for (const comment of node.property.leadingComments) {
+        if (comment.type === NodeType.block_comment) {
+          this.layout.add((comment as BlockCommentNode).text, WS.SPACE);
+        } else if (comment.type === NodeType.line_comment) {
+          this.layout.add((comment as LineCommentNode).text, WS.SPACE);
+        }
+      }
+      // Format property without leading comments (already handled above)
+      this.formatNodeWithoutComments(node.property);
+      this.formatComments(node.property.trailingComments);
+    } else {
+      this.formatNode(node.property);
+    }
   }
 
   private formatParenthesis(node: ParenthesisNode) {
@@ -345,13 +375,36 @@ export default class ExpressionFormatter {
         // Look ahead: check if next node is a line comment
         this.formatLeadingComma();
       } else {
-        this.layout.add(WS.NO_SPACE, ',', WS.NEWLINE, WS.INDENT);
+        this.formatTrailingComma();
       }
     } else {
       this.layout.add(WS.NO_SPACE, ',', WS.SPACE);
     }
   }
-
+  private formatTrailingComma() {
+    if (this.pendingLineComment && this.pendingLineComment.length > 0) {
+      // We have a line comment that should come after the comma
+      while (this.pendingLineComment.length > 0) {
+        const comment = this.pendingLineComment.shift();
+        if (comment) {
+          if (comment.type === NodeType.line_comment) {
+            this.layout.add(
+              WS.NO_SPACE,
+              ',',
+              WS.SPACE,
+              (comment as LineCommentNode).text,
+              WS.MANDATORY_NEWLINE,
+              WS.INDENT
+            );
+          } else {
+            this.layout.add((comment as BlockCommentNode).text, WS.MANDATORY_NEWLINE, WS.INDENT);
+          }
+        }
+      }
+    } else {
+      this.layout.add(WS.NO_SPACE, ',', WS.NEWLINE, WS.INDENT);
+    }
+  }
   private formatLeadingComma() {
     const comments: AstNode[] = [];
     let lookAheadIndex = this.index + 1;
@@ -433,6 +486,11 @@ export default class ExpressionFormatter {
   }
 
   private formatLineComment(node: LineCommentNode) {
+    if (!this.inline && this.cfg.commaPosition === 'trailing' && this.isNextNonCommentNodeComma()) {
+      // Store the comment to be output after the comma
+      this.pendingLineComment.push(node);
+      return;
+    }
     if (isMultiline(node.precedingWhitespace || '')) {
       this.layout.add(WS.NEWLINE, WS.INDENT, node.text, WS.MANDATORY_NEWLINE, WS.INDENT);
     } else if (this.layout.getLayoutItems().length > 0) {
@@ -442,8 +500,25 @@ export default class ExpressionFormatter {
       this.layout.add(node.text, WS.MANDATORY_NEWLINE, WS.INDENT);
     }
   }
+  private isNextNonCommentNodeComma(): boolean {
+    let lookAheadIndex = this.index + 1;
+    while (lookAheadIndex < this.nodes.length) {
+      const nextNode = this.nodes[lookAheadIndex];
+      if (nextNode.type === NodeType.line_comment || nextNode.type === NodeType.block_comment) {
+        lookAheadIndex++;
+      } else {
+        return nextNode.type === NodeType.comma;
+      }
+    }
+    return false;
+  }
 
   private formatBlockComment(node: BlockCommentNode | DisableCommentNode) {
+    if (!this.inline && this.cfg.commaPosition === 'trailing' && this.isNextNonCommentNodeComma()) {
+      // Store the comment to be output after the comma
+      this.pendingLineComment.push(node);
+      return;
+    }
     if (node.type === NodeType.block_comment && this.isMultilineBlockComment(node)) {
       this.splitBlockComment(node.text).forEach(line => {
         this.layout.add(WS.NEWLINE, WS.INDENT, line);

test/features/comments.ts

@@ -93,8 +93,7 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
     `)
     ).toBe(dedent`
       SELECT
-        a --comment
-      ,
+        a, --comment
         b
     `);
   });

test/options/commaPosition.ts

@@ -242,4 +242,31 @@ export default function supportsCommaPosition(format: FormatFn) {
     `
     );
   });
+
+  it('supports trailing comma with comments from a leading comma formatted query', () => {
+    const result = format(
+      `SELECT
+        id -- comment 1
+        ,first_name -- comment 2
+        /* block comment */
+        ,last_name
+        ,email
+      FROM
+        users;
+    `,
+      { commaPosition: 'trailing' }
+    );
+    expect(result).toBe(
+      dedent`
+    SELECT
+      id, -- comment 1
+      first_name, -- comment 2
+      /* block comment */
+      last_name,
+      email
+    FROM
+      users;
+    `
+    );
+  });
 }