Hi Jielin,
please find my remarks below. Most of the remarks are about taking line
and column information from a different node than the current one.
EJBQLException.java/EJBQLExceptionResource.java:
- I propose to add line and column information for error message 8018
invalidMultipleUseOfSameParameter. Method defineParameterType of
ParseTreeContext could take line and column as additional parameters and
pass it to the error message. The method is called from ParameterNode.
- The same appies for error message 8019 multipleVariableDeclaration.
Methods registerSchema and registerJoinVariable of ParseTreeContext
could get additional line and column parameters.
EJBQLExceptionResource.java:
- The error message of 8032 does not use the attribute name. I propose
to change the text to: Invalid access of attribute [{0}] in SET clause
target [{1}], only state fields ...
- The error message of entry 8033 is broken.
- In the definition of field contents there is a typo in the second
entry from the bottom: it should have the number 8037 instead of 8034.
- The error message of 8038 should start with an 'a': "line {1}, column
{2}: a problem ...
DotNode.java:
Using getLine() and getColumn in the class DotNode returns the position
of the dot itself. In some cases we can give a better error message when
using the position of the erroneous syntax element mentioned in the
error message. E.g. if the error message says unknown attribute we
should use the position of the attribute instead of the position of the dot:
- Method validate, exception unknownAttribute: right.getLine(),
right.getColumn()
- Method validate, exception resolveEnumConstant: right.getLine(),
right.getColumn()
- Method validate, exception aliasResolutionException:
leftMost.getLine(), leftMost.getColumn()
- Method checkNavigation, exception invalidCollectionNavigation
right.getLine(), right.getColumn()
EqualsAssignmentNode.java:
- Method validateTarget
I propose to use the line and column information of the attribute node
for the invalidSetClauseTarget exception. You could intoduce a new local
variable attributeNode and then use attributeNode.getLine() and
attributeNode.getColumn() for the error message:
Node attributeNode = (AttributeNode)node.getRight();
String attribute = attributeNode.getAttributeName();
...
- Method validateNavigation
Same as above: use attributeNode.getLine() and attributeNode.getColumn()
Node attributeNode = (AttributeNode)qualifier.getRight();
String attribute = attributeNode.getAttributeName();
...
GroupByNode.java:
- Method validate: use line and column information of the select
expression (selectExpr.getLine() and selectExpr.getColumn()).
HavingNode.java:
- Method validate: use line and column information of the having
expression (having.getLine() and having.getColumn()).
InNode.java:
- Method validate: use line and column information of the node in the IN
expression list (node.getLine() and node.getColumn()).
ModNode.java:
- Method validate: use line and column information of the mode argument
nodes (left.getLine() and left.getColumn(), denominator.getLine() and
denominator.getColumn()).
OrderByItemNode.java:
- Method validate: use line and column information of the order by item
node (orderByItem.getLine() and orderByItem.getColumn()).
Regards Michael
> Hi, Michael,
>
> Attched is the changes for issue 1355. The issue is about adding line
> and column information when there is any error in a Java Persistence
> query results in a EJBQLException.
>
> Basically, there are two type of error handling for jpql in the
> current code: one is thrown from EJBQLParser.handleANTLRException();
> the other is all validation methods in the Node subclasses. Line and
> column information are added in these two category. If the same error
> method is used in other place and don't have line and column
> information, a new method is added with same same having suffix 2.
>
> Would you please review the changes?
>
> Thanks.
>
> Jielin