Skip to content

Commit

Permalink
- Add warnings for deprecated DRL syntax when used in DRL6
Browse files Browse the repository at this point in the history
  • Loading branch information
tkobayas committed Feb 3, 2025
1 parent 37a218d commit 428a1e6
Show file tree
Hide file tree
Showing 12 changed files with 1,821 additions and 1,534 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,14 @@ public DescrBuilder<P, T> endLocation( int line,
* @return
*/
public P end();

/**
* Returns the parent container of this descr builder.
* Example: ruleDescrBuilder.getParent() will return the
* PackageDescrBuilder as that is its parent container
* without ending the current construction.
*
* @return
*/
public P getParent();
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,8 @@ public P end() {
return parent;
}

@Override
public P getParent() {
return parent;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -576,4 +576,48 @@ void newBigDecimal() {
assertThat(bind.getVariable()).isEqualTo("$bd");
assertThat(bind.getExpression()).isEqualTo("new BigDecimal(30)");
}

@Test
void halfConstraintAnd() {
String source = "age > 10 && < 20";
parser.parse(source);

if (DrlParser.ANTLR4_PARSER_ENABLED) {
// half constraint is dropped in DRL10
assertThat(parser.hasErrors()).isTrue();
} else {
assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse();
}
}

@Test
void halfConstraintOr() {
String source = "name == \"John\" || == \"Paul\"";
parser.parse(source);

if (DrlParser.ANTLR4_PARSER_ENABLED) {
// half constraint is dropped in DRL10
assertThat(parser.hasErrors()).isTrue();
} else {
assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse();
}
}

@Test
void customOperator() {
Operator.addOperatorToRegistry("supersetOf", false);
// prefix '##' is required for custom operators in DRL10
String source = DrlParser.ANTLR4_PARSER_ENABLED ? "this ##supersetOf $list" : "this supersetOf $list";
ConstraintConnectiveDescr result = parser.parse(source);
assertThat(parser.hasErrors()).as(parser.getErrors().toString()).isFalse();

RelationalExprDescr rel = (RelationalExprDescr) result.getDescrs().get(0);
assertThat(rel.getOperator()).isEqualTo("supersetOf");

AtomicExprDescr left = (AtomicExprDescr) rel.getLeft();
assertThat(left.getExpression()).isEqualTo("this");

AtomicExprDescr right = (AtomicExprDescr) rel.getRight();
assertThat(right.getExpression()).isEqualTo("$list");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5320,4 +5320,24 @@ void typeDeclarationWithTypeToken() {
TypeFieldDescr typeFieldDescr = typeDeclarationDescr.getFields().get("id");
assertThat(typeFieldDescr.getPattern().getObjectType()).isEqualTo("int");
}

@Test
void agendaGroup() {
final String drl = "rule R1\n" +
" agenda-group \"group1\"\n" +
" when\n" +
" then\n" +
"end";
PackageDescr pkg = parseAndGetPackageDescrWithoutErrorCheck(drl);
if (DrlParser.ANTLR4_PARSER_ENABLED) {
// agenda-group is dropped in DRL10
assertThat(parser.hasErrors()).isTrue();
} else {
RuleDescr rule = pkg.getRules().get(0);
assertThat(rule).isNotNull();
final AttributeDescr att = rule.getAttributes().get("agenda-group");
assertThat(att.getValue()).isEqualTo("group1");
assertThat(att.getName()).isEqualTo("agenda-group");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ public DrlParser() {

public DrlParser(LanguageLevelOption languageLevel) {
this.languageLevel = languageLevel;

if (languageLevel == LanguageLevelOption.DRL5 || languageLevel == LanguageLevelOption.DRL6_STRICT) {
LOG.warn("{} is deprecated and will be removed in future versions. Please use the default {} or newly introduced {} instead.",
languageLevel, LanguageLevelOption.DRL6, LanguageLevelOption.DRL10);
}
}

/** Parse a rule from text */
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,7 @@ public AttributeDescr attribute(AttributeSupportBuilder<?> as) {
DroolsSoftKeywords.GROUP)) {
attribute = stringAttribute(as,
new String[]{DroolsSoftKeywords.AGENDA, "-", DroolsSoftKeywords.GROUP});
helper.logAgendaGroupWarn(attribute);
} else if (helper.validateIdentifierKey(DroolsSoftKeywords.ACTIVATION) &&
helper.validateLT(2,
"-") &&
Expand Down Expand Up @@ -2155,6 +2156,7 @@ private BaseDescr lhsOr(final CEDescrBuilder<?, ?> ce,
while (input.LA(1) == DRL6Lexer.AT) {
// annotation*
annotation(or);
helper.logAnnotationInLhsPatternWarn(or);
if (state.failed)
return null;
}
Expand Down Expand Up @@ -2212,6 +2214,7 @@ private BaseDescr lhsOr(final CEDescrBuilder<?, ?> ce,
null,
null,
DroolsEditorType.SYMBOL);
helper.logInfixOrWarn(or);
} else {
match(input,
DRL6Lexer.ID,
Expand All @@ -2225,6 +2228,7 @@ private BaseDescr lhsOr(final CEDescrBuilder<?, ?> ce,
while (input.LA(1) == DRL6Lexer.AT) {
// annotation*
annotation(or);
helper.logAnnotationInLhsPatternWarn(or);
if (state.failed)
return null;
}
Expand Down Expand Up @@ -2299,6 +2303,7 @@ private BaseDescr lhsAnd(final CEDescrBuilder<?, ?> ce,
while (input.LA(1) == DRL6Lexer.AT) {
// annotation*
annotation(and);
helper.logAnnotationInLhsPatternWarn(and);
if (state.failed)
return null;
}
Expand Down Expand Up @@ -2354,6 +2359,7 @@ private BaseDescr lhsAnd(final CEDescrBuilder<?, ?> ce,
null,
null,
DroolsEditorType.SYMBOL);
helper.logInfixAndWarn(and);
} else {
match(input,
DRL6Lexer.ID,
Expand All @@ -2367,6 +2373,7 @@ private BaseDescr lhsAnd(final CEDescrBuilder<?, ?> ce,
while (input.LA(1) == DRL6Lexer.AT) {
// annotation*
annotation(and);
helper.logAnnotationInLhsPatternWarn(and);
if (state.failed)
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,26 @@

import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Deque;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import org.antlr.runtime.CommonToken;
import org.antlr.runtime.RecognitionException;
import org.antlr.runtime.RecognizerSharedState;
import org.antlr.runtime.Token;
import org.antlr.runtime.TokenStream;
import org.drools.drl.ast.descr.AnnotatedBaseDescr;
import org.drools.drl.ast.descr.AttributeDescr;
import org.drools.drl.ast.descr.BaseDescr;
import org.drools.drl.ast.descr.RelationalExprDescr;
import org.drools.drl.ast.descr.RuleDescr;
import org.drools.drl.ast.dsl.AbstractClassTypeDeclarationBuilder;
import org.drools.drl.ast.dsl.AccumulateDescrBuilder;
import org.drools.drl.ast.dsl.AccumulateImportDescrBuilder;
Expand Down Expand Up @@ -65,13 +72,18 @@
import org.drools.drl.ast.dsl.UnitDescrBuilder;
import org.drools.drl.ast.dsl.WindowDeclarationDescrBuilder;
import org.drools.drl.parser.DroolsParserException;
import org.drools.drl.parser.impl.Operator;
import org.kie.internal.builder.conf.LanguageLevelOption;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This is a class to hold all the helper functions/methods used
* by the DRL parser
*/
public class ParserHelper {
public static final Logger LOG = LoggerFactory.getLogger(ParserHelper.class );

public final String[] statementKeywords = new String[]{
DroolsSoftKeywords.PACKAGE,
DroolsSoftKeywords.UNIT,
Expand Down Expand Up @@ -100,6 +112,17 @@ public class ParserHelper {

private final LanguageLevelOption languageLevel;

private static final Set<String> BUILT_IN_OPERATORS = Arrays.stream(Operator.BuiltInOperator.values())
.map(Operator.BuiltInOperator::getSymbol)
.collect(Collectors.toSet());

private static int logCounterHalfConstraint = 0;
private static int logCounterCustomOperator = 0;
private static int logCounterInfixAnd = 0;
private static int logCounterInfixOr = 0;
private static int logCounterAnnotationInLhsPattern = 0;
private static int logCounterAgendaGroup = 0;

public ParserHelper(TokenStream input,
RecognizerSharedState state,
LanguageLevelOption languageLevel) {
Expand Down Expand Up @@ -656,4 +679,123 @@ void setEnd( DescrBuilder< ? , ? > db ) {
public String[] getStatementKeywords() {
return statementKeywords;
}

public static void logHalfConstraintWarn(String logicalConstraint, BaseDescr descr) {
if (descr instanceof RelationalExprDescr relational) {
String halfConstraintStr = logicalConstraint + " " + relational.getOperator() + " " + relational.getRight().toString();
logHalfConstraintWarn("The use of a half constraint '" + halfConstraintStr + "' is deprecated" +
" and will be removed in the future version (LanguageLevel.DRL10)." +
" Please add a left operand.");
}
}

public static void logHalfConstraintWarn(String message) {
if (logCounterHalfConstraint > 10) {
return; // suppress further warnings
}

logCounterHalfConstraint++;
LOG.warn(message);
if (logCounterHalfConstraint == 10) {
LOG.warn("Further warnings about half constraints will be suppressed.");
}
}

public static void logCustomOperatorWarn(Token token) {
if (logCounterCustomOperator > 1) {
return; // suppress further warnings
}

String operator = token.getText();
if (BUILT_IN_OPERATORS.contains(operator)) {
return; // built-in operator
}
logCounterCustomOperator++;
LOG.warn("Custom operator will require a prefix '##' in the future version (LanguageLevel.DRL10)." +
" If you use LanguageLevel.DRL10, you need to change '{}' to '##{}'." +
" You don't need to change the rule while you use the default LanguageLevel.DRL6.", operator, operator);
}

public static void logInfixOrWarn(CEDescrBuilder descrBuilder) {
if (logCounterInfixOr > 5) {
return; // suppress further warnings
}

Optional<RuleDescr> ruleDescrOpt = getParentRuleDescr(descrBuilder);
if (ruleDescrOpt.isEmpty()) {
return;
}

logCounterInfixOr++;
LOG.warn("Connecting patterns with '||' is deprecated and will be removed in the future version (LanguageLevel.DRL10)." +
" Please replace '||' with 'or' in rule '{}'. '||' in a constraint will remain supported.", ruleDescrOpt.get().getName());
if (logCounterInfixOr == 5) {
LOG.warn("Further warnings about '||' will be suppressed.");
}
}

public static void logInfixAndWarn(CEDescrBuilder descrBuilder) {
if (logCounterInfixAnd > 5) {
return; // suppress further warnings
}

Optional<RuleDescr> ruleDescrOpt = getParentRuleDescr(descrBuilder);
if (ruleDescrOpt.isEmpty()) {
return;
}

logCounterInfixAnd++;
LOG.warn("Connecting patterns with '&&' is deprecated and will be removed in the future version (LanguageLevel.DRL10)." +
" Please replace '&&' with 'and' in rule '{}'. '&&' in a constraint will remain supported.", ruleDescrOpt.get().getName());
if (logCounterInfixAnd == 5) {
LOG.warn("Further warnings about '&&' will be suppressed.");
}
}

private static Optional<RuleDescr> getParentRuleDescr(DescrBuilder<?, ?> descrBuilder) {
while (descrBuilder != null) {
if (descrBuilder instanceof RuleDescrBuilder) {
return Optional.of(((RuleDescrBuilder) descrBuilder).getDescr());
} else if (descrBuilder instanceof PackageDescrBuilder) {
return Optional.empty();
}
descrBuilder = descrBuilder.getParent();
}
return Optional.empty();
}

public static void logAnnotationInLhsPatternWarn(CEDescrBuilder descrBuilder) {
if (logCounterAnnotationInLhsPattern > 5) {
return; // suppress further warnings
}

Optional<RuleDescr> ruleDescrOpt = getParentRuleDescr(descrBuilder);
if (ruleDescrOpt.isEmpty()) {
return;
}

BaseDescr descr = descrBuilder.getDescr();
if (descr instanceof AnnotatedBaseDescr annotated) {
String annotationNames = annotated.getAnnotationNames().stream().collect(Collectors.joining(", "));
logCounterAnnotationInLhsPattern++;
LOG.warn("Annotation inside LHS patterns is deprecated and will be removed in the future version (LanguageLevel.DRL10)." +
" Found '{}' in rule '{}'. Annotation in other places will remain supported.", annotationNames, ruleDescrOpt.get().getName());
if (logCounterAnnotationInLhsPattern == 5) {
LOG.warn("Further warnings about Annotation inside LHS patterns will be suppressed.");
}
}
}

public static void logAgendaGroupWarn(AttributeDescr attributeDescr) {
if (logCounterAgendaGroup > 5) {
return; // suppress further warnings
}

logCounterAgendaGroup++;
LOG.warn("'agenda-group \"{}\"' is found. 'agenda-group' is deprecated and will be dropped in the future version (LanguageLevel.DRL10)." +
" Please replace 'agenda-group' with 'ruleflow-group', which works as the same as 'agenda-group'.", attributeDescr.getValue());
if (logCounterAgendaGroup == 5) {
LOG.warn("Further warnings about 'agenda-group' will be suppressed.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ orRestriction returns [BaseDescr result]
: left=andRestriction { if( buildDescr ) { $result = $left.result; } }
( (DOUBLE_PIPE fullAnnotation[null]? andRestriction)=>lop=DOUBLE_PIPE args=fullAnnotation[null]? right=andRestriction
{ if( buildDescr ) {
helper.logHalfConstraintWarn("||", right);
ConstraintConnectiveDescr descr = ConstraintConnectiveDescr.newOr();
descr.addOrMerge( $result );
descr.addOrMerge( $right.result );
Expand All @@ -448,6 +449,7 @@ andRestriction returns [BaseDescr result]
{ if ( isNotEOF() ) helper.emit( Location.LOCATION_LHS_INSIDE_CONDITION_OPERATOR ); }
args=fullAnnotation[null]?right=singleRestriction
{ if( buildDescr ) {
helper.logHalfConstraintWarn("&&", right);
ConstraintConnectiveDescr descr = ConstraintConnectiveDescr.newAnd();
descr.addOrMerge( $result );
descr.addOrMerge( $right.result );
Expand Down Expand Up @@ -847,9 +849,9 @@ in_key
;

operator_key
: {(helper.isPluggableEvaluator(false))}?=> id=ID { helper.emit($ID, DroolsEditorType.KEYWORD); }
: {(helper.isPluggableEvaluator(false))}?=> id=ID { helper.logCustomOperatorWarn(id); helper.emit($ID, DroolsEditorType.KEYWORD); }
;

neg_operator_key
: {(helper.isPluggableEvaluator(true))}?=> id=ID { helper.emit($ID, DroolsEditorType.KEYWORD); }
: {(helper.isPluggableEvaluator(true))}?=> id=ID { helper.logCustomOperatorWarn(id); helper.emit($ID, DroolsEditorType.KEYWORD); }
;
Loading

0 comments on commit 428a1e6

Please sign in to comment.