Skip to content

Commit

Permalink
[incubator-kie-issues#1546] Fix DMN TCK range equality test failures …
Browse files Browse the repository at this point in the history
…for unary test ranges (#6134)

* Introduce undefined boundaries for range instances.

Signed-off-by: Tibor Zimányi <tiborzimanyi@ibm.com>

* Update inclusion and equality methods for FEEL range.

Signed-off-by: Tibor Zimányi <tiborzimanyi@ibm.com>

* Fix compiled FEEL scenarios with ranges.

* Restore FEELRangesTest.

* Add RangeImpl unit tests.

* [range-problems-gc] WIP

* [range-problems-gc] WIP

* [incubator-kie-issues#1546] Add UndefinedValueComparable UndefinedValueNode

* [incubator-kie-issues#1546] Fixing tests - WIP

* [incubator-kie-issues#1546] Fixing tests - WIP

* [incubator-kie-issues#1546] Fixing tests

* [incubator-kie-issues#1546] Fixing tests

---------

Signed-off-by: Tibor Zimányi <tiborzimanyi@ibm.com>
Co-authored-by: Tibor Zimányi <tiborzimanyi@ibm.com>
Co-authored-by: Gabriele-Cardosi <gabriele.cardosi@ibm.com>
  • Loading branch information
3 people authored Oct 23, 2024
1 parent 9a7f01f commit 1689794
Show file tree
Hide file tree
Showing 25 changed files with 317 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ literal
| BooleanLiteral #boolLiteral
| atLiteral #atLiteralLabel
| StringLiteral #stringLiteral
| NULL #nullLiteral
| NULL #nullLiteral
| UNDEFINEDVALUE #undefined
;

atLiteral
Expand All @@ -340,6 +341,7 @@ BooleanLiteral
| FALSE
;


/**************************
* OTHER CONSTRUCTS
**************************/
Expand Down Expand Up @@ -463,6 +465,7 @@ reusableKeywords
| BETWEEN
| NOT
| NULL
| UNDEFINEDVALUE
| TRUE
| FALSE
;
Expand Down Expand Up @@ -536,6 +539,10 @@ NULL
: 'null'
;

UNDEFINEDVALUE
: 'undefined'
;

TRUE
: 'true'
;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import org.kie.dmn.feel.lang.ast.TypeNode;
import org.kie.dmn.feel.lang.ast.UnaryTestListNode;
import org.kie.dmn.feel.lang.ast.UnaryTestNode;
import org.kie.dmn.feel.lang.ast.UndefinedValueNode;
import org.kie.dmn.feel.lang.impl.JavaBackedType;
import org.kie.dmn.feel.lang.impl.MapBackedType;
import org.kie.dmn.feel.lang.types.AliasFEELType;
Expand Down Expand Up @@ -144,6 +145,7 @@
import static org.kie.dmn.feel.codegen.feel11.DMNCodegenConstants.TYPE_CT;
import static org.kie.dmn.feel.codegen.feel11.DMNCodegenConstants.UNARYTESTLISTNODE_CT;
import static org.kie.dmn.feel.codegen.feel11.DMNCodegenConstants.UNARYTESTNODE_CT;
import static org.kie.dmn.feel.codegen.feel11.DMNCodegenConstants.UNDEFINEDVALUENODE_CT;
import static org.kie.dmn.feel.util.CodegenUtils.getEnumExpression;
import static org.kie.dmn.feel.util.CodegenUtils.getListExpression;
import static org.kie.dmn.feel.util.CodegenUtils.getStringLiteralExpr;
Expand Down Expand Up @@ -488,6 +490,10 @@ public BlockStmt add(UnaryTestNode n) {
n.getText());
}

public BlockStmt add(UndefinedValueNode n) {
return addVariableDeclaratorWithObjectCreation(UNDEFINEDVALUENODE_CT, NodeList.nodeList());
}

public String getLastVariableName() {
return lastVariableName.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.kie.dmn.feel.lang.ast.TemporalConstantNode;
import org.kie.dmn.feel.lang.ast.UnaryTestListNode;
import org.kie.dmn.feel.lang.ast.UnaryTestNode;
import org.kie.dmn.feel.lang.ast.UndefinedValueNode;
import org.kie.dmn.feel.lang.ast.Visitor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -290,6 +291,12 @@ public BlockStmt visit(UnaryTestNode n) {
return compilerHelper.add(n);
}

@Override
public BlockStmt visit(UndefinedValueNode n) {
LOGGER.trace("visit {}", n);
return compilerHelper.add(n);
}

public String getLastVariableName() {
LOGGER.trace("getLastVariableName");
return compilerHelper.getLastVariableName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.kie.dmn.feel.lang.ast.StringNode;
import org.kie.dmn.feel.lang.ast.UnaryTestListNode;
import org.kie.dmn.feel.lang.ast.UnaryTestNode;
import org.kie.dmn.feel.lang.ast.UndefinedValueNode;
import org.kie.dmn.feel.lang.ast.visitor.DefaultedVisitor;

public class ASTUnaryTestTransform extends DefaultedVisitor<ASTUnaryTestTransform.UnaryTestSubexpr> {
Expand Down Expand Up @@ -136,6 +137,11 @@ public UnaryTestSubexpr visit(QualifiedNameNode n) {
}
}

@Override
public UnaryTestSubexpr visit(UndefinedValueNode n) {
return new SimpleUnaryExpression(n);
}

private UnaryTestSubexpr propagateWildcard(ASTNode n) {
return Arrays.stream(n.getChildrenNode()).map(e -> e.accept(this)).anyMatch(UnaryTestSubexpr::isWildcard) ?
new WildCardUnaryExpression((BaseNode) n) : new SimpleUnaryExpression((BaseNode) n);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.kie.dmn.feel.lang.ast.TemporalConstantNode;
import org.kie.dmn.feel.lang.ast.UnaryTestListNode;
import org.kie.dmn.feel.lang.ast.UnaryTestNode;
import org.kie.dmn.feel.lang.ast.UndefinedValueNode;
import org.kie.dmn.feel.lang.impl.JavaBackedType;
import org.kie.dmn.feel.lang.impl.MapBackedType;
import org.kie.dmn.feel.lang.types.AliasFEELType;
Expand Down Expand Up @@ -172,6 +173,7 @@ public class DMNCodegenConstants {
parseClassOrInterfaceType(UnaryTestListNode.class.getCanonicalName());
public static final ClassOrInterfaceType UNARYTESTNODE_CT =
parseClassOrInterfaceType(UnaryTestNode.class.getCanonicalName());
public static final ClassOrInterfaceType UNDEFINEDVALUENODE_CT = parseClassOrInterfaceType(UndefinedValueNode.class.getCanonicalName());

private DMNCodegenConstants() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ public static NullNode newNullNode(ParserRuleContext ctx) {
return new NullNode( ctx );
}

public static UndefinedValueNode newUndefinedValueNode() {
return new UndefinedValueNode();
}

public static StringNode newStringNode(ParserRuleContext ctx) {
return new StringNode( ctx );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.kie.dmn.feel.lang.types.impl.ComparablePeriod;
import org.kie.dmn.feel.runtime.Range;
import org.kie.dmn.feel.runtime.impl.RangeImpl;
import org.kie.dmn.feel.runtime.impl.UndefinedValueComparable;
import org.kie.dmn.feel.util.Msg;

public class RangeNode
Expand Down Expand Up @@ -113,13 +114,17 @@ public Range evaluate(EvaluationContext ctx) {

Type sType = BuiltInType.determineTypeFromInstance(s);
Type eType = BuiltInType.determineTypeFromInstance(e);
if (s != null && e != null && sType != eType && !s.getClass().isAssignableFrom(e.getClass())) {
boolean withUndefined = s instanceof UndefinedValueComparable || e instanceof UndefinedValueComparable;
if (s != null && e != null &&
!withUndefined &&
sType != eType &&
!s.getClass().isAssignableFrom(e.getClass())) {
ctx.notifyEvt( astEvent(Severity.ERROR, Msg.createMessage(Msg.X_TYPE_INCOMPATIBLE_WITH_Y_TYPE, "Start", "End")));
return null;
}

Comparable start = convertToComparable( ctx, s );
Comparable end = convertToComparable( ctx, e );
Comparable start = s instanceof UndefinedValueComparable ? (Comparable) s : convertToComparable(ctx, s );
Comparable end = e instanceof UndefinedValueComparable ? (Comparable) e : convertToComparable( ctx, e );

return new RangeImpl( lowerBound==IntervalBoundary.OPEN ? Range.RangeBoundary.OPEN : Range.RangeBoundary.CLOSED,
start,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* 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.kie.dmn.feel.lang.ast;

import org.kie.dmn.feel.lang.EvaluationContext;
import org.kie.dmn.feel.runtime.impl.UndefinedValueComparable;

public class UndefinedValueNode
extends BaseNode {

public UndefinedValueNode() { }

@Override
public Object evaluate(EvaluationContext ctx) {
return new UndefinedValueComparable();
}

@Override
public <T> T accept(Visitor<T> v) {
return v.visit(this);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,5 @@ default T visit(TemporalConstantNode n) {

T visit(UnaryTestListNode n);
T visit(UnaryTestNode n);
T visit(UndefinedValueNode n);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,15 @@
package org.kie.dmn.feel.lang.ast.visitor;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import org.kie.dmn.api.feel.runtime.events.FEELEvent;
import org.kie.dmn.api.feel.runtime.events.FEELEvent.Severity;
import org.kie.dmn.feel.lang.ast.ASTNode;
import org.kie.dmn.feel.lang.ast.InfixOpNode;
import org.kie.dmn.feel.lang.ast.NullNode;
import org.kie.dmn.feel.lang.ast.RangeNode;
import org.kie.dmn.feel.lang.ast.UnaryTestNode;
import org.kie.dmn.feel.lang.ast.UndefinedValueNode;
import org.kie.dmn.feel.runtime.events.ASTHeuristicCheckEvent;
import org.kie.dmn.feel.util.Msg;

Expand Down Expand Up @@ -63,8 +62,8 @@ public List<FEELEvent> visit(UnaryTestNode n) {

@Override
public List<FEELEvent> visit(RangeNode n) {
if ((n.getStart() instanceof NullNode && n.getEnd() instanceof RangeNode)
|| (n.getStart() instanceof RangeNode && n.getEnd() instanceof NullNode)) {
if ((n.getStart() instanceof UndefinedValueNode && n.getEnd() instanceof RangeNode)
|| (n.getStart() instanceof RangeNode && n.getEnd() instanceof UndefinedValueNode)) {
return List.of(new ASTHeuristicCheckEvent(Severity.WARN, Msg.createMessage(Msg.UT_OF_UT, n.getText()), n));
}
return defaultVisit(n);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.kie.dmn.feel.lang.ast.StringNode;
import org.kie.dmn.feel.lang.ast.UnaryTestListNode;
import org.kie.dmn.feel.lang.ast.UnaryTestNode;
import org.kie.dmn.feel.lang.ast.UndefinedValueNode;
import org.kie.dmn.feel.lang.ast.Visitor;

public abstract class DefaultedVisitor<T> implements Visitor<T> {
Expand Down Expand Up @@ -240,4 +241,9 @@ public T visit(FunctionTypeNode n) {
return defaultVisit(n);
}

@Override
public T visit(UndefinedValueNode n) {
return defaultVisit(n);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ public BaseNode visitNullLiteral(FEEL_1_1Parser.NullLiteralContext ctx) {
return ASTBuilderFactory.newNullNode( ctx );
}

@Override
public BaseNode visitUndefined(FEEL_1_1Parser.UndefinedContext ctx) {
return ASTBuilderFactory.newUndefinedValueNode();
}

@Override
public BaseNode visitStringLiteral(FEEL_1_1Parser.StringLiteralContext ctx) {
return ASTBuilderFactory.newStringNode( ctx );
Expand Down Expand Up @@ -188,13 +193,13 @@ public BaseNode visitPositiveUnaryTestIneqInterval(FEEL_1_1Parser.PositiveUnaryT
String op = ctx.op.getText();
switch (UnaryOperator.determineOperator(op)) {
case GT:
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.OPEN, value, ASTBuilderFactory.newNullNode(ctx), RangeNode.IntervalBoundary.OPEN);
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.OPEN, value, ASTBuilderFactory.newUndefinedValueNode(), RangeNode.IntervalBoundary.OPEN);
case GTE:
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.CLOSED, value, ASTBuilderFactory.newNullNode(ctx), RangeNode.IntervalBoundary.OPEN);
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.CLOSED, value, ASTBuilderFactory.newUndefinedValueNode(), RangeNode.IntervalBoundary.OPEN);
case LT:
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.OPEN, ASTBuilderFactory.newNullNode(ctx), value, RangeNode.IntervalBoundary.OPEN);
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.OPEN, ASTBuilderFactory.newUndefinedValueNode(), value, RangeNode.IntervalBoundary.OPEN);
case LTE:
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.OPEN, ASTBuilderFactory.newNullNode(ctx), value, RangeNode.IntervalBoundary.CLOSED);
return ASTBuilderFactory.newIntervalNode(ctx, RangeNode.IntervalBoundary.OPEN, ASTBuilderFactory.newUndefinedValueNode(), value, RangeNode.IntervalBoundary.CLOSED);
default:
throw new UnsupportedOperationException("by the parser rule FEEL grammar rule 7.a for range syntax should not have determined the operator " + op);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,6 @@ enum RangeBoundary {

Boolean includes(Object param);

boolean isWithUndefined();

}
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public FEELFnResult<Range> invoke(@ParameterName("from") String from) {
return FEELFnResult.ofError(new InvalidParametersEvent(FEELEvent.Severity.ERROR, "from", "endpoints must be of equivalent types"));
}

// Boundary values need to be always defined in range string. They can be undefined only in unary test, that represents range, e.g. (<10).
return FEELFnResult.ofResult(new RangeImpl(startBoundary, (Comparable) left, (Comparable) right, endBoundary));
}

Expand Down
Loading

0 comments on commit 1689794

Please sign in to comment.