Repository: sql-formatter-org/sql-formatter
Pull request #462 (2022-09-26 16:14:48)

Better formatting of inline block-comments
Previously all block-comments were formatted on a separate line, like:

```sql
SELECT 1 /*com1*/ + 2 /*com2*/;
```

would get formatted as:

```sql
SELECT
  1
  /*com1*/
  + 2
  /*com2*/;
```

After this change the result is more like one would expect:

```sql
SELECT
  1 /*com1*/ + 2 /*com2*/;
```

Only when the block-comment spans multiple lines or is preceded by a newline will it be formatted as before: on a separate line of its own.

The formatting is still not ideal, but it should work well for most common scenarios. Sometimes the comments don't get nicely padded by spaces on both sides, as one can see in the tests, but these are some pretty odd scenarios, like placing comments between table and column name `table /* */ . /* */ col`, something one really rarely does in real world.




Patch:
diff --git a/.babelrc b/.babelrc
index f9eefdda..fe12103f 100644
--- a/.babelrc
+++ b/.babelrc
@@ -1,6 +1,7 @@
 {
-  "presets": [["@babel/preset-env", { "modules": false }], "@babel/preset-typescript"],
+  "presets": ["@babel/preset-env", "@babel/preset-typescript"],
   "plugins": [
+    "@babel/plugin-proposal-class-properties",
     "add-module-exports",
     [
       "module-resolver",
diff --git a/bin/sql-formatter-cli.cjs b/bin/sql-formatter-cli.js
similarity index 97%
rename from bin/sql-formatter-cli.cjs
rename to bin/sql-formatter-cli.js
index 6301af37..aaa13315 100755
--- a/bin/sql-formatter-cli.cjs
+++ b/bin/sql-formatter-cli.js
@@ -2,7 +2,7 @@
 
 'use strict';
 
-const { format, supportedDialects } = require('../dist/sql-formatter.min.cjs');
+const { format, supportedDialects } = require('../lib/index');
 const fs = require('fs');
 const tty = require('tty');
 const { version } = require('../package.json');
diff --git a/package.json b/package.json
index f52884f7..65cd32ab 100644
--- a/package.json
+++ b/package.json
@@ -1,24 +1,13 @@
 {
   "name": "sql-formatter",
-  "version": "10.8.0-beta.3",
+  "version": "10.7.1",
   "description": "Format whitespace in a SQL query to make it more readable",
   "license": "MIT",
-  "main": "dist/sql-formatter.min.cjs",
-  "unpkg": "dist/sql-formatter.min.js",
-  "module": "lib/index.js",
+  "main": "lib/index.js",
   "types": "lib/src/index.d.ts",
-  "exports": {
-    ".": {
-      "types": "./lib/src/index.d.ts",
-      "import": "./lib/index.js",
-      "require": "./dist/sql-formatter.min.cjs"
-    }
-  },
   "bin": {
-    "sql-formatter": "./bin/sql-formatter-cli.cjs"
+    "sql-formatter": "./bin/sql-formatter-cli.js"
   },
-  "type": "module",
-  "sideEffects": false,
   "keywords": [
     "sql",
     "formatter",
@@ -102,7 +91,7 @@
     "grammar": "nearleyc src/parser/grammar.ne -o src/parser/grammar.ts",
     "build:babel": "babel src --out-dir lib --extensions .ts --source-maps",
     "build:types": "ttsc --module commonjs --emitDeclarationOnly --isolatedModules",
-    "build:minified": "webpack --config webpack.prod.js && cp dist/sql-formatter.min.cjs dist/sql-formatter.min.js",
+    "build:minified": "webpack --config webpack.prod.js",
     "build": "yarn grammar && npm-run-all --parallel build:babel build:types build:minified",
     "release": "release-it"
   },
diff --git a/src/formatter/ExpressionFormatter.ts b/src/formatter/ExpressionFormatter.ts
index ee119b0e..f899d2df 100644
--- a/src/formatter/ExpressionFormatter.ts
+++ b/src/formatter/ExpressionFormatter.ts
@@ -1,5 +1,5 @@
 import { FormatOptions } from 'src/FormatOptions';
-import { equalizeWhitespace } from 'src/utils';
+import { equalizeWhitespace, isMultiline } from 'src/utils';
 
 import Params from 'src/formatter/Params';
 import { isTabularStyle } from 'src/formatter/config';
@@ -309,7 +309,7 @@ export default class ExpressionFormatter {
   }
 
   private formatLineComment(node: LineCommentNode) {
-    if (/\n/.test(node.precedingWhitespace || '')) {
+    if (isMultiline(node.precedingWhitespace || '')) {
       this.layout.add(WS.NEWLINE, WS.INDENT, node.text, WS.MANDATORY_NEWLINE, WS.INDENT);
     } else {
       this.layout.add(WS.NO_NEWLINE, WS.SPACE, node.text, WS.MANDATORY_NEWLINE, WS.INDENT);
@@ -317,10 +317,18 @@ export default class ExpressionFormatter {
   }
 
   private formatBlockComment(node: BlockCommentNode) {
-    this.splitBlockComment(node.text).forEach(line => {
-      this.layout.add(WS.NEWLINE, WS.INDENT, line);
-    });
-    this.layout.add(WS.NEWLINE, WS.INDENT);
+    if (this.isMultilineBlockComment(node)) {
+      this.splitBlockComment(node.text).forEach(line => {
+        this.layout.add(WS.NEWLINE, WS.INDENT, line);
+      });
+      this.layout.add(WS.NEWLINE, WS.INDENT);
+    } else {
+      this.layout.add(node.text, WS.SPACE);
+    }
+  }
+
+  private isMultilineBlockComment(node: BlockCommentNode): boolean {
+    return isMultiline(node.text) || isMultiline(node.precedingWhitespace || '');
   }
 
   // Breaks up block comment to multiple lines.
diff --git a/src/parser/ast.ts b/src/parser/ast.ts
index 27998900..7d1a129e 100644
--- a/src/parser/ast.ts
+++ b/src/parser/ast.ts
@@ -159,6 +159,7 @@ export interface LineCommentNode extends BaseNode {
 export interface BlockCommentNode extends BaseNode {
   type: NodeType.block_comment;
   text: string;
+  precedingWhitespace: string;
 }
 
 export type CommentNode = LineCommentNode | BlockCommentNode;
diff --git a/src/parser/grammar.ne b/src/parser/grammar.ne
index d5fbbcd1..a8e81ead 100644
--- a/src/parser/grammar.ne
+++ b/src/parser/grammar.ne
@@ -101,13 +101,6 @@ select_clause -> %RESERVED_SELECT (all_columns_asterisk expression:* | asteriskl
     children: [exp, ...expressions],
   })
 %}
-select_clause -> %RESERVED_SELECT {%
-  ([nameToken]) => ({
-    type: NodeType.clause,
-    name: toKeywordNode(nameToken),
-    children: [],
-  })
-%}
 
 all_columns_asterisk -> %ASTERISK {%
   () => ({ type: NodeType.all_columns_asterisk })
@@ -308,5 +301,9 @@ comment -> %LINE_COMMENT {%
   })
 %}
 comment -> %BLOCK_COMMENT {%
-  ([token]) => ({ type: NodeType.block_comment, text: token.text })
+  ([token]) => ({
+    type: NodeType.block_comment,
+    text: token.text,
+    precedingWhitespace: token.precedingWhitespace,
+  })
 %}
diff --git a/src/utils.ts b/src/utils.ts
index 74c46755..b608c27f 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -28,6 +28,9 @@ export const sum = (arr: number[]): number => {
 export const flatKeywordList = (obj: Record<string, string[]>): string[] =>
   dedupe(Object.values(obj).flat());
 
+// True when string contains multiple lines
+export const isMultiline = (text: string): boolean => /\n/.test(text);
+
 // Given a type and a field name, returns a type where this field is optional
 //
 // For example, these two type definitions are equivalent:
diff --git a/webpack.common.js b/webpack.common.js
index a3f73c4f..aba4152c 100644
--- a/webpack.common.js
+++ b/webpack.common.js
@@ -1,18 +1,17 @@
-import path from 'path';
+const path = require('path');
 
-export default {
+module.exports = {
   entry: './src/index.ts',
   output: {
-    path: path.resolve('./dist'),
-    filename: 'sql-formatter.cjs',
+    path: path.join(__dirname, 'dist'),
+    filename: 'sql-formatter.js',
     library: 'sqlFormatter',
     libraryTarget: 'umd',
-    globalObject: 'this',
   },
   resolve: {
     extensions: ['.js', '.ts'],
     alias: {
-      src: path.resolve('./src'),
+      src: path.resolve(__dirname, 'src/'),
     },
   },
   module: {
diff --git a/webpack.prod.js b/webpack.prod.js
index 68e90de0..473cfa31 100644
--- a/webpack.prod.js
+++ b/webpack.prod.js
@@ -1,11 +1,10 @@
-import { merge } from 'webpack-merge';
+const merge = require('webpack-merge').merge;
+const common = require('./webpack.common.js');
 
-import common from './webpack.common.js';
-
-export default merge(common, {
+module.exports = merge(common, {
   mode: 'production',
   devtool: 'source-map',
   output: {
-    filename: 'sql-formatter.min.cjs',
+    filename: 'sql-formatter.min.js',
   },
 });


Test patch:
diff --git a/test/features/arrayAndMapAccessors.ts b/test/features/arrayAndMapAccessors.ts
index e75db0b1..021fb111 100644
--- a/test/features/arrayAndMapAccessors.ts
+++ b/test/features/arrayAndMapAccessors.ts
@@ -35,9 +35,7 @@ export default function supportsArrayAndMapAccessors(format: FormatFn) {
     const result = format(`SELECT arr /* comment */ [1];`);
     expect(result).toBe(dedent`
       SELECT
-        arr
-        /* comment */
-        [1];
+        arr/* comment */ [1];
     `);
   });
 
@@ -45,9 +43,7 @@ export default function supportsArrayAndMapAccessors(format: FormatFn) {
     const result = format(`SELECT foo./* comment */arr[1];`);
     expect(result).toBe(dedent`
       SELECT
-        foo.
-        /* comment */
-        arr[1];
+        foo./* comment */ arr[1];
     `);
   });
 }
diff --git a/test/features/between.ts b/test/features/between.ts
index f3057854..7850112c 100644
--- a/test/features/between.ts
+++ b/test/features/between.ts
@@ -13,13 +13,7 @@ export default function supportsBetween(format: FormatFn) {
   it('formats BETWEEN with comments inside', () => {
     expect(format('WHERE foo BETWEEN /*C1*/ t.bar /*C2*/ AND /*C3*/ t.baz')).toBe(dedent`
       WHERE
-        foo BETWEEN
-        /*C1*/
-        t.bar
-        /*C2*/
-       AND
-        /*C3*/
-        t.baz
+        foo BETWEEN /*C1*/ t.bar /*C2*/ AND /*C3*/ t.baz
     `);
   });
 }
diff --git a/test/features/case.ts b/test/features/case.ts
index c356aed5..b2545a3c 100644
--- a/test/features/case.ts
+++ b/test/features/case.ts
@@ -116,23 +116,10 @@ export default function supportsCase(format: FormatFn) {
 
     expect(result).toBe(dedent`
       SELECT
-        CASE
-        /*c1*/
-        foo
-          /*c2*/
-          WHEN
-          /*c3*/
-          1
-          /*c4*/
-          THEN
-          /*c5*/
-          2
-          /*c6*/
-          ELSE
-          /*c7*/
-          3
-        /*c8*/
-        END;
+        CASE /*c1*/ foo
+          /*c2*/ WHEN /*c3*/ 1 /*c4*/ THEN /*c5*/ 2
+          /*c6*/ ELSE /*c7*/ 3
+        /*c8*/ END;
     `);
   });
 
diff --git a/test/features/comments.ts b/test/features/comments.ts
index aa3833cd..ce93f842 100644
--- a/test/features/comments.ts
+++ b/test/features/comments.ts
@@ -48,6 +48,18 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
     expect(format(sql)).toBe(sql);
   });
 
+  it('keeps block comment on separate line when it is separate in original SQL', () => {
+    const sql = dedent`
+      SELECT
+        /* separate-line block comment */
+        foo,
+        bar /* inline block comment */
+      FROM
+        tbl;
+    `;
+    expect(format(sql)).toBe(sql);
+  });
+
   it('formats tricky line comments', () => {
     expect(format('SELECT a--comment, here\nFROM b--comment')).toBe(dedent`
       SELECT
@@ -167,9 +179,7 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
     `);
     expect(result).toBe(dedent`
       SELECT
-        count
-        /* comment */
-        (*);
+        count/* comment */ (*);
     `);
   });
 
@@ -179,18 +189,10 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
     `);
     expect(result).toBe(dedent`
       SELECT
-        foo
-        /* com1 */
-      .bar,
-        count()
-        /* com2 */
-      .bar,
-        foo.bar
-        /* com3 */
-      .baz,
-        (1, 2)
-        /* com4 */
-      .foo;
+        foo /* com1 */.bar,
+        count() /* com2 */.bar,
+        foo.bar /* com3 */.baz,
+        (1, 2) /* com4 */.foo;
     `);
   });
 
@@ -200,12 +202,8 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
     `);
     expect(result).toBe(dedent`
       SELECT
-        foo.
-        /* com1 */
-        bar,
-        foo.
-        /* com2 */
-        *;
+        foo./* com1 */ bar,
+        foo./* com2 */ *;
     `);
   });
 
@@ -226,8 +224,7 @@ export default function supportsComments(format: FormatFn, opts: CommentsConfig
       const result = format('SELECT alpha /* /* commment */ */ FROM beta');
       expect(result).toBe(dedent`
         SELECT
-          alpha
-          /* /* commment */ */
+          alpha /* /* commment */ */
         FROM
           beta
       `);
diff --git a/test/postgresql.test.ts b/test/postgresql.test.ts
index 075e1fd6..7839ee62 100644
--- a/test/postgresql.test.ts
+++ b/test/postgresql.test.ts
@@ -204,13 +204,6 @@ describe('PostgreSqlFormatter', () => {
     `);
   });
 
-  // Regression test for issue #447
-  it('formats empty SELECT', () => {
-    expect(format('SELECT;')).toBe(dedent`
-      SELECT;
-    `);
-  });
-
   // Regression test for issue #391
   it('formats TIMESTAMP WITH TIME ZONE syntax', () => {
     expect(
diff --git a/test/unit/__snapshots__/Parser.test.ts.snap b/test/unit/__snapshots__/Parser.test.ts.snap
index 212c62dc..c4af0125 100644
--- a/test/unit/__snapshots__/Parser.test.ts.snap
+++ b/test/unit/__snapshots__/Parser.test.ts.snap
@@ -362,6 +362,7 @@ Array [
               "text": "my_array",
               "trailingComments": Array [
                 Object {
+                  "precedingWhitespace": " ",
                   "text": "/*haha*/",
                   "type": "block_comment",
                 },