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.

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

[DO-NOT-MERGE][WIP][incubator-kie-drools-6220] Slim down DRL syntax with New Antlr4 Parser #6225

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/pr-downstream.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ on:

jobs:
kogito-downstream-build:
if: false
concurrency:
group: pr-${{ matrix.job_name }}_${{ matrix.os }}_${{ matrix.java-version }}_${{ matrix.maven-version }}_${{ github.head_ref }}
cancel-in-progress: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr-drools.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ jobs:
- name: Build Chain
uses: apache/incubator-kie-kogito-pipelines/.ci/actions/build-chain@main
env:
BUILD_MVN_OPTS_CURRENT: '-Dfull -Dreproducible'
BUILD_MVN_OPTS_CURRENT: '-Dfull -Dreproducible -DenableNewParser'
MAVEN_OPTS: "-Dfile.encoding=UTF-8"
with:
definition-file: https://raw.githubusercontent.com/${GROUP:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/pull-request-config.yaml
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ nbproject
*.ipr
*.iws
*.iml
build.log

# generated files
dependency-reduced-pom.xml
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
public class OperatorDescr extends BaseDescr {
private static final long serialVersionUID = 520l;

// prefix for custom operators which are not built-in
public static final String CUSTOM_OPERATOR_PREFIX = "##";

private String operator;
private boolean negated;
private List<String> parameters;
Expand Down Expand Up @@ -55,7 +58,15 @@ public String getOperator() {
}

public void setOperator( String operator ) {
this.operator = operator != null ? operator.trim() : null;
this.operator = operator != null ? trimOperator(operator) : null;
}

private String trimOperator(String operator) {
operator = operator.trim();
if (operator.startsWith(CUSTOM_OPERATOR_PREFIX)) {
operator = operator.substring(CUSTOM_OPERATOR_PREFIX.length());
}
return operator;
}

public boolean isNegated() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.drools.drl.parser.antlr4.DRLParserHelper.computeTokenIndex;
import static org.drools.drl.parser.antlr4.DRLParserHelper.createDrlParser;
import static org.drools.drl.parser.antlr4.DRLParserHelper.parse;
import static org.drools.drl.parser.antlr4.DRL10ParserHelper.computeTokenIndex;
import static org.drools.drl.parser.antlr4.DRL10ParserHelper.createDrlParser;
import static org.drools.drl.parser.antlr4.DRL10ParserHelper.parse;

/**
* This test class is specific to new antlr4 parser.
*/
class DRLParserTest {
class DRL10ParserTest {

private static final String drl =
"package org.test;\n" +
Expand Down Expand Up @@ -94,7 +94,7 @@ void parse_basicRule() {

@Test
void computeTokenIndex_basicRule() {
DRLParser parser = createDrlParser(drl);
DRL10Parser parser = createDrlParser(drl);
parser.compilationUnit();

assertThat((int) computeTokenIndex(parser, 1, 0)).isEqualTo(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.drools.drl.ast.descr.ConstraintConnectiveDescr;
import org.drools.drl.ast.descr.RelationalExprDescr;
import org.drools.drl.parser.DrlExprParser;
import org.drools.drl.parser.DrlExprParserFactory;
import org.drools.drl.parser.DrlParser;
import org.drools.drl.parser.DroolsParserException;
import org.drools.drl.parser.impl.Operator;
Expand All @@ -38,7 +37,6 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.kie.internal.builder.conf.LanguageLevelOption;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
Expand All @@ -52,7 +50,7 @@ class DRLExprParserTest {

@BeforeEach
void setUp() {
this.parser = DrlExprParserFactory.getDrlExprParser(LanguageLevelOption.DRL6);
this.parser = ParserTestUtils.getExprParser();
}

@AfterEach
Expand Down Expand Up @@ -175,7 +173,7 @@ void bindingConstraint() {

@Test
void bindingWithRestrictions() {
String source = "$x : property > value && < 20";
String source = "$x : property > value && property < 20";
ConstraintConnectiveDescr result = parser.parse( source );
assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse();

Expand Down Expand Up @@ -458,7 +456,7 @@ void noViableAlt() {
assertThat(exception.getColumn()).isEqualTo(2);
assertThat(exception.getOffset()).isEqualTo(2);
assertThat(exception.getMessage())
.isEqualToIgnoringCase("[ERR 101] Line 1:2 no viable alternative at input 'a'");
.isEqualToIgnoringCase("[ERR 101] Line 1:2 no viable alternative at input '~a'");
} else {
assertThat(parser.hasErrors()).isFalse();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@
import org.drools.drl.ast.descr.BindingDescr;
import org.drools.drl.ast.descr.ConstraintConnectiveDescr;
import org.drools.drl.parser.DrlExprParser;
import org.drools.drl.parser.DrlExprParserFactory;
import org.drools.mvel.evaluators.MatchesEvaluatorsDefinition;
import org.drools.mvel.evaluators.SetEvaluatorsDefinition;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.kie.internal.builder.conf.LanguageLevelOption;
import org.junit.jupiter.api.condition.DisabledIfSystemProperty;

import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -47,6 +46,7 @@ void setUp() {
dumper = new DescrDumper();
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void dump() {
String input = "price > 10 && < 20 || == $val || == 30";
Expand Down Expand Up @@ -147,6 +147,7 @@ void dumpWithDateAttr() {
assertThat(result).isEqualTo(expected);
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void dumpComplex() {
String input = "a ( > 60 && < 70 ) || ( > 50 && < 55 ) && a3 == \"black\" || a == 40 && a3 == \"pink\" || a == 12 && a3 == \"yellow\" || a3 == \"blue\"";
Expand Down Expand Up @@ -220,6 +221,7 @@ void dumpBindings4() {
assertThat(result).isEqualTo(expected);
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void dumpBindingsWithRestriction() {
String input = "$x : age > 10 && < 20 || > 30";
Expand Down Expand Up @@ -360,7 +362,7 @@ void processImplicitConstraints() {
}

public ConstraintConnectiveDescr parse(final String constraint) {
DrlExprParser parser = DrlExprParserFactory.getDrlExprParser(LanguageLevelOption.DRL6);
DrlExprParser parser = ParserTestUtils.getExprParser();
ConstraintConnectiveDescr result = parser.parse(constraint);
assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@
import org.drools.drl.parser.DroolsParserException;
import org.drools.drl.parser.impl.Operator;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand Down Expand Up @@ -873,6 +874,7 @@ void simpleRuleWithBindings() {
assertThat((String) rule.getConsequence()).isEqualToIgnoringWhitespace("if ( a == b ) { " + " assert( foo3 );" + "} else {" + " retract( foo4 );" + "}" + " System.out.println( a4 );");
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void multipleRestrictionsConstraint() {
RuleDescr rule = parseAndGetFirstRuleDescrFromFile("restrictions_test.drl");
Expand Down Expand Up @@ -2444,7 +2446,7 @@ void inOperator() {
assertThat(pattern.getConstraint().getDescrs()).hasSize(1);

ExprConstraintDescr fld = (ExprConstraintDescr) pattern.getConstraint().getDescrs().get(0);
assertThat(fld.getExpression()).isEqualTo("age > 30 && < 40");
assertThat(fld.getExpression()).isEqualTo("age > 30 && age < 40");

// the second col, with 2 fields, the first with 2 restrictions, the
// second field with one
Expand Down Expand Up @@ -2478,7 +2480,7 @@ void notInOperator() {
assertThat(pattern.getConstraint().getDescrs()).hasSize(1);

ExprConstraintDescr fld = (ExprConstraintDescr) pattern.getConstraint().getDescrs().get(0);
assertThat(fld.getExpression()).isEqualTo("age > 30 && < 40");
assertThat(fld.getExpression()).isEqualTo("age > 30 && age < 40");

// the second col, with 2 fields, the first with 2 restrictions, the
// second field with one
Expand Down Expand Up @@ -2528,6 +2530,7 @@ void constraintOrConnective() {
assertThat(fcd.getExpression()).isEqualToIgnoringWhitespace("age < 42 || location==\"atlanta\"");
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void restrictions() {
final String text = "rule X when Foo( bar > 1 || == 1 ) then end\n";
Expand Down Expand Up @@ -2979,7 +2982,7 @@ void pluggableOperators() {
assertThat(eventE.getConstraint().getDescrs()).hasSize(1);

ExprConstraintDescr fcdE = (ExprConstraintDescr) eventE.getConstraint().getDescrs().get(0);
assertThat(fcdE.getExpression()).isEqualTo("this not before[1, 10] $b || after[1, 10] $c && this after[1, 5] $d");
assertThat(fcdE.getExpression()).isEqualTo("this not before[1, 10] $b || this after[1, 10] $c && this after[1, 5] $d");
}

@Test
Expand Down Expand Up @@ -3888,6 +3891,7 @@ void constraintOperators(String constraint) {
assertThat(exprConstraintDescr.getExpression()).isEqualToIgnoringWhitespace(constraint);
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@ParameterizedTest
@ValueSource(strings = {
"country matches \"[a-z]*\" || matches \"[A-Z]*\"",
Expand Down Expand Up @@ -4053,6 +4057,7 @@ void lhsPatternAnnotation() {
assertThat(annotationDescr.getSingleValueAsString()).isEqualTo("!*, age");
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void prefixAndDescrAnnotation() {
final String text =
Expand All @@ -4074,6 +4079,7 @@ void prefixAndDescrAnnotation() {
assertThat(andDescr.getDescrs()).hasSize(2);
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void prefixOrDescrAnnotation() {
final String text =
Expand All @@ -4096,6 +4102,7 @@ void prefixOrDescrAnnotation() {
assertThat(orDescr.getDescrs()).hasSize(2);
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void infixAndDescrAnnotation() {
final String text =
Expand All @@ -4116,6 +4123,7 @@ void infixAndDescrAnnotation() {
assertThat(andDescr.getDescrs()).hasSize(3);
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void infixOrDescrAnnotation() {
final String text =
Expand Down Expand Up @@ -4411,8 +4419,9 @@ void traitExtendsMultiple() {
.containsExactlyInAnyOrder("com.sample.ParentTrait", "UncleTrait", "org.test.GrandParentTrait");
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void pluggableEvaluator() {
void pluggableEvaluatorOldParser() {
final String source = "package org.drools\n" +
"rule R\n" +
"when\n" +
Expand All @@ -4431,6 +4440,27 @@ void pluggableEvaluator() {
.containsExactly("$c : core", "this not isA t.x.E.class", "this isA t.x.D.class");
}

@EnabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void pluggableEvaluatorNewParser() {
final String source = "package org.drools\n" +
"rule R\n" +
"when\n" +
" $t : Thing( $c : core, this not ##isA t.x.E.class, this ##isA t.x.D.class )\n" +
"then\n" +
" list.add( \"E\" ); \n" +
" don( $t, E.class ); \n" +
"end\n";

Operator.addOperatorToRegistry("isA", false);
Operator.addOperatorToRegistry("isA", true);

PatternDescr pattern = (PatternDescr) parseAndGetFirstRuleDescr(source).getLhs().getDescrs().get(0);
assertThat(pattern.getConstraint().getDescrs())
.extracting(Object::toString)
.containsExactly("$c : core", "this not ##isA t.x.E.class", "this ##isA t.x.D.class");
}

@Test
void namedConsequenceDo() {
final String text =
Expand Down Expand Up @@ -5114,6 +5144,7 @@ void accumulateEmptyChunks() {
assertThat(accumulateDescr.getResultCode()).isEqualTo("null");
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void doublePipeInfixOr() {
final String text =
Expand All @@ -5135,6 +5166,7 @@ void doublePipeInfixOr() {
});
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void doubleAmpersandInfixAnd() {
final String text =
Expand All @@ -5154,6 +5186,7 @@ void doubleAmpersandInfixAnd() {
});
}

@DisabledIfSystemProperty(named = "drools.drl.antlr4.parser.enabled", matches = "true")
@Test
void doubleAmpersandInfixAndInAccumulate() {
final String text =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@
import java.util.Arrays;
import java.util.List;

import org.drools.drl.parser.DrlExprParser;
import org.drools.drl.parser.DrlExprParserFactory;
import org.drools.drl.parser.DrlParser;
import org.kie.internal.builder.conf.LanguageLevelOption;

public class ParserTestUtils {

Expand All @@ -45,14 +48,22 @@ private ParserTestUtils() {
* Returns a DrlParser which encapsulates an old or new parser depending on system property
*/
public static DrlParser getParser() {
return new DrlParser();
if (DrlParser.ANTLR4_PARSER_ENABLED) {
return new DrlParser(LanguageLevelOption.DRL10);
} else {
return new DrlParser(LanguageLevelOption.DRL6);
}
}

/**
* Enables the old parser. Just for quick testing purposes.
* Returns a DrlExprParser which encapsulates an old or new parser depending on system property
*/
public static void enableOldParser() {
DrlParser.ANTLR4_PARSER_ENABLED = false;
public static DrlExprParser getExprParser() {
if (DrlParser.ANTLR4_PARSER_ENABLED) {
return DrlExprParserFactory.getDrlExprParser(LanguageLevelOption.DRL10);
} else {
return DrlExprParserFactory.getDrlExprParser(LanguageLevelOption.DRL6);
}
}

public static List<String> javaKeywords() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
rule "AccumulateParserTest"
when
// below statement makes no sense, but is useful to test parsing recursiveness
$personList : ArrayList() from accumulate( Person( $age : age > 21 || < 10 ) from collect( People() from $town.getPeople() ),
$personList : ArrayList() from accumulate( Person( $age : age > 21 || age < 10 ) from collect( People() from $town.getPeople() ),
max( $age ) );
then
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

rule simple_rule
when
Person(age > 30 && < 40)
Person(age > 30 && age < 40)
Vehicle(type in ( "sedan", "wagon" ), age < 3)
then
consequence();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

rule simple_rule
when
Person(age > 30 && < 40)
Person(age > 30 && age < 40)
Vehicle(type not in ( "sedan", "wagon" ), age < 3)
then
consequence();
Expand Down
Loading
Loading