Merge PR #388: Preserve line-comment positions

sql-formatter-org · Aug 14, 2022 · caadc6a · caadc6a
2 parents 6ee1dab + 6dd87c3
commit caadc6a
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 8 deletions.
diff --git a/src/formatter/ExpressionFormatter.ts b/src/formatter/ExpressionFormatter.ts
@@ -230,7 +230,11 @@ export default class ExpressionFormatter {
 
   /** Formats a line comment onto query */
   private formatLineComment(token: Token) {
-    this.layout.add(this.show(token), WS.MANDATORY_NEWLINE, WS.INDENT);
+    if (/\n/.test(token.precedingWhitespace || '')) {
+      this.layout.add(WS.NEWLINE, WS.INDENT, this.show(token), WS.MANDATORY_NEWLINE, WS.INDENT);
+    } else {
+      this.layout.add(WS.NO_NEWLINE, WS.SPACE, this.show(token), WS.MANDATORY_NEWLINE, WS.INDENT);
+    }
   }
 
   /** Formats a block comment onto query */

diff --git a/src/lexer/TokenizerEngine.ts b/src/lexer/TokenizerEngine.ts
@@ -34,7 +34,7 @@ export default class TokenizerEngine {
     // Keep processing the string until end is reached
     while (this.index < this.input.length) {
       // skip any preceding whitespace
-      this.skipWhitespace();
+      const precedingWhitespace = this.getWhitespace();
 
       if (this.index < this.input.length) {
         // Get the next token and the token type
@@ -43,20 +43,22 @@ export default class TokenizerEngine {
           throw new Error(`Parse error: Unexpected "${input.slice(this.index, 100)}"`);
         }
 
-        tokens.push(token);
+        tokens.push({ ...token, precedingWhitespace });
       }
     }
     return tokens;
   }
 
-  private skipWhitespace(): void {
+  private getWhitespace(): string | undefined {
     WHITESPACE_REGEX.lastIndex = this.index;
 
     const matches = WHITESPACE_REGEX.exec(this.input);
     if (matches) {
       // Advance current position by matched whitespace length
       this.index += matches[0].length;
+      return matches[0];
     }
+    return undefined;
   }
 
   private getNextToken(previousToken?: Token): Token | undefined {

diff --git a/src/lexer/token.ts b/src/lexer/token.ts
@@ -37,6 +37,7 @@ export interface Token {
   key?: string;
   start: number; // 0-based index of the token in the whole query string
   end: number; // 0-based index of where the token ends in the query string
+  precedingWhitespace?: string; // Whitespace before this token, if any
 }
 
 /**

diff --git a/test/features/comments.ts b/test/features/comments.ts
@@ -59,8 +59,7 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
   it('formats line comments followed by semicolon', () => {
     expect(
       format(`
-      SELECT a FROM b
-      --comment
+      SELECT a FROM b --comment
       ;
     `)
     ).toBe(dedent`
@@ -103,6 +102,48 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
     `);
   });
 
+  it('preserves single-line comments at the end of lines', () => {
+    expect(
+      format(`
+        SELECT
+          a, --comment1
+          b --comment2
+        FROM --comment3
+          my_table;
+      `)
+    ).toBe(dedent`
+      SELECT
+        a, --comment1
+        b --comment2
+      FROM --comment3
+        my_table;
+    `);
+  });
+
+  it('preserves single-line comments on separate lines', () => {
+    expect(
+      format(`
+        SELECT
+          --comment1
+          a,
+          --comment2
+          b
+        FROM
+          --comment3
+          my_table;
+      `)
+    ).toBe(dedent`
+      SELECT
+        --comment1
+        a,
+        --comment2
+        b
+      FROM
+        --comment3
+        my_table;
+    `);
+  });
+
   it('recognizes line-comments with Windows line-endings (converts them to UNIX)', () => {
     const result = format('SELECT * FROM\r\n-- line comment 1\r\nMyTable -- line comment 2\r\n');
     expect(result).toBe('SELECT\n  *\nFROM\n  -- line comment 1\n  MyTable -- line comment 2');

diff --git a/test/features/limiting.ts b/test/features/limiting.ts
@@ -44,8 +44,7 @@ export default function supportsLimiting(format: FormatFn, types: LimitingTypes)
             *
           FROM
             tbl
-          LIMIT
-            --comment
+          LIMIT --comment
             5, --comment
             6;
       `);

diff --git a/test/unit/Parser.test.ts b/test/unit/Parser.test.ts
@@ -30,6 +30,7 @@ describe('Parser', () => {
             Object {
               "token": Object {
                 "end": 3,
+                "precedingWhitespace": undefined,
                 "raw": "foo",
                 "start": 0,
                 "text": "foo",
@@ -46,6 +47,7 @@ describe('Parser', () => {
             Object {
               "token": Object {
                 "end": 8,
+                "precedingWhitespace": " ",
                 "raw": "bar",
                 "start": 5,
                 "text": "bar",
@@ -71,6 +73,7 @@ describe('Parser', () => {
                 Object {
                   "nameToken": Object {
                     "end": 11,
+                    "precedingWhitespace": " ",
                     "raw": "SQRT",
                     "start": 7,
                     "text": "SQRT",
@@ -81,6 +84,7 @@ describe('Parser', () => {
                       Object {
                         "token": Object {
                           "end": 13,
+                          "precedingWhitespace": undefined,
                           "raw": "2",
                           "start": 12,
                           "text": "2",
@@ -98,6 +102,7 @@ describe('Parser', () => {
               ],
               "nameToken": Object {
                 "end": 6,
+                "precedingWhitespace": undefined,
                 "raw": "SELECT",
                 "start": 0,
                 "text": "SELECT",
@@ -123,6 +128,7 @@ describe('Parser', () => {
                 Object {
                   "arrayToken": Object {
                     "end": 15,
+                    "precedingWhitespace": " ",
                     "raw": "my_array",
                     "start": 7,
                     "text": "my_array",
@@ -133,6 +139,7 @@ describe('Parser', () => {
                       Object {
                         "nameToken": Object {
                           "end": 22,
+                          "precedingWhitespace": undefined,
                           "raw": "OFFSET",
                           "start": 16,
                           "text": "OFFSET",
@@ -143,6 +150,7 @@ describe('Parser', () => {
                             Object {
                               "token": Object {
                                 "end": 24,
+                                "precedingWhitespace": undefined,
                                 "raw": "5",
                                 "start": 23,
                                 "text": "5",
@@ -167,6 +175,7 @@ describe('Parser', () => {
               ],
               "nameToken": Object {
                 "end": 6,
+                "precedingWhitespace": undefined,
                 "raw": "SELECT",
                 "start": 0,
                 "text": "SELECT",
@@ -194,6 +203,7 @@ describe('Parser', () => {
                     Object {
                       "token": Object {
                         "end": 18,
+                        "precedingWhitespace": undefined,
                         "raw": "birth_year",
                         "start": 8,
                         "text": "birth_year",
@@ -204,6 +214,7 @@ describe('Parser', () => {
                     Object {
                       "token": Object {
                         "end": 20,
+                        "precedingWhitespace": " ",
                         "raw": "-",
                         "start": 19,
                         "text": "-",
@@ -216,6 +227,7 @@ describe('Parser', () => {
                         Object {
                           "token": Object {
                             "end": 34,
+                            "precedingWhitespace": undefined,
                             "raw": "CURRENT_DATE",
                             "start": 22,
                             "text": "CURRENT_DATE",
@@ -226,6 +238,7 @@ describe('Parser', () => {
                         Object {
                           "token": Object {
                             "end": 36,
+                            "precedingWhitespace": " ",
                             "raw": "+",
                             "start": 35,
                             "text": "+",
@@ -236,6 +249,7 @@ describe('Parser', () => {
                         Object {
                           "token": Object {
                             "end": 38,
+                            "precedingWhitespace": " ",
                             "raw": "1",
                             "start": 37,
                             "text": "1",
@@ -256,6 +270,7 @@ describe('Parser', () => {
               ],
               "nameToken": Object {
                 "end": 6,
+                "precedingWhitespace": undefined,
                 "raw": "SELECT",
                 "start": 0,
                 "text": "SELECT",
@@ -281,6 +296,7 @@ describe('Parser', () => {
                 Object {
                   "token": Object {
                     "end": 9,
+                    "precedingWhitespace": " ",
                     "raw": "age",
                     "start": 6,
                     "text": "age",
@@ -291,27 +307,31 @@ describe('Parser', () => {
                 Object {
                   "andToken": Object {
                     "end": 24,
+                    "precedingWhitespace": " ",
                     "raw": "and",
                     "start": 21,
                     "text": "AND",
                     "type": "RESERVED_LOGICAL_OPERATOR",
                   },
                   "betweenToken": Object {
                     "end": 17,
+                    "precedingWhitespace": " ",
                     "raw": "BETWEEN",
                     "start": 10,
                     "text": "BETWEEN",
                     "type": "RESERVED_KEYWORD",
                   },
                   "expr1": Object {
                     "end": 20,
+                    "precedingWhitespace": " ",
                     "raw": "10",
                     "start": 18,
                     "text": "10",
                     "type": "NUMBER",
                   },
                   "expr2": Object {
                     "end": 27,
+                    "precedingWhitespace": " ",
                     "raw": "15",
                     "start": 25,
                     "text": "15",
@@ -322,6 +342,7 @@ describe('Parser', () => {
               ],
               "nameToken": Object {
                 "end": 5,
+                "precedingWhitespace": undefined,
                 "raw": "WHERE",
                 "start": 0,
                 "text": "WHERE",
@@ -347,6 +368,7 @@ describe('Parser', () => {
                 Object {
                   "token": Object {
                     "end": 8,
+                    "precedingWhitespace": " ",
                     "raw": "10",
                     "start": 6,
                     "text": "10",
@@ -357,6 +379,7 @@ describe('Parser', () => {
               ],
               "limitToken": Object {
                 "end": 5,
+                "precedingWhitespace": undefined,
                 "raw": "LIMIT",
                 "start": 0,
                 "text": "LIMIT",
@@ -382,6 +405,7 @@ describe('Parser', () => {
                 Object {
                   "token": Object {
                     "end": 13,
+                    "precedingWhitespace": " ",
                     "raw": "10",
                     "start": 11,
                     "text": "10",
@@ -392,6 +416,7 @@ describe('Parser', () => {
               ],
               "limitToken": Object {
                 "end": 5,
+                "precedingWhitespace": undefined,
                 "raw": "LIMIT",
                 "start": 0,
                 "text": "LIMIT",
@@ -401,6 +426,7 @@ describe('Parser', () => {
                 Object {
                   "token": Object {
                     "end": 9,
+                    "precedingWhitespace": " ",
                     "raw": "200",
                     "start": 6,
                     "text": "200",
@@ -432,6 +458,7 @@ describe('Parser', () => {
               ],
               "nameToken": Object {
                 "end": 6,
+                "precedingWhitespace": undefined,
                 "raw": "SELECT",
                 "start": 0,
                 "text": "SELECT",
@@ -460,6 +487,7 @@ describe('Parser', () => {
               ],
               "nameToken": Object {
                 "end": 15,
+                "precedingWhitespace": undefined,
                 "raw": "SELECT DISTINCT",
                 "start": 0,
                 "text": "SELECT DISTINCT",