Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DROOLS-7310] parse nested parentheses #42

Merged
merged 2 commits into from
Oct 27, 2023
Merged

[DROOLS-7310] parse nested parentheses #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4473ecd
[DROOLS-7310] parse nested parentheses
tkobayas Oct 20, 2023
5eace20
- Adding code commentes and explanations
tkobayas Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions drools-parser/src/main/antlr4/org/drools/parser/DRLParser.g4
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading