Skip to content

Commit a6ba5c9

Browse files
dmitriplotnikovcopybara-github
authored andcommitted
Introduce versioning for the CEL "strings" extension library.
Tests are added to ensure that each function/macro is only available in the versions it's expected to be in. PiperOrigin-RevId: 875348651
1 parent a69d6b1 commit a6ba5c9

16 files changed

Lines changed: 622 additions & 77 deletions

checker/optional.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class OptionalOverloads {
116116
static constexpr char kOptionalMapIndexValue[] = "optional_map_index_value";
117117
};
118118

119-
absl::Status RegisterOptionalDecls(TypeCheckerBuilder& builder) {
119+
absl::Status RegisterOptionalDecls(TypeCheckerBuilder& builder, int version) {
120120
CEL_ASSIGN_OR_RETURN(
121121
auto of,
122122
MakeFunctionDecl(OptionalNames::kOptionalOf,
@@ -219,19 +219,26 @@ absl::Status RegisterOptionalDecls(TypeCheckerBuilder& builder) {
219219
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(or_value)));
220220
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(opt_index)));
221221
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(select)));
222+
CEL_RETURN_IF_ERROR(builder.MergeFunction(std::move(index)));
223+
224+
if (version == 0 || version == 1) {
225+
return absl::OkStatus();
226+
}
227+
222228
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(first)));
223229
CEL_RETURN_IF_ERROR(builder.AddFunction(std::move(last)));
224-
CEL_RETURN_IF_ERROR(builder.MergeFunction(std::move(index)));
225230

226231
return absl::OkStatus();
227232
}
228233

229234
} // namespace
230235

231-
CheckerLibrary OptionalCheckerLibrary() {
236+
CheckerLibrary OptionalCheckerLibrary(int version) {
232237
return CheckerLibrary({
233238
"optional",
234-
&RegisterOptionalDecls,
239+
[version](TypeCheckerBuilder& builder) {
240+
return RegisterOptionalDecls(builder, version);
241+
},
235242
});
236243
}
237244

checker/optional.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919

2020
namespace cel {
2121

22+
constexpr int kOptionalExtensionLatestVersion = 2;
23+
2224
// Library for CEL optional definitions.
23-
CheckerLibrary OptionalCheckerLibrary();
25+
CheckerLibrary OptionalCheckerLibrary(
26+
int version = kOptionalExtensionLatestVersion);
2427

2528
} // namespace cel
2629

compiler/BUILD

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ cc_test(
105105
":compiler",
106106
":compiler_factory",
107107
":optional",
108+
":standard_library",
109+
"//checker:optional",
108110
"//checker:standard_library",
109111
"//checker:type_check_issue",
110112
"//checker:validation_result",
@@ -114,6 +116,7 @@ cc_test(
114116
"//internal:testing",
115117
"//internal:testing_descriptor_pool",
116118
"//testutil:baseline_tests",
119+
"@com_google_absl//absl/algorithm:container",
117120
"@com_google_absl//absl/status:status_matchers",
118121
"@com_google_absl//absl/status:statusor",
119122
"@com_google_absl//absl/strings",

compiler/optional.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,18 @@
2222

2323
namespace cel {
2424

25-
CompilerLibrary OptionalCompilerLibrary() {
25+
CompilerLibrary OptionalCompilerLibrary(int version) {
2626
CompilerLibrary library =
27-
CompilerLibrary::FromCheckerLibrary(OptionalCheckerLibrary());
27+
CompilerLibrary::FromCheckerLibrary(OptionalCheckerLibrary(version));
2828

29-
library.configure_parser = [](ParserBuilder& builder) {
29+
library.configure_parser = [version](ParserBuilder& builder) {
3030
builder.GetOptions().enable_optional_syntax = true;
3131
absl::Status status;
32-
status.Update(builder.AddMacro(OptFlatMapMacro()));
3332
status.Update(builder.AddMacro(OptMapMacro()));
33+
if (version == 0) {
34+
return status;
35+
}
36+
status.Update(builder.AddMacro(OptFlatMapMacro()));
3437
return status;
3538
};
3639

compiler/optional.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@
1414
#ifndef THIRD_PARTY_CEL_CPP_COMPILER_OPTIONALS_H_
1515
#define THIRD_PARTY_CEL_CPP_COMPILER_OPTIONALS_H_
1616

17+
#include "checker/optional.h"
1718
#include "compiler/compiler.h"
1819

1920
namespace cel {
2021

2122
// CompilerLibrary that enables support for CEL optional types.
22-
CompilerLibrary OptionalCompilerLibrary();
23+
CompilerLibrary OptionalCompilerLibrary(
24+
int version = kOptionalExtensionLatestVersion);
2325

2426
} // namespace cel
2527

compiler/optional_test.cc

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
#include <memory>
1717
#include <string>
1818
#include <utility>
19+
#include <vector>
1920

21+
#include "absl/algorithm/container.h"
2022
#include "absl/status/status_matchers.h"
2123
#include "absl/status/statusor.h"
2224
#include "absl/strings/ascii.h"
2325
#include "absl/strings/str_cat.h"
2426
#include "absl/strings/str_join.h"
27+
#include "checker/optional.h"
2528
#include "checker/standard_library.h"
2629
#include "checker/type_check_issue.h"
2730
#include "checker/validation_result.h"
@@ -30,6 +33,7 @@
3033
#include "common/type.h"
3134
#include "compiler/compiler.h"
3235
#include "compiler/compiler_factory.h"
36+
#include "compiler/standard_library.h"
3337
#include "internal/testing.h"
3438
#include "internal/testing_descriptor_pool.h"
3539
#include "testutil/baseline_tests.h"
@@ -42,6 +46,8 @@ using ::absl_testing::IsOk;
4246
using ::cel::expr::conformance::proto3::TestAllTypes;
4347
using ::cel::test::FormatBaselineAst;
4448
using ::testing::HasSubstr;
49+
using ::testing::IsEmpty;
50+
using ::testing::ValuesIn;
4551

4652
struct TestCase {
4753
std::string expr;
@@ -271,5 +277,108 @@ TEST(OptionalTest, NotEnabled) {
271277
HasSubstr("undeclared reference to 'optional'"));
272278
}
273279

280+
struct OptionalExtensionVersionTestCase {
281+
std::string expr;
282+
std::vector<int> expected_supported_versions;
283+
};
284+
285+
class OptionalExtensionVersionTest
286+
: public ::testing::TestWithParam<OptionalExtensionVersionTestCase> {};
287+
288+
TEST_P(OptionalExtensionVersionTest, OptionalExtensionVersions) {
289+
const OptionalExtensionVersionTestCase& test_case = GetParam();
290+
for (int version = 0; version <= cel::kOptionalExtensionLatestVersion;
291+
++version) {
292+
CompilerLibrary compiler_library = OptionalCompilerLibrary(version);
293+
294+
CompilerOptions compiler_options;
295+
compiler_options.parser_options.enable_optional_syntax = true;
296+
297+
ASSERT_OK_AND_ASSIGN(
298+
std::unique_ptr<CompilerBuilder> builder,
299+
cel::NewCompilerBuilder(internal::GetTestingDescriptorPool(),
300+
compiler_options));
301+
ASSERT_THAT(builder->AddLibrary(StandardCompilerLibrary()), IsOk());
302+
ASSERT_THAT(builder->AddLibrary(std::move(compiler_library)), IsOk());
303+
304+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Compiler> compiler, builder->Build());
305+
ASSERT_OK_AND_ASSIGN(ValidationResult result,
306+
compiler->Compile(test_case.expr));
307+
if (absl::c_contains(test_case.expected_supported_versions, version)) {
308+
EXPECT_THAT(result.GetIssues(), IsEmpty())
309+
<< "Expected no issues for expr: " << test_case.expr
310+
<< " at version: " << version << " but got: " << result.FormatError();
311+
} else {
312+
EXPECT_THAT(result.GetIssues(),
313+
Contains(Property(&TypeCheckIssue::message,
314+
HasSubstr("undeclared reference"))))
315+
<< "Expected undeclared reference for expr: " << test_case.expr
316+
<< " at version: " << version;
317+
}
318+
}
319+
};
320+
321+
std::vector<OptionalExtensionVersionTestCase>
322+
CreateOptionalExtensionVersionParams() {
323+
return {
324+
OptionalExtensionVersionTestCase{
325+
.expr = "optional_type",
326+
.expected_supported_versions = {0, 1, 2},
327+
},
328+
OptionalExtensionVersionTestCase{
329+
.expr = "optional.of('foo').optMap(x, x)",
330+
.expected_supported_versions = {0, 1, 2},
331+
},
332+
OptionalExtensionVersionTestCase{
333+
.expr = "optional.of('foo')",
334+
.expected_supported_versions = {0, 1, 2},
335+
},
336+
OptionalExtensionVersionTestCase{
337+
.expr = "optional.ofNonZeroValue(1)",
338+
.expected_supported_versions = {0, 1, 2},
339+
},
340+
OptionalExtensionVersionTestCase{
341+
.expr = "optional.of('foo').value()",
342+
.expected_supported_versions = {0, 1, 2},
343+
},
344+
OptionalExtensionVersionTestCase{
345+
.expr = "optional.of('foo').hasValue()",
346+
.expected_supported_versions = {0, 1, 2},
347+
},
348+
OptionalExtensionVersionTestCase{
349+
.expr = "optional.of(1).or(optional.of(2))",
350+
.expected_supported_versions = {0, 1, 2},
351+
},
352+
OptionalExtensionVersionTestCase{
353+
.expr = "optional.of(1).orValue(2)",
354+
.expected_supported_versions = {0, 1, 2},
355+
},
356+
OptionalExtensionVersionTestCase{
357+
.expr = "[1, 2, 3][?5]",
358+
.expected_supported_versions = {0, 1, 2},
359+
},
360+
OptionalExtensionVersionTestCase{
361+
.expr = "dyn(1).?bar",
362+
.expected_supported_versions = {0, 1, 2},
363+
},
364+
OptionalExtensionVersionTestCase{
365+
.expr = "optional.of('foo').optFlatMap(x, optional.of(x))",
366+
.expected_supported_versions = {1, 2},
367+
},
368+
OptionalExtensionVersionTestCase{
369+
.expr = "[1, 2, 3].first()",
370+
.expected_supported_versions = {2},
371+
},
372+
OptionalExtensionVersionTestCase{
373+
.expr = "[1, 2, 3].last()",
374+
.expected_supported_versions = {2},
375+
},
376+
};
377+
}
378+
379+
INSTANTIATE_TEST_SUITE_P(OptionalExtensionVersionTest,
380+
OptionalExtensionVersionTest,
381+
ValuesIn(CreateOptionalExtensionVersionParams()));
382+
274383
} // namespace
275384
} // namespace cel

extensions/BUILD

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,14 @@ cc_test(
143143
":math_ext_decls",
144144
":math_ext_macros",
145145
"//checker:standard_library",
146+
"//checker:type_check_issue",
146147
"//checker:validation_result",
147148
"//common:decl",
148149
"//common:function_descriptor",
150+
"//common:type",
151+
"//compiler",
149152
"//compiler:compiler_factory",
153+
"//compiler:standard_library",
150154
"//eval/public:activation",
151155
"//eval/public:builtin_func_registrar",
152156
"//eval/public:cel_expr_builder_factory",
@@ -162,6 +166,7 @@ cc_test(
162166
"//runtime:activation",
163167
"//runtime:runtime_options",
164168
"//runtime:standard_runtime_builder_factory",
169+
"@com_google_absl//absl/algorithm:container",
165170
"@com_google_absl//absl/status",
166171
"@com_google_absl//absl/status:status_matchers",
167172
"@com_google_absl//absl/strings",
@@ -456,6 +461,7 @@ cc_test(
456461
srcs = ["lists_functions_test.cc"],
457462
deps = [
458463
":lists_functions",
464+
"//checker:type_check_issue",
459465
"//checker:validation_result",
460466
"//common:source",
461467
"//common:value",
@@ -476,6 +482,7 @@ cc_test(
476482
"//runtime:runtime_builder",
477483
"//runtime:runtime_options",
478484
"//runtime:standard_runtime_builder_factory",
485+
"@com_google_absl//absl/algorithm:container",
479486
"@com_google_absl//absl/status",
480487
"@com_google_absl//absl/status:status_matchers",
481488
"@com_google_absl//absl/strings:string_view",
@@ -595,11 +602,13 @@ cc_test(
595602
deps = [
596603
":strings",
597604
"//checker:standard_library",
605+
"//checker:type_check_issue",
598606
"//checker:type_checker_builder",
599607
"//checker:validation_result",
600608
"//common:decl",
601609
"//common:type",
602610
"//common:value",
611+
"//compiler",
603612
"//compiler:compiler_factory",
604613
"//compiler:standard_library",
605614
"//extensions/protobuf:runtime_adapter",
@@ -613,6 +622,7 @@ cc_test(
613622
"//runtime:runtime_options",
614623
"//runtime:standard_runtime_builder_factory",
615624
"//testutil:baseline_tests",
625+
"@com_google_absl//absl/algorithm:container",
616626
"@com_google_absl//absl/status",
617627
"@com_google_absl//absl/status:status_matchers",
618628
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",

0 commit comments

Comments
 (0)