Skip to content

Commit 80cade2

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Enable conformance tests for two-var comprehensions.
Add 2-var existsOne, alternative to exists_one that is more consistent with standard naming conventions. PiperOrigin-RevId: 853942944
1 parent a1508fd commit 80cade2

File tree

10 files changed

+135
-32
lines changed

10 files changed

+135
-32
lines changed

‎bundle/src/test/java/dev/cel/bundle/CelEnvironmentExporterTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public void standardLibrarySubset_favorExclusion() throws Exception {
110110
FunctionSelector.create("matches", ImmutableSet.of()),
111111
FunctionSelector.create(
112112
"timestamp", ImmutableSet.of("string_to_timestamp"))))
113-
.setExcludedMacros(ImmutableSet.of("map", "filter"))
113+
.setExcludedMacros(ImmutableSet.of("map", "existsOne", "filter"))
114114
.build());
115115
}
116116

‎common/src/main/java/dev/cel/common/Operator.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ public enum Operator {
4545
HAS("has"),
4646
ALL("all"),
4747
EXISTS("exists"),
48+
@Deprecated // Prefer EXISTS_ONE_NEW.
4849
EXISTS_ONE("exists_one"),
50+
EXISTS_ONE_NEW("existsOne"),
4951
MAP("map"),
5052
FILTER("filter"),
5153
NOT_STRICTLY_FALSE("@not_strictly_false"),
@@ -109,6 +111,7 @@ public static Optional<Operator> find(String text) {
109111
.put(EQUALS.getFunction(), EQUALS)
110112
.put(EXISTS.getFunction(), EXISTS)
111113
.put(EXISTS_ONE.getFunction(), EXISTS_ONE)
114+
.put(EXISTS_ONE_NEW.getFunction(), EXISTS_ONE_NEW)
112115
.put(FILTER.getFunction(), FILTER)
113116
.put(GREATER.getFunction(), GREATER)
114117
.put(GREATER_EQUALS.getFunction(), GREATER_EQUALS)

‎conformance/src/test/java/dev/cel/conformance/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ java_library(
2222
"//common:options",
2323
"//common/types:cel_proto_types",
2424
"//compiler",
25+
"//compiler:compiler_builder",
2526
"//extensions",
2627
"//extensions:optional_library",
2728
"//parser:macro",
@@ -49,6 +50,7 @@ java_library(
4950
tags = ["conformance_maven"],
5051
deps = MAVEN_JAR_DEPS + [
5152
"//:java_truth",
53+
"//compiler:compiler_builder",
5254
"//testing:expr_value_utils",
5355
"@cel_spec//proto/cel/expr:expr_java_proto",
5456
"@cel_spec//proto/cel/expr/conformance/proto2:test_all_types_java_proto",
@@ -75,6 +77,7 @@ _ALL_TESTS = [
7577
"@cel_spec//tests/simple:testdata/lists.textproto",
7678
"@cel_spec//tests/simple:testdata/logic.textproto",
7779
"@cel_spec//tests/simple:testdata/macros.textproto",
80+
"@cel_spec//tests/simple:testdata/macros2.textproto",
7881
"@cel_spec//tests/simple:testdata/math_ext.textproto",
7982
"@cel_spec//tests/simple:testdata/namespace.textproto",
8083
"@cel_spec//tests/simple:testdata/optionals.textproto",

‎conformance/src/test/java/dev/cel/conformance/ConformanceTest.java‎

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import dev.cel.common.CelValidationResult;
3636
import dev.cel.common.types.CelProtoTypes;
3737
import dev.cel.compiler.CelCompilerFactory;
38+
import dev.cel.compiler.CelCompilerLibrary;
3839
import dev.cel.expr.conformance.test.SimpleTest;
3940
import dev.cel.extensions.CelExtensions;
4041
import dev.cel.extensions.CelOptionalLibrary;
@@ -45,6 +46,7 @@
4546
import dev.cel.runtime.CelRuntime;
4647
import dev.cel.runtime.CelRuntime.Program;
4748
import dev.cel.runtime.CelRuntimeFactory;
49+
import dev.cel.runtime.CelRuntimeLibrary;
4850
import java.util.Map;
4951
import org.junit.runners.model.Statement;
5052

@@ -61,31 +63,37 @@ public final class ConformanceTest extends Statement {
6163
.enableQuotedIdentifierSyntax(true)
6264
.build();
6365

66+
private static final ImmutableList<CelCompilerLibrary> CANONICAL_COMPILER_EXTENSIONS =
67+
ImmutableList.of(
68+
CelExtensions.bindings(),
69+
CelExtensions.comprehensions(),
70+
CelExtensions.encoders(OPTIONS),
71+
CelExtensions.math(OPTIONS),
72+
CelExtensions.protos(),
73+
CelExtensions.sets(OPTIONS),
74+
CelExtensions.strings(),
75+
CelOptionalLibrary.INSTANCE);
76+
77+
private static final ImmutableList<CelRuntimeLibrary> CANONICAL_RUNTIME_EXTENSIONS =
78+
ImmutableList.of(
79+
CelExtensions.comprehensions(),
80+
CelExtensions.encoders(OPTIONS),
81+
CelExtensions.math(OPTIONS),
82+
CelExtensions.sets(OPTIONS),
83+
CelExtensions.strings(),
84+
CelOptionalLibrary.INSTANCE);
85+
6486
private static final CelParser PARSER_WITH_MACROS =
6587
CelParserFactory.standardCelParserBuilder()
6688
.setOptions(OPTIONS)
67-
.addLibraries(
68-
CelExtensions.bindings(),
69-
CelExtensions.encoders(OPTIONS),
70-
CelExtensions.math(OPTIONS),
71-
CelExtensions.protos(),
72-
CelExtensions.sets(OPTIONS),
73-
CelExtensions.strings(),
74-
CelOptionalLibrary.INSTANCE)
89+
.addLibraries(CANONICAL_COMPILER_EXTENSIONS)
7590
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
7691
.build();
7792

7893
private static final CelParser PARSER_WITHOUT_MACROS =
7994
CelParserFactory.standardCelParserBuilder()
8095
.setOptions(OPTIONS)
81-
.addLibraries(
82-
CelExtensions.bindings(),
83-
CelExtensions.encoders(OPTIONS),
84-
CelExtensions.math(OPTIONS),
85-
CelExtensions.protos(),
86-
CelExtensions.sets(OPTIONS),
87-
CelExtensions.strings(),
88-
CelOptionalLibrary.INSTANCE)
96+
.addLibraries(CANONICAL_COMPILER_EXTENSIONS)
8997
.setStandardMacros()
9098
.build();
9199

@@ -104,13 +112,7 @@ private static CelChecker getChecker(SimpleTest test) throws Exception {
104112
.setContainer(CelContainer.ofName(test.getContainer()))
105113
.addDeclarations(decls.build())
106114
.addFileTypes(dev.cel.expr.conformance.proto2.TestAllTypesExtensions.getDescriptor())
107-
.addLibraries(
108-
CelExtensions.bindings(),
109-
CelExtensions.encoders(OPTIONS),
110-
CelExtensions.math(OPTIONS),
111-
CelExtensions.sets(OPTIONS),
112-
CelExtensions.strings(),
113-
CelOptionalLibrary.INSTANCE)
115+
.addLibraries(CANONICAL_COMPILER_EXTENSIONS)
114116
.addMessageTypes(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor())
115117
.addMessageTypes(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor())
116118
.build();
@@ -119,12 +121,7 @@ private static CelChecker getChecker(SimpleTest test) throws Exception {
119121
private static final CelRuntime RUNTIME =
120122
CelRuntimeFactory.standardCelRuntimeBuilder()
121123
.setOptions(OPTIONS)
122-
.addLibraries(
123-
CelExtensions.encoders(OPTIONS),
124-
CelExtensions.math(OPTIONS),
125-
CelExtensions.sets(OPTIONS),
126-
CelExtensions.strings(),
127-
CelOptionalLibrary.INSTANCE)
124+
.addLibraries(CANONICAL_RUNTIME_EXTENSIONS)
128125
.setExtensionRegistry(DEFAULT_EXTENSION_REGISTRY)
129126
.addMessageTypes(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor())
130127
.addMessageTypes(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor())

‎extensions/src/main/java/dev/cel/extensions/CelComprehensionsExtensions.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ public ImmutableSet<CelMacro> macros() {
159159
Operator.EXISTS_ONE.getFunction(),
160160
3,
161161
CelComprehensionsExtensions::expandExistsOneMacro),
162+
CelMacro.newReceiverMacro(
163+
Operator.EXISTS_ONE_NEW.getFunction(),
164+
3,
165+
CelComprehensionsExtensions::expandExistsOneMacro),
162166
CelMacro.newReceiverMacro(
163167
"transformList", 3, CelComprehensionsExtensions::transformListMacro),
164168
CelMacro.newReceiverMacro(

‎parser/src/main/java/dev/cel/parser/CelStandardMacro.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ public enum CelStandardMacro {
5454
CelMacro.newReceiverMacro(
5555
Operator.EXISTS_ONE.getFunction(), 2, CelStandardMacro::expandExistsOneMacro)),
5656

57+
/**
58+
* Boolean comprehension which asserts that a predicate holds true for exactly one element in the
59+
* input range.
60+
*/
61+
EXISTS_ONE_NEW(
62+
CelMacro.newReceiverMacro(
63+
Operator.EXISTS_ONE_NEW.getFunction(), 2, CelStandardMacro::expandExistsOneMacro)),
64+
5765
/**
5866
* Comprehension which applies a transform to each element in the input range and produces a list
5967
* of equivalent size as output.

‎parser/src/test/java/dev/cel/parser/CelParserImplTest.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.truth.Truth.assertThat;
1818
import static org.junit.Assert.assertThrows;
1919

20+
import com.google.common.collect.ImmutableSet;
2021
import com.google.testing.junit.testparameterinjector.TestParameter;
2122
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
2223
import com.google.testing.junit.testparameterinjector.TestParameters;
@@ -260,10 +261,17 @@ public void parse_exprUnderMaxRecursionLimit_doesNotThrow(
260261
@TestParameters("{expression: 'A.all(a?b, c)'}")
261262
@TestParameters("{expression: 'A.exists(a?b, c)'}")
262263
@TestParameters("{expression: 'A.exists_one(a?b, c)'}")
264+
@TestParameters("{expression: 'A.existsOne(a?b, c)'}")
263265
@TestParameters("{expression: 'A.filter(a?b, c)'}")
264266
public void parse_macroArgumentContainsSyntaxError_throws(String expression) {
265267
CelParser parser =
266-
CelParserImpl.newBuilder().setStandardMacros(CelStandardMacro.STANDARD_MACROS).build();
268+
CelParserImpl.newBuilder()
269+
.setStandardMacros(
270+
ImmutableSet.<CelStandardMacro>builder()
271+
.addAll(CelStandardMacro.STANDARD_MACROS)
272+
.add(CelStandardMacro.EXISTS_ONE_NEW)
273+
.build())
274+
.build();
267275

268276
CelValidationResult parseResult = parser.parse(expression);
269277

‎parser/src/test/java/dev/cel/parser/CelParserParameterizedTest.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.auto.value.AutoValue;
2727
import com.google.common.base.Ascii;
2828
import com.google.common.base.Joiner;
29+
import com.google.common.collect.ImmutableSet;
2930
import com.google.errorprone.annotations.Immutable;
3031
import com.google.protobuf.Descriptors.Descriptor;
3132
import com.google.protobuf.Descriptors.EnumDescriptor;
@@ -57,7 +58,11 @@
5758
public final class CelParserParameterizedTest extends BaselineTestCase {
5859
private static final CelParser PARSER =
5960
CelParserFactory.standardCelParserBuilder()
60-
.setStandardMacros(CelStandardMacro.STANDARD_MACROS)
61+
.setStandardMacros(
62+
ImmutableSet.<CelStandardMacro>builder()
63+
.addAll(CelStandardMacro.STANDARD_MACROS)
64+
.add(CelStandardMacro.EXISTS_ONE_NEW)
65+
.build())
6166
.addLibraries(CelOptionalLibrary.INSTANCE)
6267
.addMacros(
6368
CelMacro.newGlobalVarArgMacro("noop_macro", (a, b, c) -> Optional.empty()),
@@ -162,6 +167,7 @@ public void parser() {
162167
runTest(PARSER, "aaa.bbb(ccc)");
163168
runTest(PARSER, "has(m.f)");
164169
runTest(PARSER, "m.exists_one(v, f)");
170+
runTest(PARSER, "m.existsOne(v, f)");
165171
runTest(PARSER, "m.map(v, f)");
166172
runTest(PARSER, "m.map(v, p, f)");
167173
runTest(PARSER, "m.filter(v, p)");

‎parser/src/test/java/dev/cel/parser/CelStandardMacroTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ public void getFunction() {
3232
assertThat(CelStandardMacro.EXISTS.getFunction()).isEqualTo(Operator.EXISTS.getFunction());
3333
assertThat(CelStandardMacro.EXISTS_ONE.getFunction())
3434
.isEqualTo(Operator.EXISTS_ONE.getFunction());
35+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getFunction())
36+
.isEqualTo(Operator.EXISTS_ONE_NEW.getFunction());
3537
assertThat(CelStandardMacro.FILTER.getFunction()).isEqualTo(Operator.FILTER.getFunction());
3638
assertThat(CelStandardMacro.MAP.getFunction()).isEqualTo(Operator.MAP.getFunction());
3739
assertThat(CelStandardMacro.MAP_FILTER.getFunction()).isEqualTo(Operator.MAP.getFunction());
@@ -90,6 +92,21 @@ public void testExistsOne() {
9092
.isEqualTo(CelStandardMacro.EXISTS_ONE.getDefinition().getKey().hashCode());
9193
}
9294

95+
@Test
96+
public void testExistsOneNew() {
97+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getFunction())
98+
.isEqualTo(Operator.EXISTS_ONE_NEW.getFunction());
99+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().getArgumentCount()).isEqualTo(2);
100+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().isReceiverStyle()).isTrue();
101+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().getKey())
102+
.isEqualTo("existsOne:2:true");
103+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().isVariadic()).isFalse();
104+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().toString())
105+
.isEqualTo(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().getKey());
106+
assertThat(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().hashCode())
107+
.isEqualTo(CelStandardMacro.EXISTS_ONE_NEW.getDefinition().getKey().hashCode());
108+
}
109+
93110
@Test
94111
public void testMap2() {
95112
assertThat(CelStandardMacro.MAP.getFunction()).isEqualTo(Operator.MAP.getFunction());

‎parser/src/test/resources/parser.baseline‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,63 @@ M: m^#1:Expr.Ident#.exists_one(
784784
f^#4:Expr.Ident#
785785
)^#0:Expr.Call#
786786

787+
I: m.existsOne(v, f)
788+
=====>
789+
P: __comprehension__(
790+
// Variable
791+
v,
792+
// Target
793+
m^#1:Expr.Ident#,
794+
// Accumulator
795+
@result,
796+
// Init
797+
0^#5:int64#,
798+
// LoopCondition
799+
true^#6:bool#,
800+
// LoopStep
801+
_?_:_(
802+
f^#4:Expr.Ident#,
803+
_+_(
804+
@result^#7:Expr.Ident#,
805+
1^#8:int64#
806+
)^#9:Expr.Call#,
807+
@result^#10:Expr.Ident#
808+
)^#11:Expr.Call#,
809+
// Result
810+
_==_(
811+
@result^#12:Expr.Ident#,
812+
1^#13:int64#
813+
)^#14:Expr.Call#)^#15:Expr.Comprehension#
814+
L: __comprehension__(
815+
// Variable
816+
v,
817+
// Target
818+
m^#1[1,0]#,
819+
// Accumulator
820+
@result,
821+
// Init
822+
0^#5[1,11]#,
823+
// LoopCondition
824+
true^#6[1,11]#,
825+
// LoopStep
826+
_?_:_(
827+
f^#4[1,15]#,
828+
_+_(
829+
@result^#7[1,11]#,
830+
1^#8[1,11]#
831+
)^#9[1,11]#,
832+
@result^#10[1,11]#
833+
)^#11[1,11]#,
834+
// Result
835+
_==_(
836+
@result^#12[1,11]#,
837+
1^#13[1,11]#
838+
)^#14[1,11]#)^#15[1,11]#
839+
M: m^#1:Expr.Ident#.existsOne(
840+
v^#3:Expr.Ident#,
841+
f^#4:Expr.Ident#
842+
)^#0:Expr.Call#
843+
787844
I: m.map(v, f)
788845
=====>
789846
P: __comprehension__(

0 commit comments

Comments
 (0)