Polishing.

Align ANTLR version with Hibernate's ANTLR version to avoid version mismatch reports to syserr. Rename types for consistent naming. Avoid duplicate creation of DeclaredQuery within the parsers. Lazify parsing and reuse cached results to avoid parser overhead. Add common configuration for parsers for consistent parser and lexer configurations. Simplify factory methods for HQL and JPQL parsers. Simplify condition flow for query enhancer creation. Refine fields for non-nullability requirements, avoid handing out null for a List.
spring-projects · Mar 14, 2023 · 4aa2af5 · 4aa2af5
1 parent a13a9bc
commit 4aa2af5
Show file tree

Hide file tree

Showing 21 changed files with 650 additions and 663 deletions.
diff --git a/pom.xml b/pom.xml
@@ -28,9 +28,9 @@
 
 	<properties>
 		<source.level>16</source.level>
-    <!-- AspectJ maven plugin can't handle 17 yet -->
+		<!-- AspectJ maven plugin can't handle 17 yet -->
 
-		<antlr>4.11.1</antlr>
+		<antlr>4.10.1</antlr> <!-- align with Hibernate's parser -->
 		<eclipselink>3.0.3</eclipselink>
 		<hibernate>6.1.4.Final</hibernate>
 		<hsqldb>2.7.1</hsqldb>

diff --git a/...y/JpaQueryParsingSyntaxErrorListener.java → ...ry/query/BadJpqlGrammarErrorListener.java b/...y/JpaQueryParsingSyntaxErrorListener.java → ...ry/query/BadJpqlGrammarErrorListener.java
@@ -20,16 +20,23 @@
 import org.antlr.v4.runtime.Recognizer;
 
 /**
- * A {@link BaseErrorListener} that will throw a {@link JpaQueryParsingSyntaxError} if the query is invalid.
+ * A {@link BaseErrorListener} that will throw a {@link BadJpqlGrammarException} if the query is invalid.
  *
  * @author Greg Turnquist
  * @since 3.1
  */
-class JpaQueryParsingSyntaxErrorListener extends BaseErrorListener {
+class BadJpqlGrammarErrorListener extends BaseErrorListener {
+
+	private final String query;
+
+	BadJpqlGrammarErrorListener(String query) {
+		this.query = query;
+	}
 
 	@Override
 	public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int line, int charPositionInLine,
 			String msg, RecognitionException e) {
-		throw new JpaQueryParsingSyntaxError("line " + line + ":" + charPositionInLine + " " + msg);
+		throw new BadJpqlGrammarException("Line " + line + ":" + charPositionInLine + " " + msg, query, null);
 	}
+
 }
diff --git a/...ory/query/JpaQueryParsingSyntaxError.java → ...sitory/query/BadJpqlGrammarException.java b/...ory/query/JpaQueryParsingSyntaxError.java → ...sitory/query/BadJpqlGrammarException.java
@@ -16,17 +16,26 @@
 package org.springframework.data.jpa.repository.query;
 
 import org.springframework.dao.InvalidDataAccessResourceUsageException;
+import org.springframework.lang.Nullable;
 
 /**
  * An exception thrown if the JPQL query is invalid.
  *
  * @author Greg Turnquist
+ * @author Mark Paluch
  * @since 3.1
  */
-class JpaQueryParsingSyntaxError extends InvalidDataAccessResourceUsageException {
+public class BadJpqlGrammarException extends InvalidDataAccessResourceUsageException {
 
-	public JpaQueryParsingSyntaxError(String message) {
-		super(message);
+	private final String jpql;
+
+	public BadJpqlGrammarException(String message, String jpql, @Nullable Throwable cause) {
+		super(message + "; Bad JPQL grammar [" + jpql + "]", cause);
+		this.jpql = jpql;
+	}
+
+	public String getJpql() {
+		return this.jpql;
 	}
 
 }
diff --git a/...-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryParser.java b/...-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryParser.java
@@ -24,46 +24,43 @@
 import org.springframework.lang.Nullable;
 
 /**
- * Implements the parsing operations of a {@link JpaQueryParser} using the ANTLR-generated {@link HqlParser} and
- * {@link HqlQueryTransformer}.
- * 
+ * Implements the {@code HQL} parsing operations of a {@link JpaQueryParserSupport} using the ANTLR-generated
+ * {@link HqlParser} and {@link HqlQueryTransformer}.
+ *
  * @author Greg Turnquist
+ * @author Mark Paluch
  * @since 3.1
  */
-class HqlQueryParser extends JpaQueryParser {
-
-	HqlQueryParser(DeclaredQuery declaredQuery) {
-		super(declaredQuery);
-	}
+class HqlQueryParser extends JpaQueryParserSupport {
 
 	HqlQueryParser(String query) {
 		super(query);
 	}
 
 	/**
-	 * Convenience method to parse an HQL query. Will throw a {@link JpaQueryParsingSyntaxError} if the query is invalid.
+	 * Convenience method to parse an HQL query. Will throw a {@link BadJpqlGrammarException} if the query is invalid.
 	 *
 	 * @param query
 	 * @return a parsed query, ready for postprocessing
 	 */
-	static ParserRuleContext parse(String query) {
+	public static ParserRuleContext parseQuery(String query) {
 
 		HqlLexer lexer = new HqlLexer(CharStreams.fromString(query));
 		HqlParser parser = new HqlParser(new CommonTokenStream(lexer));
 
-		parser.addErrorListener(new JpaQueryParsingSyntaxErrorListener());
+		configureParser(query, lexer, parser);
 
 		return parser.start();
 	}
 
 	/**
-	 * Parse the query using {@link #parse(String)}.
+	 * Parse the query using {@link #parseQuery(String)}.
 	 *
 	 * @return a parsed query
 	 */
 	@Override
-	protected ParserRuleContext parse() {
-		return parse(getQuery());
+	protected ParserRuleContext parse(String query) {
+		return parseQuery(query);
 	}
 
 	/**
@@ -74,7 +71,7 @@ protected ParserRuleContext parse() {
 	 * @return list of {@link JpaQueryParsingToken}s
 	 */
 	@Override
-	protected List<JpaQueryParsingToken> doCreateQuery(ParserRuleContext parsedQuery, Sort sort) {
+	protected List<JpaQueryParsingToken> applySort(ParserRuleContext parsedQuery, Sort sort) {
 		return new HqlQueryTransformer(sort).visit(parsedQuery);
 	}
 

diff --git a/...-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java b/...-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java
@@ -18,11 +18,13 @@
 import static org.springframework.data.jpa.repository.query.JpaQueryParsingToken.*;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.antlr.v4.runtime.ParserRuleContext;
 import org.springframework.data.domain.Sort;
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 
 /**
  * An ANTLR {@link org.antlr.v4.runtime.tree.ParseTreeVisitor} that transforms a parsed HQL query.
@@ -32,30 +34,35 @@
  */
 class HqlQueryTransformer extends HqlQueryRenderer {
 
-	@Nullable private Sort sort;
-	private boolean countQuery;
+	// TODO: Separate input from result parameters, encapsulation...
 
-	@Nullable private String countProjection;
+	private final Sort sort;
+	private final boolean countQuery;
 
-	@Nullable private String alias = null;
+	private final @Nullable String countProjection;
 
-	private List<JpaQueryParsingToken> projection = null;
+	private @Nullable String alias = null;
+
+	private List<JpaQueryParsingToken> projection = Collections.emptyList();
+	private boolean projectionProcessed;
 
 	private boolean hasConstructorExpression = false;
 
 	HqlQueryTransformer() {
-		this(null, false, null);
+		this(Sort.unsorted(), false, null);
 	}
 
-	HqlQueryTransformer(@Nullable Sort sort) {
+	HqlQueryTransformer(Sort sort) {
 		this(sort, false, null);
 	}
 
 	HqlQueryTransformer(boolean countQuery, @Nullable String countProjection) {
-		this(null, countQuery, countProjection);
+		this(Sort.unsorted(), countQuery, countProjection);
 	}
 
-	private HqlQueryTransformer(@Nullable Sort sort, boolean countQuery, @Nullable String countProjection) {
+	private HqlQueryTransformer(Sort sort, boolean countQuery, @Nullable String countProjection) {
+
+		Assert.notNull(sort, "Sort must not be null");
 
 		this.sort = sort;
 		this.countQuery = countQuery;
@@ -94,7 +101,7 @@ private static boolean isSubquery(ParserRuleContext ctx) {
 	@Override
 	public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContext ctx) {
 
-		List<JpaQueryParsingToken> tokens = new ArrayList<>();
+		List<JpaQueryParsingToken> tokens = newArrayList();
 
 		if (ctx.query() != null) {
 			tokens.addAll(visit(ctx.query()));
@@ -111,7 +118,7 @@ public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContex
 				tokens.addAll(visit(ctx.queryOrder()));
 			}
 
-			if (this.sort != null && this.sort.isSorted()) {
+			if (this.sort.isSorted()) {
 
 				if (ctx.queryOrder() != null) {
 
@@ -125,7 +132,7 @@ public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContex
 
 				this.sort.forEach(order -> {
 
-					JpaQueryParser.checkSortExpression(order);
+					JpaQueryParserSupport.checkSortExpression(order);
 
 					if (order.isIgnoreCase()) {
 						tokens.add(TOKEN_LOWER_FUNC);
@@ -160,7 +167,7 @@ public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContex
 	@Override
 	public List<JpaQueryParsingToken> visitFromQuery(HqlParser.FromQueryContext ctx) {
 
-		List<JpaQueryParsingToken> tokens = new ArrayList<>();
+		List<JpaQueryParsingToken> tokens = newArrayList();
 
 		if (countQuery && !isSubquery(ctx) && ctx.selectClause() == null) {
 
@@ -201,7 +208,7 @@ public List<JpaQueryParsingToken> visitFromQuery(HqlParser.FromQueryContext ctx)
 	@Override
 	public List<JpaQueryParsingToken> visitQueryOrder(HqlParser.QueryOrderContext ctx) {
 
-		List<JpaQueryParsingToken> tokens = new ArrayList<>();
+		List<JpaQueryParsingToken> tokens = newArrayList();
 
 		if (!countQuery) {
 			tokens.addAll(visit(ctx.orderByClause()));
@@ -224,7 +231,7 @@ public List<JpaQueryParsingToken> visitQueryOrder(HqlParser.QueryOrderContext ct
 	@Override
 	public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
 
-		List<JpaQueryParsingToken> tokens = new ArrayList<>();
+		List<JpaQueryParsingToken> tokens = newArrayList();
 
 		if (ctx.entityName() != null) {
 
@@ -261,7 +268,7 @@ public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
 	@Override
 	public List<JpaQueryParsingToken> visitAlias(HqlParser.AliasContext ctx) {
 
-		List<JpaQueryParsingToken> tokens = new ArrayList<>();
+		List<JpaQueryParsingToken> tokens = newArrayList();
 
 		if (ctx.AS() != null) {
 			tokens.add(new JpaQueryParsingToken(ctx.AS()));
@@ -279,7 +286,7 @@ public List<JpaQueryParsingToken> visitAlias(HqlParser.AliasContext ctx) {
 	@Override
 	public List<JpaQueryParsingToken> visitSelectClause(HqlParser.SelectClauseContext ctx) {
 
-		List<JpaQueryParsingToken> tokens = new ArrayList<>();
+		List<JpaQueryParsingToken> tokens = newArrayList();
 
 		tokens.add(new JpaQueryParsingToken(ctx.SELECT()));
 
@@ -321,8 +328,9 @@ public List<JpaQueryParsingToken> visitSelectClause(HqlParser.SelectClauseContex
 			tokens.addAll(selectionListTokens);
 		}
 
-		if (projection == null && !isSubquery(ctx)) {
+		if (!projectionProcessed && !isSubquery(ctx)) {
 			this.projection = selectionListTokens;
+			this.projectionProcessed = true;
 		}
 
 		return tokens;
@@ -335,4 +343,8 @@ public List<JpaQueryParsingToken> visitInstantiation(HqlParser.InstantiationCont
 
 		return super.visitInstantiation(ctx);
 	}
+
+	static <T> ArrayList<T> newArrayList() {
+		return new ArrayList<>();
+	}
 }