This is an automated email from the ASF dual-hosted git repository.
porcelli pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-kie-drools.git
The following commit(s) were added to refs/heads/main by this push:
new 9b99105f42 Add configurable compile-time warning for self-referencing
constraints (fieldA == fieldA) (#6618)
9b99105f42 is described below
commit 9b99105f42006a9d01d6e31eb7ab5eb57509e06c
Author: Alex Porcelli <[email protected]>
AuthorDate: Thu Mar 19 14:35:12 2026 -0400
Add configurable compile-time warning for self-referencing constraints
(fieldA == fieldA) (#6618)
---
.../compiler/SelfReferencingConstraint.java | 63 ++++++++++++++++
.../generator/drlxparse/ConstraintParser.java | 25 +++++++
.../generator/drlxparse/ConstraintParserTest.java | 85 ++++++++++++++++++++++
.../builder/impl/KnowledgeBuilderTest.java | 2 +
4 files changed, 175 insertions(+)
diff --git
a/drools-compiler/src/main/java/org/drools/compiler/compiler/SelfReferencingConstraint.java
b/drools-compiler/src/main/java/org/drools/compiler/compiler/SelfReferencingConstraint.java
new file mode 100644
index 0000000000..862deffcda
--- /dev/null
+++
b/drools-compiler/src/main/java/org/drools/compiler/compiler/SelfReferencingConstraint.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * 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.
+ */
+package org.drools.compiler.compiler;
+
+import org.drools.drl.parser.BaseKnowledgeBuilderResultImpl;
+import org.kie.api.io.Resource;
+import org.kie.internal.builder.KnowledgeBuilderConfiguration;
+import org.kie.internal.builder.ResultSeverity;
+import org.kie.internal.builder.conf.KBuilderSeverityOption;
+
+public class SelfReferencingConstraint extends BaseKnowledgeBuilderResultImpl {
+
+ public static final String KEY = "selfReferencingConstraint";
+ private static final ResultSeverity DEFAULT_SEVERITY =
ResultSeverity.WARNING;
+
+ private final ResultSeverity severity;
+ private final int[] lines;
+
+ public SelfReferencingConstraint(Resource resource,
KnowledgeBuilderConfiguration config, String fieldName, String expression) {
+ super(resource, getMessage(fieldName, expression));
+ this.severity = resolveSeverity(config);
+ this.lines = new int[0];
+ }
+
+ private static ResultSeverity
resolveSeverity(KnowledgeBuilderConfiguration config) {
+ if (config != null &&
config.getOptionSubKeys(KBuilderSeverityOption.KEY).contains(KEY)) {
+ return config.getOption(KBuilderSeverityOption.KEY,
KEY).getSeverity();
+ }
+ return DEFAULT_SEVERITY;
+ }
+
+ @Override
+ public ResultSeverity getSeverity() {
+ return severity;
+ }
+
+ @Override
+ public int[] getLines() {
+ return lines;
+ }
+
+ private static String getMessage(String fieldName, String expression) {
+ return "Constraint '" + expression + "' compares field '" + fieldName +
+ "' to itself within the same pattern, which is always true and
likely a bug. " +
+ "Consider using a bound variable (e.g. $" + fieldName + ")
from another pattern instead.";
+ }
+}
diff --git
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
index f8f50cd684..ac5369f6dc 100644
---
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
+++
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParser.java
@@ -53,6 +53,9 @@ import org.drools.util.DateUtils;
import org.drools.model.Index;
import org.drools.model.codegen.execmodel.PackageModel;
import org.drools.model.codegen.execmodel.errors.ParseExpressionErrorResult;
+import org.drools.compiler.builder.impl.TypeDeclarationContext;
+import org.drools.compiler.compiler.SelfReferencingConstraint;
+import org.kie.internal.builder.KnowledgeBuilderConfiguration;
import org.drools.model.codegen.execmodel.errors.VariableUsedInBindingError;
import org.drools.model.codegen.execmodel.generator.TypedDeclarationSpec;
import org.drools.model.codegen.execmodel.generator.DrlxParseUtil;
@@ -690,6 +693,19 @@ public class ConstraintParser {
boolean equalityExpr = operator == EQUALS || operator == NOT_EQUALS;
+ try {
+ if (isBooleanOperator(operator) && isSelfComparison(left, right)) {
+ String fieldName = printNode(binaryExpr.getLeft());
+ TypeDeclarationContext typeDeclarationContext =
context.getTypeDeclarationContext();
+ KnowledgeBuilderConfiguration config = typeDeclarationContext
!= null ? typeDeclarationContext.getBuilderConfiguration() : null;
+ context.addCompilationWarning(new SelfReferencingConstraint(
+ context.getRuleDescr() != null ?
context.getRuleDescr().getResource() : null,
+ config, fieldName, printNode(binaryExpr)));
+ }
+ } catch (Exception e) {
+ LOG.debug("Unable to check for self-comparison: {}",
e.getMessage());
+ }
+
CoercedExpression.CoercedExpressionResult coerced;
try {
coerced = new CoercedExpression(left, right,
equalityExpr).coerce();
@@ -799,6 +815,15 @@ public class ConstraintParser {
return op == AND || op == OR || op == EQUALS || op == NOT_EQUALS || op
== LESS || op == GREATER || op == LESS_EQUALS || op == GREATER_EQUALS;
}
+ private static boolean isSelfComparison(TypedExpression left,
TypedExpression right) {
+ if (left.getExpression() == null || right.getExpression() == null) {
+ return false;
+ }
+ String leftStr = printNode(left.getExpression());
+ String rightStr = printNode(right.getExpression());
+ return leftStr.equals(rightStr) && leftStr.contains(THIS_PLACEHOLDER +
".");
+ }
+
public static TypedExpression getCoercedRightExpression( PackageModel
packageModel, CoercedExpression.CoercedExpressionResult coerced ) {
if ( coerced.isRightAsStaticField()) {
TypedExpression expr = coerced.getCoercedRight();
diff --git
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParserTest.java
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParserTest.java
index 80fef32d67..21f1e24d10 100644
---
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParserTest.java
+++
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/generator/drlxparse/ConstraintParserTest.java
@@ -24,6 +24,10 @@ import java.util.Optional;
import java.util.Set;
import com.github.javaparser.ast.expr.Expression;
+import org.drools.compiler.builder.impl.BuildResultCollectorImpl;
+import org.drools.compiler.builder.impl.KnowledgeBuilderConfigurationImpl;
+import org.drools.compiler.compiler.SelfReferencingConstraint;
+import org.drools.drl.ast.descr.RuleDescr;
import org.drools.model.codegen.execmodel.PackageModel;
import org.drools.model.codegen.execmodel.domain.Person;
import org.drools.model.codegen.execmodel.generator.DRLIdGenerator;
@@ -31,8 +35,11 @@ import
org.drools.model.codegen.execmodel.generator.RuleContext;
import org.drools.modelcompiler.util.EvaluationUtil;
import org.drools.util.ClassTypeResolver;
import org.drools.util.TypeResolver;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.kie.internal.builder.KnowledgeBuilderFactory;
+import org.kie.internal.builder.ResultSeverity;
import static org.assertj.core.api.Assertions.assertThat;
@@ -40,6 +47,11 @@ public class ConstraintParserTest {
private ConstraintParser parser;
+ @AfterEach
+ public void tearDown() {
+ System.getProperties().remove("drools.kbuilder.severity." +
SelfReferencingConstraint.KEY);
+ }
+
@BeforeEach
public void setup() {
PackageModel packageModel = new
PackageModel("org.kie.test:constraint-parser-test:1.0.0", "org.kie.test", null,
null, new DRLIdGenerator());
@@ -248,4 +260,77 @@ public class ConstraintParserTest {
assertThat(result.getExpr().toString()).isEqualTo("D.eval(org.drools.model.operators.InOperator.INSTANCE,
_this.getMoney(), new java.math.BigDecimal(\"100\"), new
java.math.BigDecimal(\"200\"))");
}
+
+ @Test
+ public void testSelfComparisonWarning() {
+ BuildResultCollectorImpl resultCollector = new
BuildResultCollectorImpl();
+ PackageModel packageModel = new
PackageModel("org.kie.test:constraint-parser-test:1.0.0", "org.kie.test", null,
null, new DRLIdGenerator());
+ Set<String> imports = new HashSet<>();
+ imports.add(Person.class.getCanonicalName());
+ TypeResolver typeResolver = new ClassTypeResolver(imports,
getClass().getClassLoader());
+ RuleContext context = new RuleContext(null, resultCollector,
packageModel, typeResolver, new RuleDescr("testRule"), 0);
+ ConstraintParser selfCompParser =
ConstraintParser.defaultConstraintParser(context, packageModel);
+
+ SingleDrlxParseSuccess result = (SingleDrlxParseSuccess)
selfCompParser.drlxParse(Person.class, "$p", "name == name");
+
+ assertThat(result).isNotNull();
+
assertThat(resultCollector.hasResults(ResultSeverity.WARNING)).isTrue();
+ assertThat(resultCollector.getAllResults()).anyMatch(r ->
r.getSeverity() == ResultSeverity.WARNING
+ && r.getMessage().contains("compares field")
+ && r.getMessage().contains("to itself"));
+ }
+
+ @Test
+ public void testNoWarningForNormalConstraint() {
+ BuildResultCollectorImpl resultCollector = new
BuildResultCollectorImpl();
+ PackageModel packageModel = new
PackageModel("org.kie.test:constraint-parser-test:1.0.0", "org.kie.test", null,
null, new DRLIdGenerator());
+ Set<String> imports = new HashSet<>();
+ imports.add(Person.class.getCanonicalName());
+ TypeResolver typeResolver = new ClassTypeResolver(imports,
getClass().getClassLoader());
+ RuleContext context = new RuleContext(null, resultCollector,
packageModel, typeResolver, new RuleDescr("testRule"), 0);
+ ConstraintParser selfCompParser =
ConstraintParser.defaultConstraintParser(context, packageModel);
+
+ SingleDrlxParseSuccess result = (SingleDrlxParseSuccess)
selfCompParser.drlxParse(Person.class, "$p", "name == \"John\"");
+
+ assertThat(result).isNotNull();
+
assertThat(resultCollector.hasResults(ResultSeverity.WARNING)).isFalse();
+ }
+
+ @Test
+ public void testSelfReferencingConstraintDefaultSeverityIsWarning() {
+ SelfReferencingConstraint constraint = new
SelfReferencingConstraint(null, null, "name", "name == name");
+ assertThat(constraint.getSeverity()).isEqualTo(ResultSeverity.WARNING);
+ assertThat(constraint.getMessage()).contains("compares field");
+ assertThat(constraint.getMessage()).contains("to itself");
+ }
+
+ @Test
+ public void testSelfReferencingConstraintConfiguredAsError() {
+ System.setProperty("drools.kbuilder.severity." +
SelfReferencingConstraint.KEY, "ERROR");
+ KnowledgeBuilderConfigurationImpl config =
KnowledgeBuilderFactory.newKnowledgeBuilderConfiguration()
+ .as(KnowledgeBuilderConfigurationImpl.KEY);
+
+ SelfReferencingConstraint constraint = new
SelfReferencingConstraint(null, config, "name", "name == name");
+ assertThat(constraint.getSeverity()).isEqualTo(ResultSeverity.ERROR);
+ }
+
+ @Test
+ public void testSelfReferencingConstraintConfiguredAsInfo() {
+ System.setProperty("drools.kbuilder.severity." +
SelfReferencingConstraint.KEY, "INFO");
+ KnowledgeBuilderConfigurationImpl config =
KnowledgeBuilderFactory.newKnowledgeBuilderConfiguration()
+ .as(KnowledgeBuilderConfigurationImpl.KEY);
+
+ SelfReferencingConstraint constraint = new
SelfReferencingConstraint(null, config, "name", "name == name");
+ assertThat(constraint.getSeverity()).isEqualTo(ResultSeverity.INFO);
+ }
+
+ @Test
+ public void testSelfReferencingConstraintDefaultsToWarningWithoutConfig() {
+ // When config exists but property is not set, severity defaults to
WARNING
+ KnowledgeBuilderConfigurationImpl config =
KnowledgeBuilderFactory.newKnowledgeBuilderConfiguration()
+ .as(KnowledgeBuilderConfigurationImpl.KEY);
+
+ SelfReferencingConstraint constraint = new
SelfReferencingConstraint(null, config, "age", "age == age");
+ assertThat(constraint.getSeverity()).isEqualTo(ResultSeverity.WARNING);
+ }
}
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/compiler/builder/impl/KnowledgeBuilderTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/compiler/builder/impl/KnowledgeBuilderTest.java
index c8d7e24c52..2be671028d 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/compiler/builder/impl/KnowledgeBuilderTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/mvel/compiler/builder/impl/KnowledgeBuilderTest.java
@@ -33,6 +33,7 @@ import org.drools.compiler.compiler.Dialect;
import org.drools.compiler.compiler.DialectCompiletimeRegistry;
import org.drools.compiler.compiler.DuplicateFunction;
import org.drools.compiler.compiler.DuplicateRule;
+import org.drools.compiler.compiler.SelfReferencingConstraint;
import org.drools.base.base.ClassObjectType;
import org.drools.core.common.ActivationGroupNode;
import org.drools.core.common.ActivationNode;
@@ -113,6 +114,7 @@ public class KnowledgeBuilderTest {
System.getProperties().remove( "drools.warning.filters" );
System.getProperties().remove( "drools.kbuilder.severity." +
DuplicateFunction.KEY);
System.getProperties().remove( "drools.kbuilder.severity." +
DuplicateRule.KEY);
+ System.getProperties().remove( "drools.kbuilder.severity." +
SelfReferencingConstraint.KEY);
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
