From 38d3d0867e47d5d29dfdaec5e75bc4f7003d8f3e Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Tue, 15 Sep 2026 16:43:10 -0700 Subject: [PATCH] Consolidate CelIdentDecl into CelVarDecl, extract CelProtoDeclConverter PiperOrigin-RevId: 982118176 --- .../src/main/java/dev/cel/bundle/BUILD.bazel | 10 +- .../java/dev/cel/bundle/CelEnvironment.java | 6 +- .../cel/bundle/CelEnvironmentExporter.java | 31 ++--- checker/BUILD.bazel | 5 - .../src/main/java/dev/cel/checker/BUILD.bazel | 45 +++---- .../dev/cel/checker/CelCheckerLegacyImpl.java | 63 ++++------ .../java/dev/cel/checker/CelIdentDecl.java | 103 ---------------- .../cel/checker/CelStandardDeclarations.java | 29 ++--- .../src/main/java/dev/cel/checker/Env.java | 99 +++++++-------- .../java/dev/cel/checker/ExprChecker.java | 20 ++-- .../checker/ProtoTypeMaskTypeProvider.java | 7 +- .../src/test/java/dev/cel/checker/BUILD.bazel | 8 +- .../dev/cel/checker/CelOverloadDeclTest.java | 11 +- .../java/dev/cel/checker/ExprCheckerTest.java | 22 +--- .../ProtoTypeMaskTypeProviderTest.java | 5 +- common/BUILD.bazel | 21 ++++ .../src/main/java/dev/cel/common/BUILD.bazel | 64 +++++++++- .../java/dev/cel/common/CelFunctionDecl.java | 18 --- .../java/dev/cel/common/CelOverloadDecl.java | 31 ----- .../dev/cel/common/CelProtoDeclConverter.java | 113 ++++++++++++++++++ .../main/java/dev/cel/common/CelVarDecl.java | 55 ++++++++- .../src/test/java/dev/cel/common/BUILD.bazel | 5 +- .../java/dev/cel/common/CelVarDeclTest.java | 52 ++++---- 23 files changed, 434 insertions(+), 389 deletions(-) delete mode 100644 checker/src/main/java/dev/cel/checker/CelIdentDecl.java create mode 100644 common/src/main/java/dev/cel/common/CelProtoDeclConverter.java rename checker/src/test/java/dev/cel/checker/CelIdentDeclTest.java => common/src/test/java/dev/cel/common/CelVarDeclTest.java (62%) diff --git a/bundle/src/main/java/dev/cel/bundle/BUILD.bazel b/bundle/src/main/java/dev/cel/bundle/BUILD.bazel index be4fade3d..40ce8df32 100644 --- a/bundle/src/main/java/dev/cel/bundle/BUILD.bazel +++ b/bundle/src/main/java/dev/cel/bundle/BUILD.bazel @@ -108,7 +108,9 @@ java_library( "//bundle:cel", "//checker:proto_type_mask", "//checker:standard_decl", - "//common:compiler_common", + "//common:cel_function_decl", + "//common:cel_overload_decl", + "//common:cel_var_decl", "//common:container", "//common:options", "//common:source", @@ -172,10 +174,12 @@ java_library( "//bundle:cel", "//checker:checker_builder", "//checker:standard_decl", - "//common:compiler_common", + "//common:cel_function_decl", + "//common:cel_overload_decl", + "//common:cel_proto_decl_converter", + "//common:cel_var_decl", "//common:options", "//common/internal:env_visitor", - "//common/types:cel_proto_types", "//common/types:type_providers", "//compiler:compiler_builder", "//extensions", diff --git a/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java b/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java index f26d4e3fd..192f368b3 100644 --- a/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java +++ b/bundle/src/main/java/dev/cel/bundle/CelEnvironment.java @@ -478,7 +478,11 @@ public static VariableDecl create(String name, TypeDecl type) { /** Converts this policy variable declaration into a {@link CelVarDecl}. */ public CelVarDecl toCelVarDecl(CelTypeProvider celTypeProvider) { - return CelVarDecl.newVarDeclaration(name(), type().toCelType(celTypeProvider)); + return CelVarDecl.newBuilder() + .setName(name()) + .setType(type().toCelType(celTypeProvider)) + .setDoc(description().orElse("")) + .build(); } } diff --git a/bundle/src/main/java/dev/cel/bundle/CelEnvironmentExporter.java b/bundle/src/main/java/dev/cel/bundle/CelEnvironmentExporter.java index 6e10edd92..462de653d 100644 --- a/bundle/src/main/java/dev/cel/bundle/CelEnvironmentExporter.java +++ b/bundle/src/main/java/dev/cel/bundle/CelEnvironmentExporter.java @@ -37,11 +37,11 @@ import dev.cel.common.CelFunctionDecl; import dev.cel.common.CelOptions; import dev.cel.common.CelOverloadDecl; +import dev.cel.common.CelProtoDeclConverter; import dev.cel.common.CelVarDecl; import dev.cel.common.internal.EnvVisitable; import dev.cel.common.internal.EnvVisitor; import dev.cel.common.types.CelKind; -import dev.cel.common.types.CelProtoTypes; import dev.cel.common.types.CelType; import dev.cel.compiler.CelCompiler; import dev.cel.extensions.CelExtensionLibrary; @@ -262,13 +262,11 @@ public void visitDecl(String name, List decls) { for (Overload overload : function.getOverloadsList()) { inventory.add( NamedOverload.create( - decl.getName(), CelOverloadDecl.overloadToCelOverload(overload))); + decl.getName(), + CelProtoDeclConverter.overloadToCelOverload(overload))); } } else if (decl.hasIdent()) { - inventory.add( - CelVarDecl.newVarDeclaration( - decl.getName(), - CelProtoTypes.typeToCelType(decl.getIdent().getType()))); + inventory.add(CelProtoDeclConverter.declToCelVarDecl(decl)); } } } @@ -299,7 +297,7 @@ private void addExtensionConfigsAndRemoveFromInventory( featureSets.sort( Comparator.comparing(NamedFeatureSet::name) - .thenComparing(nfs -> nfs.featureSet().version()) + .thenComparingInt(nfs -> nfs.featureSet().version()) .reversed()); Set includedExtensions = new HashSet<>(); @@ -348,8 +346,7 @@ private void addStandardLibrarySubsetAndRemoveFromInventory( CelEnvironment.Builder envBuilder, Set inventory) { // Claim standard identifiers for the standard library for (StandardIdentifier value : StandardIdentifier.values()) { - inventory.remove( - CelVarDecl.newVarDeclaration(value.identDecl().name(), value.identDecl().type())); + inventory.remove(value.identDecl()); } Set excludedFunctions = new HashSet<>(); @@ -431,13 +428,13 @@ private ImmutableSet buildFunctionSelectors( private void addCustomDecls(CelEnvironment.Builder envBuilder, Set inventory) { // Group "orphaned" function overloads and vars by their names ListMultimap extraOverloads = ArrayListMultimap.create(); - Map extraVars = new HashMap<>(); + Map extraVars = new HashMap<>(); for (Object item : inventory) { if (item instanceof NamedOverload) { extraOverloads.put( ((NamedOverload) item).functionName(), ((NamedOverload) item).overload()); } else if (item instanceof CelVarDecl) { - extraVars.put(((CelVarDecl) item).name(), ((CelVarDecl) item).type()); + extraVars.put(((CelVarDecl) item).name(), (CelVarDecl) item); } } @@ -457,9 +454,15 @@ private void addCustomDecls(CelEnvironment.Builder envBuilder, Set inven if (!extraVars.isEmpty()) { ImmutableSet.Builder varDeclBuilder = ImmutableSet.builder(); - for (String ident : extraVars.keySet()) { - varDeclBuilder.add( - CelEnvironment.VariableDecl.create(ident, toCelEnvTypeDecl(extraVars.get(ident)))); + for (CelVarDecl varDecl : extraVars.values()) { + CelEnvironment.VariableDecl.Builder builder = + CelEnvironment.VariableDecl.newBuilder() + .setName(varDecl.name()) + .setType(toCelEnvTypeDecl(varDecl.type())); + if (!varDecl.doc().isEmpty()) { + builder.setDescription(varDecl.doc()); + } + varDeclBuilder.add(builder.build()); } envBuilder.setVariables(varDeclBuilder.build()); } diff --git a/checker/BUILD.bazel b/checker/BUILD.bazel index 5fa3c6c05..ac00ddff1 100644 --- a/checker/BUILD.bazel +++ b/checker/BUILD.bazel @@ -32,11 +32,6 @@ java_library( exports = ["//checker/src/main/java/dev/cel/checker:type_provider_legacy_impl"], ) -java_library( - name = "cel_ident_decl", - exports = ["//checker/src/main/java/dev/cel/checker:cel_ident_decl"], -) - java_library( name = "checker_legacy_environment", deprecation = "See go/cel-java-migration-guide. Please use CEL-Java Fluent APIs //compiler instead", diff --git a/checker/src/main/java/dev/cel/checker/BUILD.bazel b/checker/src/main/java/dev/cel/checker/BUILD.bazel index 99d4da586..304ce0ec4 100644 --- a/checker/src/main/java/dev/cel/checker/BUILD.bazel +++ b/checker/src/main/java/dev/cel/checker/BUILD.bazel @@ -65,22 +65,23 @@ java_library( tags = [ ], deps = [ - ":cel_ident_decl", ":checker_builder", ":checker_legacy_environment", ":proto_type_mask", ":standard_decl", ":type_provider_legacy_impl", - "//:auto_value", "//common:cel_ast", "//common:cel_descriptor_util", + "//common:cel_function_decl", + "//common:cel_issue", + "//common:cel_proto_decl_converter", "//common:cel_source", - "//common:compiler_common", + "//common:cel_validation_result", + "//common:cel_var_decl", "//common:container", "//common:options", "//common:source_location", "//common/annotations", - "//common/ast:expr_converter", "//common/internal:env_visitor", "//common/internal:errors", "//common/types", @@ -105,7 +106,9 @@ java_library( ":proto_type_mask", ":standard_decl", "//common:cel_ast", - "//common:compiler_common", + "//common:cel_function_decl", + "//common:cel_validation_result", + "//common:cel_var_decl", "//common:container", "//common:options", "//common/types:type_providers", @@ -128,26 +131,6 @@ java_library( ], ) -java_library( - name = "cel_ident_decl", - srcs = [ - "CelIdentDecl.java", - ], - tags = [ - ], - deps = [ - "//:auto_value", - "//common/annotations", - "//common/ast", - "//common/ast:expr_converter", - "//common/types:cel_proto_types", - "//common/types:type_providers", - "@cel_spec//proto/cel/expr:checked_java_proto", - "@maven//:com_google_errorprone_error_prone_annotations", - "@maven//:com_google_guava_guava", - ], -) - java_library( name = "type_provider_legacy_impl", srcs = ["TypeProviderLegacyImpl.java"], @@ -155,7 +138,6 @@ java_library( ], deps = [ ":checker_legacy_environment", - "//:auto_value", "//common/annotations", "//common/types", "//common/types:cel_proto_types", @@ -173,12 +155,14 @@ java_library( tags = [ ], deps = [ - ":cel_ident_decl", ":standard_decl", "//:auto_value", "//common:cel_ast", + "//common:cel_function_decl", + "//common:cel_overload_decl", + "//common:cel_proto_decl_converter", "//common:cel_source", - "//common:compiler_common", + "//common:cel_var_decl", "//common:container", "//common:mutable_ast", "//common:operator", @@ -240,8 +224,9 @@ java_library( tags = [ ], deps = [ - ":cel_ident_decl", - "//common:compiler_common", + "//common:cel_function_decl", + "//common:cel_overload_decl", + "//common:cel_var_decl", "//common:operator", "//common/types", "//common/types:cel_types", diff --git a/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java b/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java index 329725e42..3384ccd63 100644 --- a/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java +++ b/checker/src/main/java/dev/cel/checker/CelCheckerLegacyImpl.java @@ -36,13 +36,12 @@ import dev.cel.common.CelFunctionDecl; import dev.cel.common.CelIssue; import dev.cel.common.CelOptions; -import dev.cel.common.CelOverloadDecl; +import dev.cel.common.CelProtoDeclConverter; import dev.cel.common.CelSource; import dev.cel.common.CelSourceLocation; import dev.cel.common.CelValidationResult; import dev.cel.common.CelVarDecl; import dev.cel.common.annotations.Internal; -import dev.cel.common.ast.CelExprConverter; import dev.cel.common.internal.EnvVisitable; import dev.cel.common.internal.EnvVisitor; import dev.cel.common.internal.Errors; @@ -70,7 +69,7 @@ public final class CelCheckerLegacyImpl implements CelChecker, EnvVisitable { private final CelOptions celOptions; private final CelContainer container; - private final ImmutableSet identDeclarations; + private final ImmutableSet identDeclarations; private final ImmutableSet functionDeclarations; private final Optional expectedResultType; @@ -115,7 +114,7 @@ public CelTypeProvider getTypeProvider() { public CelCheckerBuilder toCheckerBuilder() { CelCheckerBuilder builder = new Builder() - .addIdentDeclarations(identDeclarations) + .addVarDeclarations(identDeclarations) .setOptions(celOptions) .setTypeProvider(celTypeProvider) .setContainer(container) @@ -146,14 +145,14 @@ public void accept(EnvVisitor envVisitor) { names.addAll(declGroup.getIdents().keySet()); names.addAll(declGroup.getFunctions().keySet()); for (String name : names) { - CelIdentDecl ident = declGroup.getIdent(name); + CelVarDecl ident = declGroup.getIdent(name); CelFunctionDecl func = declGroup.getFunction(name); List decls = new ArrayList<>(); if (ident != null) { - decls.add(CelIdentDecl.celIdentToDecl(ident)); + decls.add(CelProtoDeclConverter.celVarDeclToDecl(ident)); } if (func != null) { - decls.add(CelFunctionDecl.celFunctionDeclToDecl(func)); + decls.add(CelProtoDeclConverter.celFunctionDeclToDecl(func)); } envVisitor.visitDecl(name, decls); } @@ -182,7 +181,7 @@ public static CelCheckerBuilder newBuilder() { /** Builder class for the legacy {@code CelChecker} implementation. */ public static final class Builder implements CelCheckerBuilder { - private final ImmutableSet.Builder identDeclarations; + private final ImmutableSet.Builder identDeclarations; private final ImmutableSet.Builder functionDeclarations; private final ImmutableSet.Builder protoTypeMasks; private final ImmutableSet.Builder messageTypes; @@ -231,27 +230,10 @@ public CelCheckerBuilder addDeclarations(Iterable declarations) { for (Decl decl : declarations) { switch (decl.getDeclKindCase()) { case IDENT: - CelIdentDecl.Builder identBuilder = - CelIdentDecl.newBuilder() - .setName(decl.getName()) - .setType(CelProtoTypes.typeToCelType(decl.getIdent().getType())) - // Note: Setting doc and constant value exists for compatibility reason. This - // should not be set by the users. - .setDoc(decl.getIdent().getDoc()); - if (decl.getIdent().hasValue()) { - identBuilder.setConstant( - CelExprConverter.exprConstantToCelConstant(decl.getIdent().getValue())); - } - - this.identDeclarations.add(identBuilder.build()); + this.identDeclarations.add(CelProtoDeclConverter.declToCelVarDecl(decl)); break; case FUNCTION: - addFunctionDeclarations( - CelFunctionDecl.newFunctionDeclaration( - decl.getName(), - decl.getFunction().getOverloadsList().stream() - .map(CelOverloadDecl::overloadToCelOverload) - .collect(toImmutableList()))); + addFunctionDeclarations(CelProtoDeclConverter.declToCelFunctionDecl(decl)); break; default: throw new IllegalArgumentException("unexpected decl kind: " + decl.getDeclKindCase()); @@ -283,10 +265,7 @@ public CelCheckerBuilder addVarDeclarations(CelVarDecl... celVarDecls) { @Override public CelCheckerBuilder addVarDeclarations(Iterable celVarDecls) { checkNotNull(celVarDecls); - for (CelVarDecl celVarDecl : celVarDecls) { - this.identDeclarations.add( - CelIdentDecl.newIdentDeclaration(celVarDecl.name(), celVarDecl.type())); - } + this.identDeclarations.addAll(celVarDecls); return this; } @@ -384,12 +363,6 @@ public CelCheckerBuilder addLibraries(Iterable libr return this; } - @CanIgnoreReturnValue - Builder addIdentDeclarations(ImmutableSet identDeclarations) { - this.identDeclarations.addAll(identDeclarations); - return this; - } - // The following getters marked @VisibleForTesting exist for testing toCheckerBuilder copies // over all properties. Do not expose these to public @VisibleForTesting @@ -398,7 +371,7 @@ ImmutableSet.Builder functionDecls() { } @VisibleForTesting - ImmutableSet.Builder identDecls() { + ImmutableSet.Builder identDecls() { return this.identDeclarations; } @@ -468,15 +441,20 @@ public CelCheckerLegacyImpl build() { // Configure the declaration set, and possibly alter the type provider if ProtoDecl values // are provided as they may prevent the use of certain field selection patterns against the // proto. - ImmutableSet identDeclarationSet = identDeclarations.build(); + ImmutableSet identDeclarationSet = identDeclarations.build(); ImmutableSet protoTypeMaskSet = protoTypeMasks.build(); if (!protoTypeMaskSet.isEmpty()) { ProtoTypeMaskTypeProvider protoTypeMaskTypeProvider = new ProtoTypeMaskTypeProvider(messageTypeProvider, protoTypeMaskSet); + ImmutableSet declaredNames = + identDeclarationSet.stream().map(CelVarDecl::name).collect(toImmutableSet()); identDeclarationSet = - ImmutableSet.builder() + ImmutableSet.builder() .addAll(identDeclarationSet) - .addAll(protoTypeMaskTypeProvider.computeDeclsFromProtoTypeMasks()) + .addAll( + protoTypeMaskTypeProvider.computeDeclsFromProtoTypeMasks().stream() + .filter(decl -> !declaredNames.contains(decl.name())) + .collect(toImmutableSet())) .build(); messageTypeProvider = protoTypeMaskTypeProvider; } @@ -518,7 +496,7 @@ private Builder() { private CelCheckerLegacyImpl( CelOptions celOptions, CelContainer container, - ImmutableSet identDeclarations, + ImmutableSet identDeclarations, ImmutableSet functionDeclarations, Optional expectedResultType, TypeProvider typeProvider, @@ -528,6 +506,7 @@ private CelCheckerLegacyImpl( ImmutableSet checkerLibraries, ImmutableSet fileDescriptors, ImmutableSet protoTypeMasks) { + this.celOptions = celOptions; this.container = container; this.identDeclarations = identDeclarations; diff --git a/checker/src/main/java/dev/cel/checker/CelIdentDecl.java b/checker/src/main/java/dev/cel/checker/CelIdentDecl.java deleted file mode 100644 index 60f747b77..000000000 --- a/checker/src/main/java/dev/cel/checker/CelIdentDecl.java +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright 2023 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package dev.cel.checker; - -import dev.cel.expr.Decl; -import dev.cel.expr.Decl.IdentDecl; -import com.google.auto.value.AutoValue; -import com.google.errorprone.annotations.CanIgnoreReturnValue; -import com.google.errorprone.annotations.CheckReturnValue; -import com.google.errorprone.annotations.Immutable; -import dev.cel.common.annotations.Internal; -import dev.cel.common.ast.CelConstant; -import dev.cel.common.ast.CelExprConverter; -import dev.cel.common.types.CelProtoTypes; -import dev.cel.common.types.CelType; -import java.util.Optional; - -/** - * Abstract representation of a CEL identifier declaration. - * - *

CEL Library Internals. Do Not Use. - */ -@AutoValue -@Immutable -@Internal -public abstract class CelIdentDecl { - - /** Fully qualified variable name. */ - public abstract String name(); - - /** The type of the variable. */ - public abstract CelType type(); - - /** - * The constant value of the identifier. If not specified, the identifier must be supplied at - * evaluation time. - */ - public abstract Optional constant(); - - /** Documentation string for the identifier. */ - public abstract String doc(); - - /** Converts a {@link CelIdentDecl} to a protobuf equivalent form {@code Decl} */ - public static Decl celIdentToDecl(CelIdentDecl identDecl) { - IdentDecl.Builder identBuilder = - IdentDecl.newBuilder() - .setDoc(identDecl.doc()) - .setType(CelProtoTypes.celTypeToType(identDecl.type())); - if (identDecl.constant().isPresent()) { - identBuilder.setValue(CelExprConverter.celConstantToExprConstant(identDecl.constant().get())); - } - return Decl.newBuilder().setName(identDecl.name()).setIdent(identBuilder).build(); - } - - /** Create a new {@code CelIdentDecl} with a given {@code name} and {@code type}. */ - @CheckReturnValue - public static CelIdentDecl newIdentDeclaration(String name, CelType type) { - return newBuilder().setName(name).setType(type).build(); - } - - public static Builder newBuilder() { - return new AutoValue_CelIdentDecl.Builder().setDoc(""); - } - - /** Builder for configuring the {@link CelIdentDecl}. */ - @AutoValue.Builder - public abstract static class Builder { - @CanIgnoreReturnValue - public abstract Builder setName(String name); - - @CanIgnoreReturnValue - public abstract Builder setType(CelType name); - - @CanIgnoreReturnValue - public abstract Builder setConstant(CelConstant constant); - - @CanIgnoreReturnValue - public abstract Builder setConstant(Optional constant); - - @CanIgnoreReturnValue - public abstract Builder setDoc(String value); - - @CanIgnoreReturnValue - public Builder clearConstant() { - return setConstant(Optional.empty()); - } - - @CheckReturnValue - public abstract CelIdentDecl build(); - } -} diff --git a/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java b/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java index bd63c4279..218135c1d 100644 --- a/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java +++ b/checker/src/main/java/dev/cel/checker/CelStandardDeclarations.java @@ -25,6 +25,7 @@ import com.google.errorprone.annotations.Immutable; import dev.cel.common.CelFunctionDecl; import dev.cel.common.CelOverloadDecl; +import dev.cel.common.CelVarDecl; import dev.cel.common.Operator; import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypes; @@ -56,7 +57,7 @@ public final class CelStandardDeclarations { new CelStandardDeclarations(ImmutableSet.of(), ImmutableSet.of()); private final ImmutableSet celFunctionDecls; - private final ImmutableSet celIdentDecls; + private final ImmutableSet celIdentDecls; /** Enumeration of Standard Functions. */ public enum StandardFunction implements CelFunctionDecl.Declarer { @@ -1559,25 +1560,25 @@ public enum StandardIdentifier { MAP(newStandardIdentDecl("map", MapType.create(SimpleType.DYN, SimpleType.DYN))), ; - private static CelIdentDecl newStandardIdentDecl(CelType celType) { + private final CelVarDecl identDecl; + + public CelVarDecl identDecl() { + return identDecl; + } + + private static CelVarDecl newStandardIdentDecl(CelType celType) { return newStandardIdentDecl(CelTypes.format(celType), celType); } - private static CelIdentDecl newStandardIdentDecl(String identName, CelType celType) { - return CelIdentDecl.newBuilder() + private static CelVarDecl newStandardIdentDecl(String identName, CelType celType) { + return CelVarDecl.newBuilder() .setName(identName) .setType(TypeType.create(celType)) .setDoc("type denotation") .build(); } - private final CelIdentDecl identDecl; - - public CelIdentDecl identDecl() { - return identDecl; - } - - StandardIdentifier(CelIdentDecl identDecl) { + StandardIdentifier(CelVarDecl identDecl) { this.identDecl = identDecl; } } @@ -1740,7 +1741,7 @@ public CelStandardDeclarations build() { functionDeclBuilder.add(standardFunction.celFunctionDecl); } - ImmutableSet.Builder identBuilder = ImmutableSet.builder(); + ImmutableSet.Builder identBuilder = ImmutableSet.builder(); for (StandardIdentifier standardIdentifier : StandardIdentifier.values()) { if (hasIncludeIdentifiers) { if (this.includeIdentifiers.contains(standardIdentifier)) { @@ -1804,12 +1805,12 @@ ImmutableSet functionDecls() { return celFunctionDecls; } - ImmutableSet identifierDecls() { + ImmutableSet identifierDecls() { return celIdentDecls; } private CelStandardDeclarations( - ImmutableSet celFunctionDecls, ImmutableSet celIdentDecls) { + ImmutableSet celFunctionDecls, ImmutableSet celIdentDecls) { this.celFunctionDecls = celFunctionDecls; this.celIdentDecls = celIdentDecls; } diff --git a/checker/src/main/java/dev/cel/checker/Env.java b/checker/src/main/java/dev/cel/checker/Env.java index 75ed7cf1f..81fec362c 100644 --- a/checker/src/main/java/dev/cel/checker/Env.java +++ b/checker/src/main/java/dev/cel/checker/Env.java @@ -34,6 +34,8 @@ import dev.cel.common.CelFunctionDecl; import dev.cel.common.CelOptions; import dev.cel.common.CelOverloadDecl; +import dev.cel.common.CelProtoDeclConverter; +import dev.cel.common.CelVarDecl; import dev.cel.common.annotations.Internal; import dev.cel.common.ast.CelConstant; import dev.cel.common.ast.CelExpr; @@ -71,8 +73,8 @@ public class Env { public static final int ROOT_SCOPE = 0; /** An ident declaration to represent an error. */ - public static final CelIdentDecl ERROR_IDENT_DECL = - CelIdentDecl.newBuilder().setName("*error*").setType(SimpleType.ERROR).build(); + public static final CelVarDecl ERROR_IDENT_DECL = + CelVarDecl.newBuilder().setName("*error*").setType(SimpleType.ERROR).build(); /** A function declaration to represent an error. */ public static final CelFunctionDecl ERROR_FUNCTION_DECL = @@ -347,28 +349,9 @@ void setRef(CelMutableExpr expr, CelReference reference) { public Env add(Decl decl) { switch (decl.getDeclKindCase()) { case IDENT: - CelIdentDecl.Builder identBuilder = - CelIdentDecl.newBuilder() - .setName(decl.getName()) - .setType(CelProtoTypes.typeToCelType(decl.getIdent().getType())) - // Note: Setting doc and constant value exists for compatibility reason. This should - // not be set by the users. - .setDoc(decl.getIdent().getDoc()); - if (decl.getIdent().hasValue()) { - identBuilder.setConstant( - CelExprConverter.exprConstantToCelConstant(decl.getIdent().getValue())); - } - return add(identBuilder.build()); + return add(CelProtoDeclConverter.declToCelVarDecl(decl)); case FUNCTION: - ImmutableList.Builder overloadDeclBuilder = new ImmutableList.Builder<>(); - for (Overload overload : decl.getFunction().getOverloadsList()) { - overloadDeclBuilder.add(CelOverloadDecl.overloadToCelOverload(overload)); - } - return add( - CelFunctionDecl.newBuilder() - .setName(decl.getName()) - .addOverloads(overloadDeclBuilder.build()) - .build()); + return add(CelProtoDeclConverter.declToCelFunctionDecl(decl)); default: break; } @@ -381,8 +364,8 @@ public Env add(CelFunctionDecl celFunctionDecl) { } @CanIgnoreReturnValue - public Env add(CelIdentDecl celIdentDecl) { - return addIdent(sanitizeIdent(celIdentDecl)); + public Env add(CelVarDecl celVarDecl) { + return addIdent(sanitizeIdent(celVarDecl)); } /** @@ -395,7 +378,7 @@ public Env add(CelIdentDecl celIdentDecl) { @CanIgnoreReturnValue @Deprecated public Env add(String name, Type type) { - return add(CelIdentDecl.newIdentDeclaration(name, CelProtoTypes.typeToCelType(type))); + return add(CelVarDecl.newVarDeclaration(name, CelProtoTypes.typeToCelType(type))); } /** @@ -410,7 +393,7 @@ public Env add(String name, Type type) { return null; } - return CelFunctionDecl.celFunctionDeclToDecl(decl); + return CelProtoDeclConverter.celFunctionDeclToDecl(decl); } /** @@ -439,12 +422,12 @@ public Env add(String name, Type type) { */ @Deprecated public @Nullable Decl tryLookupIdent(CelContainer container, String name) { - CelIdentDecl decl = tryLookupCelIdent(container, name); + CelVarDecl decl = tryLookupCelIdent(container, name); if (decl == null) { return null; } - return CelIdentDecl.celIdentToDecl(decl); + return CelProtoDeclConverter.celVarDeclToDecl(decl); } /** @@ -457,7 +440,7 @@ public Env add(String name, Type type) { * *

Returns {@code null} if the ident cannot be found. */ - public @Nullable CelIdentDecl tryLookupCelIdent(CelContainer container, String name) { + public @Nullable CelVarDecl tryLookupCelIdent(CelContainer container, String name) { // A name with a leading '.' always resolves in the root scope, bypassing local scopes. if (!name.startsWith(".")) { // Check if this is a qualified ident, or a field selection. @@ -468,7 +451,7 @@ public Env add(String name, Type type) { } // Attempt to find the decl with just the ident name to account for shadowed variables. - CelIdentDecl decl = tryLookupCelIdentFromLocalScopes(simpleName); + CelVarDecl decl = tryLookupCelIdentFromLocalScopes(simpleName); if (decl != null) { // Appears to be a field selection on a local. // Return null instead of attempting to resolve qualified name at the root scope @@ -477,7 +460,7 @@ public Env add(String name, Type type) { } for (String cand : container.resolveCandidateNames(name)) { - CelIdentDecl decl = tryLookupCelIdent(cand); + CelVarDecl decl = tryLookupCelIdent(cand); if (decl != null) { return decl; } @@ -486,9 +469,9 @@ public Env add(String name, Type type) { return null; } - private @Nullable CelIdentDecl tryLookupCelIdent(String cand) { + private @Nullable CelVarDecl tryLookupCelIdent(String cand) { // First determine whether we know this name already. - CelIdentDecl decl = findIdentDecl(cand); + CelVarDecl decl = findIdentDecl(cand); if (decl != null) { return decl; } @@ -497,7 +480,7 @@ public Env add(String name, Type type) { // This is done via the type provider. Optional type = typeProvider.lookupCelType(cand); if (type.isPresent()) { - decl = CelIdentDecl.newIdentDeclaration(cand, type.get()); + decl = CelVarDecl.newVarDeclaration(cand, type.get()); decls.get(0).putIdent(decl); return decl; } @@ -507,7 +490,7 @@ public Env add(String name, Type type) { Integer enumValue = typeProvider.lookupEnumValue(cand); if (enumValue != null) { decl = - CelIdentDecl.newBuilder() + CelVarDecl.newBuilder() .setName(cand) .setType(SimpleType.INT) .setConstant(CelConstant.ofValue(enumValue)) @@ -525,7 +508,7 @@ public Env add(String name, Type type) { * *

Returns {@code null} if not found in local scopes. */ - @Nullable CelIdentDecl tryLookupCelIdentFromLocalScopes(String name) { + @Nullable CelVarDecl tryLookupCelIdentFromLocalScopes(String name) { int firstUserSpaceScope = 2; // Iterate from the top of the stack down to the first local scope. // Note that: @@ -533,7 +516,7 @@ public Env add(String name, Type type) { // Scope 1: User defined environment // Scope 2 and onwards: comprehension scopes for (int i = decls.size() - 1; i >= firstUserSpaceScope; i--) { - CelIdentDecl ident = decls.get(i).getIdent(name); + CelVarDecl ident = decls.get(i).getIdent(name); if (ident != null) { return ident; } @@ -545,8 +528,8 @@ public Env add(String name, Type type) { * Lookup a name like {@link #tryLookupCelIdent}, but report an error if the name is not found and * return the {@link #ERROR_IDENT_DECL}. */ - public CelIdentDecl lookupIdent(long exprId, int position, CelContainer container, String name) { - CelIdentDecl result = tryLookupCelIdent(container, name); + public CelVarDecl lookupIdent(long exprId, int position, CelContainer container, String name) { + CelVarDecl result = tryLookupCelIdent(container, name); if (result == null) { reportError( exprId, @@ -607,18 +590,18 @@ boolean enableNamespacedDeclarations() { /** Add an identifier {@code decl} to the environment. */ @CanIgnoreReturnValue - private Env addIdent(CelIdentDecl celIdentDecl) { - CelIdentDecl current = getDeclGroup().getIdent(celIdentDecl.name()); + private Env addIdent(CelVarDecl celVarDecl) { + CelVarDecl current = getDeclGroup().getIdent(celVarDecl.name()); if (current == null) { - getDeclGroup().putIdent(celIdentDecl); + getDeclGroup().putIdent(celVarDecl); } else { reportError( /* exprId= */ 0, /* position= */ 0, "overlapping declaration name '%s' (type '%s' cannot be distinguished from '%s')", - celIdentDecl.name(), + celVarDecl.name(), CelTypes.format(current.type()), - CelTypes.format(celIdentDecl.type())); + CelTypes.format(celVarDecl.type())); } return this; } @@ -688,9 +671,9 @@ private void addOverload(CelFunctionDecl.Builder builder, CelOverloadDecl overlo } /** Search for the named identifier declaration. */ - private @Nullable CelIdentDecl findIdentDecl(String name) { + private @Nullable CelVarDecl findIdentDecl(String name) { for (DeclGroup declGroup : Lists.reverse(decls)) { - CelIdentDecl ident = declGroup.getIdent(name); + CelVarDecl ident = declGroup.getIdent(name); if (ident != null) { return ident; } @@ -749,7 +732,7 @@ private void addOverload(CelFunctionDecl.Builder builder, CelOverloadDecl overlo */ @Deprecated public static final class IdentBuilder { - private final CelIdentDecl.Builder builder = CelIdentDecl.newBuilder(); + private final CelVarDecl.Builder builder = CelVarDecl.newBuilder(); /** Create an identifier builder. */ public IdentBuilder(String name) { @@ -788,7 +771,7 @@ public IdentBuilder doc(@Nullable String value) { /** Build the ident {@code Decl}. */ public Decl build() { - return CelIdentDecl.celIdentToDecl(builder.build()); + return CelProtoDeclConverter.celVarDeclToDecl(builder.build()); } } @@ -824,7 +807,7 @@ public FunctionBuilder sameAs(Decl func, String idPart, String idPartReplace) { Preconditions.checkNotNull(func); for (Overload overload : func.getFunction().getOverloadsList()) { this.overloads.add( - CelOverloadDecl.overloadToCelOverload(overload).toBuilder() + CelProtoDeclConverter.overloadToCelOverload(overload).toBuilder() .setOverloadId(overload.getOverloadId().replace(idPart, idPartReplace)) .build()); } @@ -896,7 +879,7 @@ public FunctionBuilder doc(@Nullable String value) { /** Build the function {@code Decl}. */ @CheckReturnValue public Decl build() { - return CelFunctionDecl.celFunctionDeclToDecl( + return CelProtoDeclConverter.celFunctionDeclToDecl( CelFunctionDecl.newBuilder().setName(name).addOverloads(overloads).build()); } } @@ -914,7 +897,7 @@ public Decl build() { */ public static class DeclGroup { - private final Map idents; + private final Map idents; private final Map functions; /** Construct an empty {@code DeclGroup}. */ @@ -923,7 +906,7 @@ public DeclGroup() { } /** Construct a new {@code DeclGroup} from the input {@code idents} and {@code functions}. */ - public DeclGroup(Map idents, Map functions) { + public DeclGroup(Map idents, Map functions) { this.functions = functions; this.idents = idents; } @@ -931,7 +914,7 @@ public DeclGroup(Map idents, Map /** * Get an immutable map of the identifiers in the {@code DeclGroup} keyed by declaration name. */ - public Map getIdents() { + public Map getIdents() { return ImmutableMap.copyOf(idents); } @@ -941,12 +924,12 @@ public Map getFunctions() { } /** Get an identifier declaration by {@code name}. Returns {@code null} if absent. */ - public @Nullable CelIdentDecl getIdent(String name) { + public @Nullable CelVarDecl getIdent(String name) { return idents.get(name); } /** Put an identifier declaration into the {@code DeclGroup}. */ - public void putIdent(CelIdentDecl ident) { + public void putIdent(CelVarDecl ident) { idents.put(ident.name(), ident); } @@ -970,13 +953,13 @@ public DeclGroup immutableCopy() { * Sanitize the identifier declaration type making sure that proto-based message names are mapped * to the appropriate CEL type. */ - private static CelIdentDecl sanitizeIdent(CelIdentDecl decl) { + private static CelVarDecl sanitizeIdent(CelVarDecl decl) { CelType type = decl.type(); if (!isWellKnownType(type)) { return decl; } - return CelIdentDecl.newIdentDeclaration(decl.name(), getWellKnownType(decl.type())); + return decl.toBuilder().setType(getWellKnownType(decl.type())).build(); } /** diff --git a/checker/src/main/java/dev/cel/checker/ExprChecker.java b/checker/src/main/java/dev/cel/checker/ExprChecker.java index e3ce99e67..f72919f4e 100644 --- a/checker/src/main/java/dev/cel/checker/ExprChecker.java +++ b/checker/src/main/java/dev/cel/checker/ExprChecker.java @@ -33,6 +33,7 @@ import dev.cel.common.CelOverloadDecl; import dev.cel.common.CelProtoAbstractSyntaxTree; import dev.cel.common.CelSource; +import dev.cel.common.CelVarDecl; import dev.cel.common.Operator; import dev.cel.common.annotations.Internal; import dev.cel.common.ast.CelConstant; @@ -262,7 +263,7 @@ private void visit(CelMutableExpr expr, CelConstant constant) { } private void visit(CelMutableExpr expr, CelMutableIdent ident) { - CelIdentDecl decl = env.lookupIdent(expr.id(), getPosition(expr), container, ident.name()); + CelVarDecl decl = env.lookupIdent(expr.id(), getPosition(expr), container, ident.name()); checkNotNull(decl); if (decl.equals(Env.ERROR_IDENT_DECL)) { // error reported @@ -285,7 +286,7 @@ private void visit(CelMutableExpr expr, CelMutableSelect select) { // Before traversing down the tree, try to interpret as qualified name. String qname = asQualifiedName(expr); if (qname != null) { - CelIdentDecl decl = env.tryLookupCelIdent(container, qname); + CelVarDecl decl = env.tryLookupCelIdent(container, qname); if (decl != null) { if (select.testOnly()) { env.reportError(expr.id(), getPosition(expr), "expression does not select a field"); @@ -299,6 +300,7 @@ private void visit(CelMutableExpr expr, CelMutableSelect select) { expr.setIdent(CelMutableIdent.create(refName)); } env.setType(expr, decl.type()); + env.setRef(expr, makeReference(refName, decl)); } return; @@ -375,7 +377,7 @@ private void visit(CelMutableExpr expr, CelMutableCall call) { private void visit(CelMutableExpr expr, CelMutableStruct struct) { // Determine the type of the message. CelType messageType = SimpleType.ERROR; - CelIdentDecl decl = + CelVarDecl decl = env.lookupIdent(expr.id(), getPosition(expr), container, struct.messageName()); if (!struct.messageName().equals(decl.name())) { struct.setMessageName(decl.name()); @@ -545,12 +547,12 @@ private void visit(CelMutableExpr expr, CelMutableComprehension compre) { // Declare accumulation variable on outer scope. env.enterScope(); - env.add(CelIdentDecl.newIdentDeclaration(compre.accuVar(), accuType)); + env.add(CelVarDecl.newVarDeclaration(compre.accuVar(), accuType)); // Declare iteration variable on inner scope. env.enterScope(); - env.add(CelIdentDecl.newIdentDeclaration(compre.iterVar(), varType)); + env.add(CelVarDecl.newVarDeclaration(compre.iterVar(), varType)); if (!Strings.isNullOrEmpty(compre.iterVar2())) { - env.add(CelIdentDecl.newIdentDeclaration(compre.iterVar2(), varType2)); + env.add(CelVarDecl.newVarDeclaration(compre.iterVar2(), varType2)); } visit(compre.loopCondition()); assertType(compre.loopCondition(), SimpleType.BOOL); @@ -564,11 +566,9 @@ private void visit(CelMutableExpr expr, CelMutableComprehension compre) { env.setType(expr, inferenceContext.specialize(env.getType(compre.result()))); } - private CelReference makeReference(String name, CelIdentDecl decl) { + private CelReference makeReference(String name, CelVarDecl decl) { CelReference.Builder ref = CelReference.newBuilder().setName(name); - if (decl.constant().isPresent()) { - ref.setValue(decl.constant().get()); - } + decl.constant().ifPresent(ref::setValue); return ref.build(); } diff --git a/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java b/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java index 025dfaf1a..fd5c87bde 100644 --- a/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java +++ b/checker/src/main/java/dev/cel/checker/ProtoTypeMaskTypeProvider.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.Immutable; +import dev.cel.common.CelVarDecl; import dev.cel.common.types.CelKind; import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypeProvider; @@ -69,8 +70,8 @@ public Optional findType(String typeName) { *

All top-level fields in {@link ProtoTypeMask#getTypeName} definition which are also exposed * via a {@code FieldMask} are converted into {@code Decl} values. */ - ImmutableList computeDeclsFromProtoTypeMasks() { - ImmutableList.Builder decls = ImmutableList.builder(); + ImmutableList computeDeclsFromProtoTypeMasks() { + ImmutableList.Builder decls = ImmutableList.builder(); for (ProtoTypeMask typeMask : protoTypeMasks) { if (!typeMask.fieldsAreVariableDeclarations()) { continue; @@ -82,7 +83,7 @@ ImmutableList computeDeclsFromProtoTypeMasks() { StructType celStruct = (StructType) celType.get(); // The fieldNames cannot be null based on the checking provided by the computeVisibleFieldsMap for (StructType.Field field : celStruct.fields()) { - decls.add(CelIdentDecl.newIdentDeclaration(field.name(), field.type())); + decls.add(CelVarDecl.newVarDeclaration(field.name(), field.type())); } } return decls.build(); diff --git a/checker/src/test/java/dev/cel/checker/BUILD.bazel b/checker/src/test/java/dev/cel/checker/BUILD.bazel index 22b70210d..3028d6a2a 100644 --- a/checker/src/test/java/dev/cel/checker/BUILD.bazel +++ b/checker/src/test/java/dev/cel/checker/BUILD.bazel @@ -14,7 +14,6 @@ java_library( resources = ["//checker/src/test/resources:baselines"], deps = [ "//checker", - "//checker:cel_ident_decl", "//checker:checker_builder", "//checker:checker_legacy_environment", "//checker:proto_expr_visitor", @@ -23,8 +22,13 @@ java_library( "//checker:type_inferencer", "//checker:type_provider_legacy_impl", "//common:cel_ast", + "//common:cel_function_decl", + "//common:cel_issue", + "//common:cel_overload_decl", + "//common:cel_proto_decl_converter", "//common:cel_source", - "//common:compiler_common", + "//common:cel_validation_exception", + "//common:cel_var_decl", "//common:container", "//common:mutable_ast", "//common:operator", diff --git a/checker/src/test/java/dev/cel/checker/CelOverloadDeclTest.java b/checker/src/test/java/dev/cel/checker/CelOverloadDeclTest.java index 0dd7d83df..4e851a56f 100644 --- a/checker/src/test/java/dev/cel/checker/CelOverloadDeclTest.java +++ b/checker/src/test/java/dev/cel/checker/CelOverloadDeclTest.java @@ -22,6 +22,7 @@ import dev.cel.expr.Decl.FunctionDecl.Overload; import com.google.common.collect.ImmutableList; import dev.cel.common.CelOverloadDecl; +import dev.cel.common.CelProtoDeclConverter; import dev.cel.common.types.CelProtoTypes; import dev.cel.common.types.SimpleType; import dev.cel.common.types.TypeParamType; @@ -30,7 +31,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class CelOverloadDeclTest { +public final class CelOverloadDeclTest { @Test public void newGlobalFunction_success() { CelOverloadDecl overloadDecl = @@ -69,16 +70,16 @@ public void newMemberFunction_success() { @Test public void toProtoOverload_withTypeParams() { - CelOverloadDecl.Builder celOverloadDeclBuilder = + CelOverloadDecl celOverloadDecl = CelOverloadDecl.newBuilder() .setOverloadId("overloadId") .setResultType(TypeParamType.create("A")) .addParameterTypes(SimpleType.STRING, SimpleType.DOUBLE, TypeParamType.create("B")) - .setIsInstanceFunction(true); + .setIsInstanceFunction(true) + .build(); - CelOverloadDecl celOverloadDecl = celOverloadDeclBuilder.build(); + Overload protoOverload = CelProtoDeclConverter.celOverloadToOverload(celOverloadDecl); - Overload protoOverload = CelOverloadDecl.celOverloadToOverload(celOverloadDecl); assertThat(protoOverload.getOverloadId()).isEqualTo("overloadId"); assertThat(protoOverload.getIsInstanceFunction()).isTrue(); assertThat(protoOverload.getResultType()).isEqualTo(CelProtoTypes.createTypeParam("A")); diff --git a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java index 846201d32..b9a7f66df 100644 --- a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java +++ b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java @@ -15,7 +15,6 @@ package dev.cel.checker; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.collect.ImmutableList.toImmutableList; import static dev.cel.common.types.CelProtoTypes.format; import dev.cel.expr.CheckedExpr; @@ -31,16 +30,13 @@ // import com.google.testing.testsize.MediumTest; import dev.cel.common.CelAbstractSyntaxTree; import dev.cel.common.CelContainer; -import dev.cel.common.CelFunctionDecl; import dev.cel.common.CelMutableAst; -import dev.cel.common.CelOverloadDecl; import dev.cel.common.CelProtoAbstractSyntaxTree; -import dev.cel.common.CelVarDecl; +import dev.cel.common.CelProtoDeclConverter; import dev.cel.common.ast.CelConstant; import dev.cel.common.internal.EnvVisitable; import dev.cel.common.internal.EnvVisitor; import dev.cel.common.internal.Errors; -import dev.cel.common.types.CelProtoTypes; import dev.cel.common.types.CelType; import dev.cel.common.types.ListType; import dev.cel.common.types.MapType; @@ -116,18 +112,12 @@ public void visitDecl(String name, List decls) { // interface for (Decl decl : decls) { if (decl.hasFunction()) { - CelFunctionDecl celFunctionDecl = - CelFunctionDecl.newFunctionDeclaration( - decl.getName(), - decl.getFunction().getOverloadsList().stream() - .map(CelOverloadDecl::overloadToCelOverload) - .collect(toImmutableList())); - testOutput().println(formatFunctionDecl(celFunctionDecl)); + testOutput() + .println( + formatFunctionDecl(CelProtoDeclConverter.declToCelFunctionDecl(decl))); } else if (decl.hasIdent()) { - CelVarDecl celVarDecl = - CelVarDecl.newVarDeclaration( - decl.getName(), CelProtoTypes.typeToCelType(decl.getIdent().getType())); - testOutput().println(formatVarDecl(celVarDecl)); + testOutput() + .println(formatVarDecl(CelProtoDeclConverter.declToCelVarDecl(decl))); } else { throw new IllegalArgumentException("Invalid declaration: " + decl); } diff --git a/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java b/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java index b4b52bd26..2ac3d1190 100644 --- a/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java +++ b/checker/src/test/java/dev/cel/checker/ProtoTypeMaskTypeProviderTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableSet; import com.google.protobuf.FieldMask; import com.google.rpc.context.AttributeContext; +import dev.cel.common.CelVarDecl; import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypeProvider; import dev.cel.common.types.MapType; @@ -219,8 +220,8 @@ public void computeDecls() { ProtoMessageType requestType = (ProtoMessageType) celTypeProvider.findType(REQUEST_TYPE).get(); assertThat(protoTypeMaskProvider.computeDeclsFromProtoTypeMasks()) .containsExactly( - CelIdentDecl.newBuilder().setName("resource").setType(resourceType).build(), - CelIdentDecl.newBuilder().setName("request").setType(requestType).build()); + CelVarDecl.newBuilder().setName("resource").setType(resourceType).build(), + CelVarDecl.newBuilder().setName("request").setType(requestType).build()); } @Test diff --git a/common/BUILD.bazel b/common/BUILD.bazel index 9cb0c2f7b..c84701d17 100644 --- a/common/BUILD.bazel +++ b/common/BUILD.bazel @@ -13,6 +13,7 @@ java_library( ":cel_function_decl", ":cel_issue", ":cel_overload_decl", + ":cel_proto_decl_converter", ":cel_validation_exception", ":cel_validation_result", ":cel_var_decl", @@ -24,11 +25,31 @@ java_library( exports = ["//common/src/main/java/dev/cel/common:cel_function_decl"], ) +cel_android_library( + name = "cel_function_decl_android", + exports = ["//common/src/main/java/dev/cel/common:cel_function_decl_android"], +) + java_library( name = "cel_overload_decl", exports = ["//common/src/main/java/dev/cel/common:cel_overload_decl"], ) +cel_android_library( + name = "cel_overload_decl_android", + exports = ["//common/src/main/java/dev/cel/common:cel_overload_decl_android"], +) + +java_library( + name = "cel_proto_decl_converter", + exports = ["//common/src/main/java/dev/cel/common:cel_proto_decl_converter"], +) + +cel_android_library( + name = "cel_proto_decl_converter_android", + exports = ["//common/src/main/java/dev/cel/common:cel_proto_decl_converter_android"], +) + java_library( name = "cel_var_decl", exports = ["//common/src/main/java/dev/cel/common:cel_var_decl"], diff --git a/common/src/main/java/dev/cel/common/BUILD.bazel b/common/src/main/java/dev/cel/common/BUILD.bazel index 73475e623..09dafe3cd 100644 --- a/common/src/main/java/dev/cel/common/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/BUILD.bazel @@ -64,13 +64,24 @@ java_library( deps = [ ":cel_overload_decl", "//:auto_value", - "//common/annotations", - "@cel_spec//proto/cel/expr:checked_java_proto", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", ], ) +cel_android_library( + name = "cel_function_decl_android", + srcs = ["CelFunctionDecl.java"], + tags = [ + ], + deps = [ + ":cel_overload_decl_android", + "//:auto_value", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", + ], +) + java_library( name = "cel_overload_decl", srcs = ["CelOverloadDecl.java"], @@ -78,14 +89,57 @@ java_library( ], deps = [ "//:auto_value", - "//common/types:cel_proto_types", "//common/types:type_providers", - "@cel_spec//proto/cel/expr:checked_java_proto", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", ], ) +cel_android_library( + name = "cel_overload_decl_android", + srcs = ["CelOverloadDecl.java"], + tags = [ + ], + deps = [ + "//:auto_value", + "//common/types:type_providers_android", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", + ], +) + +java_library( + name = "cel_proto_decl_converter", + srcs = ["CelProtoDeclConverter.java"], + tags = [ + ], + deps = [ + ":cel_function_decl", + ":cel_overload_decl", + ":cel_var_decl", + "//common/ast:expr_converter", + "//common/types:cel_proto_types", + "@cel_spec//proto/cel/expr:checked_java_proto", + "@maven//:com_google_guava_guava", + ], +) + +cel_android_library( + name = "cel_proto_decl_converter_android", + srcs = ["CelProtoDeclConverter.java"], + tags = [ + ], + deps = [ + ":cel_function_decl_android", + ":cel_overload_decl_android", + ":cel_var_decl_android", + "//common/ast:expr_converter_android", + "//common/types:cel_proto_types_android", + "@cel_spec//proto/cel/expr:checked_java_proto_lite", + "@maven_android//:com_google_guava_guava", + ], +) + java_library( name = "cel_var_decl", srcs = ["CelVarDecl.java"], @@ -93,6 +147,7 @@ java_library( ], deps = [ "//:auto_value", + "//common/ast", "//common/types:type_providers", "@maven//:com_google_errorprone_error_prone_annotations", ], @@ -105,6 +160,7 @@ cel_android_library( ], deps = [ "//:auto_value", + "//common/ast:ast_android", "//common/types:type_providers_android", "@maven//:com_google_errorprone_error_prone_annotations", ], diff --git a/common/src/main/java/dev/cel/common/CelFunctionDecl.java b/common/src/main/java/dev/cel/common/CelFunctionDecl.java index ea10366ff..c8cc79908 100644 --- a/common/src/main/java/dev/cel/common/CelFunctionDecl.java +++ b/common/src/main/java/dev/cel/common/CelFunctionDecl.java @@ -15,16 +15,12 @@ package dev.cel.common; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.collect.ImmutableList.toImmutableList; -import dev.cel.expr.Decl; -import dev.cel.expr.Decl.FunctionDecl; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.CheckReturnValue; import com.google.errorprone.annotations.Immutable; -import dev.cel.common.annotations.Internal; import java.util.Arrays; /** Abstract representation of a CEL Function declaration. */ @@ -97,18 +93,4 @@ public static CelFunctionDecl newFunctionDeclaration( String functionName, Iterable overloads) { return CelFunctionDecl.newBuilder().setName(functionName).addOverloads(overloads).build(); } - - /** Converts a {@link CelFunctionDecl} to a protobuf equivalent form {@link FunctionDecl} */ - @Internal - public static Decl celFunctionDeclToDecl(CelFunctionDecl celFunctionDecl) { - return Decl.newBuilder() - .setName(celFunctionDecl.name()) - .setFunction( - FunctionDecl.newBuilder() - .addAllOverloads( - celFunctionDecl.overloads().stream() - .map(CelOverloadDecl::celOverloadToOverload) - .collect(toImmutableList()))) - .build(); - } } diff --git a/common/src/main/java/dev/cel/common/CelOverloadDecl.java b/common/src/main/java/dev/cel/common/CelOverloadDecl.java index 1d9c61e9d..c6a7ab1c3 100644 --- a/common/src/main/java/dev/cel/common/CelOverloadDecl.java +++ b/common/src/main/java/dev/cel/common/CelOverloadDecl.java @@ -15,9 +15,7 @@ package dev.cel.common; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.collect.ImmutableList.toImmutableList; -import dev.cel.expr.Decl.FunctionDecl.Overload; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -25,7 +23,6 @@ import com.google.errorprone.annotations.CheckReturnValue; import com.google.errorprone.annotations.Immutable; import dev.cel.common.types.CelKind; -import dev.cel.common.types.CelProtoTypes; import dev.cel.common.types.CelType; import java.util.Arrays; import java.util.List; @@ -225,34 +222,6 @@ private static CelOverloadDecl newOverload( .build(); } - /** Converts a {@link CelOverloadDecl} to a protobuf equivalent form {@link Overload} */ - public static Overload celOverloadToOverload(CelOverloadDecl overload) { - return Overload.newBuilder() - .setIsInstanceFunction(overload.isInstanceFunction()) - .setOverloadId(overload.overloadId()) - .setResultType(CelProtoTypes.celTypeToType(overload.resultType())) - .addAllParams( - overload.parameterTypes().stream() - .map(CelProtoTypes::celTypeToType) - .collect(toImmutableList())) - .addAllTypeParams(overload.typeParameterNames()) - .setDoc(overload.doc()) - .build(); - } - - public static CelOverloadDecl overloadToCelOverload(Overload overload) { - return CelOverloadDecl.newBuilder() - .setIsInstanceFunction(overload.getIsInstanceFunction()) - .setOverloadId(overload.getOverloadId()) - .setResultType(CelProtoTypes.typeToCelType(overload.getResultType())) - .setDoc(overload.getDoc()) - .addParameterTypes( - overload.getParamsList().stream() - .map(CelProtoTypes::typeToCelType) - .collect(toImmutableList())) - .build(); - } - private static void collectParamNames(ImmutableSet.Builder typeParamNames, CelType type) { if (type.kind().equals(CelKind.TYPE_PARAM)) { typeParamNames.add(type.name()); diff --git a/common/src/main/java/dev/cel/common/CelProtoDeclConverter.java b/common/src/main/java/dev/cel/common/CelProtoDeclConverter.java new file mode 100644 index 000000000..a452897c1 --- /dev/null +++ b/common/src/main/java/dev/cel/common/CelProtoDeclConverter.java @@ -0,0 +1,113 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dev.cel.common; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import dev.cel.expr.Decl; +import dev.cel.expr.Decl.FunctionDecl; +import dev.cel.expr.Decl.FunctionDecl.Overload; +import dev.cel.expr.Decl.IdentDecl; +import dev.cel.common.ast.CelExprConverter; +import dev.cel.common.types.CelProtoTypes; + +/** + * Utility class for converting between native CEL declarations ({@link CelVarDecl}, {@link + * CelFunctionDecl}, {@link CelOverloadDecl}) and protobuf representations ({@link Decl}, {@link + * Overload}). + */ +public final class CelProtoDeclConverter { + + /** Converts a {@link CelVarDecl} to a protobuf equivalent form {@link Decl}. */ + public static Decl celVarDeclToDecl(CelVarDecl varDecl) { + IdentDecl.Builder identBuilder = + IdentDecl.newBuilder() + .setDoc(varDecl.doc()) + .setType(CelProtoTypes.celTypeToType(varDecl.type())); + varDecl + .constant() + .ifPresent(c -> identBuilder.setValue(CelExprConverter.celConstantToExprConstant(c))); + return Decl.newBuilder().setName(varDecl.name()).setIdent(identBuilder).build(); + } + + /** Converts a protobuf {@link Decl} of kind IDENT to a {@link CelVarDecl}. */ + public static CelVarDecl declToCelVarDecl(Decl decl) { + checkArgument(decl.hasIdent(), "Decl must be of kind IDENT: %s", decl); + CelVarDecl.Builder builder = + CelVarDecl.newBuilder() + .setName(decl.getName()) + .setType(CelProtoTypes.typeToCelType(decl.getIdent().getType())) + .setDoc(decl.getIdent().getDoc()); + if (decl.getIdent().hasValue()) { + builder.setConstant(CelExprConverter.exprConstantToCelConstant(decl.getIdent().getValue())); + } + return builder.build(); + } + + /** Converts a {@link CelFunctionDecl} to a protobuf equivalent form {@link Decl}. */ + public static Decl celFunctionDeclToDecl(CelFunctionDecl celFunctionDecl) { + return Decl.newBuilder() + .setName(celFunctionDecl.name()) + .setFunction( + FunctionDecl.newBuilder() + .addAllOverloads( + celFunctionDecl.overloads().stream() + .map(CelProtoDeclConverter::celOverloadToOverload) + .collect(toImmutableList()))) + .build(); + } + + /** Converts a protobuf {@link Decl} of kind FUNCTION to a {@link CelFunctionDecl}. */ + public static CelFunctionDecl declToCelFunctionDecl(Decl decl) { + checkArgument(decl.hasFunction(), "Decl must be of kind FUNCTION: %s", decl); + return CelFunctionDecl.newFunctionDeclaration( + decl.getName(), + decl.getFunction().getOverloadsList().stream() + .map(CelProtoDeclConverter::overloadToCelOverload) + .collect(toImmutableList())); + } + + /** Converts a {@link CelOverloadDecl} to a protobuf equivalent form {@link Overload}. */ + public static Overload celOverloadToOverload(CelOverloadDecl overload) { + return Overload.newBuilder() + .setIsInstanceFunction(overload.isInstanceFunction()) + .setOverloadId(overload.overloadId()) + .setResultType(CelProtoTypes.celTypeToType(overload.resultType())) + .addAllParams( + overload.parameterTypes().stream() + .map(CelProtoTypes::celTypeToType) + .collect(toImmutableList())) + .addAllTypeParams(overload.typeParameterNames()) + .setDoc(overload.doc()) + .build(); + } + + /** Converts a protobuf {@link Overload} to a {@link CelOverloadDecl}. */ + public static CelOverloadDecl overloadToCelOverload(Overload overload) { + return CelOverloadDecl.newBuilder() + .setIsInstanceFunction(overload.getIsInstanceFunction()) + .setOverloadId(overload.getOverloadId()) + .setResultType(CelProtoTypes.typeToCelType(overload.getResultType())) + .setDoc(overload.getDoc()) + .addParameterTypes( + overload.getParamsList().stream() + .map(CelProtoTypes::typeToCelType) + .collect(toImmutableList())) + .build(); + } + + private CelProtoDeclConverter() {} +} diff --git a/common/src/main/java/dev/cel/common/CelVarDecl.java b/common/src/main/java/dev/cel/common/CelVarDecl.java index 0543ed225..370543e06 100644 --- a/common/src/main/java/dev/cel/common/CelVarDecl.java +++ b/common/src/main/java/dev/cel/common/CelVarDecl.java @@ -15,11 +15,16 @@ package dev.cel.common; import com.google.auto.value.AutoValue; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.CheckReturnValue; +import com.google.errorprone.annotations.Immutable; +import dev.cel.common.ast.CelConstant; import dev.cel.common.types.CelType; +import java.util.Optional; /** Abstract representation of a CEL variable declaration. */ @AutoValue +@Immutable public abstract class CelVarDecl { /** Fully qualified variable name. */ @@ -28,9 +33,57 @@ public abstract class CelVarDecl { /** The type of the variable. */ public abstract CelType type(); + /** + * The constant value of the identifier. If not specified, the identifier must be supplied at + * evaluation time. + */ + public abstract Optional constant(); + + /** Documentation string for the identifier. */ + public abstract String doc(); + + /** Converts this instance into a builder. */ + public abstract Builder toBuilder(); + /** Create a new {@code CelVarDecl} with a given {@code name} and {@code type}. */ @CheckReturnValue public static CelVarDecl newVarDeclaration(String name, CelType type) { - return new AutoValue_CelVarDecl(name, type); + return newBuilder().setName(name).setType(type).build(); + } + + /** Create a new builder to construct a {@link CelVarDecl} instance. */ + public static Builder newBuilder() { + return new AutoValue_CelVarDecl.Builder().setDoc(""); } + + /** Builder for configuring the {@link CelVarDecl}. */ + @AutoValue.Builder + public abstract static class Builder { + @CanIgnoreReturnValue + public abstract Builder setName(String name); + + @CanIgnoreReturnValue + public abstract Builder setType(CelType type); + + @CanIgnoreReturnValue + public abstract Builder setConstant(CelConstant constant); + + @CanIgnoreReturnValue + public abstract Builder setConstant(Optional constant); + + @CanIgnoreReturnValue + public abstract Builder setDoc(String doc); + + @CanIgnoreReturnValue + public Builder clearConstant() { + return setConstant(Optional.empty()); + } + + @CheckReturnValue + public abstract CelVarDecl build(); + + Builder() {} + } + + CelVarDecl() {} } diff --git a/common/src/test/java/dev/cel/common/BUILD.bazel b/common/src/test/java/dev/cel/common/BUILD.bazel index 98be87d17..fb918db80 100644 --- a/common/src/test/java/dev/cel/common/BUILD.bazel +++ b/common/src/test/java/dev/cel/common/BUILD.bazel @@ -16,8 +16,11 @@ java_library( "//common:cel_ast", "//common:cel_descriptor_util", "//common:cel_descriptors", + "//common:cel_issue", + "//common:cel_proto_decl_converter", "//common:cel_source", - "//common:compiler_common", + "//common:cel_validation_exception", + "//common:cel_var_decl", "//common:container", "//common:operator", "//common:options", diff --git a/checker/src/test/java/dev/cel/checker/CelIdentDeclTest.java b/common/src/test/java/dev/cel/common/CelVarDeclTest.java similarity index 62% rename from checker/src/test/java/dev/cel/checker/CelIdentDeclTest.java rename to common/src/test/java/dev/cel/common/CelVarDeclTest.java index 6dd221ae1..6280c0909 100644 --- a/checker/src/test/java/dev/cel/checker/CelIdentDeclTest.java +++ b/common/src/test/java/dev/cel/common/CelVarDeclTest.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package dev.cel.checker; +package dev.cel.common; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat; @@ -29,58 +29,58 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class CelIdentDeclTest { +public final class CelVarDeclTest { @Test - public void celIdentBuilder_success() { - CelIdentDecl stringIdent = - CelIdentDecl.newBuilder() + public void celVarBuilder_success() { + CelVarDecl stringVar = + CelVarDecl.newBuilder() .setName("ident") .setType(SimpleType.STRING) .setDoc("doc") .setConstant(CelConstant.ofValue("str")) .build(); - assertThat(stringIdent.name()).isEqualTo("ident"); - assertThat(stringIdent.type()).isEqualTo(SimpleType.STRING); - assertThat(stringIdent.doc()).isEqualTo("doc"); - assertThat(stringIdent.constant()).hasValue(CelConstant.ofValue("str")); + assertThat(stringVar.name()).isEqualTo("ident"); + assertThat(stringVar.type()).isEqualTo(SimpleType.STRING); + assertThat(stringVar.doc()).isEqualTo("doc"); + assertThat(stringVar.constant()).hasValue(CelConstant.ofValue("str")); } @Test - public void celIdentBuilder_clearConstant() { - CelIdentDecl.Builder builder = - CelIdentDecl.newBuilder() + public void celVarBuilder_clearConstant() { + CelVarDecl varDecl = + CelVarDecl.newBuilder() .setName("ident") .setType(SimpleType.STRING) - .setConstant(CelConstant.ofValue("str")); - - builder.clearConstant(); + .setConstant(CelConstant.ofValue("str")) + .clearConstant() + .build(); - assertThat(builder.build().constant()).isEmpty(); + assertThat(varDecl.constant()).isEmpty(); } @Test - public void newIdentDeclaration_success() { - CelIdentDecl intIdent = CelIdentDecl.newIdentDeclaration("ident", SimpleType.INT); + public void newVarDeclaration_success() { + CelVarDecl intVar = CelVarDecl.newVarDeclaration("ident", SimpleType.INT); - assertThat(intIdent.name()).isEqualTo("ident"); - assertThat(intIdent.type()).isEqualTo(SimpleType.INT); - assertThat(intIdent.doc()).isEmpty(); - assertThat(intIdent.constant()).isEmpty(); + assertThat(intVar.name()).isEqualTo("ident"); + assertThat(intVar.type()).isEqualTo(SimpleType.INT); + assertThat(intVar.doc()).isEmpty(); + assertThat(intVar.constant()).isEmpty(); } @Test - public void celIdentToDecl_success() { - CelIdentDecl stringIdent = - CelIdentDecl.newBuilder() + public void celVarDeclToDecl_success() { + CelVarDecl stringVar = + CelVarDecl.newBuilder() .setName("ident") .setType(SimpleType.STRING) .setDoc("doc") .setConstant(CelConstant.ofValue("str")) .build(); - Decl decl = CelIdentDecl.celIdentToDecl(stringIdent); + Decl decl = CelProtoDeclConverter.celVarDeclToDecl(stringVar); assertThat(decl) .isEqualTo(