Introduce flyweights for re-used tokens.

There are many tokens that are reusable such as "(", ",", etc. By turning them into constant flyweights, this reduces churn. Consequence is that their "space" attribute must be made immutible to avoid side effects, so I introduced a utility function.

Author: gregturn

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java

@@ -35,6 +35,12 @@
  */
 abstract class QueryParser {
 
+	private static final Pattern PUNCTUATION_PATTERN = Pattern.compile(".*((?![._])[\\p{Punct}|\\s])");
+
+	private static final String UNSAFE_PROPERTY_REFERENCE = "Sort expression '%s' must only contain property references or "
+			+ "aliases used in the select clause; If you really want to use something other than that for sorting, please use "
+			+ "JpaSort.unsafe(…)";
+
 	private final DeclaredQuery declaredQuery;
 
 	QueryParser(DeclaredQuery declaredQuery) {
@@ -56,7 +62,7 @@ String getQuery() {
 	/**
 	 * Parse the JPA query using its corresponding ANTLR parser.
 	 */
-	abstract ParserRuleContext parse();
+	abstract ParserRuleContext parse(); // TODO move details inside QueryParser
 
 	/**
 	 * Generate a query using the original query with an @literal order by} clause added (or amended) based upon the
@@ -133,12 +139,6 @@ String projection(ParserRuleContext parsedQuery) {
 	 */
 	abstract boolean hasConstructor(ParserRuleContext parsedQuery);
 
-	private static final Pattern PUNCTUATION_PATTERN = Pattern.compile(".*((?![._])[\\p{Punct}|\\s])");
-
-	private static final String UNSAFE_PROPERTY_REFERENCE = "Sort expression '%s' must only contain property references or "
-			+ "aliases used in the select clause; If you really want to use something other than that for sorting, please use "
-			+ "JpaSort.unsafe(…)";
-
 	/**
 	 * Check any given {@link JpaSort.JpaOrder#isUnsafe()} order for presence of at least one property offending the
 	 * {@link #PUNCTUATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java

@@ -28,7 +28,8 @@
  * Implementation of {@link QueryEnhancer} using a {@link QueryParser}.<br/>
  * <br/>
  * NOTE: The parser can find everything it needs to created sorted and count queries. Thus, looking up the alias or the
- * projection isn't needed for its primary function, and are simply implemented for test purposes.
+ * projection isn't needed for its primary function, and are simply implemented for test purposes. TODO: Don't LOG
+ * warning messages in the middle of actions.
  *
  * @author Greg Turnquist
  * @since 3.1
@@ -72,7 +73,6 @@ public String applySorting(Sort sort) {
 
 			return queryParser.createQuery(parsedQuery, sort);
 		} catch (QueryParsingSyntaxError e) {
-			LOG.warn(e);
 			throw new IllegalArgumentException(e);
 		}
 	}
@@ -107,7 +107,6 @@ public String detectAlias() {
 
 			return queryParser.findAlias(parsedQuery);
 		} catch (QueryParsingSyntaxError e) {
-			LOG.warn(e);
 			return null;
 		}
 	}
@@ -139,7 +138,6 @@ public String createCountQueryFor(@Nullable String countProjection) {
 
 			return queryParser.createCountQuery(parsedQuery, countProjection);
 		} catch (QueryParsingSyntaxError e) {
-			LOG.warn(e);
 			throw new IllegalArgumentException(e);
 		}
 	}
@@ -161,7 +159,6 @@ public boolean hasConstructorExpression() {
 
 			return queryParser.hasConstructor(parsedQuery);
 		} catch (QueryParsingSyntaxError e) {
-			LOG.warn(e);
 			return false;
 		}
 	}
@@ -182,7 +179,6 @@ public String getProjection() {
 
 			return queryParser.projection(parsedQuery);
 		} catch (QueryParsingSyntaxError e) {
-			LOG.debug(e);
 			return "";
 		}
 	}

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParser.java

@@ -17,6 +17,10 @@
 
 import java.util.List;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+import org.antlr.v4.runtime.Token;
+import org.antlr.v4.runtime.tree.TerminalNode;
 
 /**
  * A value type used to represent a JPA query token. NOTE: Sometimes the token's value is based upon a value found later
@@ -27,6 +31,31 @@
  */
 class QueryParsingToken {
 
+	/**
+	 * Commonly use tokens.
+	 */
+	public static final QueryParsingToken TOKEN_COMMA = new QueryParsingToken(",");
+	public static final QueryParsingToken TOKEN_DOT = new QueryParsingToken(".", false);
+	public static final QueryParsingToken TOKEN_EQUALS = new QueryParsingToken("=");
+	public static final QueryParsingToken TOKEN_OPEN_PAREN = new QueryParsingToken("(", false);
+	public static final QueryParsingToken TOKEN_CLOSE_PAREN = new QueryParsingToken(")");
+	public static final QueryParsingToken TOKEN_ORDER_BY = new QueryParsingToken("order by");
+	public static final QueryParsingToken TOKEN_LOWER_FUNC = new QueryParsingToken("lower(", false);
+	public static final QueryParsingToken TOKEN_SELECT_COUNT = new QueryParsingToken("select count(", false);
+	public static final QueryParsingToken TOKEN_PERCENT = new QueryParsingToken("%");
+	public static final QueryParsingToken TOKEN_COUNT_FUNC = new QueryParsingToken("count(", false);
+	public static final QueryParsingToken TOKEN_DOUBLE_PIPE = new QueryParsingToken("||");
+	public static final QueryParsingToken TOKEN_OPEN_SQUARE_BRACKET = new QueryParsingToken("[", false);
+	public static final QueryParsingToken TOKEN_CLOSE_SQUARE_BRACKET = new QueryParsingToken("]");
+	public static final QueryParsingToken TOKEN_COLON = new QueryParsingToken(":", false);
+	public static final QueryParsingToken TOKEN_QUESTION_MARK = new QueryParsingToken("?", false);
+	public static final QueryParsingToken TOKEN_CLOSE_BRACE = new QueryParsingToken("}");
+	public static final QueryParsingToken TOKEN_CLOSE_SQUARE_BRACKET_BRACE = new QueryParsingToken("]}");
+	public static final QueryParsingToken TOKEN_CLOSE_PAREN_BRACE = new QueryParsingToken(")}");
+
+	public static final QueryParsingToken TOKEN_DESC = new QueryParsingToken("desc", false);
+
+	public static final QueryParsingToken TOKEN_ASC = new QueryParsingToken("asc", false);
 	/**
 	 * The text value of the token.
 	 */
@@ -35,7 +64,7 @@ class QueryParsingToken {
 	/**
 	 * Space|NoSpace after token is rendered?
 	 */
-	private boolean space;
+	private final boolean space;
 
 	QueryParsingToken(Supplier<String> token, boolean space) {
 
@@ -55,6 +84,18 @@ class QueryParsingToken {
 		this(() -> token, true);
 	}
 
+	QueryParsingToken(TerminalNode node, boolean space) {
+		this(node.getText(), space);
+	}
+
+	QueryParsingToken(TerminalNode node) {
+		this(node.getText());
+	}
+
+	QueryParsingToken(Token token) {
+		this(token.getText());
+	}
+
 	/**
 	 * Extract the token's value from it's {@link Supplier}.
 	 */
@@ -69,17 +110,18 @@ boolean getSpace() {
 		return this.space;
 	}
 
-	void setSpace(boolean space) {
-		this.space = space;
-	}
-
 	/**
 	 * Switch the last {@link QueryParsingToken}'s spacing to {@literal true}.
 	 */
 	static void SPACE(List<QueryParsingToken> tokens) {
 
 		if (!tokens.isEmpty()) {
-			tokens.get(tokens.size() - 1).setSpace(true);
+
+			int index = tokens.size() - 1;
+
+			QueryParsingToken lastTokenWithSpacing = new QueryParsingToken(tokens.get(index).token);
+			tokens.remove(index);
+			tokens.add(lastTokenWithSpacing);
 		}
 	}
 
@@ -89,10 +131,43 @@ static void SPACE(List<QueryParsingToken> tokens) {
 	static void NOSPACE(List<QueryParsingToken> tokens) {
 
 		if (!tokens.isEmpty()) {
-			tokens.get(tokens.size() - 1).setSpace(false);
+
+			int index = tokens.size() - 1;
+
+			QueryParsingToken lastTokenWithNoSpacing = new QueryParsingToken(tokens.get(index).token, false);
+			tokens.remove(index);
+			tokens.add(lastTokenWithNoSpacing);
 		}
 	}
 
+	/**
+	 * Take a list of {@link QueryParsingToken}s and convert them ALL to {@code space = false} (except possibly the last
+	 * one).
+	 * 
+	 * @param tokens
+	 * @param spacelastElement
+	 */
+	static List<QueryParsingToken> NOSPACE_ALL_BUT_LAST_ELEMENT(List<QueryParsingToken> tokens,
+			boolean spacelastElement) {
+
+		List<QueryParsingToken> respacedTokens = tokens.stream() //
+				.map(queryParsingToken -> {
+
+					if (queryParsingToken.space == true) {
+						return new QueryParsingToken(queryParsingToken.token, false);
+					} else {
+						return queryParsingToken;
+					}
+				}) //
+				.collect(Collectors.toList());
+
+		if (spacelastElement) {
+			SPACE(respacedTokens);
+		}
+
+		return respacedTokens;
+	}
+
 	/**
 	 * Drop the last entry from the list of {@link QueryParsingToken}s.
 	 */

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParsingEnhancer.java

@@ -67,7 +67,6 @@ class StringQuery implements DeclaredQuery {
 	 *
 	 * @param query must not be {@literal null} or empty.
 	 */
-	@Deprecated
 	@SuppressWarnings("deprecation")
 	StringQuery(String query, boolean isNative) {
 

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryParsingToken.java

@@ -265,7 +265,8 @@ void appendsIgnoreCaseOrderingCorrectly() {
 		String query = "select p from Person p order by p.lastname asc";
 		Sort sort = Sort.by(Sort.Order.by("firstname").ignoreCase());
 
-		assertThat(createQueryFor(query, sort)).endsWith("order by p.lastname asc, lower(p.firstname) asc");
+		assertThat(createQueryFor(query, sort))
+				.isEqualTo("select p from Person p order by p.lastname asc, lower(p.firstname) asc");
 	}
 
 	@Test // DATAJPA-342

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java

@@ -254,7 +254,8 @@ void appendsIgnoreCaseOrderingCorrectly() {
 		String query = "select p from Person p order by p.lastname asc";
 		Sort sort = Sort.by(Sort.Order.by("firstname").ignoreCase());
 
-		assertThat(createQueryFor(query, sort)).endsWith("order by p.lastname asc, lower(p.firstname) asc");
+		assertThat(createQueryFor(query, sort))
+				.isEqualTo("select p from Person p order by p.lastname asc, lower(p.firstname) asc");
 	}
 
 	@Test // DATAJPA-342