yurloc commented on code in PR #5907:
URL:
https://github.com/apache/incubator-kie-drools/pull/5907#discussion_r1593830595
##########
drools-drl/drools-drl-parser/src/main/java/org/drools/drl/parser/antlr4/DRLVisitorImpl.java:
##########
@@ -803,12 +803,19 @@ public AccumulateDescr
visitFromAccumulate(DRLParser.FromAccumulateContext ctx)
@Override
public AccumulateDescr.AccumulateFunctionCallDescr
visitAccumulateFunction(DRLParser.AccumulateFunctionContext ctx) {
String function = ctx.drlIdentifier().getText();
- String bind = ctx.label() == null ? null :
ctx.label().drlIdentifier().getText();
+ String bind = null;
+ boolean unify = false;
+ if (ctx.label() != null) {
Review Comment:
@gitgabrio The fact that they can both be null and only one of them can be
non-null follows from the structure of the grammar rule:
```
accumulateFunction : (label|unif)? drlIdentifier conditionalExpressions ;
```
So if you're suggesting we should write a defensive code and add something
like
```java
if (ctx.label() != null && ctx.unif() != null) throw new
IllegalStateException();
```
then I hope this post explains that it would be redundant. Testing such a
condition is testing the ANTLR parser itself, which is unnecessary.
P.S. In other words, the fact that `label` and `unif` can't be both non-null
is guaranteed by the grammar, so no need to check that on our side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
