Hi Jielin,
looks good! Just one comment about class DotNode.java, method
checkNavigation:
For the invalidNavigation exception I propose to take line and column
information from the specified node parameter (so node.getLine(),
node.getColumn()). The other exception invalidCollectionNavigation in
method checkNavigation is fine.
Regards Michael
> Hi, Micahel,
>
> Attached is the change based on your comments. Would you please review
> them?
>
> Thanks.
>
> Jielin
>
> Michael Bouschen wrote:
>
>> 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
>>
>>