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
>
>