Fix SQL whitespace handing in comments (#897)

diffplug · Jul 20, 2021 · 5fe0566 · 5fe0566
2 parents fc350d4 + 39a1102
commit 5fe0566
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 27 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -10,6 +10,8 @@ This document is intended for Spotless developers.
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Fixed
+ * Improved [SQL formatting](https://github.com/diffplug/spotless/pull/897) with respect to comments
 
 ## [2.15.1] - 2021-07-06
 ### Changed

diff --git a/lib/src/main/java/com/diffplug/spotless/sql/dbeaver/SQLTokenizedFormatter.java b/lib/src/main/java/com/diffplug/spotless/sql/dbeaver/SQLTokenizedFormatter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 DiffPlug
+ * Copyright 2016-2021 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -263,6 +263,8 @@ private List<FormatterToken> format(final List<FormatterToken> argList) {
 					index += insertReturnAndIndent(argList, index, 0);
 				}
 				index += insertReturnAndIndent(argList, index + 1, 0);
+			} else if (token.getType() == TokenType.NAME && index > 0 && argList.get(index - 1).getType() == TokenType.COMMENT) {
+				index += insertReturnAndIndent(argList, index, indent);
 			} else {
 				if (statementDelimiters.contains(tokenString)) {
 					indent = 0;
@@ -322,6 +324,10 @@ private List<FormatterToken> format(final List<FormatterToken> argList) {
 					// Do not add space between symbols
 					continue;
 				}
+				if (prev.getType() == TokenType.COMMENT) {
+					// Do not add spaces to comments
+					continue;
+				}
 				argList.add(index, new FormatterToken(TokenType.SPACE, " "));
 			}
 		}
@@ -383,17 +389,25 @@ private int insertReturnAndIndent(final List<FormatterToken> argList, final int
 		if (functionBracket.contains(Boolean.TRUE))
 			return 0;
 		try {
-			StringBuilder s = new StringBuilder(getDefaultLineSeparator());
+			final String defaultLineSeparator = getDefaultLineSeparator();
+			StringBuilder s = new StringBuilder(defaultLineSeparator);
+			for (int index = 0; index < argIndent; index++) {
+				s.append(formatterCfg.getIndentString());
+			}
 			if (argIndex > 0) {
+				final FormatterToken token = argList.get(argIndex);
 				final FormatterToken prevToken = argList.get(argIndex - 1);
-				if (prevToken.getType() == TokenType.COMMENT &&
-						isCommentLine(sqlDialect, prevToken.getString())) {
-					s = new StringBuilder();
+				if (token.getType() == TokenType.COMMENT &&
+						isCommentLine(sqlDialect, token.getString()) &&
+						prevToken.getType() != TokenType.END) {
+					s.setCharAt(0, ' ');
+					s.setLength(1);
+
+					final String comment = token.getString();
+					final String withoutTrailingWhitespace = comment.replaceFirst("\\s*$", "");
+					token.setString(withoutTrailingWhitespace);
 				}
 			}
-			for (int index = 0; index < argIndent; index++) {
-				s.append(formatterCfg.getIndentString());
-			}
 
 			FormatterToken token = argList.get(argIndex);
 			if (token.getType() == TokenType.SPACE) {

diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md
@@ -3,6 +3,8 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).
 
 ## [Unreleased]
+### Fixed
+ * Improved [SQL formatting](https://github.com/diffplug/spotless/pull/897) with respect to comments
 
 ## [5.14.1] - 2021-07-06
 ### Changed

diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md
@@ -3,6 +3,8 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Fixed
+ * Improved [SQL formatting](https://github.com/diffplug/spotless/pull/897) with respect to comments
 
 ## [2.12.1] - 2021-06-17
 

diff --git a/testlib/src/main/resources/sql/dbeaver/V1_initial.sql.clean b/testlib/src/main/resources/sql/dbeaver/V1_initial.sql.clean
@@ -1,13 +1,12 @@
 --- Account management
- CREATE
+CREATE
     TABLE
         account(
             id serial PRIMARY KEY,
             username VARCHAR(60) NOT NULL UNIQUE,
             email VARCHAR(513) NOT NULL UNIQUE,
-            name VARCHAR(255),
-            --NULLABLE
- created_at TIMESTAMP NOT NULL,
+            name VARCHAR(255), --NULLABLE
+            created_at TIMESTAMP NOT NULL,
             created_ip inet NOT NULL,
             updated_at TIMESTAMP NOT NULL,
             updated_ip inet NOT NULL,
@@ -38,13 +37,12 @@ CREATE
         );
 
 --- Takes
- CREATE
+CREATE
     TABLE
         takerevision(
             id serial PRIMARY KEY,
-            parent_id INT REFERENCES takerevision(id),
-            --NULLABLE (null for root)
- created_at TIMESTAMP NOT NULL,
+            parent_id INT REFERENCES takerevision(id), --NULLABLE (null for root)
+            created_at TIMESTAMP NOT NULL,
             created_ip inet NOT NULL,
             title VARCHAR(255) NOT NULL,
             blocks jsonb NOT NULL
@@ -68,19 +66,17 @@ CREATE
             blocks jsonb NOT NULL,
             published_at TIMESTAMP NOT NULL,
             published_ip inet NOT NULL,
-            deleted_at TIMESTAMP,
-            --NULLABLE
- deleted_ip inet,
-            --NULLABLE
- count_view INT NOT NULL DEFAULT 0,
+            deleted_at TIMESTAMP, --NULLABLE
+            deleted_ip inet, --NULLABLE
+            count_view INT NOT NULL DEFAULT 0,
             count_like INT NOT NULL DEFAULT 0,
             count_bookmark INT NOT NULL DEFAULT 0,
             count_spam INT NOT NULL DEFAULT 0,
             count_illegal INT NOT NULL DEFAULT 0
         );
 
 -- /user/title must be unique, and fast to lookup
- CREATE
+CREATE
     UNIQUE INDEX takepublished_title_user ON
     takepublished(
         title_slug,
@@ -105,8 +101,7 @@ CREATE
                 take_id,
                 user_id,
                 kind
-            ),
-            --user can only have one of each kind of reaction per take
- reacted_at TIMESTAMP NOT NULL,
+            ), --user can only have one of each kind of reaction per take
+            reacted_at TIMESTAMP NOT NULL,
             reacted_ip inet NOT NULL
         );
diff --git a/testlib/src/main/resources/sql/dbeaver/create.clean b/testlib/src/main/resources/sql/dbeaver/create.clean
@@ -1,3 +1,5 @@
+-- multiline
+-- comment
 CREATE
     TABLE
         films(
@@ -19,7 +21,7 @@ CREATE
         );
 
 -- Create a table with a 2-dimensional array:
- CREATE
+CREATE
     TABLE
         array_int(
             vector INT [][]

diff --git a/testlib/src/main/resources/sql/dbeaver/create.clean.alternative b/testlib/src/main/resources/sql/dbeaver/create.clean.alternative
@@ -1,3 +1,5 @@
+-- multiline
+-- comment
 create
 		table
 				films(
@@ -19,7 +21,7 @@ create
 				);
 
 -- Create a table with a 2-dimensional array:
- create
+create
 		table
 				array_int(
 						vector int [][]

diff --git a/testlib/src/main/resources/sql/dbeaver/create.dirty b/testlib/src/main/resources/sql/dbeaver/create.dirty
@@ -1,3 +1,5 @@
+-- multiline
+   -- comment
 CREATE TABLE films (
     code        char(5) CONSTRAINT firstkey PRIMARY KEY,
     title       varchar(40) NOT NULL,