From 553ab00cc6dc2f00908081799e97d350c6f74545 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Tue, 24 Feb 2026 20:52:37 -0800 Subject: [PATCH 1/2] Integrate conformance test suite --- .bazelversion | 1 + .../test/java/dev/cel/conformance/BUILD.bazel | 176 +++++++++++++++++- .../dev/cel/conformance/ConformanceTest.java | 28 ++- .../conformance/ConformanceTestRunner.java | 5 +- .../dev/cel/conformance/conformance_test.bzl | 14 +- 5 files changed, 208 insertions(+), 16 deletions(-) create mode 100644 .bazelversion diff --git a/.bazelversion b/.bazelversion new file mode 100644 index 000000000..f9c71a52e --- /dev/null +++ b/.bazelversion @@ -0,0 +1 @@ +8.5.1 diff --git a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel index ab7468e54..144cca4c6 100644 --- a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel +++ b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel @@ -29,6 +29,7 @@ java_library( "//parser:parser_builder", "//parser:parser_factory", "//runtime", + "//runtime:runtime_planner_impl", "//testing:expr_value_utils", "@cel_spec//proto/cel/expr:expr_java_proto", "@cel_spec//proto/cel/expr/conformance/proto2:test_all_types_java_proto", @@ -57,6 +58,7 @@ java_library( deps = MAVEN_JAR_DEPS + [ "//:java_truth", "//compiler:compiler_builder", + "//runtime:runtime_planner_impl", "//testing:expr_value_utils", "@cel_spec//proto/cel/expr:expr_java_proto", "@cel_spec//proto/cel/expr/conformance/proto2:test_all_types_java_proto", @@ -100,14 +102,10 @@ _ALL_TESTS = [ "@cel_spec//tests/simple:testdata/wrappers.textproto", ] -_TESTS_TO_SKIP = [ +_TESTS_TO_SKIP_LEGACY = [ # Tests which require spec changes. - # TODO: Deprecate Duration.get_milliseconds - "timestamps/duration_converters/get_milliseconds", # Broken test cases which should be supported. - # TODO: Invalid bytes to string conversion should error. - "conversions/string/bytes_invalid", # TODO: Support setting / getting enum values out of the defined enum value range. "enums/legacy_proto2/select_big,select_neg", "enums/legacy_proto2/assign_standalone_int_big,assign_standalone_int_neg", @@ -159,17 +157,174 @@ _TESTS_TO_SKIP = [ "type_deductions/legacy_nullable_types/null_assignable_to_timestamp_parameter_candidate", ] +_TESTS_TO_SKIP_PLANNER = [ + "string_ext/quote", + "string_ext/format", + "string_ext/format_errors", + "string_ext/type_errors", + "basic/functions/unbound", + "basic/functions/unbound_is_runtime_error", + "comparisons/eq_wrapper/eq_proto2", + "comparisons/eq_wrapper/eq_proto3", + "comparisons/eq_wrapper/eq_proto2_missing_fields_neq", + "comparisons/eq_wrapper/eq_proto3_missing_fields_neq", + "comparisons/eq_wrapper/eq_proto_nan_equal", + "comparisons/eq_wrapper/eq_proto_different_types", + "comparisons/eq_wrapper/eq_proto2_any_unpack_equal", + "comparisons/eq_wrapper/eq_proto2_any_unpack_not_equal", + "comparisons/eq_wrapper/eq_proto2_any_unpack_bytewise_fallback_not_equal", + "comparisons/eq_wrapper/eq_proto2_any_unpack_bytewise_fallback_equal", + "comparisons/eq_wrapper/eq_proto3_any_unpack_equal", + "comparisons/eq_wrapper/eq_proto3_any_unpack_not_equal", + "comparisons/eq_wrapper/eq_proto3_any_unpack_bytewise_fallback_not_equal", + "comparisons/eq_wrapper/eq_proto3_any_unpack_bytewise_fallback_equal", + "comparisons/ne_literal/ne_proto2", + "comparisons/ne_literal/ne_proto3", + "comparisons/ne_literal/ne_proto2_missing_fields_neq", + "comparisons/ne_literal/ne_proto3_missing_fields_neq", + "comparisons/ne_literal/ne_proto_nan_not_equal", + "comparisons/ne_literal/ne_proto_different_types", + "comparisons/ne_literal/ne_proto2_any_unpack", + "comparisons/ne_literal/ne_proto2_any_unpack_bytewise_fallback", + "comparisons/ne_literal/ne_proto3_any_unpack", + "comparisons/ne_literal/ne_proto3_any_unpack_bytewise_fallback", + "conversions/int/double_int_min_range", + "conversions/type/dyn_no_denotation", + "dynamic/int32/var", + "dynamic/int64/var", + "dynamic/uint32/var", + "dynamic/uint64/var", + "dynamic/float/var", + "dynamic/double/var", + "dynamic/bool/var", + "dynamic/string/var", + "dynamic/bytes/var", + "dynamic/list/var", + "dynamic/struct/var", + "dynamic/struct/field_assign_proto2_bad", + "dynamic/struct/field_assign_proto3_bad", + "dynamic/value_null/literal_no_field_access", + "dynamic/value_null/var", + "dynamic/value_number/var", + "dynamic/value_string/var", + "dynamic/value_bool/var", + "dynamic/value_struct/var", + "dynamic/value_list/var", + "dynamic/any/var", + "enums/legacy_proto3/assign_standalone_int_too_big", + "enums/legacy_proto3/assign_standalone_int_too_neg", + "enums/strong_proto2/literal_global", + "enums/strong_proto2/literal_nested", + "enums/strong_proto2/literal_zero", + "enums/strong_proto2/type_global", + "enums/strong_proto2/type_nested", + "enums/strong_proto2/select_default", + "enums/strong_proto2/field_type", + "enums/strong_proto2/assign_standalone_int", + "enums/strong_proto2/convert_unnamed_to_int", + "enums/strong_proto2/convert_int_inrange", + "enums/strong_proto2/convert_int_big", + "enums/strong_proto2/convert_int_neg", + "enums/strong_proto2/convert_int_too_big", + "enums/strong_proto2/convert_int_too_neg", + "enums/strong_proto2/convert_string", + "enums/strong_proto2/convert_string_bad", + "enums/strong_proto3/literal_global", + "enums/strong_proto3/literal_nested", + "enums/strong_proto3/literal_zero", + "enums/strong_proto3/type_global", + "enums/strong_proto3/type_nested", + "enums/strong_proto3/select_default", + "enums/strong_proto3/select", + "enums/strong_proto3/select_big", + "enums/strong_proto3/select_neg", + "enums/strong_proto3/field_type", + "enums/strong_proto3/assign_standalone_int", + "enums/strong_proto3/assign_standalone_int_big", + "enums/strong_proto3/assign_standalone_int_neg", + "enums/strong_proto3/convert_unnamed_to_int", + "enums/strong_proto3/convert_int_inrange", + "enums/strong_proto3/convert_int_big", + "enums/strong_proto3/convert_int_neg", + "enums/strong_proto3/convert_int_too_big", + "enums/strong_proto3/convert_int_too_neg", + "enums/strong_proto3/convert_string", + "enums/strong_proto3/convert_string_bad", + "fields/qualified_identifier_resolution/map_key_float", + "fields/qualified_identifier_resolution/map_key_null", + "fields/qualified_identifier_resolution/map_value_repeat_key_heterogeneous", + "logic/AND/short_circuit_type_right", + "logic/OR/short_circuit_type_right", + "math_ext/sign/dyn_error", + "math_ext/bit_xor/dyn_dyn_error", + "math_ext/bit_not/dyn_error", + "math_ext/bit_shift_left/dyn_int_error", + "math_ext/bit_shift_right/dyn_int_error", + "namespace/namespace/self_eval_container_lookup_unchecked", + "optionals/optionals/map_null_entry_no_such_key", + "optionals/optionals/map_present_key_invalid_field", + "proto2/has/undefined", + "proto2/set_null/single_message", + "proto2/set_null/single_duration", + "proto2/set_null/single_timestamp", + "proto2/set_null/single_scalar", + "proto2/set_null/repeated", + "proto2/set_null/map", + "proto2/set_null/list_value", + "proto2/set_null/single_struct", + "proto2/quoted_fields/set_field_with_quoted_name", + "proto2/extensions_get/package_scoped_nested_ext", + "proto2/extensions_get/package_scoped_test_all_types_ext", + "proto2/extensions_get/package_scoped_repeated_test_all_types", + "proto2/extensions_get/message_scoped_nested_ext", + "proto2/extensions_get/message_scoped_repeated_test_all_types", + "proto2_ext/get_ext/package_scoped_nested_ext", + "proto2_ext/get_ext/package_scoped_test_all_types_ext", + "proto2_ext/get_ext/package_scoped_repeated_test_all_types", + "proto2_ext/get_ext/message_scoped_nested_ext", + "proto2_ext/get_ext/message_scoped_repeated_test_all_types", + "proto3/has/undefined", + "proto3/set_null/single_message", + "proto3/set_null/single_duration", + "proto3/set_null/single_timestamp", + "proto3/set_null/single_scalar", + "proto3/set_null/repeated", + "proto3/set_null/map", + "proto3/set_null/list_value", + "proto3/set_null/single_struct", + "proto3/quoted_fields/set_field", + "timestamps/duration_converters/get_milliseconds", + "timestamps/timestamp_range/sub_time_duration_over", + "timestamps/timestamp_range/sub_time_duration_under", + "type_deductions/wrappers/wrapper_promotion", + "type_deductions/legacy_nullable_types/null_assignable_to_message_parameter_candidate", + "type_deductions/legacy_nullable_types/null_assignable_to_duration_parameter_candidate", + "type_deductions/legacy_nullable_types/null_assignable_to_timestamp_parameter_candidate", + "type_deductions/legacy_nullable_types/null_assignable_to_abstract_parameter_candidate", + "wrappers/bool/to_null", + "wrappers/int32/to_null", + "wrappers/int64/to_null", + "wrappers/uint32/to_null", + "wrappers/uint64/to_null", + "wrappers/float/to_null", + "wrappers/double/to_null", + "wrappers/bytes/to_null", + "wrappers/string/to_null", + "wrappers/field_mask/to_json", + "wrappers/empty/to_json", +] + conformance_test( name = "conformance", data = _ALL_TESTS, - skip_tests = _TESTS_TO_SKIP, + skip_tests = _TESTS_TO_SKIP_LEGACY, ) conformance_test( name = "conformance_maven", data = _ALL_TESTS, mode = MODE.MAVEN_TEST, - skip_tests = _TESTS_TO_SKIP, + skip_tests = _TESTS_TO_SKIP_LEGACY, ) conformance_test( @@ -177,3 +332,10 @@ conformance_test( data = _ALL_TESTS, mode = MODE.DASHBOARD, ) + +conformance_test( + name = "conformance_planner", + data = _ALL_TESTS, + skip_tests = _TESTS_TO_SKIP_PLANNER, + use_planner = True, +) diff --git a/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java b/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java index dcd226fa5..ce52bda4d 100644 --- a/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java +++ b/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java @@ -46,6 +46,7 @@ import dev.cel.runtime.CelRuntime; import dev.cel.runtime.CelRuntime.Program; import dev.cel.runtime.CelRuntimeFactory; +import dev.cel.runtime.CelRuntimeImpl; import dev.cel.runtime.CelRuntimeLibrary; import java.util.Map; import org.junit.runners.model.Statement; @@ -128,6 +129,17 @@ private static CelChecker getChecker(SimpleTest test) throws Exception { .addFileTypes(dev.cel.expr.conformance.proto2.TestAllTypesExtensions.getDescriptor()) .build(); + private static final CelRuntime PLANNER_RUNTIME = + CelRuntimeImpl.newBuilder() + .setOptions(OPTIONS) + .addLibraries(CANONICAL_RUNTIME_EXTENSIONS) + .setExtensionRegistry(DEFAULT_EXTENSION_REGISTRY) + .addMessageTypes(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor()) + .addMessageTypes(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor()) + .addFileTypes(dev.cel.expr.conformance.proto2.TestAllTypesExtensions.getDescriptor()) + .build(); + + private static ImmutableMap getBindings(SimpleTest test) throws Exception { ImmutableMap.Builder bindings = ImmutableMap.builderWithExpectedSize(test.getBindingsCount()); @@ -157,13 +169,15 @@ private static SimpleTest defaultTestMatcherToTrueIfUnset(SimpleTest test) { private final String name; private final SimpleTest test; private final boolean skip; + private final boolean usePlanner; - public ConformanceTest(String name, SimpleTest test, boolean skip) { + public ConformanceTest(String name, SimpleTest test, boolean skip, boolean usePlanner) { this.name = Preconditions.checkNotNull(name); this.test = Preconditions.checkNotNull( defaultTestMatcherToTrueIfUnset(Preconditions.checkNotNull(test))); this.skip = skip; + this.usePlanner = usePlanner; } public String getName() { @@ -178,7 +192,9 @@ public boolean shouldSkip() { public void evaluate() throws Throwable { CelValidationResult response = getParser(test).parse(test.getExpr(), test.getName()); assertThat(response.hasError()).isFalse(); - response = getChecker(test).check(response.getAst()); + if (!test.getDisableCheck()) { + response = getChecker(test).check(response.getAst()); + } assertThat(response.hasError()).isFalse(); Type resultType = CelProtoTypes.celTypeToType(response.getAst().getResultType()); @@ -188,7 +204,13 @@ public void evaluate() throws Throwable { return; } - Program program = RUNTIME.createProgram(response.getAst()); + if (!usePlanner && test.getDisableCheck()) { + // Only planner supports parsed-only evaluation + return; + } + + CelRuntime runtime = usePlanner ? PLANNER_RUNTIME : RUNTIME; + Program program = runtime.createProgram(response.getAst()); ExprValue result = null; CelEvaluationException error = null; try { diff --git a/conformance/src/test/java/dev/cel/conformance/ConformanceTestRunner.java b/conformance/src/test/java/dev/cel/conformance/ConformanceTestRunner.java index 89598ed3e..a0610f4a8 100644 --- a/conformance/src/test/java/dev/cel/conformance/ConformanceTestRunner.java +++ b/conformance/src/test/java/dev/cel/conformance/ConformanceTestRunner.java @@ -43,6 +43,7 @@ public final class ConformanceTestRunner extends ParentRunner { private final ImmutableSortedMap testFiles; private final ImmutableList testsToSkip; + private final boolean usePlanner; private static ImmutableSortedMap loadTestFiles() { List testPaths = @@ -75,6 +76,8 @@ public ConformanceTestRunner(Class clazz) throws InitializationError { ImmutableList.copyOf( SPLITTER.splitToList( System.getProperty("dev.cel.conformance.ConformanceTests.skip_tests"))); + usePlanner = Boolean.parseBoolean( + System.getProperty("dev.cel.conformance.ConformanceTests.use_planner", "false")); } private boolean shouldSkipTest(String name) { @@ -98,7 +101,7 @@ protected List getChildren() { String name = String.format("%s/%s/%s", testFile.getName(), testSection.getName(), test.getName()); tests.add( - new ConformanceTest(name, test, test.getDisableCheck() || shouldSkipTest(name))); + new ConformanceTest(name, test, shouldSkipTest(name), usePlanner)); } } } diff --git a/conformance/src/test/java/dev/cel/conformance/conformance_test.bzl b/conformance/src/test/java/dev/cel/conformance/conformance_test.bzl index f884a2a84..e7b8acb4a 100644 --- a/conformance/src/test/java/dev/cel/conformance/conformance_test.bzl +++ b/conformance/src/test/java/dev/cel/conformance/conformance_test.bzl @@ -38,10 +38,14 @@ def _expand_tests_to_skip(tests_to_skip): result.append(test_to_skip[0:slash] + part) return result -def _conformance_test_args(data, skip_tests): +def _conformance_test_args(data, skip_tests, use_planner): args = [] args.append("-Ddev.cel.conformance.ConformanceTests.skip_tests={}".format(",".join(_expand_tests_to_skip(skip_tests)))) args.append("-Ddev.cel.conformance.ConformanceTests.tests={}".format(",".join(["$(location " + test + ")" for test in data]))) + if use_planner: + args.append("-Ddev.cel.conformance.ConformanceTests.use_planner=true") + else: + args.append("-Ddev.cel.conformance.ConformanceTests.use_planner=false") return args MODE = struct( @@ -53,7 +57,7 @@ MODE = struct( DASHBOARD = "dashboard", ) -def conformance_test(name, data, mode = MODE.TEST, skip_tests = []): +def conformance_test(name, data, mode = MODE.TEST, skip_tests = [], use_planner = False): """Executes conformance tests Args: @@ -69,7 +73,7 @@ def conformance_test(name, data, mode = MODE.TEST, skip_tests = []): if mode == MODE.DASHBOARD: java_test( name = "_" + name, - jvm_flags = _conformance_test_args(data, skip_tests), + jvm_flags = _conformance_test_args(data, skip_tests, use_planner), data = data, size = "small", test_class = "dev.cel.conformance.ConformanceTests", @@ -95,7 +99,7 @@ def conformance_test(name, data, mode = MODE.TEST, skip_tests = []): elif mode == MODE.TEST: java_test( name = name, - jvm_flags = _conformance_test_args(data, skip_tests), + jvm_flags = _conformance_test_args(data, skip_tests, use_planner), data = data, size = "small", test_class = "dev.cel.conformance.ConformanceTests", @@ -104,7 +108,7 @@ def conformance_test(name, data, mode = MODE.TEST, skip_tests = []): elif mode == MODE.MAVEN_TEST: java_test( name = name, - jvm_flags = _conformance_test_args(data, skip_tests), + jvm_flags = _conformance_test_args(data, skip_tests, use_planner), data = data, size = "small", test_class = "dev.cel.conformance.ConformanceTests", From 3a28aa956fe7b1236683a21cb779903c20bebca2 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Tue, 24 Feb 2026 21:36:13 -0800 Subject: [PATCH 2/2] Conformance fixes --- .../java/dev/cel/bundle/CelEnvironment.java | 4 + .../java/dev/cel/common/types/SimpleType.java | 1 - .../values/BaseProtoCelValueConverter.java | 17 ++ .../cel/common/values/CelValueConverter.java | 4 + .../test/java/dev/cel/conformance/BUILD.bazel | 146 ++++-------------- .../dev/cel/conformance/ConformanceTest.java | 37 +++-- .../dev/cel/extensions/CelMathExtensions.java | 12 +- .../cel/extensions/CelRegexExtensions.java | 2 +- .../cel/extensions/CelStringExtensions.java | 2 +- .../src/main/java/dev/cel/runtime/BUILD.bazel | 1 + .../java/dev/cel/runtime/CelRuntimeImpl.java | 3 +- .../java/dev/cel/runtime/planner/EvalAnd.java | 6 +- .../java/dev/cel/runtime/planner/EvalOr.java | 6 +- .../runtime/planner/NamespacedAttribute.java | 6 +- .../runtime/planner/ProgramPlannerTest.java | 1 - 15 files changed, 98 insertions(+), 150 deletions(-) diff --git a/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java b/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java index 7ec2149a7..9db8d26e6 100644 --- a/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java +++ b/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java @@ -638,6 +638,10 @@ public CelType toCelType(CelTypeProvider celTypeProvider) { return TypeParamType.create(name()); } + if (name().equals("dyn")) { + return SimpleType.DYN; + } + CelType simpleType = SimpleType.findByName(name()).orElse(null); if (simpleType != null) { return simpleType; diff --git a/common/src/main/java/dev/cel/common/types/SimpleType.java b/common/src/main/java/dev/cel/common/types/SimpleType.java index 6c43ab53f..93bd5326d 100644 --- a/common/src/main/java/dev/cel/common/types/SimpleType.java +++ b/common/src/main/java/dev/cel/common/types/SimpleType.java @@ -46,7 +46,6 @@ public abstract class SimpleType extends CelType { public static final ImmutableMap TYPE_MAP = ImmutableMap.of( - DYN.name(), DYN, BOOL.name(), BOOL, BYTES.name(), BYTES, DOUBLE.name(), DOUBLE, diff --git a/common/src/main/java/dev/cel/common/values/BaseProtoCelValueConverter.java b/common/src/main/java/dev/cel/common/values/BaseProtoCelValueConverter.java index b05a21e24..d002be885 100644 --- a/common/src/main/java/dev/cel/common/values/BaseProtoCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/BaseProtoCelValueConverter.java @@ -17,6 +17,8 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; +import com.google.common.base.CaseFormat; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -27,6 +29,8 @@ import com.google.protobuf.BytesValue; import com.google.protobuf.DoubleValue; import com.google.protobuf.Duration; +import com.google.protobuf.Empty; +import com.google.protobuf.FieldMask; import com.google.protobuf.FloatValue; import com.google.protobuf.Int32Value; import com.google.protobuf.Int64Value; @@ -41,6 +45,8 @@ import dev.cel.common.annotations.Internal; import dev.cel.common.internal.ProtoTimeUtils; import dev.cel.common.internal.WellKnownProto; +import java.util.ArrayList; +import java.util.List; /** * {@code BaseProtoCelValueConverter} contains the common logic for converting between native Java @@ -98,6 +104,17 @@ protected Object fromWellKnownProto(MessageLiteOrBuilder message, WellKnownProto return UnsignedLong.valueOf(((UInt32Value) message).getValue()); case UINT64_VALUE: return UnsignedLong.fromLongBits(((UInt64Value) message).getValue()); + case FIELD_MASK: + FieldMask fieldMask = (FieldMask) message; + List paths = new ArrayList<>(fieldMask.getPathsCount()); + for (String path : fieldMask.getPathsList()) { + if (!path.isEmpty()) { + paths.add(CaseFormat.LOWER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, path)); + } + } + return normalizePrimitive(Joiner.on(",").join(paths)); + case EMPTY: + return ImmutableMap.of(); default: throw new UnsupportedOperationException( "Unsupported well known proto conversion - " + wellKnownProto); diff --git a/common/src/main/java/dev/cel/common/values/CelValueConverter.java b/common/src/main/java/dev/cel/common/values/CelValueConverter.java index ae0b40ef7..fda014f31 100644 --- a/common/src/main/java/dev/cel/common/values/CelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/CelValueConverter.java @@ -54,6 +54,10 @@ public Object unwrap(CelValue celValue) { return Optional.of(optionalValue.value()); } + if (celValue instanceof ErrorValue) { + return celValue; + } + return celValue.value(); } diff --git a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel index 144cca4c6..faaa9d4aa 100644 --- a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel +++ b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel @@ -114,7 +114,6 @@ _TESTS_TO_SKIP_LEGACY = [ # TODO: Ensure overflow occurs on conversions of double values which might not work properly on all platforms. "conversions/int/double_int_min_range", # TODO: Duration and timestamp operations should error on overflow. - "timestamps/duration_range/from_string_under,from_string_over", "timestamps/timestamp_range/sub_time_duration_over,sub_time_duration_under", # TODO: Ensure adding negative duration values is appropriately supported. "timestamps/timestamp_arithmetic/add_time_to_duration_nanos_negative", @@ -158,112 +157,42 @@ _TESTS_TO_SKIP_LEGACY = [ ] _TESTS_TO_SKIP_PLANNER = [ + # TODO: Add strings.format and strings.quote. "string_ext/quote", "string_ext/format", "string_ext/format_errors", - "string_ext/type_errors", + + # TODO: Check behavior for go/cpp "basic/functions/unbound", "basic/functions/unbound_is_runtime_error", - "comparisons/eq_wrapper/eq_proto2", - "comparisons/eq_wrapper/eq_proto3", - "comparisons/eq_wrapper/eq_proto2_missing_fields_neq", - "comparisons/eq_wrapper/eq_proto3_missing_fields_neq", - "comparisons/eq_wrapper/eq_proto_nan_equal", - "comparisons/eq_wrapper/eq_proto_different_types", - "comparisons/eq_wrapper/eq_proto2_any_unpack_equal", - "comparisons/eq_wrapper/eq_proto2_any_unpack_not_equal", - "comparisons/eq_wrapper/eq_proto2_any_unpack_bytewise_fallback_not_equal", - "comparisons/eq_wrapper/eq_proto2_any_unpack_bytewise_fallback_equal", - "comparisons/eq_wrapper/eq_proto3_any_unpack_equal", - "comparisons/eq_wrapper/eq_proto3_any_unpack_not_equal", - "comparisons/eq_wrapper/eq_proto3_any_unpack_bytewise_fallback_not_equal", - "comparisons/eq_wrapper/eq_proto3_any_unpack_bytewise_fallback_equal", - "comparisons/ne_literal/ne_proto2", - "comparisons/ne_literal/ne_proto3", - "comparisons/ne_literal/ne_proto2_missing_fields_neq", - "comparisons/ne_literal/ne_proto3_missing_fields_neq", - "comparisons/ne_literal/ne_proto_nan_not_equal", - "comparisons/ne_literal/ne_proto_different_types", - "comparisons/ne_literal/ne_proto2_any_unpack", - "comparisons/ne_literal/ne_proto2_any_unpack_bytewise_fallback", - "comparisons/ne_literal/ne_proto3_any_unpack", - "comparisons/ne_literal/ne_proto3_any_unpack_bytewise_fallback", + + # TODO: Ensure overflow occurs on conversions of double values which might not work properly on all platforms. "conversions/int/double_int_min_range", - "conversions/type/dyn_no_denotation", - "dynamic/int32/var", - "dynamic/int64/var", - "dynamic/uint32/var", - "dynamic/uint64/var", - "dynamic/float/var", - "dynamic/double/var", - "dynamic/bool/var", - "dynamic/string/var", - "dynamic/bytes/var", - "dynamic/list/var", - "dynamic/struct/var", - "dynamic/struct/field_assign_proto2_bad", - "dynamic/struct/field_assign_proto3_bad", - "dynamic/value_null/literal_no_field_access", - "dynamic/value_null/var", - "dynamic/value_number/var", - "dynamic/value_string/var", - "dynamic/value_bool/var", - "dynamic/value_struct/var", - "dynamic/value_list/var", - "dynamic/any/var", "enums/legacy_proto3/assign_standalone_int_too_big", "enums/legacy_proto3/assign_standalone_int_too_neg", - "enums/strong_proto2/literal_global", - "enums/strong_proto2/literal_nested", - "enums/strong_proto2/literal_zero", - "enums/strong_proto2/type_global", - "enums/strong_proto2/type_nested", - "enums/strong_proto2/select_default", - "enums/strong_proto2/field_type", - "enums/strong_proto2/assign_standalone_int", - "enums/strong_proto2/convert_unnamed_to_int", - "enums/strong_proto2/convert_int_inrange", - "enums/strong_proto2/convert_int_big", - "enums/strong_proto2/convert_int_neg", - "enums/strong_proto2/convert_int_too_big", - "enums/strong_proto2/convert_int_too_neg", - "enums/strong_proto2/convert_string", - "enums/strong_proto2/convert_string_bad", - "enums/strong_proto3/literal_global", - "enums/strong_proto3/literal_nested", - "enums/strong_proto3/literal_zero", - "enums/strong_proto3/type_global", - "enums/strong_proto3/type_nested", - "enums/strong_proto3/select_default", - "enums/strong_proto3/select", - "enums/strong_proto3/select_big", - "enums/strong_proto3/select_neg", - "enums/strong_proto3/field_type", - "enums/strong_proto3/assign_standalone_int", - "enums/strong_proto3/assign_standalone_int_big", - "enums/strong_proto3/assign_standalone_int_neg", - "enums/strong_proto3/convert_unnamed_to_int", - "enums/strong_proto3/convert_int_inrange", - "enums/strong_proto3/convert_int_big", - "enums/strong_proto3/convert_int_neg", - "enums/strong_proto3/convert_int_too_big", - "enums/strong_proto3/convert_int_too_neg", - "enums/strong_proto3/convert_string", - "enums/strong_proto3/convert_string_bad", + + # Future features for CEL 1.0 + # TODO: Strong typing support for enums, specified but not implemented. + "enums/strong_proto2", + "enums/strong_proto3", + + # Skip until fixed. "fields/qualified_identifier_resolution/map_key_float", "fields/qualified_identifier_resolution/map_key_null", "fields/qualified_identifier_resolution/map_value_repeat_key_heterogeneous", - "logic/AND/short_circuit_type_right", - "logic/OR/short_circuit_type_right", + "math_ext/sign/dyn_error", "math_ext/bit_xor/dyn_dyn_error", "math_ext/bit_not/dyn_error", "math_ext/bit_shift_left/dyn_int_error", "math_ext/bit_shift_right/dyn_int_error", + "namespace/namespace/self_eval_container_lookup_unchecked", + "optionals/optionals/map_null_entry_no_such_key", "optionals/optionals/map_present_key_invalid_field", - "proto2/has/undefined", + + # TODO: Fix null assignment to a field "proto2/set_null/single_message", "proto2/set_null/single_duration", "proto2/set_null/single_timestamp", @@ -272,8 +201,15 @@ _TESTS_TO_SKIP_PLANNER = [ "proto2/set_null/map", "proto2/set_null/list_value", "proto2/set_null/single_struct", - "proto2/quoted_fields/set_field_with_quoted_name", - "proto2/extensions_get/package_scoped_nested_ext", + "proto3/set_null/single_message", + "proto3/set_null/single_duration", + "proto3/set_null/single_timestamp", + "proto3/set_null/single_scalar", + "proto3/set_null/repeated", + "proto3/set_null/map", + "proto3/set_null/list_value", + "proto3/set_null/single_struct", + "proto2/extensions_get/package_scoped_test_all_types_ext", "proto2/extensions_get/package_scoped_repeated_test_all_types", "proto2/extensions_get/message_scoped_nested_ext", @@ -283,35 +219,19 @@ _TESTS_TO_SKIP_PLANNER = [ "proto2_ext/get_ext/package_scoped_repeated_test_all_types", "proto2_ext/get_ext/message_scoped_nested_ext", "proto2_ext/get_ext/message_scoped_repeated_test_all_types", - "proto3/has/undefined", - "proto3/set_null/single_message", - "proto3/set_null/single_duration", - "proto3/set_null/single_timestamp", - "proto3/set_null/single_scalar", - "proto3/set_null/repeated", - "proto3/set_null/map", - "proto3/set_null/list_value", - "proto3/set_null/single_struct", - "proto3/quoted_fields/set_field", - "timestamps/duration_converters/get_milliseconds", + # TODO: Duration and timestamp operations should error on overflow. "timestamps/timestamp_range/sub_time_duration_over", "timestamps/timestamp_range/sub_time_duration_under", + + # Type inference edgecases around null(able) assignability. + # These type check, but resolve to a different type. + # list(int), want list(wrapper(int)) "type_deductions/wrappers/wrapper_promotion", + # list(null), want list(Message) "type_deductions/legacy_nullable_types/null_assignable_to_message_parameter_candidate", + "type_deductions/legacy_nullable_types/null_assignable_to_abstract_parameter_candidate", "type_deductions/legacy_nullable_types/null_assignable_to_duration_parameter_candidate", "type_deductions/legacy_nullable_types/null_assignable_to_timestamp_parameter_candidate", - "type_deductions/legacy_nullable_types/null_assignable_to_abstract_parameter_candidate", - "wrappers/bool/to_null", - "wrappers/int32/to_null", - "wrappers/int64/to_null", - "wrappers/uint32/to_null", - "wrappers/uint64/to_null", - "wrappers/float/to_null", - "wrappers/double/to_null", - "wrappers/bytes/to_null", - "wrappers/string/to_null", - "wrappers/field_mask/to_json", - "wrappers/empty/to_json", ] conformance_test( diff --git a/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java b/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java index ce52bda4d..42e8bde57 100644 --- a/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java +++ b/conformance/src/test/java/dev/cel/conformance/ConformanceTest.java @@ -45,6 +45,7 @@ import dev.cel.runtime.CelEvaluationException; import dev.cel.runtime.CelRuntime; import dev.cel.runtime.CelRuntime.Program; +import dev.cel.runtime.CelRuntimeBuilder; import dev.cel.runtime.CelRuntimeFactory; import dev.cel.runtime.CelRuntimeImpl; import dev.cel.runtime.CelRuntimeLibrary; @@ -119,26 +120,24 @@ private static CelChecker getChecker(SimpleTest test) throws Exception { .build(); } - private static final CelRuntime RUNTIME = - CelRuntimeFactory.standardCelRuntimeBuilder() - .setOptions(OPTIONS) - .addLibraries(CANONICAL_RUNTIME_EXTENSIONS) - .setExtensionRegistry(DEFAULT_EXTENSION_REGISTRY) - .addMessageTypes(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor()) - .addMessageTypes(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor()) - .addFileTypes(dev.cel.expr.conformance.proto2.TestAllTypesExtensions.getDescriptor()) - .build(); + private static CelRuntime getRuntime(SimpleTest test, boolean usePlanner) { + CelRuntimeBuilder builder = + usePlanner ? CelRuntimeImpl.newBuilder() : CelRuntimeFactory.standardCelRuntimeBuilder(); - private static final CelRuntime PLANNER_RUNTIME = - CelRuntimeImpl.newBuilder() - .setOptions(OPTIONS) - .addLibraries(CANONICAL_RUNTIME_EXTENSIONS) - .setExtensionRegistry(DEFAULT_EXTENSION_REGISTRY) - .addMessageTypes(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor()) - .addMessageTypes(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor()) - .addFileTypes(dev.cel.expr.conformance.proto2.TestAllTypesExtensions.getDescriptor()) - .build(); + builder + .setOptions(OPTIONS) + .addLibraries(CANONICAL_RUNTIME_EXTENSIONS) + .setExtensionRegistry(DEFAULT_EXTENSION_REGISTRY) + .addMessageTypes(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor()) + .addMessageTypes(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor()) + .addFileTypes(dev.cel.expr.conformance.proto2.TestAllTypesExtensions.getDescriptor()); + if (usePlanner) { + builder.setContainer(CelContainer.ofName(test.getContainer())); + } + + return builder.build(); + } private static ImmutableMap getBindings(SimpleTest test) throws Exception { ImmutableMap.Builder bindings = @@ -209,7 +208,7 @@ public void evaluate() throws Throwable { return; } - CelRuntime runtime = usePlanner ? PLANNER_RUNTIME : RUNTIME; + CelRuntime runtime = getRuntime(test, usePlanner); Program program = runtime.createProgram(response.getAst()); ExprValue result = null; CelEvaluationException error = null; diff --git a/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java b/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java index 57c8c1378..115fc1336 100644 --- a/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java +++ b/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java @@ -663,9 +663,15 @@ String getFunction() { ImmutableSet functionBindingsULongSigned, ImmutableSet functionBindingsULongUnsigned) { this.functionDecl = functionDecl; - this.functionBindings = functionBindings; - this.functionBindingsULongSigned = functionBindingsULongSigned; - this.functionBindingsULongUnsigned = functionBindingsULongUnsigned; + this.functionBindings = functionBindings.isEmpty() + ? ImmutableSet.of() + : CelFunctionBinding.fromOverloads(functionDecl.name(), functionBindings); + this.functionBindingsULongSigned = functionBindingsULongSigned.isEmpty() + ? ImmutableSet.of() + : CelFunctionBinding.fromOverloads(functionDecl.name(), functionBindingsULongSigned); + this.functionBindingsULongUnsigned = functionBindingsULongUnsigned.isEmpty() + ? ImmutableSet.of() + : CelFunctionBinding.fromOverloads(functionDecl.name(), functionBindingsULongUnsigned); } } diff --git a/extensions/src/main/java/dev/cel/extensions/CelRegexExtensions.java b/extensions/src/main/java/dev/cel/extensions/CelRegexExtensions.java index f1ed3b478..bfbcac487 100644 --- a/extensions/src/main/java/dev/cel/extensions/CelRegexExtensions.java +++ b/extensions/src/main/java/dev/cel/extensions/CelRegexExtensions.java @@ -123,7 +123,7 @@ String getFunction() { Function(CelFunctionDecl functionDecl, ImmutableSet functionBindings) { this.functionDecl = functionDecl; - this.functionBindings = functionBindings; + this.functionBindings = CelFunctionBinding.fromOverloads(functionDecl.name(), functionBindings); } } diff --git a/extensions/src/main/java/dev/cel/extensions/CelStringExtensions.java b/extensions/src/main/java/dev/cel/extensions/CelStringExtensions.java index 37b2a368b..4166b809a 100644 --- a/extensions/src/main/java/dev/cel/extensions/CelStringExtensions.java +++ b/extensions/src/main/java/dev/cel/extensions/CelStringExtensions.java @@ -238,7 +238,7 @@ String getFunction() { Function(CelFunctionDecl functionDecl, CelFunctionBinding... functionBindings) { this.functionDecl = functionDecl; - this.functionBindings = ImmutableSet.copyOf(functionBindings); + this.functionBindings = CelFunctionBinding.fromOverloads(functionDecl.name(), functionBindings); } } diff --git a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel index 0746a5b83..8951c2446 100644 --- a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel @@ -827,6 +827,7 @@ java_library( ":function_binding", ":function_resolver", ":program", + ":proto_message_runtime_equality", ":proto_message_runtime_helpers", ":runtime", ":runtime_equality", diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java index e910c77a9..12ac4f040 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java @@ -425,8 +425,7 @@ public CelRuntime build() { } RuntimeEquality runtimeEquality = - RuntimeEquality.create( - ProtoMessageRuntimeHelpers.create(dynamicProto, options()), options()); + ProtoMessageRuntimeEquality.create(dynamicProto, options()); ImmutableSet runtimeLibraries = runtimeLibrariesBuilder().build(); // Add libraries, such as extensions for (CelRuntimeLibrary celLibrary : runtimeLibraries) { diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalAnd.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalAnd.java index b09191e9f..04c6f7e75 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalAnd.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalAnd.java @@ -39,8 +39,10 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) { errorValue = (ErrorValue) argVal; } else { // TODO: Handle unknowns - throw new IllegalArgumentException( - String.format("Expected boolean value, found: %s", argVal)); + errorValue = ErrorValue.create( + arg.exprId(), + new IllegalArgumentException( + String.format("Expected boolean value, found: %s", argVal))); } } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalOr.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalOr.java index 8c8f5954d..b755add74 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalOr.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalOr.java @@ -39,8 +39,10 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) { errorValue = (ErrorValue) argVal; } else { // TODO: Handle unknowns - throw new IllegalArgumentException( - String.format("Expected boolean value, found: %s", argVal)); + errorValue = ErrorValue.create( + arg.exprId(), + new IllegalArgumentException( + String.format("Expected boolean value, found: %s", argVal))); } } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java index 6bdf0c072..de1a90291 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java @@ -62,11 +62,7 @@ public Object resolve(GlobalResolver ctx, ExecutionFrame frame) { Object value = resolver.resolve(name); if (value != null) { - if (!qualifiers.isEmpty()) { - return applyQualifiers(value, celValueConverter, qualifiers); - } else { - return value; - } + return applyQualifiers(value, celValueConverter, qualifiers); } // Attempt to resolve the qualify type name if the name is not a variable identifier diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java index 33500f217..7bdcaaac1 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -1055,7 +1055,6 @@ private enum TypeLiteralTestCase { INT("int", SimpleType.INT), UINT("uint", SimpleType.UINT), STRING("string", SimpleType.STRING), - DYN("dyn", SimpleType.DYN), LIST("list", ListType.create(SimpleType.DYN)), MAP("map", MapType.create(SimpleType.DYN, SimpleType.DYN)), NULL("null_type", SimpleType.NULL_TYPE),