Skip to content

Commit 55f6e80

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Various fixes to CelEnvironment and YAML serialization
Includes: - Container setting is made optional to prevent the CEL environment from overriding an already set container with an empty one in case the env is extended multiple times - Brings CEL environment YAML serialization in parity (examples, type, description) - Fixes "comprehensions" to be "two-var-comprehensions" - Partitioned newVaueString into newYamlString and newSourceString, where the former respects YAML multiline syntax PiperOrigin-RevId: 885221272
1 parent 486a380 commit 55f6e80

13 files changed

Lines changed: 220 additions & 101 deletions

File tree

‎bundle/src/main/java/dev/cel/bundle/CelEnvironment.java‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import dev.cel.common.types.OptionalType;
4444
import dev.cel.common.types.SimpleType;
4545
import dev.cel.common.types.TypeParamType;
46+
import dev.cel.common.types.TypeType;
4647
import dev.cel.compiler.CelCompiler;
4748
import dev.cel.compiler.CelCompilerBuilder;
4849
import dev.cel.compiler.CelCompilerLibrary;
@@ -71,38 +72,37 @@ public abstract class CelEnvironment {
7172
"math", CanonicalCelExtension.MATH,
7273
"optional", CanonicalCelExtension.OPTIONAL,
7374
"protos", CanonicalCelExtension.PROTOS,
75+
"regex", CanonicalCelExtension.REGEX,
7476
"sets", CanonicalCelExtension.SETS,
7577
"strings", CanonicalCelExtension.STRINGS,
76-
"comprehensions", CanonicalCelExtension.COMPREHENSIONS);
78+
"two-var-comprehensions", CanonicalCelExtension.COMPREHENSIONS);
7779

7880
private static final ImmutableMap<String, ObjIntConsumer<CelOptions.Builder>> LIMIT_HANDLERS =
7981
ImmutableMap.of(
8082
"cel.limit.expression_code_points",
81-
(options, value) -> options.maxExpressionCodePointSize(value),
83+
CelOptions.Builder::maxExpressionCodePointSize,
8284
"cel.limit.parse_error_recovery",
83-
(options, value) -> options.maxParseErrorRecoveryLimit(value),
85+
CelOptions.Builder::maxParseErrorRecoveryLimit,
8486
"cel.limit.parse_recursion_depth",
85-
(options, value) -> options.maxParseRecursionDepth(value));
87+
CelOptions.Builder::maxParseRecursionDepth);
8688

8789
private static final ImmutableMap<String, BooleanOptionConsumer> FEATURE_HANDLERS =
8890
ImmutableMap.of(
8991
"cel.feature.macro_call_tracking",
90-
(options, enabled) -> options.populateMacroCalls(enabled),
92+
CelOptions.Builder::populateMacroCalls,
9193
"cel.feature.backtick_escape_syntax",
92-
(options, enabled) -> options.enableQuotedIdentifierSyntax(enabled),
94+
CelOptions.Builder::enableQuotedIdentifierSyntax,
9395
"cel.feature.cross_type_numeric_comparisons",
94-
(options, enabled) -> options.enableHeterogeneousNumericComparisons(enabled));
96+
CelOptions.Builder::enableHeterogeneousNumericComparisons);
9597

9698
/** Environment source in textual format (ex: textproto, YAML). */
9799
public abstract Optional<Source> source();
98100

99101
/** Name of the environment. */
100102
public abstract String name();
101103

102-
/**
103-
* Container, which captures default namespace and aliases for value resolution.
104-
*/
105-
public abstract CelContainer container();
104+
/** Container, which captures default namespace and aliases for value resolution. */
105+
public abstract Optional<CelContainer> container();
106106

107107
/**
108108
* An optional description of the environment (example: location of the file containing the config
@@ -226,7 +226,6 @@ public static Builder newBuilder() {
226226
return new AutoValue_CelEnvironment.Builder()
227227
.setName("")
228228
.setDescription("")
229-
.setContainer(CelContainer.ofName(""))
230229
.setVariables(ImmutableSet.of())
231230
.setFunctions(ImmutableSet.of())
232231
.setFeatures(ImmutableSet.of())
@@ -242,7 +241,6 @@ public CelCompiler extend(CelCompiler celCompiler, CelOptions celOptions)
242241
CelCompilerBuilder compilerBuilder =
243242
celCompiler
244243
.toCompilerBuilder()
245-
.setContainer(container())
246244
.setOptions(celOptions)
247245
.setTypeProvider(celTypeProvider)
248246
.addVarDeclarations(
@@ -254,6 +252,8 @@ public CelCompiler extend(CelCompiler celCompiler, CelOptions celOptions)
254252
.map(f -> f.toCelFunctionDecl(celTypeProvider))
255253
.collect(toImmutableList()));
256254

255+
container().ifPresent(compilerBuilder::setContainer);
256+
257257
addAllCompilerExtensions(compilerBuilder, celOptions);
258258

259259
applyStandardLibrarySubset(compilerBuilder);
@@ -416,6 +416,8 @@ public abstract static class VariableDecl {
416416
/** The type of the variable. */
417417
public abstract TypeDecl type();
418418

419+
public abstract Optional<String> description();
420+
419421
/** Builder for {@link VariableDecl}. */
420422
@AutoValue.Builder
421423
public abstract static class Builder implements RequiredFieldsChecker {
@@ -428,6 +430,8 @@ public abstract static class Builder implements RequiredFieldsChecker {
428430

429431
public abstract VariableDecl.Builder setType(TypeDecl typeDecl);
430432

433+
public abstract VariableDecl.Builder setDescription(String name);
434+
431435
@Override
432436
public ImmutableList<RequiredField> requiredFields() {
433437
return ImmutableList.of(
@@ -459,6 +463,8 @@ public abstract static class FunctionDecl {
459463

460464
public abstract String name();
461465

466+
public abstract Optional<String> description();
467+
462468
public abstract ImmutableSet<OverloadDecl> overloads();
463469

464470
/** Builder for {@link FunctionDecl}. */
@@ -471,6 +477,8 @@ public abstract static class Builder implements RequiredFieldsChecker {
471477

472478
public abstract FunctionDecl.Builder setName(String name);
473479

480+
public abstract FunctionDecl.Builder setDescription(String description);
481+
474482
public abstract FunctionDecl.Builder setOverloads(ImmutableSet<OverloadDecl> overloads);
475483

476484
@Override
@@ -519,6 +527,9 @@ public abstract static class OverloadDecl {
519527
/** List of function overload type values. */
520528
public abstract ImmutableList<TypeDecl> arguments();
521529

530+
/** Examples for the overload. */
531+
public abstract ImmutableList<String> examples();
532+
522533
/** Return type of the overload. Required. */
523534
public abstract TypeDecl returnType();
524535

@@ -537,8 +548,21 @@ public abstract static class Builder implements RequiredFieldsChecker {
537548
// This should stay package-private to encourage add/set methods to be used instead.
538549
abstract ImmutableList.Builder<TypeDecl> argumentsBuilder();
539550

551+
abstract ImmutableList.Builder<String> examplesBuilder();
552+
540553
public abstract OverloadDecl.Builder setArguments(ImmutableList<TypeDecl> args);
541554

555+
@CanIgnoreReturnValue
556+
public OverloadDecl.Builder addExamples(Iterable<String> examples) {
557+
this.examplesBuilder().addAll(checkNotNull(examples));
558+
return this;
559+
}
560+
561+
@CanIgnoreReturnValue
562+
public OverloadDecl.Builder addExamples(String... examples) {
563+
return addExamples(Arrays.asList(examples));
564+
}
565+
542566
@CanIgnoreReturnValue
543567
public OverloadDecl.Builder addArguments(Iterable<TypeDecl> args) {
544568
this.argumentsBuilder().addAll(checkNotNull(args));
@@ -667,6 +691,9 @@ public CelType toCelType(CelTypeProvider celTypeProvider) {
667691
CelType keyType = params().get(0).toCelType(celTypeProvider);
668692
CelType valueType = params().get(1).toCelType(celTypeProvider);
669693
return MapType.create(keyType, valueType);
694+
case "type":
695+
checkState(params().size() == 1, "Expected 1 parameter for type, got " + params().size());
696+
return TypeType.create(params().get(0).toCelType(celTypeProvider));
670697
default:
671698
if (isTypeParam()) {
672699
return TypeParamType.create(name());
@@ -838,6 +865,7 @@ enum CanonicalCelExtension {
838865
SETS(
839866
(options, version) -> CelExtensions.sets(options),
840867
(options, version) -> CelExtensions.sets(options)),
868+
REGEX((options, version) -> CelExtensions.regex(), (options, version) -> CelExtensions.regex()),
841869
LISTS((options, version) -> CelExtensions.lists(), (options, version) -> CelExtensions.lists()),
842870
COMPREHENSIONS(
843871
(options, version) -> CelExtensions.comprehensions(),
@@ -1054,7 +1082,7 @@ public static OverloadSelector.Builder newBuilder() {
10541082
}
10551083

10561084
@FunctionalInterface
1057-
private static interface BooleanOptionConsumer {
1085+
private interface BooleanOptionConsumer {
10581086
void accept(CelOptions.Builder options, boolean value);
10591087
}
10601088
}

‎bundle/src/main/java/dev/cel/bundle/CelEnvironmentYamlParser.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,9 @@ private VariableDecl parseVariable(ParserContext<Node> ctx, Node node) {
353353
case "name":
354354
builder.setName(newString(ctx, valueNode));
355355
break;
356+
case "description":
357+
builder.setDescription(newString(ctx, valueNode));
358+
break;
356359
case "type":
357360
if (typeDeclBuilder != null) {
358361
ctx.reportError(
@@ -428,6 +431,9 @@ private FunctionDecl parseFunction(ParserContext<Node> ctx, Node node) {
428431
case "overloads":
429432
builder.setOverloads(parseOverloads(ctx, valueNode));
430433
break;
434+
case "description":
435+
builder.setDescription(newString(ctx, valueNode).trim());
436+
break;
431437
default:
432438
ctx.reportError(keyId, String.format("Unsupported function tag: %s", keyName));
433439
break;
@@ -479,6 +485,9 @@ private static ImmutableSet<OverloadDecl> parseOverloads(ParserContext<Node> ctx
479485
case "target":
480486
overloadDeclBuilder.setTarget(parseTypeDecl(ctx, valueNode));
481487
break;
488+
case "examples":
489+
overloadDeclBuilder.addExamples(parseOverloadExamples(ctx, valueNode));
490+
break;
482491
default:
483492
ctx.reportError(keyId, String.format("Unsupported overload tag: %s", fieldName));
484493
break;
@@ -494,6 +503,25 @@ private static ImmutableSet<OverloadDecl> parseOverloads(ParserContext<Node> ctx
494503
return overloadSetBuilder.build();
495504
}
496505

506+
private static ImmutableList<String> parseOverloadExamples(ParserContext<Node> ctx, Node node) {
507+
long listValueId = ctx.collectMetadata(node);
508+
if (!assertYamlType(ctx, listValueId, node, YamlNodeType.LIST)) {
509+
return ImmutableList.of();
510+
}
511+
SequenceNode paramsListNode = (SequenceNode) node;
512+
ImmutableList.Builder<String> builder = ImmutableList.builder();
513+
for (Node elementNode : paramsListNode.getValue()) {
514+
long elementNodeId = ctx.collectMetadata(elementNode);
515+
if (!assertYamlType(ctx, elementNodeId, elementNode, YamlNodeType.STRING)) {
516+
continue;
517+
}
518+
519+
builder.add(((ScalarNode) elementNode).getValue());
520+
}
521+
522+
return builder.build();
523+
}
524+
497525
private static ImmutableList<TypeDecl> parseOverloadArguments(
498526
ParserContext<Node> ctx, Node node) {
499527
long listValueId = ctx.collectMetadata(node);

‎bundle/src/main/java/dev/cel/bundle/CelEnvironmentYamlSerializer.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,8 @@ public Node representData(Object data) {
7979
if (!environment.description().isEmpty()) {
8080
configMap.put("description", environment.description());
8181
}
82-
if (!environment.container().name().isEmpty()
83-
|| !environment.container().abbreviations().isEmpty()
84-
|| !environment.container().aliases().isEmpty()) {
85-
configMap.put("container", environment.container());
82+
if (environment.container().isPresent()) {
83+
configMap.put("container", environment.container().get());
8684
}
8785
if (!environment.extensions().isEmpty()) {
8886
configMap.put("extensions", environment.extensions().asList());

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ public void container() {
333333

334334
CelEnvironmentExporter exporter = CelEnvironmentExporter.newBuilder().build();
335335
CelEnvironment celEnvironment = exporter.export(cel);
336-
CelContainer container = celEnvironment.container();
336+
CelContainer container = celEnvironment.container().get();
337337
assertThat(container.name()).isEqualTo("cntnr");
338338
assertThat(container.abbreviations()).containsExactly("foo.Bar", "baz.Qux").inOrder();
339339
assertThat(container.aliases()).containsAtLeast("nm", "user.name", "id", "user.id").inOrder();
@@ -368,4 +368,3 @@ public void options() {
368368
CelEnvironment.Limit.create("cel.limit.parse_recursion_depth", 10));
369369
}
370370
}
371-

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,7 @@ public void newBuilder_defaults() {
4444
assertThat(environment.source()).isEmpty();
4545
assertThat(environment.name()).isEmpty();
4646
assertThat(environment.description()).isEmpty();
47-
assertThat(environment.container().name()).isEmpty();
48-
assertThat(environment.container().abbreviations()).isEmpty();
49-
assertThat(environment.container().aliases()).isEmpty();
47+
assertThat(environment.container()).isEmpty();
5048
assertThat(environment.extensions()).isEmpty();
5149
assertThat(environment.variables()).isEmpty();
5250
assertThat(environment.functions()).isEmpty();
@@ -65,10 +63,10 @@ public void container() {
6563
.build())
6664
.build();
6765

68-
assertThat(environment.container().name()).isEqualTo("cntr");
69-
assertThat(environment.container().abbreviations()).containsExactly("foo.Bar", "baz.Qux");
70-
assertThat(environment.container().aliases())
71-
.containsExactly("nm", "user.name", "id", "user.id");
66+
CelContainer container = environment.container().get();
67+
assertThat(container.name()).isEqualTo("cntr");
68+
assertThat(container.abbreviations()).containsExactly("foo.Bar", "baz.Qux");
69+
assertThat(container.aliases()).containsExactly("nm", "user.name", "id", "user.id");
7270
}
7371

7472
@Test
@@ -81,9 +79,10 @@ public void extend_allExtensions() throws Exception {
8179
ExtensionConfig.latest("math"),
8280
ExtensionConfig.latest("optional"),
8381
ExtensionConfig.latest("protos"),
82+
ExtensionConfig.latest("regex"),
8483
ExtensionConfig.latest("sets"),
8584
ExtensionConfig.latest("strings"),
86-
ExtensionConfig.latest("comprehensions"));
85+
ExtensionConfig.latest("two-var-comprehensions"));
8786
CelEnvironment environment =
8887
CelEnvironment.newBuilder().addExtensions(extensionConfigs).build();
8988

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

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -675,9 +675,7 @@ private enum EnvironmentParseErrorTestcase {
675675
+ " | - version: 0\n"
676676
+ " | ..^"),
677677
ILLEGAL_LIBRARY_SUBSET_TAG(
678-
"name: 'test_suite_name'\n"
679-
+ "stdlib:\n"
680-
+ " unknown_tag: 'test_value'\n",
678+
"name: 'test_suite_name'\n" + "stdlib:\n" + " unknown_tag: 'test_value'\n",
681679
"ERROR: <input>:3:3: Unsupported library subset tag: unknown_tag\n"
682680
+ " | unknown_tag: 'test_value'\n"
683681
+ " | ..^"),
@@ -859,30 +857,40 @@ private enum EnvironmentYamlResourceTestCase {
859857
.setVariables(
860858
VariableDecl.newBuilder()
861859
.setName("msg")
860+
.setDescription(
861+
"msg represents all possible type permutation which CEL understands from a"
862+
+ " proto perspective")
862863
.setType(TypeDecl.create("cel.expr.conformance.proto3.TestAllTypes"))
863864
.build())
864865
.setFunctions(
865-
FunctionDecl.create(
866-
"isEmpty",
867-
ImmutableSet.of(
868-
OverloadDecl.newBuilder()
869-
.setId("wrapper_string_isEmpty")
870-
.setTarget(TypeDecl.create("google.protobuf.StringValue"))
871-
.setReturnType(TypeDecl.create("bool"))
872-
.build(),
873-
OverloadDecl.newBuilder()
874-
.setId("list_isEmpty")
875-
.setTarget(
876-
TypeDecl.newBuilder()
877-
.setName("list")
878-
.addParams(
879-
TypeDecl.newBuilder()
880-
.setName("T")
881-
.setIsTypeParam(true)
882-
.build())
883-
.build())
884-
.setReturnType(TypeDecl.create("bool"))
885-
.build())))
866+
FunctionDecl.newBuilder()
867+
.setName("isEmpty")
868+
.setDescription(
869+
"determines whether a list is empty,\nor a string has no characters")
870+
.setOverloads(
871+
ImmutableSet.of(
872+
OverloadDecl.newBuilder()
873+
.setId("wrapper_string_isEmpty")
874+
.setTarget(TypeDecl.create("google.protobuf.StringValue"))
875+
.addExamples("''.isEmpty() // true")
876+
.setReturnType(TypeDecl.create("bool"))
877+
.build(),
878+
OverloadDecl.newBuilder()
879+
.setId("list_isEmpty")
880+
.addExamples("[].isEmpty() // true")
881+
.addExamples("[1].isEmpty() // false")
882+
.setTarget(
883+
TypeDecl.newBuilder()
884+
.setName("list")
885+
.addParams(
886+
TypeDecl.newBuilder()
887+
.setName("T")
888+
.setIsTypeParam(true)
889+
.build())
890+
.build())
891+
.setReturnType(TypeDecl.create("bool"))
892+
.build()))
893+
.build())
886894
.setFeatures(CelEnvironment.FeatureFlag.create("cel.feature.macro_call_tracking", true))
887895
.setLimits(
888896
ImmutableSet.of(

‎common/src/main/java/dev/cel/common/formats/ParserContext.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,24 @@ public interface ParserContext<T> {
4242

4343
Map<Long, Integer> getIdToOffsetMap();
4444

45-
/** NewString creates a new ValueString from the YAML node. */
46-
ValueString newValueString(T node);
45+
/**
46+
* NewYamlString creates a new ValueString from the YAML node, evaluated according to standard
47+
* YAML parsing rules.
48+
*
49+
* <p>This respects the whitespace folding semantics defined by the node's scalar style (e.g.,
50+
* folded string {@code >} versus literal string {@code |}). Use this method for general string
51+
* fields such as {@code description}, {@code name}, or {@code id}.
52+
*/
53+
ValueString newYamlString(T node);
54+
55+
/**
56+
* NewRawString creates a new ValueString from the YAML node, preserving formatting for accurate
57+
* source mapping.
58+
*
59+
* <p>This extracts the verbatim text directly from the source file, preserving raw block
60+
* indentation and unmodified newlines. Use this method when the string represents code or a CEL
61+
* expression where precise character-level offsets must be maintained for accurate diagnostic
62+
* error reporting.
63+
*/
64+
ValueString newSourceString(T node);
4765
}

0 commit comments

Comments
 (0)