From 9333702e07368e774532dc0f839811b3d1accf65 Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Wed, 19 Aug 2026 14:21:01 -0700 Subject: [PATCH] Deprecate setStandardEnvironmentEnabled flag in favor of subsetting PiperOrigin-RevId: 967407286 --- .../main/java/dev/cel/bundle/CelBuilder.java | 13 +++- .../src/main/java/dev/cel/bundle/CelImpl.java | 1 + .../src/test/java/dev/cel/bundle/BUILD.bazel | 2 + .../test/java/dev/cel/bundle/CelImplTest.java | 25 +++++++ .../dev/cel/checker/CelCheckerBuilder.java | 12 ++- .../dev/cel/checker/CelCheckerLegacyImpl.java | 13 +--- .../cel/checker/CelStandardDeclarations.java | 4 + .../checker/CelStandardDeclarationsTest.java | 73 ++++++++++++++----- .../dev/cel/compiler/CelCompilerBuilder.java | 12 ++- .../dev/cel/compiler/CelCompilerImpl.java | 1 + .../dev/cel/runtime/CelRuntimeBuilder.java | 9 ++- .../java/dev/cel/runtime/CelRuntimeImpl.java | 1 + .../dev/cel/runtime/CelRuntimeLegacyImpl.java | 1 + .../dev/cel/runtime/CelStandardFunctions.java | 4 + .../cel/runtime/CelStandardFunctionsTest.java | 5 ++ 15 files changed, 140 insertions(+), 36 deletions(-) diff --git a/bundle/src/main/java/dev/cel/bundle/CelBuilder.java b/bundle/src/main/java/dev/cel/bundle/CelBuilder.java index a45f846e4..53eb0126b 100644 --- a/bundle/src/main/java/dev/cel/bundle/CelBuilder.java +++ b/bundle/src/main/java/dev/cel/bundle/CelBuilder.java @@ -292,7 +292,15 @@ public interface CelBuilder { @CanIgnoreReturnValue CelBuilder addFileTypes(FileDescriptorSet fileDescriptorSet); - /** Enable or disable the standard CEL library functions and variables */ + /** + * Enable or disable the standard CEL library functions and variables. + * + * @deprecated Use {@link #setStandardDeclarations(CelStandardDeclarations)} and/or {@link + * #setStandardFunctions(CelStandardFunctions)} to configure or subset the standard + * environment. Use {@link CelStandardDeclarations#EMPTY} and {@link + * CelStandardFunctions#EMPTY} to disable all standard declarations and functions. + */ + @Deprecated @CanIgnoreReturnValue CelBuilder setStandardEnvironmentEnabled(boolean value); @@ -314,8 +322,7 @@ public interface CelBuilder { /** * Override the standard declarations for the type-checker. This can be used to subset the - * standard environment to only expose the desired declarations to the type-checker. {@link - * #setStandardEnvironmentEnabled(boolean)} must be set to false for this to take effect. + * standard environment to only expose the desired declarations to the type-checker. */ @CanIgnoreReturnValue CelBuilder setStandardDeclarations(CelStandardDeclarations standardDeclarations); diff --git a/bundle/src/main/java/dev/cel/bundle/CelImpl.java b/bundle/src/main/java/dev/cel/bundle/CelImpl.java index 999f1573a..b8c7c36e9 100644 --- a/bundle/src/main/java/dev/cel/bundle/CelImpl.java +++ b/bundle/src/main/java/dev/cel/bundle/CelImpl.java @@ -379,6 +379,7 @@ public CelBuilder addFileTypes(FileDescriptorSet fileDescriptorSet) { } @Override + @Deprecated public CelBuilder setStandardEnvironmentEnabled(boolean value) { compilerBuilder.setStandardEnvironmentEnabled(value); runtimeBuilder.setStandardEnvironmentEnabled(value); diff --git a/bundle/src/test/java/dev/cel/bundle/BUILD.bazel b/bundle/src/test/java/dev/cel/bundle/BUILD.bazel index 548b4483d..ddd2e7285 100644 --- a/bundle/src/test/java/dev/cel/bundle/BUILD.bazel +++ b/bundle/src/test/java/dev/cel/bundle/BUILD.bazel @@ -27,6 +27,7 @@ java_library( "//checker", "//checker:checker_legacy_environment", "//checker:proto_type_mask", + "//checker:standard_decl", "//common:cel_ast", "//common:cel_descriptor_util", "//common:cel_source", @@ -56,6 +57,7 @@ java_library( "//runtime:evaluation_exception_builder", "//runtime:evaluation_listener", "//runtime:function_binding", + "//runtime:standard_functions", "//runtime:unknown_attributes", "//testing:cel_runtime_flavor", "//testing/protos:single_file_extension_java_proto", diff --git a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java index fbacb242a..4f82411a3 100644 --- a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java +++ b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java @@ -60,6 +60,7 @@ import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import dev.cel.checker.CelCheckerLegacyImpl; +import dev.cel.checker.CelStandardDeclarations; import dev.cel.checker.DescriptorTypeProvider; import dev.cel.checker.ProtoTypeMask; import dev.cel.checker.TypeProvider; @@ -110,6 +111,7 @@ import dev.cel.runtime.CelRuntime.Program; import dev.cel.runtime.CelRuntimeFactory; import dev.cel.runtime.CelRuntimeLegacyImpl; +import dev.cel.runtime.CelStandardFunctions; import dev.cel.runtime.CelUnknownSet; import dev.cel.runtime.CelVariableResolver; import dev.cel.runtime.UnknownContext; @@ -2294,4 +2296,27 @@ private static Cel setupEnv(CelBuilder celBuilder) { .build()) .build(); } + + @Test + public void plannerCelBuilder_setStandardDeclarationsAndFunctions_subsetsEnvironment() + throws Exception { + Cel cel = + CelFactory.plannerCelBuilder() + .setStandardDeclarations( + CelStandardDeclarations.newBuilder() + .includeFunctions(CelStandardDeclarations.StandardFunction.ADD) + .build()) + .setStandardFunctions( + CelStandardFunctions.newBuilder() + .includeFunctions(CelStandardFunctions.StandardFunction.ADD) + .build()) + .build(); + + CelAbstractSyntaxTree ast = cel.compile("1 + 1").getAst(); + assertThat(cel.createProgram(ast).eval()).isEqualTo(2L); + + CelValidationException validationException = + assertThrows(CelValidationException.class, () -> cel.compile("1 - 1").getAst()); + assertThat(validationException).hasMessageThat().contains("undeclared reference to '_-_'"); + } } diff --git a/checker/src/main/java/dev/cel/checker/CelCheckerBuilder.java b/checker/src/main/java/dev/cel/checker/CelCheckerBuilder.java index a7d531f88..b14782e27 100644 --- a/checker/src/main/java/dev/cel/checker/CelCheckerBuilder.java +++ b/checker/src/main/java/dev/cel/checker/CelCheckerBuilder.java @@ -155,14 +155,20 @@ public interface CelCheckerBuilder { @CanIgnoreReturnValue CelCheckerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet); - /** Enable or disable the standard CEL library functions and variables */ + /** + * Enable or disable the standard CEL library functions and variables. + * + * @deprecated Use {@link #setStandardDeclarations(CelStandardDeclarations)} to configure or + * subset the standard environment. Use {@link CelStandardDeclarations#EMPTY} to disable all + * standard declarations. + */ + @Deprecated @CanIgnoreReturnValue CelCheckerBuilder setStandardEnvironmentEnabled(boolean value); /** * Override the standard declarations for the type-checker. This can be used to subset the - * standard environment to only expose the desired declarations to the type-checker. {@link - * #setStandardEnvironmentEnabled(boolean)} must be set to false for this to take effect. + * standard environment to only expose the desired declarations to the type-checker. */ @CanIgnoreReturnValue CelCheckerBuilder setStandardDeclarations(CelStandardDeclarations standardDeclarations); diff --git a/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java b/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java index ceab0fa93..329725e42 100644 --- a/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java +++ b/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java @@ -162,10 +162,10 @@ public void accept(EnvVisitor envVisitor) { private Env getEnv(Errors errors) { Env env; - if (standardEnvironmentEnabled) { - env = Env.standard(errors, typeProvider, celOptions); - } else if (overriddenStandardDeclarations != null) { + if (overriddenStandardDeclarations != null) { env = Env.standard(overriddenStandardDeclarations, errors, typeProvider, celOptions); + } else if (standardEnvironmentEnabled) { + env = Env.standard(errors, typeProvider, celOptions); } else { env = Env.unconfigured(errors, typeProvider, celOptions); } @@ -359,6 +359,7 @@ public CelCheckerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet) { } @Override + @Deprecated public CelCheckerBuilder setStandardEnvironmentEnabled(boolean value) { this.standardEnvironmentEnabled = value; return this; @@ -434,12 +435,6 @@ CelTypeProvider celTypeProvider() { @Override @CheckReturnValue public CelCheckerLegacyImpl build() { - if (standardEnvironmentEnabled && standardDeclarations != null) { - throw new IllegalArgumentException( - "setStandardEnvironmentEnabled must be set to false to override standard" - + " declarations."); - } - // Add libraries, such as extensions ImmutableSet checkerLibraries = celCheckerLibraries.build(); checkerLibraries.forEach(celLibrary -> celLibrary.setCheckerOptions(this)); diff --git a/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java b/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java index 3d5175cb5..bd63c4279 100644 --- a/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java +++ b/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java @@ -51,6 +51,10 @@ public final class CelStandardDeclarations { private static final TypeParamType TYPE_PARAM_B = TypeParamType.create("B"); private static final MapType MAP_OF_AB = MapType.create(TYPE_PARAM_A, TYPE_PARAM_B); + /** An empty instance of {@link CelStandardDeclarations} with no functions or identifiers. */ + public static final CelStandardDeclarations EMPTY = + new CelStandardDeclarations(ImmutableSet.of(), ImmutableSet.of()); + private final ImmutableSet celFunctionDecls; private final ImmutableSet celIdentDecls; diff --git a/checker/src/test/java/dev/cel/checker/CelStandardDeclarationsTest.java b/checker/src/test/java/dev/cel/checker/CelStandardDeclarationsTest.java index 17a7212a1..f867728b0 100644 --- a/checker/src/test/java/dev/cel/checker/CelStandardDeclarationsTest.java +++ b/checker/src/test/java/dev/cel/checker/CelStandardDeclarationsTest.java @@ -86,24 +86,63 @@ public void standardDeclaration_moreThanOneIdentifierFilterSet_throws( } @Test - public void compiler_standardEnvironmentEnabled_throwsWhenOverridingDeclarations() { - IllegalArgumentException e = - assertThrows( - IllegalArgumentException.class, - () -> - CelCompilerFactory.standardCelCompilerBuilder() - .setStandardEnvironmentEnabled(true) - .setStandardDeclarations( - CelStandardDeclarations.newBuilder() - .includeFunctions(StandardFunction.ADD, StandardFunction.SUBTRACT) - .build()) - .build()); + public void compiler_setStandardDeclarations_overridesDefaultStandardEnvironment() + throws Exception { + CelCompiler compiler = + CelCompilerFactory.standardCelCompilerBuilder() + .setStandardDeclarations( + CelStandardDeclarations.newBuilder() + .includeFunctions(StandardFunction.ADD) + .build()) + .build(); - assertThat(e) - .hasMessageThat() - .contains( - "setStandardEnvironmentEnabled must be set to false to override standard" - + " declarations."); + assertThat(compiler.compile("1 + 1").hasError()).isFalse(); + assertThat(compiler.compile("1 - 1").hasError()).isTrue(); + } + + @Test + public void compiler_setStandardDeclarations_withStandardEnvironmentExplicitlyEnabled() + throws Exception { + CelCompiler compiler = + CelCompilerFactory.standardCelCompilerBuilder() + .setStandardEnvironmentEnabled(true) + .setStandardDeclarations( + CelStandardDeclarations.newBuilder() + .includeFunctions(StandardFunction.ADD) + .build()) + .build(); + + assertThat(compiler.compile("1 + 1").hasError()).isFalse(); + assertThat(compiler.compile("1 - 1").hasError()).isTrue(); + } + + @Test + public void compiler_setStandardDeclarations_withStandardEnvironmentExplicitlyDisabled() + throws Exception { + CelCompiler compiler = + CelCompilerFactory.standardCelCompilerBuilder() + .setStandardEnvironmentEnabled(false) + .setStandardDeclarations( + CelStandardDeclarations.newBuilder() + .includeFunctions(StandardFunction.ADD) + .build()) + .build(); + + assertThat(compiler.compile("1 + 1").hasError()).isFalse(); + assertThat(compiler.compile("1 - 1").hasError()).isTrue(); + } + + @Test + public void compiler_setStandardDeclarations_emptyDisablesAllStandardDeclarations() + throws Exception { + CelCompiler compiler = + CelCompilerFactory.standardCelCompilerBuilder() + .setStandardDeclarations(CelStandardDeclarations.EMPTY) + .build(); + + assertThat(compiler.compile("1 + 1").hasError()).isTrue(); + assertThat(compiler.compile("1 - 1").hasError()).isTrue(); + assertThat(compiler.compile("size([1])").hasError()).isTrue(); } @Test diff --git a/compiler/src/main/java/dev/cel/compiler/CelCompilerBuilder.java b/compiler/src/main/java/dev/cel/compiler/CelCompilerBuilder.java index 6dd2ee12e..08231657a 100644 --- a/compiler/src/main/java/dev/cel/compiler/CelCompilerBuilder.java +++ b/compiler/src/main/java/dev/cel/compiler/CelCompilerBuilder.java @@ -200,14 +200,20 @@ public interface CelCompilerBuilder { @CanIgnoreReturnValue CelCompilerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet); - /** Enable or disable the standard CEL library functions and variables */ + /** + * Enable or disable the standard CEL library functions and variables. + * + * @deprecated Use {@link #setStandardDeclarations(CelStandardDeclarations)} to configure or + * subset the standard environment. Use {@link CelStandardDeclarations#EMPTY} to disable all + * standard declarations. + */ + @Deprecated @CanIgnoreReturnValue CelCompilerBuilder setStandardEnvironmentEnabled(boolean value); /** * Override the standard declarations for the type-checker. This can be used to subset the - * standard environment to only expose the desired declarations to the type-checker. {@link - * #setStandardEnvironmentEnabled(boolean)} must be set to false for this to take effect. + * standard environment to only expose the desired declarations to the type-checker. */ @CanIgnoreReturnValue CelCompilerBuilder setStandardDeclarations(CelStandardDeclarations standardDeclarations); diff --git a/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java b/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java index e8804f348..eb3e1549b 100644 --- a/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java +++ b/compiler/src/main/java/dev/cel/compiler/CelCompilerImpl.java @@ -283,6 +283,7 @@ public CelCompilerBuilder addFileTypes(FileDescriptorSet fileDescriptorSet) { } @Override + @Deprecated public CelCompilerBuilder setStandardEnvironmentEnabled(boolean value) { checkerBuilder.setStandardEnvironmentEnabled(value); return this; diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java index e284b374c..00f6e3bf7 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java @@ -170,7 +170,14 @@ public interface CelRuntimeBuilder { /** Returns the configured {@link CelValueProvider}, or null if not set. */ CelValueProvider valueProvider(); - /** Enable or disable the standard CEL library functions and variables. */ + /** + * Enable or disable the standard CEL library functions and variables. + * + * @deprecated Use {@link #setStandardFunctions(CelStandardFunctions)} to configure or subset the + * standard environment. Use {@link CelStandardFunctions#EMPTY} to disable all standard + * functions. + */ + @Deprecated @CanIgnoreReturnValue CelRuntimeBuilder setStandardEnvironmentEnabled(boolean value); diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java index f934108e0..5cda25800 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java @@ -393,6 +393,7 @@ public Builder setTypeFactory(Function typeFactory) { } @Override + @Deprecated public Builder setStandardEnvironmentEnabled(boolean value) { throw new UnsupportedOperationException( "Unsupported. Subset the environment using setStandardFunctions instead."); diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java index cad7e74f8..428c6dba5 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java @@ -219,6 +219,7 @@ public CelRuntimeBuilder setTypeFactory(Function typeFa } @Override + @Deprecated public CelRuntimeBuilder setStandardEnvironmentEnabled(boolean value) { standardEnvironmentEnabled = value; return this; diff --git a/runtime/src/main/java/dev/cel/runtime/CelStandardFunctions.java b/runtime/src/main/java/dev/cel/runtime/CelStandardFunctions.java index 39797e086..c9fd4a50e 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelStandardFunctions.java +++ b/runtime/src/main/java/dev/cel/runtime/CelStandardFunctions.java @@ -140,6 +140,10 @@ public final class CelStandardFunctions { GreaterEqualsOverload.GREATER_EQUALS_UINT64_DOUBLE, GreaterEqualsOverload.GREATER_EQUALS_DOUBLE_UINT64); + /** An empty instance of {@link CelStandardFunctions} with no functions. */ + public static final CelStandardFunctions EMPTY = + new CelStandardFunctions(ImmutableMultimap.of()); + private final ImmutableMultimap standardOverloads; public static final ImmutableSet ALL_STANDARD_FUNCTIONS = diff --git a/runtime/src/test/java/dev/cel/runtime/CelStandardFunctionsTest.java b/runtime/src/test/java/dev/cel/runtime/CelStandardFunctionsTest.java index c5f5572a7..f452b99dc 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelStandardFunctionsTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelStandardFunctionsTest.java @@ -136,6 +136,11 @@ public void standardFunctions_filterFunctions() { .containsExactly(AddOverload.ADD_INT64, SubtractOverload.SUBTRACT_INT64); } + @Test + public void standardFunctions_empty() { + assertThat(CelStandardFunctions.EMPTY.getOverloads()).isEmpty(); + } + @Test public void standardEnvironment_subsetEnvironment() throws Exception { CelCompiler celCompiler = CelCompilerFactory.standardCelCompilerBuilder().build();