Skip to content

Commit 32a9ddc

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Consolidate CelIdentDecl into CelVarDecl, extract CelProtoDeclConverter
PiperOrigin-RevId: 982118176
1 parent 248b623 commit 32a9ddc

29 files changed

Lines changed: 424 additions & 404 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,10 @@ java_library(
172172
"//bundle:cel",
173173
"//checker:checker_builder",
174174
"//checker:standard_decl",
175+
"//common:cel_proto_decl_converter",
175176
"//common:compiler_common",
176177
"//common:options",
177178
"//common/internal:env_visitor",
178-
"//common/types:cel_proto_types",
179179
"//common/types:type_providers",
180180
"//compiler:compiler_builder",
181181
"//extensions",

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,11 @@ public static VariableDecl create(String name, TypeDecl type) {
478478

479479
/** Converts this policy variable declaration into a {@link CelVarDecl}. */
480480
public CelVarDecl toCelVarDecl(CelTypeProvider celTypeProvider) {
481-
return CelVarDecl.newVarDeclaration(name(), type().toCelType(celTypeProvider));
481+
return CelVarDecl.newBuilder()
482+
.setName(name())
483+
.setType(type().toCelType(celTypeProvider))
484+
.setDoc(description().orElse(""))
485+
.build();
482486
}
483487
}
484488

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

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@
3737
import dev.cel.common.CelFunctionDecl;
3838
import dev.cel.common.CelOptions;
3939
import dev.cel.common.CelOverloadDecl;
40+
import dev.cel.common.CelProtoDeclConverter;
4041
import dev.cel.common.CelVarDecl;
4142
import dev.cel.common.internal.EnvVisitable;
4243
import dev.cel.common.internal.EnvVisitor;
4344
import dev.cel.common.types.CelKind;
44-
import dev.cel.common.types.CelProtoTypes;
4545
import dev.cel.common.types.CelType;
4646
import dev.cel.compiler.CelCompiler;
4747
import dev.cel.extensions.CelExtensionLibrary;
@@ -262,13 +262,11 @@ public void visitDecl(String name, List<Decl> decls) {
262262
for (Overload overload : function.getOverloadsList()) {
263263
inventory.add(
264264
NamedOverload.create(
265-
decl.getName(), CelOverloadDecl.overloadToCelOverload(overload)));
265+
decl.getName(),
266+
CelProtoDeclConverter.overloadToCelOverload(overload)));
266267
}
267268
} else if (decl.hasIdent()) {
268-
inventory.add(
269-
CelVarDecl.newVarDeclaration(
270-
decl.getName(),
271-
CelProtoTypes.typeToCelType(decl.getIdent().getType())));
269+
inventory.add(CelProtoDeclConverter.declToCelVarDecl(decl));
272270
}
273271
}
274272
}
@@ -299,7 +297,7 @@ private void addExtensionConfigsAndRemoveFromInventory(
299297

300298
featureSets.sort(
301299
Comparator.comparing(NamedFeatureSet::name)
302-
.thenComparing(nfs -> nfs.featureSet().version())
300+
.thenComparingInt(nfs -> nfs.featureSet().version())
303301
.reversed());
304302

305303
Set<String> includedExtensions = new HashSet<>();
@@ -348,8 +346,7 @@ private void addStandardLibrarySubsetAndRemoveFromInventory(
348346
CelEnvironment.Builder envBuilder, Set<Object> inventory) {
349347
// Claim standard identifiers for the standard library
350348
for (StandardIdentifier value : StandardIdentifier.values()) {
351-
inventory.remove(
352-
CelVarDecl.newVarDeclaration(value.identDecl().name(), value.identDecl().type()));
349+
inventory.remove(value.identDecl());
353350
}
354351

355352
Set<String> excludedFunctions = new HashSet<>();
@@ -431,13 +428,13 @@ private ImmutableSet<FunctionSelector> buildFunctionSelectors(
431428
private void addCustomDecls(CelEnvironment.Builder envBuilder, Set<Object> inventory) {
432429
// Group "orphaned" function overloads and vars by their names
433430
ListMultimap<String, CelOverloadDecl> extraOverloads = ArrayListMultimap.create();
434-
Map<String, CelType> extraVars = new HashMap<>();
431+
Map<String, CelVarDecl> extraVars = new HashMap<>();
435432
for (Object item : inventory) {
436433
if (item instanceof NamedOverload) {
437434
extraOverloads.put(
438435
((NamedOverload) item).functionName(), ((NamedOverload) item).overload());
439436
} else if (item instanceof CelVarDecl) {
440-
extraVars.put(((CelVarDecl) item).name(), ((CelVarDecl) item).type());
437+
extraVars.put(((CelVarDecl) item).name(), (CelVarDecl) item);
441438
}
442439
}
443440

@@ -457,9 +454,15 @@ private void addCustomDecls(CelEnvironment.Builder envBuilder, Set<Object> inven
457454

458455
if (!extraVars.isEmpty()) {
459456
ImmutableSet.Builder<CelEnvironment.VariableDecl> varDeclBuilder = ImmutableSet.builder();
460-
for (String ident : extraVars.keySet()) {
461-
varDeclBuilder.add(
462-
CelEnvironment.VariableDecl.create(ident, toCelEnvTypeDecl(extraVars.get(ident))));
457+
for (CelVarDecl varDecl : extraVars.values()) {
458+
CelEnvironment.VariableDecl.Builder builder =
459+
CelEnvironment.VariableDecl.newBuilder()
460+
.setName(varDecl.name())
461+
.setType(toCelEnvTypeDecl(varDecl.type()));
462+
if (!varDecl.doc().isEmpty()) {
463+
builder.setDescription(varDecl.doc());
464+
}
465+
varDeclBuilder.add(builder.build());
463466
}
464467
envBuilder.setVariables(varDeclBuilder.build());
465468
}

‎checker/BUILD.bazel‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,6 @@ java_library(
3232
exports = ["//checker/src/main/java/dev/cel/checker:type_provider_legacy_impl"],
3333
)
3434

35-
java_library(
36-
name = "cel_ident_decl",
37-
exports = ["//checker/src/main/java/dev/cel/checker:cel_ident_decl"],
38-
)
39-
4035
java_library(
4136
name = "checker_legacy_environment",
4237
deprecation = "See go/cel-java-migration-guide. Please use CEL-Java Fluent APIs //compiler instead",

‎checker/src/main/java/dev/cel/checker/BUILD.bazel‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ java_library(
6565
tags = [
6666
],
6767
deps = [
68-
":cel_ident_decl",
6968
":checker_builder",
7069
":checker_legacy_environment",
7170
":proto_type_mask",
@@ -74,13 +73,14 @@ java_library(
7473
"//:auto_value",
7574
"//common:cel_ast",
7675
"//common:cel_descriptor_util",
76+
"//common:cel_proto_decl_converter",
7777
"//common:cel_source",
78+
"//common:cel_var_decl",
7879
"//common:compiler_common",
7980
"//common:container",
8081
"//common:options",
8182
"//common:source_location",
8283
"//common/annotations",
83-
"//common/ast:expr_converter",
8484
"//common/internal:env_visitor",
8585
"//common/internal:errors",
8686
"//common/types",
@@ -128,26 +128,6 @@ java_library(
128128
],
129129
)
130130

131-
java_library(
132-
name = "cel_ident_decl",
133-
srcs = [
134-
"CelIdentDecl.java",
135-
],
136-
tags = [
137-
],
138-
deps = [
139-
"//:auto_value",
140-
"//common/annotations",
141-
"//common/ast",
142-
"//common/ast:expr_converter",
143-
"//common/types:cel_proto_types",
144-
"//common/types:type_providers",
145-
"@cel_spec//proto/cel/expr:checked_java_proto",
146-
"@maven//:com_google_errorprone_error_prone_annotations",
147-
"@maven//:com_google_guava_guava",
148-
],
149-
)
150-
151131
java_library(
152132
name = "type_provider_legacy_impl",
153133
srcs = ["TypeProviderLegacyImpl.java"],
@@ -173,11 +153,12 @@ java_library(
173153
tags = [
174154
],
175155
deps = [
176-
":cel_ident_decl",
177156
":standard_decl",
178157
"//:auto_value",
179158
"//common:cel_ast",
159+
"//common:cel_proto_decl_converter",
180160
"//common:cel_source",
161+
"//common:cel_var_decl",
181162
"//common:compiler_common",
182163
"//common:container",
183164
"//common:mutable_ast",
@@ -240,7 +221,7 @@ java_library(
240221
tags = [
241222
],
242223
deps = [
243-
":cel_ident_decl",
224+
"//common:cel_var_decl",
244225
"//common:compiler_common",
245226
"//common:operator",
246227
"//common/types",

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

Lines changed: 21 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,12 @@
3636
import dev.cel.common.CelFunctionDecl;
3737
import dev.cel.common.CelIssue;
3838
import dev.cel.common.CelOptions;
39-
import dev.cel.common.CelOverloadDecl;
39+
import dev.cel.common.CelProtoDeclConverter;
4040
import dev.cel.common.CelSource;
4141
import dev.cel.common.CelSourceLocation;
4242
import dev.cel.common.CelValidationResult;
4343
import dev.cel.common.CelVarDecl;
4444
import dev.cel.common.annotations.Internal;
45-
import dev.cel.common.ast.CelExprConverter;
4645
import dev.cel.common.internal.EnvVisitable;
4746
import dev.cel.common.internal.EnvVisitor;
4847
import dev.cel.common.internal.Errors;
@@ -70,7 +69,7 @@ public final class CelCheckerLegacyImpl implements CelChecker, EnvVisitable {
7069

7170
private final CelOptions celOptions;
7271
private final CelContainer container;
73-
private final ImmutableSet<CelIdentDecl> identDeclarations;
72+
private final ImmutableSet<CelVarDecl> identDeclarations;
7473
private final ImmutableSet<CelFunctionDecl> functionDeclarations;
7574
private final Optional<CelType> expectedResultType;
7675

@@ -115,7 +114,7 @@ public CelTypeProvider getTypeProvider() {
115114
public CelCheckerBuilder toCheckerBuilder() {
116115
CelCheckerBuilder builder =
117116
new Builder()
118-
.addIdentDeclarations(identDeclarations)
117+
.addVarDeclarations(identDeclarations)
119118
.setOptions(celOptions)
120119
.setTypeProvider(celTypeProvider)
121120
.setContainer(container)
@@ -146,14 +145,14 @@ public void accept(EnvVisitor envVisitor) {
146145
names.addAll(declGroup.getIdents().keySet());
147146
names.addAll(declGroup.getFunctions().keySet());
148147
for (String name : names) {
149-
CelIdentDecl ident = declGroup.getIdent(name);
148+
CelVarDecl ident = declGroup.getIdent(name);
150149
CelFunctionDecl func = declGroup.getFunction(name);
151150
List<Decl> decls = new ArrayList<>();
152151
if (ident != null) {
153-
decls.add(CelIdentDecl.celIdentToDecl(ident));
152+
decls.add(CelProtoDeclConverter.celVarDeclToDecl(ident));
154153
}
155154
if (func != null) {
156-
decls.add(CelFunctionDecl.celFunctionDeclToDecl(func));
155+
decls.add(CelProtoDeclConverter.celFunctionDeclToDecl(func));
157156
}
158157
envVisitor.visitDecl(name, decls);
159158
}
@@ -182,7 +181,7 @@ public static CelCheckerBuilder newBuilder() {
182181
/** Builder class for the legacy {@code CelChecker} implementation. */
183182
public static final class Builder implements CelCheckerBuilder {
184183

185-
private final ImmutableSet.Builder<CelIdentDecl> identDeclarations;
184+
private final ImmutableSet.Builder<CelVarDecl> identDeclarations;
186185
private final ImmutableSet.Builder<CelFunctionDecl> functionDeclarations;
187186
private final ImmutableSet.Builder<ProtoTypeMask> protoTypeMasks;
188187
private final ImmutableSet.Builder<Descriptor> messageTypes;
@@ -231,27 +230,10 @@ public CelCheckerBuilder addDeclarations(Iterable<Decl> declarations) {
231230
for (Decl decl : declarations) {
232231
switch (decl.getDeclKindCase()) {
233232
case IDENT:
234-
CelIdentDecl.Builder identBuilder =
235-
CelIdentDecl.newBuilder()
236-
.setName(decl.getName())
237-
.setType(CelProtoTypes.typeToCelType(decl.getIdent().getType()))
238-
// Note: Setting doc and constant value exists for compatibility reason. This
239-
// should not be set by the users.
240-
.setDoc(decl.getIdent().getDoc());
241-
if (decl.getIdent().hasValue()) {
242-
identBuilder.setConstant(
243-
CelExprConverter.exprConstantToCelConstant(decl.getIdent().getValue()));
244-
}
245-
246-
this.identDeclarations.add(identBuilder.build());
233+
this.identDeclarations.add(CelProtoDeclConverter.declToCelVarDecl(decl));
247234
break;
248235
case FUNCTION:
249-
addFunctionDeclarations(
250-
CelFunctionDecl.newFunctionDeclaration(
251-
decl.getName(),
252-
decl.getFunction().getOverloadsList().stream()
253-
.map(CelOverloadDecl::overloadToCelOverload)
254-
.collect(toImmutableList())));
236+
addFunctionDeclarations(CelProtoDeclConverter.declToCelFunctionDecl(decl));
255237
break;
256238
default:
257239
throw new IllegalArgumentException("unexpected decl kind: " + decl.getDeclKindCase());
@@ -283,10 +265,7 @@ public CelCheckerBuilder addVarDeclarations(CelVarDecl... celVarDecls) {
283265
@Override
284266
public CelCheckerBuilder addVarDeclarations(Iterable<CelVarDecl> celVarDecls) {
285267
checkNotNull(celVarDecls);
286-
for (CelVarDecl celVarDecl : celVarDecls) {
287-
this.identDeclarations.add(
288-
CelIdentDecl.newIdentDeclaration(celVarDecl.name(), celVarDecl.type()));
289-
}
268+
this.identDeclarations.addAll(celVarDecls);
290269
return this;
291270
}
292271

@@ -384,12 +363,6 @@ public CelCheckerBuilder addLibraries(Iterable<? extends CelCheckerLibrary> libr
384363
return this;
385364
}
386365

387-
@CanIgnoreReturnValue
388-
Builder addIdentDeclarations(ImmutableSet<CelIdentDecl> identDeclarations) {
389-
this.identDeclarations.addAll(identDeclarations);
390-
return this;
391-
}
392-
393366
// The following getters marked @VisibleForTesting exist for testing toCheckerBuilder copies
394367
// over all properties. Do not expose these to public
395368
@VisibleForTesting
@@ -398,7 +371,7 @@ ImmutableSet.Builder<CelFunctionDecl> functionDecls() {
398371
}
399372

400373
@VisibleForTesting
401-
ImmutableSet.Builder<CelIdentDecl> identDecls() {
374+
ImmutableSet.Builder<CelVarDecl> identDecls() {
402375
return this.identDeclarations;
403376
}
404377

@@ -468,15 +441,20 @@ public CelCheckerLegacyImpl build() {
468441
// Configure the declaration set, and possibly alter the type provider if ProtoDecl values
469442
// are provided as they may prevent the use of certain field selection patterns against the
470443
// proto.
471-
ImmutableSet<CelIdentDecl> identDeclarationSet = identDeclarations.build();
444+
ImmutableSet<CelVarDecl> identDeclarationSet = identDeclarations.build();
472445
ImmutableSet<ProtoTypeMask> protoTypeMaskSet = protoTypeMasks.build();
473446
if (!protoTypeMaskSet.isEmpty()) {
474447
ProtoTypeMaskTypeProvider protoTypeMaskTypeProvider =
475448
new ProtoTypeMaskTypeProvider(messageTypeProvider, protoTypeMaskSet);
449+
ImmutableSet<String> declaredNames =
450+
identDeclarationSet.stream().map(CelVarDecl::name).collect(toImmutableSet());
476451
identDeclarationSet =
477-
ImmutableSet.<CelIdentDecl>builder()
452+
ImmutableSet.<CelVarDecl>builder()
478453
.addAll(identDeclarationSet)
479-
.addAll(protoTypeMaskTypeProvider.computeDeclsFromProtoTypeMasks())
454+
.addAll(
455+
protoTypeMaskTypeProvider.computeDeclsFromProtoTypeMasks().stream()
456+
.filter(decl -> !declaredNames.contains(decl.name()))
457+
.collect(toImmutableSet()))
480458
.build();
481459
messageTypeProvider = protoTypeMaskTypeProvider;
482460
}
@@ -518,7 +496,7 @@ private Builder() {
518496
private CelCheckerLegacyImpl(
519497
CelOptions celOptions,
520498
CelContainer container,
521-
ImmutableSet<CelIdentDecl> identDeclarations,
499+
ImmutableSet<CelVarDecl> identDeclarations,
522500
ImmutableSet<CelFunctionDecl> functionDeclarations,
523501
Optional<CelType> expectedResultType,
524502
TypeProvider typeProvider,
@@ -528,6 +506,7 @@ private CelCheckerLegacyImpl(
528506
ImmutableSet<CelCheckerLibrary> checkerLibraries,
529507
ImmutableSet<FileDescriptor> fileDescriptors,
530508
ImmutableSet<ProtoTypeMask> protoTypeMasks) {
509+
531510
this.celOptions = celOptions;
532511
this.container = container;
533512
this.identDeclarations = identDeclarations;

0 commit comments

Comments
 (0)