From d637f88a0fec8bd6fad1dfe57aa21a46202f2718 Mon Sep 17 00:00:00 2001 From: Toshiya Kobayashi Date: Thu, 12 Jan 2023 17:41:33 +0900 Subject: [PATCH] [DROOLS-7270] Enhance test/grammar coverage : andRestriction, orRestriction (#4) - Removed some unused cast --- .../antlr4/org/drools/parser/DRLParser.g4 | 15 ++- .../org/drools/parser/DRLVisitorImpl.java | 74 +++++++------- .../org/drools/parser/MiscDRLParserTest.java | 96 +++++++++++++------ .../org/drools/parser/restrictions_test.drl | 24 +++++ 4 files changed, 137 insertions(+), 72 deletions(-) create mode 100644 drools-drl/drools-drl10-parser/src/test/resources/org/drools/parser/restrictions_test.drl diff --git a/drools-drl/drools-drl10-parser/src/main/antlr4/org/drools/parser/DRLParser.g4 b/drools-drl/drools-drl10-parser/src/main/antlr4/org/drools/parser/DRLParser.g4 index 5c81fff3662..a15d73dc7d6 100644 --- a/drools-drl/drools-drl10-parser/src/main/antlr4/org/drools/parser/DRLParser.g4 +++ b/drools-drl/drools-drl10-parser/src/main/antlr4/org/drools/parser/DRLParser.g4 @@ -77,8 +77,19 @@ andExpression : left=equalityExpression (BITAND right=equalityExpression)* ; equalityExpression : left=instanceOfExpression ( ( op=EQUAL | op=NOTEQUAL ) right=instanceOfExpression )* ; instanceOfExpression : left=inExpression ( 'instanceof' right=type )? ; inExpression : left=relationalExpression ( 'not'? 'in' LPAREN drlExpression (COMMA drlExpression)* RPAREN )? ; -relationalExpression : drlExpression? ; - +relationalExpression : left=drlExpression (right=orRestriction)* ; +orRestriction : left=andRestriction (OR right=andRestriction)* ; +andRestriction : left=singleRestriction (AND right=singleRestriction)* ; +singleRestriction : op=relationalOperator drlExpression ; + +relationalOperator + : EQUAL + | NOTEQUAL + | LE + | GE + | GT + | LT + ; /* function := FUNCTION type? ID parameters(typed) chunk_{_} */ functiondef : DRL_FUNCTION typeTypeOrVoid? IDENTIFIER formalParameters block ; diff --git a/drools-drl/drools-drl10-parser/src/main/java/org/drools/parser/DRLVisitorImpl.java b/drools-drl/drools-drl10-parser/src/main/java/org/drools/parser/DRLVisitorImpl.java index 236478d7fb8..12fcc510a08 100644 --- a/drools-drl/drools-drl10-parser/src/main/java/org/drools/parser/DRLVisitorImpl.java +++ b/drools-drl/drools-drl10-parser/src/main/java/org/drools/parser/DRLVisitorImpl.java @@ -244,6 +244,9 @@ public PatternDescr visitLhsPattern(DRLParser.LhsPatternContext ctx) { @Override public List visitConstraints(DRLParser.ConstraintsContext ctx) { + if (ctx == null) { + return new ArrayList<>(); + } List descrList = visitDescrChildren(ctx); return descrList.stream() .filter(ExprConstraintDescr.class::isInstance) @@ -253,57 +256,20 @@ public List visitConstraints(DRLParser.ConstraintsContext c @Override public ExprConstraintDescr visitConstraint(DRLParser.ConstraintContext ctx) { - Object constraint = super.visitConstraint(ctx); - if (constraint != null) { - String constraintString = constraint.toString(); - DRLParser.LabelContext label = ctx.label(); - if (label != null) { - constraintString = label.getText() + constraintString; - } - ExprConstraintDescr constraintDescr = new ExprConstraintDescr(constraintString); + String constraint = visitConstraintChildren(ctx); + if (!constraint.isEmpty()) { + ExprConstraintDescr constraintDescr = new ExprConstraintDescr(constraint); constraintDescr.setType(ExprConstraintDescr.Type.NAMED); return constraintDescr; } return null; } - @Override - public String visitDrlExpression(DRLParser.DrlExpressionContext ctx) { - return ctx.children.stream() - .map(c -> c instanceof TerminalNode ? c : c.accept(this)) - .filter(Objects::nonNull) - .map(Object::toString) - .collect(Collectors.joining(" ")); - } - - @Override - public String visitDrlPrimary(DRLParser.DrlPrimaryContext ctx) { - return ctx.children.stream() - .map(c -> c instanceof TerminalNode ? c : c.accept(this)) - .filter(Objects::nonNull) - .map(Object::toString) - .collect(Collectors.joining(" ")); - } - @Override public String visitDrlIdentifier(DRLParser.DrlIdentifierContext ctx) { return ctx.getText(); } - @Override - public String visitDrlLiteral(DRLParser.DrlLiteralContext ctx) { - ParseTree node = ctx; - while (true) { - if (node instanceof TerminalNode) { - return node.toString(); - } - if (node.getChildCount() != 1) { - return super.visitDrlLiteral(ctx).toString(); - } - node = node.getChild(0); - } - } - @Override public ExistsDescr visitLhsExists(DRLParser.LhsExistsContext ctx) { ExistsDescr existsDescr = new ExistsDescr(); @@ -372,6 +338,34 @@ private List visitDescrChildren(RuleNode node) { return aggregator; } + // leaves of constraint concatenate return Strings + private String visitConstraintChildren(RuleNode node) { + return ((ParserRuleContext) node).children.stream() + .map(c -> c instanceof TerminalNode ? c : c.accept(this)) + .filter(Objects::nonNull) + .map(Object::toString) + .collect(Collectors.joining(" ")); + } + + @Override + public Object visitChildren(RuleNode node) { + if (hasConstraintAsAncestor(node)) { + return visitConstraintChildren(node); + } + return super.visitChildren(node); + } + + private boolean hasConstraintAsAncestor(RuleNode node) { + ParseTree parent = node.getParent(); + if (parent instanceof DRLParser.ConstraintContext) { + return true; + } else if (parent == null) { + return false; + } else { + return hasConstraintAsAncestor((RuleNode) parent); + } + } + private Optional visitFirstDescrChild(RuleNode node) { int n = node.getChildCount(); diff --git a/drools-drl/drools-drl10-parser/src/test/java/org/drools/parser/MiscDRLParserTest.java b/drools-drl/drools-drl10-parser/src/test/java/org/drools/parser/MiscDRLParserTest.java index 82404dee71a..da89c442562 100644 --- a/drools-drl/drools-drl10-parser/src/test/java/org/drools/parser/MiscDRLParserTest.java +++ b/drools-drl/drools-drl10-parser/src/test/java/org/drools/parser/MiscDRLParserTest.java @@ -13,7 +13,6 @@ import org.drools.drl.ast.descr.ExprConstraintDescr; import org.drools.drl.ast.descr.FromDescr; import org.drools.drl.ast.descr.FunctionDescr; -import org.drools.drl.ast.descr.FunctionImportDescr; import org.drools.drl.ast.descr.GlobalDescr; import org.drools.drl.ast.descr.ImportDescr; import org.drools.drl.ast.descr.MVELExprDescr; @@ -24,6 +23,7 @@ import org.drools.drl.ast.descr.RuleDescr; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -49,7 +49,8 @@ private String readResource(final String filename) throws Exception { final StringBuilder sb = new StringBuilder(); try (BufferedReader br = Files.newBufferedReader(path, StandardCharsets.UTF_8)) { for (String line; (line = br.readLine()) != null; ) { - sb.append(line + "\n"); + sb.append(line); + sb.append("\n"); } } catch (IOException e) { e.printStackTrace(); @@ -183,16 +184,16 @@ void parse_globalWithOrWithoutSemi() throws Exception { assertThat(pkg.getRules()).hasSize(1); - final RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + final RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getLhs().getDescrs()).hasSize(1); assertThat(pkg.getImports()).hasSize(1); assertThat(pkg.getGlobals()).hasSize(2); - final GlobalDescr foo = (GlobalDescr) pkg.getGlobals().get(0); + final GlobalDescr foo = pkg.getGlobals().get(0); assertThat(foo.getType()).isEqualTo("java.lang.String"); assertThat(foo.getIdentifier()).isEqualTo("foo"); - final GlobalDescr bar = (GlobalDescr) pkg.getGlobals().get(1); + final GlobalDescr bar = pkg.getGlobals().get(1); assertThat(bar.getType()).isEqualTo("java.lang.Integer"); assertThat(bar.getIdentifier()).isEqualTo("bar"); } @@ -204,12 +205,12 @@ void parse_functionImportWithNotExist() throws Exception { assertThat(pkg.getFunctionImports()).hasSize(2); - assertThat(((FunctionImportDescr) pkg.getFunctionImports().get(0)).getTarget()).isEqualTo("abd.def.x"); - assertThat(((FunctionImportDescr) pkg.getFunctionImports().get(0)).getStartCharacter()).isNotSameAs(-1); - assertThat(((FunctionImportDescr) pkg.getFunctionImports().get(0)).getEndCharacter()).isNotSameAs(-1); - assertThat(((FunctionImportDescr) pkg.getFunctionImports().get(1)).getTarget()).isEqualTo("qed.wah.*"); - assertThat(((FunctionImportDescr) pkg.getFunctionImports().get(1)).getStartCharacter()).isNotSameAs(-1); - assertThat(((FunctionImportDescr) pkg.getFunctionImports().get(1)).getEndCharacter()).isNotSameAs(-1); + assertThat(pkg.getFunctionImports().get(0).getTarget()).isEqualTo("abd.def.x"); + assertThat(pkg.getFunctionImports().get(0).getStartCharacter()).isNotSameAs(-1); + assertThat(pkg.getFunctionImports().get(0).getEndCharacter()).isNotSameAs(-1); + assertThat(pkg.getFunctionImports().get(1).getTarget()).isEqualTo("qed.wah.*"); + assertThat(pkg.getFunctionImports().get(1).getStartCharacter()).isNotSameAs(-1); + assertThat(pkg.getFunctionImports().get(1).getEndCharacter()).isNotSameAs(-1); } @Test @@ -227,7 +228,7 @@ void parse_fromComplexAccessor() { assertThat(parser.hasErrors()).as(parser.getErrorMessages().toString()).isFalse(); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getName()).isEqualTo("Invalid customer id"); assertThat(rule.getLhs().getDescrs()).hasSize(2); @@ -252,7 +253,7 @@ void parse_fromWithInlineList() { PackageDescr pkg = parser.parse(source); assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getName()).isEqualTo("XYZ"); PatternDescr number = (PatternDescr) ((NotDescr) rule.getLhs().getDescrs().get(1)).getDescrs().get(0); @@ -269,10 +270,11 @@ void parse_fromWithInlineListMethod() { " System.err.println(\"Invalid customer id found!\"); \n" + " o.addError(\"Invalid customer id\"); \n" + "end \n"; + System.out.println(source); PackageDescr pkg = parser.parse(source); assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getName()).isEqualTo("XYZ"); assertThat(parser.hasErrors()).isFalse(); @@ -295,7 +297,7 @@ void parse_fromWithInlineListIndex() { assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getName()).isEqualTo("XYZ"); assertThat(parser.hasErrors()).isFalse(); @@ -348,7 +350,7 @@ void parse_compatibleRestriction() { PackageDescr pkg = parser.parse(source); assertThat(pkg.getName()).isEqualTo("com.sample"); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getName()).isEqualTo("test"); ExprConstraintDescr expr = (ExprConstraintDescr) ((PatternDescr) rule.getLhs().getDescrs().get(0)).getDescrs().get(0); assertThat(expr.getText()).isEqualTo("( text == null || text2 matches \"\" )"); @@ -360,7 +362,7 @@ void parse_simpleConstraint() { PackageDescr pkg = parser.parse(source); assertThat(pkg.getName()).isEqualTo("com.sample"); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getName()).isEqualTo("test"); assertThat(rule.getLhs().getDescrs()).hasSize(1); @@ -377,7 +379,7 @@ void parse_stringEscapes() { String source = "package com.sample rule test when Cheese( type matches \"\\..*\\\\.\" ) then end"; PackageDescr pkg = parser.parse(source); assertThat(pkg.getName()).isEqualTo("com.sample"); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(rule.getName()).isEqualTo("test"); assertThat(rule.getLhs().getDescrs()).hasSize(1); @@ -392,7 +394,7 @@ void parse_stringEscapes() { void parse_dialectWithSingleQuotation() { final String source = "dialect 'mvel'"; PackageDescr pkg = parser.parse(source); - AttributeDescr attr = (AttributeDescr) pkg.getAttributes().get(0); + AttributeDescr attr = pkg.getAttributes().get(0); assertThat(attr.getName()).isEqualTo("dialect"); assertThat(attr.getValue()).isEqualTo("mvel"); } @@ -409,7 +411,7 @@ void parse_dialectWithDoubleQuotation() { @Test void parse_emptyRuleWithoutWhen() throws Exception { String source = readResource("empty_rule.drl"); // without WHEN - PackageDescr pkg = parser.parse(source); + parser.parse(source); assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isTrue(); @@ -435,7 +437,7 @@ void parse_ternaryExpression() throws Exception { String source = readResource("ternary_expression.drl"); PackageDescr pkg = parser.parse(source); - final RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + final RuleDescr rule = pkg.getRules().get(0); assertThat(pkg.getRules()).hasSize(1); assertThat((String) rule.getConsequence()).isEqualToIgnoringWhitespace("if (speed > speedLimit ? true : false;) pullEmOver();"); @@ -454,11 +456,11 @@ void parse_functionWithArrays() throws Exception { assertThat(pkg.getName()).isEqualTo("foo"); assertThat(pkg.getRules()).hasSize(1); - final RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + final RuleDescr rule = pkg.getRules().get(0); assertThat((String) rule.getConsequence()).isEqualToIgnoringWhitespace("yourFunction(new String[] {\"a\",\"b\",\"c\"});"); - final FunctionDescr func = (FunctionDescr) pkg.getFunctions().get(0); + final FunctionDescr func = pkg.getFunctions().get(0); assertThat(func.getReturnType()).isEqualTo("String[]"); assertThat(func.getParameterNames().get(0)).isEqualTo("args[]"); @@ -506,7 +508,7 @@ void parse_noLoop() throws Exception { assertThat(rule).isNotNull(); assertThat(rule.getName()).isEqualTo("rule1"); - final AttributeDescr att = (AttributeDescr) rule.getAttributes().get("no-loop"); + final AttributeDescr att = rule.getAttributes().get("no-loop"); assertThat(att.getValue()).isEqualTo("false"); assertThat(att.getName()).isEqualTo("no-loop"); } @@ -522,7 +524,7 @@ void parse_autofocus() throws Exception { assertThat(rule).isNotNull(); assertThat(rule.getName()).isEqualTo("rule1"); - final AttributeDescr att = (AttributeDescr) rule.getAttributes().get("auto-focus"); + final AttributeDescr att = rule.getAttributes().get("auto-focus"); assertThat(att.getValue()).isEqualTo("true"); assertThat(att.getName()).isEqualTo("auto-focus"); } @@ -538,7 +540,7 @@ void parse_ruleFlowGroup() throws Exception { assertThat(rule).isNotNull(); assertThat(rule.getName()).isEqualTo("rule1"); - final AttributeDescr att = (AttributeDescr) rule.getAttributes().get("ruleflow-group"); + final AttributeDescr att = rule.getAttributes().get("ruleflow-group"); assertThat(att.getValue()).isEqualTo("a group"); assertThat(att.getName()).isEqualTo("ruleflow-group"); } @@ -578,7 +580,7 @@ void parse_consequenceWithDeclaration() throws Exception { void parse_or() { final String text = "rule X when Person(age < 42, location==\"atlanta\") \nor\nPerson(name==\"bob\") then end"; PackageDescr pkg = parser.parse(text); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); @@ -593,7 +595,7 @@ void parse_or() { void parse_lhsWithStringQuotes() { final String text = "rule X when Person( location==\"atlanta\\\"\") then end\n"; PackageDescr pkg = parser.parse(text); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); @@ -609,7 +611,7 @@ void parse_lhsWithStringQuotes() { void parse_lhsWithStringQuotesEscapeChars() { final String text = "rule X when Cheese( $x: type, type == \"s\\tti\\\"lto\\nn\" ) then end\n"; PackageDescr pkg = parser.parse(text); - RuleDescr rule = (RuleDescr) pkg.getRules().get(0); + RuleDescr rule = pkg.getRules().get(0); assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); assertThat(rule).isNotNull(); @@ -665,7 +667,7 @@ void parse_emptyPattern() throws Exception { assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse(); assertThat(pkg.getRules()).hasSize(1); - final RuleDescr ruleDescr = (RuleDescr) pkg.getRules().get(0); + final RuleDescr ruleDescr = pkg.getRules().get(0); assertThat(ruleDescr.getName()).isEqualTo("simple rule"); assertThat(ruleDescr.getLhs()).isNotNull(); assertThat(ruleDescr.getLhs().getDescrs()).hasSize(1); @@ -783,4 +785,38 @@ void parse_simpleRuleWithBindings() throws Exception { assertThat((String) rule.getConsequence()).isEqualToIgnoringWhitespace("if ( a == b ) { " + " assert( foo3 );" + "} else {" + " retract( foo4 );" + "}" + " System.out.println( a4 );"); } + + @Test + void parse_multipleRestrictionsConstraint() throws Exception { + RuleDescr rule = parseAndGetFirstRuleFromFile("restrictions_test.drl"); + assertThat(rule).isNotNull(); + + assertThat((String) rule.getConsequence()).isEqualToIgnoringWhitespace("consequence();"); + assertThat(rule.getName()).isEqualTo("simple_rule"); + assertThat(rule.getLhs().getDescrs()).hasSize(2); + + // The first pattern, with 2 restrictions on a single field (plus a + // connective) + PatternDescr pattern = (PatternDescr) rule.getLhs().getDescrs().get( 0 ); + assertThat(pattern.getObjectType()).isEqualTo("Person"); + assertThat( pattern.getConstraint().getDescrs()).hasSize(1); + + AndDescr and = (AndDescr) pattern.getConstraint(); + ExprConstraintDescr fld = (ExprConstraintDescr) and.getDescrs().get( 0 ); + assertThat(fld.getExpression()).isEqualTo("age > 30 && < 40"); + + // the second col, with 2 fields, the first with 2 restrictions, the + // second field with one + pattern = (PatternDescr) rule.getLhs().getDescrs().get( 1 ); + assertThat(pattern.getObjectType()).isEqualTo("Vehicle"); + assertThat(pattern.getConstraint().getDescrs()).hasSize(2); + + and = (AndDescr) pattern.getConstraint(); + fld = (ExprConstraintDescr) and.getDescrs().get( 0 ); + assertThat(fld.getExpression()).isEqualToIgnoringWhitespace( "type == \"sedan\" || == \"wagon\""); + + // now the second field + fld = (ExprConstraintDescr) and.getDescrs().get( 1 ); + assertThat(fld.getExpression()).isEqualToIgnoringWhitespace( "age < 3"); + } } diff --git a/drools-drl/drools-drl10-parser/src/test/resources/org/drools/parser/restrictions_test.drl b/drools-drl/drools-drl10-parser/src/test/resources/org/drools/parser/restrictions_test.drl new file mode 100644 index 00000000000..8a6d403cf30 --- /dev/null +++ b/drools-drl/drools-drl10-parser/src/test/resources/org/drools/parser/restrictions_test.drl @@ -0,0 +1,24 @@ +/* + * Copyright 2015 Red Hat, Inc. and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +//this is for showing off all the new multi restriction stuff + +rule simple_rule + when + Person(age > 30 && < 40) + Vehicle(type == "sedan" || == "wagon", age < 3) + then + consequence(); +end