Skip to content

Commit 04ba6cb

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Deprecate setStandardEnvironmentEnabled flag in favor of subsetting
PiperOrigin-RevId: 967247609
1 parent 426fa24 commit 04ba6cb

15 files changed

Lines changed: 140 additions & 36 deletions

File tree

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,15 @@ public interface CelBuilder {
292292
@CanIgnoreReturnValue
293293
CelBuilder addFileTypes(FileDescriptorSet fileDescriptorSet);
294294

295-
/** Enable or disable the standard CEL library functions and variables */
295+
/**
296+
* Enable or disable the standard CEL library functions and variables.
297+
*
298+
* @deprecated Use {@link #setStandardDeclarations(CelStandardDeclarations)} and/or {@link
299+
* #setStandardFunctions(CelStandardFunctions)} to configure or subset the standard
300+
* environment. Use {@link CelStandardDeclarations#EMPTY} and {@link
301+
* CelStandardFunctions#EMPTY} to disable all standard declarations and functions.
302+
*/
303+
@Deprecated
296304
@CanIgnoreReturnValue
297305
CelBuilder setStandardEnvironmentEnabled(boolean value);
298306

@@ -314,8 +322,7 @@ public interface CelBuilder {
314322

315323
/**
316324
* Override the standard declarations for the type-checker. This can be used to subset the
317-
* standard environment to only expose the desired declarations to the type-checker. {@link
318-
* #setStandardEnvironmentEnabled(boolean)} must be set to false for this to take effect.
325+
* standard environment to only expose the desired declarations to the type-checker.
319326
*/
320327
@CanIgnoreReturnValue
321328
CelBuilder setStandardDeclarations(CelStandardDeclarations standardDeclarations);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ public CelBuilder addFileTypes(FileDescriptorSet fileDescriptorSet) {
379379
}
380380

381381
@Override
382+
@Deprecated
382383
public CelBuilder setStandardEnvironmentEnabled(boolean value) {
383384
compilerBuilder.setStandardEnvironmentEnabled(value);
384385
runtimeBuilder.setStandardEnvironmentEnabled(value);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ java_library(
2727
"//checker",
2828
"//checker:checker_legacy_environment",
2929
"//checker:proto_type_mask",
30+
"//checker:standard_decl",
3031
"//common:cel_ast",
3132
"//common:cel_descriptor_util",
3233
"//common:cel_source",
@@ -56,6 +57,7 @@ java_library(
5657
"//runtime:evaluation_exception_builder",
5758
"//runtime:evaluation_listener",
5859
"//runtime:function_binding",
60+
"//runtime:standard_functions",
5961
"//runtime:unknown_attributes",
6062
"//testing:cel_runtime_flavor",
6163
"//testing/protos:single_file_extension_java_proto",

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import com.google.testing.junit.testparameterinjector.TestParameter;
6161
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
6262
import dev.cel.checker.CelCheckerLegacyImpl;
63+
import dev.cel.checker.CelStandardDeclarations;
6364
import dev.cel.checker.DescriptorTypeProvider;
6465
import dev.cel.checker.ProtoTypeMask;
6566
import dev.cel.checker.TypeProvider;
@@ -110,6 +111,7 @@
110111
import dev.cel.runtime.CelRuntime.Program;
111112
import dev.cel.runtime.CelRuntimeFactory;
112113
import dev.cel.runtime.CelRuntimeLegacyImpl;
114+
import dev.cel.runtime.CelStandardFunctions;
113115
import dev.cel.runtime.CelUnknownSet;
114116
import dev.cel.runtime.CelVariableResolver;
115117
import dev.cel.runtime.UnknownContext;
@@ -2294,4 +2296,27 @@ private static Cel setupEnv(CelBuilder celBuilder) {
22942296
.build())
22952297
.build();
22962298
}
2299+
2300+
@Test
2301+
public void plannerCelBuilder_setStandardDeclarationsAndFunctions_subsetsEnvironment()
2302+
throws Exception {
2303+
Cel cel =
2304+
CelFactory.plannerCelBuilder()
2305+
.setStandardDeclarations(
2306+
CelStandardDeclarations.newBuilder()
2307+
.includeFunctions(CelStandardDeclarations.StandardFunction.ADD)
2308+
.build())
2309+
.setStandardFunctions(
2310+
CelStandardFunctions.newBuilder()
2311+
.includeFunctions(CelStandardFunctions.StandardFunction.ADD)
2312+
.build())
2313+
.build();
2314+
2315+
CelAbstractSyntaxTree ast = cel.compile("1 + 1").getAst();
2316+
assertThat(cel.createProgram(ast).eval()).isEqualTo(2L);
2317+
2318+
CelValidationException validationException =
2319+
assertThrows(CelValidationException.class, () -> cel.compile("1 - 1").getAst());
2320+
assertThat(validationException).hasMessageThat().contains("undeclared reference to '_-_'");
2321+
}
22972322
}

‎checker/src/main/java/dev/cel/checker/CelCheckerBuilder.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,14 +155,20 @@ public interface CelCheckerBuilder {
155155
@CanIgnoreReturnValue
156156
CelCheckerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet);
157157

158-
/** Enable or disable the standard CEL library functions and variables */
158+
/**
159+
* Enable or disable the standard CEL library functions and variables.
160+
*
161+
* @deprecated Use {@link #setStandardDeclarations(CelStandardDeclarations)} to configure or
162+
* subset the standard environment. Use {@link CelStandardDeclarations#EMPTY} to disable all
163+
* standard declarations.
164+
*/
165+
@Deprecated
159166
@CanIgnoreReturnValue
160167
CelCheckerBuilder setStandardEnvironmentEnabled(boolean value);
161168

162169
/**
163170
* Override the standard declarations for the type-checker. This can be used to subset the
164-
* standard environment to only expose the desired declarations to the type-checker. {@link
165-
* #setStandardEnvironmentEnabled(boolean)} must be set to false for this to take effect.
171+
* standard environment to only expose the desired declarations to the type-checker.
166172
*/
167173
@CanIgnoreReturnValue
168174
CelCheckerBuilder setStandardDeclarations(CelStandardDeclarations standardDeclarations);

‎checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ public void accept(EnvVisitor envVisitor) {
162162

163163
private Env getEnv(Errors errors) {
164164
Env env;
165-
if (standardEnvironmentEnabled) {
166-
env = Env.standard(errors, typeProvider, celOptions);
167-
} else if (overriddenStandardDeclarations != null) {
165+
if (overriddenStandardDeclarations != null) {
168166
env = Env.standard(overriddenStandardDeclarations, errors, typeProvider, celOptions);
167+
} else if (standardEnvironmentEnabled) {
168+
env = Env.standard(errors, typeProvider, celOptions);
169169
} else {
170170
env = Env.unconfigured(errors, typeProvider, celOptions);
171171
}
@@ -359,6 +359,7 @@ public CelCheckerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet) {
359359
}
360360

361361
@Override
362+
@Deprecated
362363
public CelCheckerBuilder setStandardEnvironmentEnabled(boolean value) {
363364
this.standardEnvironmentEnabled = value;
364365
return this;
@@ -434,12 +435,6 @@ CelTypeProvider celTypeProvider() {
434435
@Override
435436
@CheckReturnValue
436437
public CelCheckerLegacyImpl build() {
437-
if (standardEnvironmentEnabled && standardDeclarations != null) {
438-
throw new IllegalArgumentException(
439-
"setStandardEnvironmentEnabled must be set to false to override standard"
440-
+ " declarations.");
441-
}
442-
443438
// Add libraries, such as extensions
444439
ImmutableSet<CelCheckerLibrary> checkerLibraries = celCheckerLibraries.build();
445440
checkerLibraries.forEach(celLibrary -> celLibrary.setCheckerOptions(this));

‎checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ public final class CelStandardDeclarations {
5151
private static final TypeParamType TYPE_PARAM_B = TypeParamType.create("B");
5252
private static final MapType MAP_OF_AB = MapType.create(TYPE_PARAM_A, TYPE_PARAM_B);
5353

54+
/** An empty instance of {@link CelStandardDeclarations} with no functions or identifiers. */
55+
public static final CelStandardDeclarations EMPTY =
56+
new CelStandardDeclarations(ImmutableSet.of(), ImmutableSet.of());
57+
5458
private final ImmutableSet<CelFunctionDecl> celFunctionDecls;
5559
private final ImmutableSet<CelIdentDecl> celIdentDecls;
5660

‎checker/src/test/java/dev/cel/checker/CelStandardDeclarationsTest.java‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,24 +86,63 @@ public void standardDeclaration_moreThanOneIdentifierFilterSet_throws(
8686
}
8787

8888
@Test
89-
public void compiler_standardEnvironmentEnabled_throwsWhenOverridingDeclarations() {
90-
IllegalArgumentException e =
91-
assertThrows(
92-
IllegalArgumentException.class,
93-
() ->
94-
CelCompilerFactory.standardCelCompilerBuilder()
95-
.setStandardEnvironmentEnabled(true)
96-
.setStandardDeclarations(
97-
CelStandardDeclarations.newBuilder()
98-
.includeFunctions(StandardFunction.ADD, StandardFunction.SUBTRACT)
99-
.build())
100-
.build());
89+
public void compiler_setStandardDeclarations_overridesDefaultStandardEnvironment()
90+
throws Exception {
91+
CelCompiler compiler =
92+
CelCompilerFactory.standardCelCompilerBuilder()
93+
.setStandardDeclarations(
94+
CelStandardDeclarations.newBuilder()
95+
.includeFunctions(StandardFunction.ADD)
96+
.build())
97+
.build();
10198

102-
assertThat(e)
103-
.hasMessageThat()
104-
.contains(
105-
"setStandardEnvironmentEnabled must be set to false to override standard"
106-
+ " declarations.");
99+
assertThat(compiler.compile("1 + 1").hasError()).isFalse();
100+
assertThat(compiler.compile("1 - 1").hasError()).isTrue();
101+
}
102+
103+
@Test
104+
public void compiler_setStandardDeclarations_withStandardEnvironmentExplicitlyEnabled()
105+
throws Exception {
106+
CelCompiler compiler =
107+
CelCompilerFactory.standardCelCompilerBuilder()
108+
.setStandardEnvironmentEnabled(true)
109+
.setStandardDeclarations(
110+
CelStandardDeclarations.newBuilder()
111+
.includeFunctions(StandardFunction.ADD)
112+
.build())
113+
.build();
114+
115+
assertThat(compiler.compile("1 + 1").hasError()).isFalse();
116+
assertThat(compiler.compile("1 - 1").hasError()).isTrue();
117+
}
118+
119+
@Test
120+
public void compiler_setStandardDeclarations_withStandardEnvironmentExplicitlyDisabled()
121+
throws Exception {
122+
CelCompiler compiler =
123+
CelCompilerFactory.standardCelCompilerBuilder()
124+
.setStandardEnvironmentEnabled(false)
125+
.setStandardDeclarations(
126+
CelStandardDeclarations.newBuilder()
127+
.includeFunctions(StandardFunction.ADD)
128+
.build())
129+
.build();
130+
131+
assertThat(compiler.compile("1 + 1").hasError()).isFalse();
132+
assertThat(compiler.compile("1 - 1").hasError()).isTrue();
133+
}
134+
135+
@Test
136+
public void compiler_setStandardDeclarations_emptyDisablesAllStandardDeclarations()
137+
throws Exception {
138+
CelCompiler compiler =
139+
CelCompilerFactory.standardCelCompilerBuilder()
140+
.setStandardDeclarations(CelStandardDeclarations.EMPTY)
141+
.build();
142+
143+
assertThat(compiler.compile("1 + 1").hasError()).isTrue();
144+
assertThat(compiler.compile("1 - 1").hasError()).isTrue();
145+
assertThat(compiler.compile("size([1])").hasError()).isTrue();
107146
}
108147

109148
@Test

‎compiler/src/main/java/dev/cel/compiler/CelCompilerBuilder.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,20 @@ public interface CelCompilerBuilder {
200200
@CanIgnoreReturnValue
201201
CelCompilerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet);
202202

203-
/** Enable or disable the standard CEL library functions and variables */
203+
/**
204+
* Enable or disable the standard CEL library functions and variables.
205+
*
206+
* @deprecated Use {@link #setStandardDeclarations(CelStandardDeclarations)} to configure or
207+
* subset the standard environment. Use {@link CelStandardDeclarations#EMPTY} to disable all
208+
* standard declarations.
209+
*/
210+
@Deprecated
204211
@CanIgnoreReturnValue
205212
CelCompilerBuilder setStandardEnvironmentEnabled(boolean value);
206213

207214
/**
208215
* Override the standard declarations for the type-checker. This can be used to subset the
209-
* standard environment to only expose the desired declarations to the type-checker. {@link
210-
* #setStandardEnvironmentEnabled(boolean)} must be set to false for this to take effect.
216+
* standard environment to only expose the desired declarations to the type-checker.
211217
*/
212218
@CanIgnoreReturnValue
213219
CelCompilerBuilder setStandardDeclarations(CelStandardDeclarations standardDeclarations);

‎compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ public CelCompilerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet) {
283283
}
284284

285285
@Override
286+
@Deprecated
286287
public CelCompilerBuilder setStandardEnvironmentEnabled(boolean value) {
287288
checkerBuilder.setStandardEnvironmentEnabled(value);
288289
return this;

0 commit comments

Comments
 (0)