From e09d6855a2855aec178aa857115ca81f0adfb06e Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 26 Apr 2013 11:04:40 -0500 Subject: [PATCH] HHH-8159 - Apply fixups indicated by analysis tools --- .../java/org/hibernate/QueryException.java | 27 +++++++-- .../hibernate/QueryParameterException.java | 2 +- .../hql/internal/ast/ErrorCounter.java | 55 ++++++++++++++----- .../hibernate/hql/internal/ast/HqlParser.java | 36 ++++++------ .../hql/internal/ast/HqlSqlWalker.java | 2 +- .../internal/ast/QuerySyntaxException.java | 43 +++++++++++++-- 6 files changed, 125 insertions(+), 40 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/QueryException.java b/hibernate-core/src/main/java/org/hibernate/QueryException.java index eca6070aa4..dd59a7ed0c 100644 --- a/hibernate-core/src/main/java/org/hibernate/QueryException.java +++ b/hibernate-core/src/main/java/org/hibernate/QueryException.java @@ -102,19 +102,38 @@ protected final String getOriginalMessage() { } /** + * Wraps this exception with another, of same kind, with the specified queryString. If this exception already + * has a queryString defined, the same exception ({@code this}) is returned. Otherwise the protected + * {@link #generateQueryException(String)} is called, to allow subclasses to properly create the correct + * subclass for return. * - * @param queryString - * @return + * @param queryString The query string that led to the QueryException + * + * @return {@code this}, if {@code this} has {@code null} for {@link #getQueryString()}; otherwise a new + * QueryException (or subclass) is returned. */ public final QueryException wrapWithQueryString(String queryString) { if ( this.getQueryString() != null ) { return this; } - return doWrapWithQueryString( queryString ); + return generateQueryException( queryString ); } - protected QueryException doWrapWithQueryString(String queryString) { + /** + * Called from {@link #wrapWithQueryString(String)} when we really need to generate a new QueryException + * (or subclass). + *

+ * NOTE : implementors should take care to use {@link #getOriginalMessage()} for the message, not + * {@link #getMessage()} + * + * @param queryString The query string + * + * @return The generated QueryException (or subclass) + * + * @see #getOriginalMessage() + */ + protected QueryException generateQueryException(String queryString) { return new QueryException( getOriginalMessage(), queryString, this ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/QueryParameterException.java b/hibernate-core/src/main/java/org/hibernate/QueryParameterException.java index b86ea91c5b..3f07de8ebc 100644 --- a/hibernate-core/src/main/java/org/hibernate/QueryParameterException.java +++ b/hibernate-core/src/main/java/org/hibernate/QueryParameterException.java @@ -50,7 +50,7 @@ public QueryParameterException(String message, String queryString, Exception cau } @Override - protected QueryException doWrapWithQueryString(String queryString) { + protected QueryException generateQueryException(String queryString) { return new QueryParameterException( super.getOriginalMessage(), queryString, this ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/ErrorCounter.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/ErrorCounter.java index 9a28c8e3dd..a520c54347 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/ErrorCounter.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/ErrorCounter.java @@ -37,48 +37,77 @@ * An error handler that counts parsing errors and warnings. */ public class ErrorCounter implements ParseErrorHandler { + private static final CoreMessageLogger LOG = Logger.getMessageLogger( + CoreMessageLogger.class, + ErrorCounter.class.getName() + ); - private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, ErrorCounter.class.getName()); + private final String hql; private List errorList = new ArrayList(); - private List warningList = new ArrayList(); private List recognitionExceptions = new ArrayList(); + /** + * Constructs an ErrorCounter without knowledge of the HQL, meaning that generated QueryException + * instances *will not* contain the HQL (and will need to be wrapped at a higher level in another + * QueryException). + */ + public ErrorCounter() { + this( null ); + } + + /** + * Constructs an ErrorCounter with knowledge of the HQL, meaning that generated QueryException + * instances *will* contain the HQL. + */ + public ErrorCounter(String hql) { + this.hql = hql; + } + + @Override public void reportError(RecognitionException e) { reportError( e.toString() ); recognitionExceptions.add( e ); - LOG.error(e.toString(), e); + LOG.error( e.toString(), e ); } + @Override public void reportError(String message) { - LOG.error(message); + LOG.error( message ); errorList.add( message ); } + @Override public int getErrorCount() { return errorList.size(); } + @Override public void reportWarning(String message) { - LOG.debug(message); - warningList.add( message ); + LOG.debug( message ); } private String getErrorString() { - StringBuilder buf = new StringBuilder(); - for ( Iterator iterator = errorList.iterator(); iterator.hasNext(); ) { + final StringBuilder buf = new StringBuilder(); + final Iterator iterator = errorList.iterator(); + while ( iterator.hasNext() ) { buf.append( iterator.next() ); - if ( iterator.hasNext() ) buf.append( "\n" ); + if ( iterator.hasNext() ) { + buf.append( "\n" ); + } } return buf.toString(); } + @Override public void throwQueryException() throws QueryException { if ( getErrorCount() > 0 ) { - if (recognitionExceptions.size() > 0) throw QuerySyntaxException.convert(recognitionExceptions.get(0)); - throw new QueryException(getErrorString()); - } - LOG.debug("throwQueryException() : no errors"); + if ( recognitionExceptions.size() > 0 ) { + throw QuerySyntaxException.convert( recognitionExceptions.get( 0 ), hql ); + } + throw new QueryException( getErrorString(), hql ); + } + LOG.debug( "throwQueryException() : no errors" ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlParser.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlParser.java index b9f0bbd8e5..0a886e0e13 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlParser.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlParser.java @@ -52,25 +52,35 @@ * @author Joshua Davis (pgmjsd@sourceforge.net) */ public final class HqlParser extends HqlBaseParser { + private static final CoreMessageLogger LOG = Logger.getMessageLogger( + CoreMessageLogger.class, + HqlParser.class.getName() + ); - private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, HqlParser.class.getName()); - - private ParseErrorHandler parseErrorHandler; - private ASTPrinter printer = getASTPrinter(); + private final ParseErrorHandler parseErrorHandler; + private final ASTPrinter printer = getASTPrinter(); private static ASTPrinter getASTPrinter() { return new ASTPrinter( org.hibernate.hql.internal.antlr.HqlTokenTypes.class ); } + /** + * Get a HqlParser instance for the given HQL string. + * + * @param hql The HQL query string + * + * @return The parser. + */ public static HqlParser getInstance(String hql) { - // [jsd] The fix for HHH-558... - HqlLexer lexer = new HqlLexer( new StringReader( hql ) ); - return new HqlParser( lexer ); + return new HqlParser( hql ); } - private HqlParser(TokenStream lexer) { - super( lexer ); - initialize(); + private HqlParser(String hql) { + // The fix for HHH-558... + super( new HqlLexer( new StringReader( hql ) ) ); + parseErrorHandler = new ErrorCounter( hql ); + // Create nodes that track line and column number. + setASTFactory( new HqlASTFactory() ); } @@ -326,12 +336,6 @@ private void showAst(AST ast, PrintWriter pw) { printer.showAst( ast, pw ); } - private void initialize() { - // Initialize the error handling delegate. - parseErrorHandler = new ErrorCounter(); - setASTFactory(new HqlASTFactory()); // Create nodes that track line and column number. - } - @Override public void weakKeywords() throws TokenStreamException { diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java index 95cba36400..8f32783230 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java @@ -170,7 +170,7 @@ public HqlSqlWalker( String collectionRole) { setASTFactory( new SqlASTFactory( this ) ); // Initialize the error handling delegate. - this.parseErrorHandler = new ErrorCounter(); + this.parseErrorHandler = new ErrorCounter( qti.getQueryString() ); this.queryTranslatorImpl = qti; this.sessionFactoryHelper = new SessionFactoryHelper( sfi ); this.literalProcessor = new LiteralProcessor( this ); diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QuerySyntaxException.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QuerySyntaxException.java index ea97a8ce49..c651ecdd15 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QuerySyntaxException.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QuerySyntaxException.java @@ -1,10 +1,10 @@ /* * Hibernate, Relational Persistence for Idiomatic Java * - * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as + * Copyright (c) 2008, 2013, Red Hat Inc. or third-party contributors as * indicated by the @author tags or express copyright attribution * statements applied by the authors. All third-party contributions are - * distributed under license by Red Hat Middleware LLC. + * distributed under license by Red Hat Inc. * * This copyrighted material is made available to anyone wishing to use, modify, * copy, or redistribute it subject to the terms and conditions of the GNU @@ -20,7 +20,6 @@ * Free Software Foundation, Inc. * 51 Franklin Street, Fifth Floor * Boston, MA 02110-1301 USA - * */ package org.hibernate.hql.internal.ast; import antlr.RecognitionException; @@ -33,23 +32,57 @@ * @author josh */ public class QuerySyntaxException extends QueryException { - + /** + * Constructs a QuerySyntaxException + * + * @param message Message explaining the condition that led to the exception + */ public QuerySyntaxException(String message) { super( message ); } + /** + * Constructs a QuerySyntaxException + * + * @param message Message explaining the condition that led to the exception + * @param hql The hql query that was being parsed/analyzed + */ public QuerySyntaxException(String message, String hql) { super( message, hql ); } + /** + * Intended for use solely from {@link #generateQueryException(String)} + * + * @param message Message explaining the condition that led to the exception + * @param queryString The hql query that was being parsed/analyzed + * @param cause The cause, generally another QuerySyntaxException + */ protected QuerySyntaxException(String message, String queryString, Exception cause) { super( message, queryString, cause ); } + /** + * Converts the given ANTLR RecognitionException into a QuerySyntaxException. The RecognitionException + * does not become the cause because ANTLR exceptions are not serializable. + * + * @param e The ANTLR exception + * + * @return The QuerySyntaxException + */ public static QuerySyntaxException convert(RecognitionException e) { return convert( e, null ); } + /** + * Converts the given ANTLR RecognitionException into a QuerySyntaxException. The RecognitionException + * does not become the cause because ANTLR exceptions are not serializable. + * + * @param e The ANTLR exception + * @param hql The query string + * + * @return The QuerySyntaxException + */ public static QuerySyntaxException convert(RecognitionException e, String hql) { String positionInfo = e.getLine() > 0 && e.getColumn() > 0 ? " near line " + e.getLine() + ", column " + e.getColumn() @@ -58,7 +91,7 @@ public static QuerySyntaxException convert(RecognitionException e, String hql) { } @Override - protected QueryException doWrapWithQueryString(String queryString) { + protected QueryException generateQueryException(String queryString) { return new QuerySyntaxException( getOriginalMessage(), queryString, this ); } }