Skip to content

Commit c673db8

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Handle custom functions returning unknowns in planner
PiperOrigin-RevId: 963756455
1 parent 30f8e6d commit c673db8

8 files changed

Lines changed: 251 additions & 20 deletions

File tree

‎extensions/src/test/java/dev/cel/extensions/CelOptionalLibraryTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
import dev.cel.runtime.CelEvaluationException;
5555
import dev.cel.runtime.CelFunctionBinding;
5656
import dev.cel.runtime.CelRuntime;
57-
import dev.cel.runtime.InterpreterUtil;
57+
import dev.cel.runtime.CelUnknownSet;
5858
import dev.cel.runtime.PartialVars;
5959
import java.time.Duration;
6060
import java.time.Instant;
@@ -937,7 +937,7 @@ public void optionalIndex_onMapWithUnknownInput_returnsUnknownResult(String sour
937937
cel.createProgram(ast)
938938
.eval(PartialVars.of(CelAttributePattern.fromQualifiedIdentifier("x")));
939939

940-
assertThat(InterpreterUtil.isUnknown(result)).isTrue();
940+
assertThat(result).isInstanceOf(CelUnknownSet.class);
941941
}
942942

943943
@Test
@@ -1029,7 +1029,7 @@ public void optionalIndex_onListWithUnknownInput_returnsUnknownResult() throws E
10291029
cel.createProgram(ast)
10301030
.eval(PartialVars.of(CelAttributePattern.fromQualifiedIdentifier("x")));
10311031

1032-
assertThat(InterpreterUtil.isUnknown(result)).isTrue();
1032+
assertThat(result).isInstanceOf(CelUnknownSet.class);
10331033
}
10341034

10351035
@Test
@@ -1066,7 +1066,7 @@ public void optionalFieldSelect_fieldMarkedUnknown_returnsUnknownSet() throws Ex
10661066
ImmutableMap.of("msg", TestAllTypes.newBuilder().setSingleInt32(42).build()),
10671067
CelAttributePattern.fromQualifiedIdentifier("msg.single_int32")));
10681068

1069-
assertThat(InterpreterUtil.isUnknown(result)).isTrue();
1069+
assertThat(result).isInstanceOf(CelUnknownSet.class);
10701070
}
10711071

10721072
@Test
@@ -1089,7 +1089,7 @@ public void optionalChainedFunctions_lhsIsUnknown_returnsUnknown(String expressi
10891089
cel.createProgram(ast)
10901090
.eval(PartialVars.of(CelAttributePattern.fromQualifiedIdentifier("optx")));
10911091

1092-
assertThat(InterpreterUtil.isUnknown(result)).isTrue();
1092+
assertThat(result).isInstanceOf(CelUnknownSet.class);
10931093
}
10941094

10951095
@Test

‎runtime/src/main/java/dev/cel/runtime/CallArgumentChecker.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ void checkArg(DefaultInterpreter.IntermediateResult arg) {
7373
unknowns = mergeOptionalUnknowns(unknowns, argUnknowns);
7474

7575
// support for ExprValue unknowns.
76-
if (InterpreterUtil.isAccumulatedUnknowns(arg.value())) {
76+
if (arg.value() instanceof AccumulatedUnknowns) {
7777
AccumulatedUnknowns unknownSet = (AccumulatedUnknowns) arg.value();
7878
exprIds.addAll(unknownSet.exprIds());
7979
}

‎runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ private IntermediateResult evalInternal(ExecutionFrame frame, CelExpr expr)
282282
}
283283

284284
private static boolean isUnknownValue(Object value) {
285-
return InterpreterUtil.isAccumulatedUnknowns(value);
285+
return value instanceof AccumulatedUnknowns;
286286
}
287287

288288
private static boolean isUnknownOrError(Object value) {

‎runtime/src/main/java/dev/cel/runtime/InterpreterUtil.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.google.common.collect.ImmutableSet;
1818
import com.google.errorprone.annotations.CheckReturnValue;
19+
import com.google.errorprone.annotations.InlineMe;
1920
import dev.cel.common.annotations.Internal;
2021
import org.jspecify.annotations.Nullable;
2122

@@ -51,15 +52,14 @@ public static Object strict(Object valueOrThrowable) throws CelEvaluationExcepti
5152
*
5253
* @param obj Object to check.
5354
* @return boolean value if object is unknown.
55+
* @deprecated Perform {@code obj instanceof CelUnknownSet} directly instead.
5456
*/
57+
@Deprecated
58+
@InlineMe(replacement = "obj instanceof CelUnknownSet", imports = "dev.cel.runtime.CelUnknownSet")
5559
public static boolean isUnknown(Object obj) {
5660
return obj instanceof CelUnknownSet;
5761
}
5862

59-
public static boolean isAccumulatedUnknowns(Object obj) {
60-
return obj instanceof AccumulatedUnknowns;
61-
}
62-
6363
/** If the argument is {@link CelUnknownSet}, adapts it into {@link AccumulatedUnknowns} */
6464
public static Object maybeAdaptToAccumulatedUnknowns(Object val) {
6565
if (!(val instanceof CelUnknownSet)) {
@@ -102,7 +102,7 @@ public static Object enforceStrictness(Object left, Object right) throws CelEval
102102

103103
public static Object valueOrUnknown(@Nullable Object valueOrThrowable, Long id) {
104104
// Handle the unknown value case.
105-
if (isAccumulatedUnknowns(valueOrThrowable)) {
105+
if (valueOrThrowable instanceof AccumulatedUnknowns) {
106106
return AccumulatedUnknowns.create(id);
107107
}
108108
// Handle the null value case.

‎runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ java_library(
344344
"//common/values",
345345
"//runtime:evaluation_exception",
346346
"//runtime:interpretable",
347+
"//runtime:interpreter_util",
347348
"//runtime:resolved_overload",
348349
"@maven//:com_google_guava_guava",
349350
],
@@ -853,6 +854,7 @@ cel_android_library(
853854
"//common/values:values_android",
854855
"//runtime:evaluation_exception",
855856
"//runtime:interpretable_android",
857+
"//runtime:interpreter_util_android",
856858
"//runtime:resolved_overload_android",
857859
"@maven_android//:com_google_guava_guava",
858860
],

‎runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import dev.cel.runtime.CelEvaluationException;
2323
import dev.cel.runtime.CelResolvedOverload;
2424
import dev.cel.runtime.GlobalResolver;
25+
import dev.cel.runtime.InterpreterUtil;
2526

2627
final class EvalHelpers {
2728

@@ -63,7 +64,7 @@ static Object dispatch(
6364
throws CelEvaluationException {
6465
try {
6566
Object result = overload.invoke(args);
66-
return valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result));
67+
return adaptResult(valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result)));
6768
} catch (RuntimeException e) {
6869
throw handleDispatchException(e, overload, args);
6970
}
@@ -77,7 +78,7 @@ static Object dispatch(
7778
throws CelEvaluationException {
7879
try {
7980
Object result = overload.invoke(arg);
80-
return valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result));
81+
return adaptResult(valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result)));
8182
} catch (RuntimeException e) {
8283
throw handleDispatchException(e, overload, arg);
8384
}
@@ -92,12 +93,16 @@ static Object dispatch(
9293
throws CelEvaluationException {
9394
try {
9495
Object result = overload.invoke(arg1, arg2);
95-
return valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result));
96+
return adaptResult(valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result)));
9697
} catch (RuntimeException e) {
9798
throw handleDispatchException(e, overload, arg1, arg2);
9899
}
99100
}
100101

102+
private static Object adaptResult(Object result) {
103+
return InterpreterUtil.maybeAdaptToAccumulatedUnknowns(result);
104+
}
105+
101106
private static RuntimeException handleDispatchException(
102107
RuntimeException e, CelResolvedOverload overload, Object... args) {
103108
if (e instanceof CelRuntimeException) {

‎runtime/src/test/java/dev/cel/runtime/CelRuntimeTest.java‎

Lines changed: 149 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ public void trace_shortCircuitingDisabledWithUnknownsAndedToFalse_returnsFalse(S
522522
(expr, res) -> {
523523
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)
524524
|| expr.identOrDefault().name().equals("x")) {
525-
if (InterpreterUtil.isUnknown(res)) {
525+
if (res instanceof CelUnknownSet) {
526526
branchResults.add("x"); // Swap unknown result with a sentinel value for testing
527527
} else {
528528
branchResults.add(res);
@@ -577,7 +577,7 @@ public void trace_shortCircuitingDisabledWithUnknownAndedToTrue_returnsUnknown(S
577577
PartialVars partialVars = PartialVars.of(CelAttributePattern.create("x"));
578578
Object unknownResult = cel.createProgram(ast).trace(partialVars, listener);
579579

580-
assertThat(InterpreterUtil.isUnknown(unknownResult)).isTrue();
580+
assertThat(unknownResult).isInstanceOf(CelUnknownSet.class);
581581
assertThat(branchResults.build()).containsExactly(true, true, unknownResult);
582582
}
583583

@@ -653,7 +653,7 @@ public void trace_shortCircuitingDisabledWithUnknownsOredToFalse_returnsUnknown(
653653
PartialVars partialVars = PartialVars.of(CelAttributePattern.create("x"));
654654
Object unknownResult = cel.createProgram(ast).trace(partialVars, listener);
655655

656-
assertThat(InterpreterUtil.isUnknown(unknownResult)).isTrue();
656+
assertThat(unknownResult).isInstanceOf(CelUnknownSet.class);
657657
assertThat(branchResults.build()).containsExactly(false, false, unknownResult);
658658
}
659659

@@ -668,7 +668,7 @@ public void trace_shortCircuitingDisabledWithUnknownOredToTrue_returnsTrue(Strin
668668
(expr, res) -> {
669669
if (expr.constantOrDefault().getKind().equals(CelConstant.Kind.BOOLEAN_VALUE)
670670
|| expr.identOrDefault().name().equals("x")) {
671-
if (InterpreterUtil.isUnknown(res)) {
671+
if (res instanceof CelUnknownSet) {
672672
branchResults.add("x"); // Swap unknown result with a sentinel value for testing
673673
} else {
674674
branchResults.add(res);
@@ -748,7 +748,7 @@ public void trace_shortCircuitingDisabled_ternaryWithUnknowns(String source) thr
748748
PartialVars partialVars = PartialVars.of(CelAttributePattern.create("x"));
749749
Object unknownResult = cel.createProgram(ast).trace(partialVars, listener);
750750

751-
assertThat(InterpreterUtil.isUnknown(unknownResult)).isTrue();
751+
assertThat(unknownResult).isInstanceOf(CelUnknownSet.class);
752752
assertThat(branchResults.build()).containsExactly(false, unknownResult, true);
753753
}
754754

@@ -944,4 +944,148 @@ public void trace_shortCircuitingDisabled_logicalOrPrefersFirstError() throws Ex
944944
CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> program.eval());
945945
assertThat(e).hasCauseThat().hasMessageThat().contains("error 1");
946946
}
947+
948+
@Test
949+
// Field selection
950+
@TestParameters("{expression: 'getMsg().single_int32'}")
951+
@TestParameters("{expression: 'getMsg().single_nested_message.bb'}")
952+
// Binary & unary operators
953+
@TestParameters("{expression: 'getMsg().single_int32 == 100'}")
954+
@TestParameters("{expression: 'getMsg().single_int32 + 5 == 10'}")
955+
@TestParameters("{expression: '-getMsg().single_int32 == -10'}")
956+
// Boolean operators & ternary
957+
@TestParameters("{expression: 'true && (getMsg().single_int32 == 100)'}")
958+
@TestParameters("{expression: 'false || (getMsg().single_int32 == 100)'}")
959+
@TestParameters("{expression: '(getMsg().single_int32 == 100) ? \"match\" : \"no-match\"'}")
960+
// Comprehensions
961+
@TestParameters("{expression: '[1, 2, 3].exists(x, x == getMsg().single_int32)'}")
962+
@TestParameters("{expression: '[1, 2, 3].all(x, x > 0 && getMsg().single_int32 > 0)'}")
963+
@TestParameters("{expression: '[1, 2, 3].map(x, x + getMsg().single_int32)'}")
964+
@TestParameters("{expression: '[1, 2, 3].filter(x, x == getMsg().single_int32)'}")
965+
public void evaluate_customFunctionReturningCelUnknownSet_propagatesUnknown(String expression)
966+
throws Exception {
967+
Cel cel =
968+
runtimeFlavor
969+
.builder()
970+
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
971+
.addMessageTypes(TestAllTypes.getDescriptor())
972+
.addFunctionDeclarations(
973+
CelFunctionDecl.newFunctionDeclaration(
974+
"getMsg",
975+
CelOverloadDecl.newGlobalOverload(
976+
"getMsg_overload",
977+
StructTypeReference.create(TestAllTypes.getDescriptor().getFullName()),
978+
ImmutableList.of())))
979+
.addFunctionBindings(
980+
CelFunctionBinding.from(
981+
"getMsg_overload",
982+
ImmutableList.of(),
983+
args -> CelUnknownSet.create(CelAttribute.create("custom_msg"))))
984+
.build();
985+
986+
Object result = cel.createProgram(cel.compile(expression).getAst()).eval();
987+
988+
assertThat(result).isInstanceOf(CelUnknownSet.class);
989+
}
990+
991+
@Test
992+
// Short-circuited boolean operators
993+
@TestParameters("{expression: 'false && (getMsg().single_int32 == 100)', expected: false}")
994+
@TestParameters("{expression: 'true || (getMsg().single_int32 == 100)', expected: true}")
995+
// Short-circuited comprehensions
996+
@TestParameters(
997+
"{expression: '[1, 2, 3].exists(x, x == 1 || x == getMsg().single_int32)', expected: true}")
998+
@TestParameters(
999+
"{expression: '[1, 2, 3].all(x, x == 0 && getMsg().single_int32 > 0)', expected: false}")
1000+
public void evaluate_customFunctionReturningCelUnknownSet_shortCircuits(
1001+
String expression, boolean expected) throws Exception {
1002+
Cel cel =
1003+
runtimeFlavor
1004+
.builder()
1005+
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
1006+
.addMessageTypes(TestAllTypes.getDescriptor())
1007+
.addFunctionDeclarations(
1008+
CelFunctionDecl.newFunctionDeclaration(
1009+
"getMsg",
1010+
CelOverloadDecl.newGlobalOverload(
1011+
"getMsg_overload",
1012+
StructTypeReference.create(TestAllTypes.getDescriptor().getFullName()),
1013+
ImmutableList.of())))
1014+
.addFunctionBindings(
1015+
CelFunctionBinding.from(
1016+
"getMsg_overload",
1017+
ImmutableList.of(),
1018+
args -> CelUnknownSet.create(CelAttribute.create("custom_msg"))))
1019+
.build();
1020+
1021+
Object result = cel.createProgram(cel.compile(expression).getAst()).eval();
1022+
1023+
assertThat(result).isEqualTo(expected);
1024+
}
1025+
1026+
@Test
1027+
public void evaluate_customFunctionReturningCelUnknownSet_differentArities() throws Exception {
1028+
Cel cel =
1029+
runtimeFlavor
1030+
.builder()
1031+
.addFunctionDeclarations(
1032+
CelFunctionDecl.newFunctionDeclaration(
1033+
"unkZero",
1034+
CelOverloadDecl.newGlobalOverload(
1035+
"unk_zero", SimpleType.INT, ImmutableList.of())),
1036+
CelFunctionDecl.newFunctionDeclaration(
1037+
"unkUnary",
1038+
CelOverloadDecl.newGlobalOverload("unk_unary", SimpleType.INT, SimpleType.INT)),
1039+
CelFunctionDecl.newFunctionDeclaration(
1040+
"unkBinary",
1041+
CelOverloadDecl.newGlobalOverload(
1042+
"unk_binary", SimpleType.INT, SimpleType.INT, SimpleType.INT)),
1043+
CelFunctionDecl.newFunctionDeclaration(
1044+
"unkMember",
1045+
CelOverloadDecl.newMemberOverload(
1046+
"unk_member", SimpleType.INT, SimpleType.STRING, SimpleType.INT)),
1047+
CelFunctionDecl.newFunctionDeclaration(
1048+
"unkVarargs",
1049+
CelOverloadDecl.newGlobalOverload(
1050+
"unk_varargs",
1051+
SimpleType.INT,
1052+
SimpleType.INT,
1053+
SimpleType.INT,
1054+
SimpleType.INT)))
1055+
.addFunctionBindings(
1056+
CelFunctionBinding.from(
1057+
"unk_zero",
1058+
ImmutableList.of(),
1059+
args -> CelUnknownSet.create(CelAttribute.create("attr_zero"))),
1060+
CelFunctionBinding.from(
1061+
"unk_unary",
1062+
Long.class,
1063+
arg -> CelUnknownSet.create(CelAttribute.create("attr_unary"))),
1064+
CelFunctionBinding.from(
1065+
"unk_binary",
1066+
Long.class,
1067+
Long.class,
1068+
(a, b) -> CelUnknownSet.create(CelAttribute.create("attr_binary"))),
1069+
CelFunctionBinding.from(
1070+
"unk_member",
1071+
String.class,
1072+
Long.class,
1073+
(target, arg) -> CelUnknownSet.create(CelAttribute.create("attr_member"))),
1074+
CelFunctionBinding.from(
1075+
"unk_varargs",
1076+
ImmutableList.of(Long.class, Long.class, Long.class),
1077+
args -> CelUnknownSet.create(CelAttribute.create("attr_varargs"))))
1078+
.build();
1079+
1080+
assertThat(cel.createProgram(cel.compile("unkZero() + 1").getAst()).eval())
1081+
.isEqualTo(CelUnknownSet.create(CelAttribute.create("attr_zero")));
1082+
assertThat(cel.createProgram(cel.compile("unkUnary(1) + 1").getAst()).eval())
1083+
.isEqualTo(CelUnknownSet.create(CelAttribute.create("attr_unary")));
1084+
assertThat(cel.createProgram(cel.compile("unkBinary(1, 2) + 1").getAst()).eval())
1085+
.isEqualTo(CelUnknownSet.create(CelAttribute.create("attr_binary")));
1086+
assertThat(cel.createProgram(cel.compile("'target'.unkMember(1) + 1").getAst()).eval())
1087+
.isEqualTo(CelUnknownSet.create(CelAttribute.create("attr_member")));
1088+
assertThat(cel.createProgram(cel.compile("unkVarargs(1, 2, 3) + 1").getAst()).eval())
1089+
.isEqualTo(CelUnknownSet.create(CelAttribute.create("attr_varargs")));
1090+
}
9471091
}

0 commit comments

Comments
 (0)