Skip to content

Commit f70af47

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

9 files changed

Lines changed: 306 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: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,12 @@ java_library(
342342
"//common:error_codes",
343343
"//common/exceptions:runtime_exception",
344344
"//common/values",
345+
"//runtime:accumulated_unknowns",
345346
"//runtime:evaluation_exception",
346347
"//runtime:interpretable",
348+
"//runtime:interpreter_util",
347349
"//runtime:resolved_overload",
350+
"//runtime:unknown_attributes",
348351
"@maven//:com_google_guava_guava",
349352
],
350353
)
@@ -851,9 +854,12 @@ cel_android_library(
851854
"//common:error_codes",
852855
"//common/exceptions:runtime_exception",
853856
"//common/values:values_android",
857+
"//runtime:accumulated_unknowns_android",
854858
"//runtime:evaluation_exception",
855859
"//runtime:interpretable_android",
860+
"//runtime:interpreter_util_android",
856861
"//runtime:resolved_overload_android",
862+
"//runtime:unknown_attributes_android",
857863
"@maven_android//:com_google_guava_guava",
858864
],
859865
)

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,12 @@
1919
import dev.cel.common.exceptions.CelRuntimeException;
2020
import dev.cel.common.values.CelValueConverter;
2121
import dev.cel.common.values.ErrorValue;
22+
import dev.cel.runtime.AccumulatedUnknowns;
2223
import dev.cel.runtime.CelEvaluationException;
2324
import dev.cel.runtime.CelResolvedOverload;
25+
import dev.cel.runtime.CelUnknownSet;
2426
import dev.cel.runtime.GlobalResolver;
27+
import dev.cel.runtime.InterpreterUtil;
2528

2629
final class EvalHelpers {
2730

@@ -63,7 +66,7 @@ static Object dispatch(
6366
throws CelEvaluationException {
6467
try {
6568
Object result = overload.invoke(args);
66-
return valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result));
69+
return convertAndAdaptResult(valueConverter, result);
6770
} catch (RuntimeException e) {
6871
throw handleDispatchException(e, overload, args);
6972
}
@@ -77,7 +80,7 @@ static Object dispatch(
7780
throws CelEvaluationException {
7881
try {
7982
Object result = overload.invoke(arg);
80-
return valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result));
83+
return convertAndAdaptResult(valueConverter, result);
8184
} catch (RuntimeException e) {
8285
throw handleDispatchException(e, overload, arg);
8386
}
@@ -92,12 +95,22 @@ static Object dispatch(
9295
throws CelEvaluationException {
9396
try {
9497
Object result = overload.invoke(arg1, arg2);
95-
return valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result));
98+
return convertAndAdaptResult(valueConverter, result);
9699
} catch (RuntimeException e) {
97100
throw handleDispatchException(e, overload, arg1, arg2);
98101
}
99102
}
100103

104+
/**
105+
* Converts the raw invocation result into a CEL runtime value, unwraps it if necessary, and
106+
* adapts any public {@link CelUnknownSet} instances into internal {@link AccumulatedUnknowns} for
107+
* AST evaluation.
108+
*/
109+
private static Object convertAndAdaptResult(CelValueConverter valueConverter, Object result) {
110+
return InterpreterUtil.maybeAdaptToAccumulatedUnknowns(
111+
valueConverter.maybeUnwrap(valueConverter.toRuntimeValue(result)));
112+
}
113+
101114
private static RuntimeException handleDispatchException(
102115
RuntimeException e, CelResolvedOverload overload, Object... args) {
103116
if (e instanceof CelRuntimeException) {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,9 @@ public NamespacedAttribute addQualifier(Qualifier qualifier) {
184184

185185
private static Object applyQualifiers(
186186
Object value, CelValueConverter celValueConverter, ImmutableList<Qualifier> qualifiers) {
187+
if (value instanceof AccumulatedUnknowns) {
188+
return value;
189+
}
187190
Object obj = celValueConverter.toRuntimeValue(value);
188191

189192
// Avoid enhanced for loop to prevent UnmodifiableIterator from being allocated

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

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

0 commit comments

Comments
 (0)