[DROOLS-7310] parse nested parentheses #42
Conversation
There was a problem hiding this comment.
@tkobayas @mariofusco
Sorry guys, but this code is getting impossible to understand.
Please add comments/explanations to methods/variables, otherwise it will become another only-one-person-in-the-world can manage it.
| BaseDescr descr = visitLhsPatternBind(ctx.lhsPatternBind()); | ||
| existsDescr.addDescr(descr); | ||
| if (ctx.lhsOrDef() != null) { | ||
| BaseDescr descr = visitDescrChildren(ctx.lhsOrDef()).get(0); |
There was a problem hiding this comment.
@tkobayas
Please check and manage the case where this -> visitDescrChildren(ctx.lhsOrDef()) returns an empty list
There was a problem hiding this comment.
Thanks, I'll work on it.
@gitgabrio Thank you very much for the suggestion. I'll add more comments/explanation for this PR. And also I will add comments/explanation on overall drools-parser codebase (and probably "developer notes") as a separated JIRA https://issues.redhat.com/browse/DROOLS-7577 |
Great, many thanks @tkobayas !!! |
| @@ -2835,7 +2833,6 @@ public void parse_TypeDeclarationWithFields() throws Exception { | |||
|
|
|||
| } | |||
|
|
|||
| @Disabled("Priority : High | Failed to parse or with parentheses in LHS") | |||
| @Test | |||
| public void parse_RuleWithLHSNesting() throws Exception { | |||
There was a problem hiding this comment.
Hi @tkobayas : Does this (or other) test cover multi-level nesting ?
If not, could you please add one that cover that specific case (multiple level of nested parentheses) ?
There was a problem hiding this comment.
The test case is one level nest (renamed to parenthesesOneLevelNestWithThreeSiblings).
However, we already had multipleLevelNestAndOrOrOrAnd() (renamed).
I also added 2 test cases of tricky multiple level nesting : multipleLevelNestWithThreeOrSiblings and existsMultipleLevelNestWithThreeOrSiblings
Thanks!
- Adding more nested level tests
|
Updated with more comments (only for this PR related part. I'll do overall review with https://issues.redhat.com/browse/DROOLS-7577). @gitgabrio , feel free to let me know if you want more explanations on some specific places. Thanks! |
No description provided.