Skip to content

Commit

Permalink
[DROOLS-7310] parse nested parentheses (#42)
Browse files Browse the repository at this point in the history 
* [DROOLS-7310] parse nested parentheses

* - Adding code commentes and explanations
- Adding more nested level tests
  • Loading branch information
@tkobayas
tkobayas authored Oct 27, 2023
1 parent 921f15a commit 42b1183
Show file tree
Hide file tree
Showing 3 changed files with 225 additions and 28 deletions.
20 changes: 16 additions & 4 deletions drools-parser/src/main/antlr4/org/drools/parser/DRLParser.g4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ lhsExpression : LPAREN lhsExpression RPAREN #lhsExpressionEnclosed
| lhsExpression (DRL_OR lhsExpression)+ #lhsOr
;

// lhsAnd is used as a label in lhsExpression rule. But some other rules also use it.
lhsAndDef : lhsUnary (DRL_AND lhsUnary)*
// lhsAnd is used as a label in lhsExpression rule. But some other rules explicitly use the def, so lhsAndDef is declared.
lhsAndDef : LPAREN lhsAndDef RPAREN
| lhsUnary (DRL_AND lhsUnary)*
| LPAREN DRL_AND lhsUnary+ RPAREN
;

Expand Down Expand Up @@ -380,15 +381,26 @@ fromEntryPoint : DRL_ENTRY_POINT stringId ;
| lhsPatternBind
)
*/
lhsExists : DRL_EXISTS lhsPatternBind ;
// Use lhsExpression instead of lhsOr because lhsExpression has good enough structure
lhsExists : DRL_EXISTS
( LPAREN lhsExpression RPAREN
| lhsPatternBind
)
;

/*
lhsNot := NOT
( (LEFT_PAREN (or_key|and_key))=> lhsOr // prevents '((' for prefixed and/or
| LEFT_PAREN lhsOr RIGHT_PAREN
| lhsPatternBind
)
*/
lhsNot : DRL_NOT lhsPatternBind ;
// Use lhsExpression instead of lhsOr because lhsExpression has good enough structure
lhsNot : DRL_NOT
( LPAREN lhsExpression RPAREN
| lhsPatternBind
)
;

/**
* lhsEval := EVAL LEFT_PAREN conditionalExpression RIGHT_PAREN
Expand Down
148 changes: 133 additions & 15 deletions drools-parser/src/main/java/org/drools/parser/DRLVisitorImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.drools.drl.ast.descr.TypeFieldDescr;
import org.drools.drl.ast.descr.UnitDescr;
import org.drools.drl.ast.descr.WindowDeclarationDescr;
import org.drools.util.StringUtils;

import static org.drools.parser.DRLParserHelper.getTextWithoutErrorNode;
import static org.drools.parser.ParserStringUtils.getTextPreservingWhitespace;
Expand Down Expand Up @@ -577,16 +576,38 @@ public String visitDrlIdentifier(DRLParser.DrlIdentifierContext ctx) {
@Override
public ExistsDescr visitLhsExists(DRLParser.LhsExistsContext ctx) {
ExistsDescr existsDescr = new ExistsDescr();
BaseDescr descr = visitLhsPatternBind(ctx.lhsPatternBind());
existsDescr.addDescr(descr);
if (ctx.lhsExpression() != null) {
// exists( A() or B() )
List<BaseDescr> baseDescrs = visitDescrChildren(ctx);
if (baseDescrs.size() == 1) {
existsDescr.addDescr(baseDescrs.get(0));
} else {
throw new IllegalStateException("'exists()' children descr size must be 1 : " + ctx.getText());
}
} else {
// exists A()
BaseDescr descr = visitLhsPatternBind(ctx.lhsPatternBind());
existsDescr.addDescr(descr);
}
return existsDescr;
}

@Override
public NotDescr visitLhsNot(DRLParser.LhsNotContext ctx) {
NotDescr notDescr = new NotDescr();
BaseDescr descr = visitLhsPatternBind(ctx.lhsPatternBind());
notDescr.addDescr(descr);
if (ctx.lhsExpression() != null) {
// not ( A() or B() )
List<BaseDescr> baseDescrs = visitDescrChildren(ctx);
if (baseDescrs.size() == 1) {
notDescr.addDescr(baseDescrs.get(0));
} else {
throw new IllegalStateException("'not()' children descr size must be 1 : " + ctx.getText());
}
} else {
// not A()
BaseDescr descr = visitLhsPatternBind(ctx.lhsPatternBind());
notDescr.addDescr(descr);
}
return notDescr;
}

Expand All @@ -602,26 +623,87 @@ public BaseDescr visitLhsExpressionEnclosed(DRLParser.LhsExpressionEnclosedConte

@Override
public BaseDescr visitLhsOr(DRLParser.LhsOrContext ctx) {
OrDescr orDescr = new OrDescr();
List<BaseDescr> descrList = visitDescrChildren(ctx);
descrList.forEach(orDescr::addDescr);
return orDescr;
// For flatten nested OrDescr logic, we call visitDescrChildrenForDescrNodePair instead of usual visitDescrChildren
List<DescrNodePair> descrList = visitDescrChildrenForDescrNodePair(ctx);
if (descrList.size() == 1) {
// Avoid nested OrDescr
return descrList.get(0).getDescr();
} else {
OrDescr orDescr = new OrDescr();
// For example, in case of A() or B() or C(),
// Parser creates AST like this:
// lhsOr
// / \
// A() lhsOr
// / \
// B() C()
// So, we need to flatten it so that OrDescr has A(), B() and C() as children.
List<BaseDescr> flattenedDescrs = flattenOrDescr(descrList);
flattenedDescrs.forEach(orDescr::addDescr);
return orDescr;
}
}

private List<BaseDescr> flattenOrDescr(List<DescrNodePair> descrList) {
List<BaseDescr> flattenedDescrs = new ArrayList<>();
for (DescrNodePair descrNodePair : descrList) {
BaseDescr descr = descrNodePair.getDescr();
ParseTree node = descrNodePair.getNode(); // parser node corresponding to the descr
if (descr instanceof OrDescr && !(node instanceof DRLParser.LhsExpressionEnclosedContext)) {
// sibling OrDescr should be flattened unless it's explicitly enclosed by parenthesis
flattenedDescrs.addAll(((OrDescr) descr).getDescrs());
} else {
flattenedDescrs.add(descr);
}
}
return flattenedDescrs;
}

@Override
public BaseDescr visitLhsAnd(DRLParser.LhsAndContext ctx) {
return createAndDescr(visitDescrChildren(ctx));
return createAndDescr(ctx);
}

private BaseDescr createAndDescr(ParserRuleContext ctx) {
// For flatten nested AndDescr logic, we call visitDescrChildrenForDescrNodePair instead of usual visitDescrChildren
List<DescrNodePair> descrList = visitDescrChildrenForDescrNodePair(ctx);
if (descrList.size() == 1) {
// Avoid nested AndDescr
return descrList.get(0).getDescr();
} else {
AndDescr andDescr = new AndDescr();
// For example, in case of A() and B() and C(),
// Parser creates AST like this:
// lhsAnd
// / \
// A() lhsAnd
// / \
// B() C()
// So, we need to flatten it so that AndDescr has A(), B() and C() as children.
List<BaseDescr> flattenedDescrs = flattenAndDescr(descrList);
flattenedDescrs.forEach(andDescr::addDescr);
return andDescr;
}
}

private AndDescr createAndDescr(List<BaseDescr> descrList) {
AndDescr andDescr = new AndDescr();
descrList.forEach(andDescr::addDescr);
return andDescr;
private List<BaseDescr> flattenAndDescr(List<DescrNodePair> descrList) {
List<BaseDescr> flattenedDescrs = new ArrayList<>();
for (DescrNodePair descrNodePair : descrList) {
BaseDescr descr = descrNodePair.getDescr();
ParseTree node = descrNodePair.getNode(); // parser node corresponding to the descr
if (descr instanceof AndDescr && !(node instanceof DRLParser.LhsExpressionEnclosedContext)) {
// sibling AndDescr should be flattened unless it's explicitly enclosed by parenthesis
flattenedDescrs.addAll(((AndDescr) descr).getDescrs());
} else {
flattenedDescrs.add(descr);
}
}
return flattenedDescrs;
}

@Override
public BaseDescr visitLhsAndDef(DRLParser.LhsAndDefContext ctx) {
return createAndDescr(visitDescrChildren(ctx));
return createAndDescr(ctx);
}

@Override
Expand Down Expand Up @@ -650,6 +732,42 @@ private List<BaseDescr> visitDescrChildren(RuleNode node) {
return aggregator;
}

// This method is used when the parent descr requires children parser node information for composing the descr.
// Ideally, we should use visitDescrChildren as possible and use the returned Descr object to compose the parent descr.
// However, for example, in flatten OrDescr/AndDescr logic,
// enhancing the returned Descr object of visitLhsExpressionEnclosed() (e.g. adding 'enclosed' flag to OrDescr/AndDescr) could be intrusive just for composing the parent descr.
private List<DescrNodePair> visitDescrChildrenForDescrNodePair(RuleNode node) {
List<DescrNodePair> aggregator = new ArrayList<>();
int n = node.getChildCount();

for (int i = 0; i < n && this.shouldVisitNextChild(node, aggregator); ++i) {
ParseTree c = node.getChild(i);
Object childResult = c.accept(this);
if (childResult instanceof BaseDescr) {
aggregator.add(new DescrNodePair((BaseDescr) childResult, c)); // pairing the returned Descr and the parser node
}
}
return aggregator;
}

private static class DescrNodePair {
private final BaseDescr descr; // returned Descr object
private final ParseTree node; // parser node corresponding to the descr. This is used for composing the parent descr.

private DescrNodePair(BaseDescr descr, ParseTree node) {
this.descr = descr;
this.node = node;
}

public BaseDescr getDescr() {
return descr;
}

public ParseTree getNode() {
return node;
}
}

// leaves of constraint concatenate return Strings
private String visitConstraintChildren(ParserRuleContext ctx) {
return getTokenTextPreservingWhitespace(ctx, tokenStream);
Expand Down
85 changes: 76 additions & 9 deletions drools-parser/src/test/java/org/drools/parser/MiscDRLParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1477,7 +1477,7 @@ void parenthesesOrAndAnd() {
}

@Test
void parenthesesAndOrOrOrAnd() throws Exception {
void multipleLevelNestAndOrOrOrAnd() throws Exception {
final String drl = "rule and_or_or_or_and\n" +
" when\n" +
" (Foo(x == 1) and (Bar(x == 2) or Foo(x == 3))) or (Bar(x == 4) or (Foo(x == 5) and Bar(x == 6)))\n" +
Expand Down Expand Up @@ -1516,6 +1516,79 @@ void parenthesesAndOrOrOrAnd() throws Exception {
assertThat(bar3.getObjectType()).isEqualTo("Bar");
}

@Test
void multipleLevelNestWithThreeOrSiblings() throws Exception {
final String drl = "rule nest_or_siblings\n" +
" when\n" +
" (A() or (B() or C() or (D() and E())))\n" +
" then\n" +
"end";
PackageDescr pkg = parser.parse(drl);

assertThat(pkg.getRules()).hasSize(1);
final RuleDescr rule = (RuleDescr) pkg.getRules().get(0);
final AndDescr rootAnd = (AndDescr) rule.getLhs();
assertThat(rootAnd.getDescrs()).hasSize(1);

final OrDescr topOr = (OrDescr) rootAnd.getDescrs().get(0);
assertThat(topOr.getDescrs()).hasSize(2);

final PatternDescr leftPattern = (PatternDescr) topOr.getDescrs().get(0);
assertThat(leftPattern.getObjectType()).isEqualTo("A");

final OrDescr rightOr = (OrDescr) topOr.getDescrs().get(1);
assertThat(rightOr.getDescrs()).as("top level Or has 3 sibling children").hasSize(3);
final PatternDescr bPattern = (PatternDescr) rightOr.getDescrs().get(0);
assertThat(bPattern.getObjectType()).isEqualTo("B");
final PatternDescr cPattern = (PatternDescr) rightOr.getDescrs().get(1);
assertThat(cPattern.getObjectType()).isEqualTo("C");
final AndDescr deAnd = (AndDescr) rightOr.getDescrs().get(2);
assertThat(deAnd.getDescrs()).hasSize(2);

final PatternDescr dPattern = (PatternDescr) deAnd.getDescrs().get(0);
assertThat(dPattern.getObjectType()).isEqualTo("D");
final PatternDescr ePattern = (PatternDescr) deAnd.getDescrs().get(1);
assertThat(ePattern.getObjectType()).isEqualTo("E");
}

@Test
public void existsMultipleLevelNestWithThreeOrSiblings() throws Exception {
final String drl = "rule nest_or_siblings\n" +
" when\n" +
" exists(A() or (B() or C() or (D() and E())))\n" +
" then\n" +
"end";
PackageDescr pkg = parser.parse(drl);

assertThat(pkg.getRules()).hasSize(1);
final RuleDescr rule = (RuleDescr) pkg.getRules().get(0);
final AndDescr rootAnd = (AndDescr) rule.getLhs();
assertThat(rootAnd.getDescrs()).hasSize(1);

final ExistsDescr topExists = (ExistsDescr) rootAnd.getDescrs().get(0);
assertThat(topExists.getDescrs()).hasSize(1);

final OrDescr topOr = (OrDescr) topExists.getDescrs().get(0);
assertThat(topOr.getDescrs()).hasSize(2);

final PatternDescr leftPattern = (PatternDescr) topOr.getDescrs().get(0);
assertThat(leftPattern.getObjectType()).isEqualTo("A");

final OrDescr rightOr = (OrDescr) topOr.getDescrs().get(1);
assertThat(rightOr.getDescrs()).hasSize(3);
final PatternDescr bPattern = (PatternDescr) rightOr.getDescrs().get(0);
assertThat(bPattern.getObjectType()).isEqualTo("B");
final PatternDescr cPattern = (PatternDescr) rightOr.getDescrs().get(1);
assertThat(cPattern.getObjectType()).isEqualTo("C");
final AndDescr deAnd = (AndDescr) rightOr.getDescrs().get(2);
assertThat(deAnd.getDescrs()).hasSize(2);

final PatternDescr dPattern = (PatternDescr) deAnd.getDescrs().get(0);
assertThat(dPattern.getObjectType()).isEqualTo("D");
final PatternDescr ePattern = (PatternDescr) deAnd.getDescrs().get(1);
assertThat(ePattern.getObjectType()).isEqualTo("E");
}

@Test
public void parse_EvalMultiple() throws Exception {
final PackageDescr pkg = parseAndGetPackageDescrFromFile(
Expand Down Expand Up @@ -1686,7 +1759,6 @@ public void parse_Attributes() throws Exception {
assertThat(at.getValue()).isEqualTo("true");
}

@Disabled("Priority : High | Parse attribute with parentheses")
@Test
public void parse_Attributes2() throws Exception {
final PackageDescr pkg = parseAndGetPackageDescrFromFile(
Expand Down Expand Up @@ -2084,7 +2156,6 @@ public void parse_EscapedStrings() throws Exception {
assertThat((String) rule.getConsequence()).isEqualToIgnoringWhitespace( expected);
}

@Disabled("Priority : High | parse nested parentheses")
@Test
public void parse_NestedCEs() throws Exception {
final RuleDescr rule = parseAndGetFirstRuleDescrFromFile(
Expand Down Expand Up @@ -2835,11 +2906,9 @@ public void parse_TypeDeclarationWithFields() throws Exception {

}

@Disabled("Priority : High | Failed to parse or with parentheses in LHS")
@Test
public void parse_RuleWithLHSNesting() throws Exception {
final PackageDescr pkg = parseAndGetPackageDescrFromFile(
"Rule_with_nested_LHS.drl" );
public void parenthesesOneLevelNestWithThreeSiblings() throws Exception {
final PackageDescr pkg = parseAndGetPackageDescrFromFile( "Rule_with_nested_LHS.drl" );

assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse();

Expand Down Expand Up @@ -2941,7 +3010,6 @@ public void parse_SlidingWindow() throws Exception {
assertThat(descr.getParameters().get(0)).isEqualTo("10");
}

@Disabled("Priority : Mid | outmost parentheses")
@Test
public void parse_RuleOldSyntax1() throws Exception {
final String source = "rule \"Test\" when ( not $r :LiteralRestriction( operator == Operator.EQUAL ) ) then end";
Expand All @@ -2963,7 +3031,6 @@ public void parse_RuleOldSyntax1() throws Exception {
assertThat(fieldConstraintDescr.getExpression()).isEqualToIgnoringWhitespace("operator == Operator.EQUAL");
}

@Disabled("Priority : Mid | outmost parentheses")
@Test
public void parse_RuleOldSyntax2() throws Exception {
final String source = "rule \"Test\" when ( $r :LiteralRestriction( operator == Operator.EQUAL ) ) then end";
Expand Down

0 comments on commit 42b1183

Please sign in to comment.