Merge pull request #1140 from domaframework/feat/reimplement-removal-of-sql-comments

Reimplement "Enable removal of comments from SQL templates"

Author: nakamura-to

doma-core/src/main/java/org/seasar/doma/internal/jdbc/sql/NodePreparedSqlBuilder.java

@@ -29,6 +29,7 @@
 import org.seasar.doma.internal.jdbc.sql.node.BindVariableNode;
 import org.seasar.doma.internal.jdbc.sql.node.BlankNode;
 import org.seasar.doma.internal.jdbc.sql.node.CommentNode;
+import org.seasar.doma.internal.jdbc.sql.node.CommentType;
 import org.seasar.doma.internal.jdbc.sql.node.DistinctNode;
 import org.seasar.doma.internal.jdbc.sql.node.ElseNode;
 import org.seasar.doma.internal.jdbc.sql.node.ElseifNode;
@@ -69,6 +70,7 @@
 import org.seasar.doma.jdbc.InParameter;
 import org.seasar.doma.jdbc.JdbcException;
 import org.seasar.doma.jdbc.PreparedSql;
+import org.seasar.doma.jdbc.SqlBuilderSettings;
 import org.seasar.doma.jdbc.SqlKind;
 import org.seasar.doma.jdbc.SqlLogFormattingFunction;
 import org.seasar.doma.jdbc.SqlLogType;
@@ -86,7 +88,7 @@ public class NodePreparedSqlBuilder
 
   protected final Config config;
 
-  protected final boolean shouldRemoveBlankLines;
+  protected final SqlBuilderSettings sqlBuilderSettings;
 
   protected final SqlKind kind;
 
@@ -156,7 +158,7 @@ public NodePreparedSqlBuilder(
       BiConsumer<PopulateNode, SqlContext> valuesPopulater) {
     assertNotNull(config, kind, evaluator, columnsExpander, valuesPopulater);
     this.config = config;
-    this.shouldRemoveBlankLines = config.getSqlBuilderSettings().shouldRemoveBlankLines();
+    this.sqlBuilderSettings = config.getSqlBuilderSettings();
     this.kind = kind;
     this.sqlFilePath = sqlFilePath;
     this.evaluator = evaluator;
@@ -205,8 +207,21 @@ public Void visitWhitespaceNode(WhitespaceNode node, Context p) {
   @Override
   public Void visitCommentNode(CommentNode node, Context p) {
     String comment = node.getComment();
-    p.appendRawSql(comment);
-    p.appendFormattedSql(comment);
+    CommentType commentType = node.getCommentType();
+    switch (commentType) {
+      case BLOCK:
+        if (!sqlBuilderSettings.shouldRemoveBlockComment(comment)) {
+          p.appendRawSql(comment);
+          p.appendFormattedSql(comment);
+        }
+        break;
+      case LINE:
+        if (!sqlBuilderSettings.shouldRemoveLineComment(comment)) {
+          p.appendRawSql(comment);
+          p.appendFormattedSql(comment);
+        }
+        break;
+    }
     return null;
   }
 
@@ -769,14 +784,14 @@ public Void visitExpandNode(ExpandNode node, Context p) {
   }
 
   private Context createContext(Context context) {
-    if (shouldRemoveBlankLines) {
+    if (sqlBuilderSettings.shouldRemoveBlankLines()) {
       return new BlankLineRemovalContext(context);
     }
     return new DefaultContext(context);
   }
 
   private Context createContext(Config config, ExpressionEvaluator evaluator) {
-    if (shouldRemoveBlankLines) {
+    if (sqlBuilderSettings.shouldRemoveBlankLines()) {
       return new BlankLineRemovalContext(config, evaluator);
     }
     return new DefaultContext(config, evaluator);
@@ -827,9 +842,6 @@ interface Context {
     Value removeValue(String variableName);
 
     EvaluationResult evaluate(SqlLocation location, String expression);
-
-    @Override
-    String toString();
   }
 
   protected static class DefaultContext implements Context {
@@ -1093,7 +1105,7 @@ private void flushBlankNodes() {
     }
 
     private static String toString(List<BlankNode> nodes, int eolNodeCount) {
-      if (eolNodeCount > 1) {
+      if (eolNodeCount > 0) {
         int seenEolNodeCount = 0;
         ListIterator<BlankNode> iterator = nodes.listIterator();
         while (iterator.hasNext()) {
@@ -1189,7 +1201,6 @@ public EvaluationResult evaluate(SqlLocation location, String expression) {
 
     @Override
     public String toString() {
-      flushBlankNodes();
       return context.toString();
     }
   }

doma-core/src/main/java/org/seasar/doma/internal/jdbc/sql/SqlParser.java

@@ -12,6 +12,7 @@
 import org.seasar.doma.internal.jdbc.sql.node.BindVariableNode;
 import org.seasar.doma.internal.jdbc.sql.node.BlockNode;
 import org.seasar.doma.internal.jdbc.sql.node.CommentNode;
+import org.seasar.doma.internal.jdbc.sql.node.CommentType;
 import org.seasar.doma.internal.jdbc.sql.node.DistinctNode;
 import org.seasar.doma.internal.jdbc.sql.node.ElseNode;
 import org.seasar.doma.internal.jdbc.sql.node.ElseifNode;
@@ -48,7 +49,6 @@
 import org.seasar.doma.internal.util.StringUtil;
 import org.seasar.doma.jdbc.JdbcException;
 import org.seasar.doma.jdbc.SqlNode;
-import org.seasar.doma.jdbc.SqlParserConfig;
 import org.seasar.doma.message.Message;
 
 public class SqlParser {
@@ -60,8 +60,6 @@ public class SqlParser {
 
   protected final String sql;
 
-  protected final SqlParserConfig config;
-
   protected final SqlTokenizer tokenizer;
 
   protected final AnonymousNode rootNode;
@@ -71,13 +69,8 @@ public class SqlParser {
   protected String token;
 
   public SqlParser(String sql) {
-    this(sql, SqlParserConfig.DEFAULT);
-  }
-
-  public SqlParser(String sql, SqlParserConfig config) {
-    assertNotNull(sql, config);
+    assertNotNull(sql);
     this.sql = sql;
-    this.config = config;
     tokenizer = new SqlTokenizer(sql);
     rootNode = new AnonymousNode();
     nodeStack.push(rootNode);
@@ -239,6 +232,7 @@ public SqlNode parse() {
             parseBlockComment();
             break;
           }
+
         case LINE_COMMENT:
           {
             parseLineComment();
@@ -424,17 +418,13 @@ protected void parseDistinctWord() {
   }
 
   protected void parseBlockComment() {
-    if (!config.shouldRemoveBlockComment(token)) {
-      CommentNode node = new CommentNode(token);
-      appendNode(node);
-    }
+    CommentNode node = new CommentNode(token, CommentType.BLOCK);
+    appendNode(node);
   }
 
   protected void parseLineComment() {
-    if (!config.shouldRemoveLineComment(token)) {
-      CommentNode node = new CommentNode(token);
-      appendNode(node);
-    }
+    CommentNode node = new CommentNode(token, CommentType.LINE);
+    appendNode(node);
   }
 
   protected void parseOpenedParens() {

doma-core/src/main/java/org/seasar/doma/internal/jdbc/sql/node/CommentNode.java

@@ -11,15 +11,22 @@ public class CommentNode extends AbstractSqlNode {
 
   protected final String comment;
 
-  public CommentNode(String comment) {
-    assertNotNull(comment);
+  protected final CommentType commentType;
+
+  public CommentNode(String comment, CommentType commentType) {
+    assertNotNull(comment, commentType);
     this.comment = comment;
+    this.commentType = commentType;
   }
 
   public String getComment() {
     return comment;
   }
 
+  public CommentType getCommentType() {
+    return commentType;
+  }
+
   @Override
   public void appendNode(SqlNode child) {
     throw new JdbcUnsupportedOperationException(getClass().getName(), "addNode");

doma-core/src/main/java/org/seasar/doma/internal/jdbc/sql/node/CommentType.java

@@ -0,0 +1,6 @@
+package org.seasar.doma.internal.jdbc.sql.node;
+
+public enum CommentType {
+  BLOCK,
+  LINE
+}

doma-core/src/main/java/org/seasar/doma/jdbc/AbstractSqlFileRepository.java

@@ -16,16 +16,6 @@
 /** A skeletal implementation of the {@link SqlFileRepository} interface. */
 public abstract class AbstractSqlFileRepository implements SqlFileRepository {
 
-  protected final SqlParserConfig sqlParserConfig;
-
-  protected AbstractSqlFileRepository() {
-    this(SqlParserConfig.DEFAULT);
-  }
-
-  protected AbstractSqlFileRepository(SqlParserConfig sqlParserConfig) {
-    this.sqlParserConfig = Objects.requireNonNull(sqlParserConfig);
-  }
-
   @Override
   public final SqlFile getSqlFile(Method method, String path, Dialect dialect) {
     if (method == null) {
@@ -110,7 +100,7 @@ protected final String getPrimaryPath(String path, Dialect dialect) {
    * @return the SQL node
    */
   protected final SqlNode parse(String sql) {
-    SqlParser parser = new SqlParser(sql, sqlParserConfig);
+    SqlParser parser = new SqlParser(sql);
     return parser.parse();
   }
 

doma-core/src/main/java/org/seasar/doma/jdbc/Config.java

@@ -226,6 +226,11 @@ default EntityListenerProvider getEntityListenerProvider() {
     return ConfigSupport.defaultEntityListenerProvider;
   }
 
+  /**
+   * Returns the context for SQL builder settings.
+   *
+   * @return the context for SQL builder settings
+   */
   default SqlBuilderSettings getSqlBuilderSettings() {
     return ConfigSupport.defaultSqlBuilderSettings;
   }

doma-core/src/main/java/org/seasar/doma/jdbc/GreedyCacheSqlFileRepository.java

@@ -10,12 +10,6 @@ public class GreedyCacheSqlFileRepository extends AbstractSqlFileRepository {
 
   protected final ConcurrentMap<CacheKey, SqlFile> sqlFileMap = new ConcurrentHashMap<>(200);
 
-  public GreedyCacheSqlFileRepository() {}
-
-  public GreedyCacheSqlFileRepository(SqlParserConfig sqlParserConfig) {
-    super(sqlParserConfig);
-  }
-
   @Override
   protected SqlFile getSqlFileWithCacheControl(Method method, String path, Dialect dialect) {
     CacheKey key = new CacheKey(method, path);

doma-core/src/main/java/org/seasar/doma/jdbc/NoCacheSqlFileRepository.java

@@ -6,12 +6,6 @@
 /** An SQL file repository that does not cache the results of SQL parsing. */
 public class NoCacheSqlFileRepository extends AbstractSqlFileRepository {
 
-  public NoCacheSqlFileRepository() {}
-
-  public NoCacheSqlFileRepository(SqlParserConfig sqlParserConfig) {
-    super(sqlParserConfig);
-  }
-
   @Override
   protected SqlFile getSqlFileWithCacheControl(Method method, String path, Dialect dialect) {
     return createSqlFile(method, path, dialect);

doma-core/src/main/java/org/seasar/doma/jdbc/SqlBuilderSettings.java

@@ -1,6 +1,35 @@
 package org.seasar.doma.jdbc;
 
+/** A context for SQL builder settings. */
 public interface SqlBuilderSettings {
+
+  /**
+   * Returns whether the block comment should be removed. The beginning of the {@code comment}
+   * parameter string must always start with "/*".
+   *
+   * @param comment the block comment, never null
+   * @return true if the block comment should be removed
+   */
+  default boolean shouldRemoveBlockComment(String comment) {
+    return false;
+  }
+
+  /**
+   * Returns whether the line comment should be removed. The beginning of the {@code comment}
+   * parameter string must always start with "--".
+   *
+   * @param comment the line comment, never null
+   * @return true if the line comment should be removed
+   */
+  default boolean shouldRemoveLineComment(String comment) {
+    return false;
+  }
+
+  /**
+   * Returns whether the blank lines should be removed.
+   *
+   * @return true if the blank lines should be removed
+   */
   default boolean shouldRemoveBlankLines() {
     return false;
   }