Skip to content

Commit 6a7c232

Browse files
l46kokcopybara-github
authored andcommitted
TypeChecker Refactor
PiperOrigin-RevId: 861155220
1 parent b8eafbf commit 6a7c232

7 files changed

Lines changed: 236 additions & 235 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ java_library(
190190
"//common/types",
191191
"//common/types:cel_proto_types",
192192
"//common/types:cel_types",
193+
"//common/types:message_type_provider",
193194
"//common/types:type_providers",
194195
"//parser:macro",
195196
"@cel_spec//proto/cel/expr:checked_java_proto",

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,11 @@ public void accept(EnvVisitor envVisitor) {
163163
private Env getEnv(Errors errors) {
164164
Env env;
165165
if (standardEnvironmentEnabled) {
166-
env = Env.standard(errors, typeProvider, celOptions);
166+
env = Env.standard(errors, celTypeProvider, celOptions);
167167
} else if (overriddenStandardDeclarations != null) {
168-
env = Env.standard(overriddenStandardDeclarations, errors, typeProvider, celOptions);
168+
env = Env.standard(overriddenStandardDeclarations, errors, celTypeProvider, celOptions);
169169
} else {
170-
env = Env.unconfigured(errors, typeProvider, celOptions);
170+
env = Env.unconfigured(errors, celTypeProvider, celOptions);
171171
}
172172
identDeclarations.forEach(env::add);
173173
functionDeclarations.forEach(env::add);
@@ -483,11 +483,11 @@ public CelCheckerLegacyImpl build() {
483483
messageTypeProvider = protoTypeMaskTypeProvider;
484484
}
485485

486-
TypeProvider legacyProvider = new TypeProviderLegacyImpl(messageTypeProvider);
487486
if (customTypeProvider != null) {
488-
legacyProvider =
489-
new TypeProvider.CombinedTypeProvider(
490-
ImmutableList.of(customTypeProvider, legacyProvider));
487+
messageTypeProvider = new CelTypeProvider.CombinedCelTypeProvider(
488+
messageTypeProvider,
489+
new TypeProviderLegacyImpl(customTypeProvider)
490+
);
491491
}
492492

493493
return new CelCheckerLegacyImpl(
@@ -496,7 +496,7 @@ public CelCheckerLegacyImpl build() {
496496
identDeclarationSet,
497497
functionDeclarations.build(),
498498
Optional.fromNullable(expectedResultType),
499-
legacyProvider,
499+
customTypeProvider,
500500
messageTypeProvider,
501501
standardEnvironmentEnabled,
502502
standardDeclarations,

checker/src/main/java/dev/cel/checker/Env.java

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import dev.cel.expr.Decl.FunctionDecl.Overload;
2020
import dev.cel.expr.Expr;
2121
import dev.cel.expr.Type;
22-
import com.google.common.annotations.VisibleForTesting;
2322
import com.google.common.base.Preconditions;
2423
import com.google.common.collect.ImmutableList;
2524
import com.google.common.collect.ImmutableMap;
@@ -43,7 +42,9 @@
4342
import dev.cel.common.types.CelKind;
4443
import dev.cel.common.types.CelProtoTypes;
4544
import dev.cel.common.types.CelType;
45+
import dev.cel.common.types.CelTypeProvider;
4646
import dev.cel.common.types.CelTypes;
47+
import dev.cel.common.types.ProtoMessageTypeProvider;
4748
import dev.cel.common.types.SimpleType;
4849
import dev.cel.parser.CelStandardMacro;
4950
import java.util.ArrayList;
@@ -78,7 +79,7 @@ public class Env {
7879
CelFunctionDecl.newBuilder().setName("*error*").build();
7980

8081
/** Type provider responsible for resolving CEL message references to strong types. */
81-
private final TypeProvider typeProvider;
82+
private final CelTypeProvider typeProvider;
8283

8384
/**
8485
* Stack of declaration groups where each entry in stack represents a scope capable of hinding
@@ -105,7 +106,7 @@ public class Env {
105106
.build();
106107

107108
private Env(
108-
Errors errors, TypeProvider typeProvider, DeclGroup declGroup, CelOptions celOptions) {
109+
Errors errors, CelTypeProvider typeProvider, DeclGroup declGroup, CelOptions celOptions) {
109110
this.celOptions = celOptions;
110111
this.errors = Preconditions.checkNotNull(errors);
111112
this.typeProvider = Preconditions.checkNotNull(typeProvider);
@@ -118,27 +119,10 @@ private Env(
118119
*/
119120
@Deprecated
120121
public static Env unconfigured(Errors errors) {
121-
return unconfigured(errors, LEGACY_TYPE_CHECKER_OPTIONS);
122+
return unconfigured(errors, new ProtoMessageTypeProvider(), LEGACY_TYPE_CHECKER_OPTIONS);
122123
}
123124

124-
/**
125-
* Creates an unconfigured {@code Env} value without the standard CEL types, functions, and
126-
* operators with a reference to the configured {@code celOptions}.
127-
*/
128-
@VisibleForTesting
129-
static Env unconfigured(Errors errors, CelOptions celOptions) {
130-
return unconfigured(errors, new DescriptorTypeProvider(), celOptions);
131-
}
132-
133-
/**
134-
* Creates an unconfigured {@code Env} value without the standard CEL types, functions, and
135-
* operators using a custom {@code typeProvider}.
136-
*
137-
* @deprecated Do not use. This exists for compatibility reasons. Migrate to CEL-Java fluent APIs.
138-
* See {@code CelCompilerFactory}.
139-
*/
140-
@Deprecated
141-
public static Env unconfigured(Errors errors, TypeProvider typeProvider, CelOptions celOptions) {
125+
static Env unconfigured(Errors errors, CelTypeProvider typeProvider, CelOptions celOptions) {
142126
return new Env(errors, typeProvider, new DeclGroup(), celOptions);
143127
}
144128

@@ -148,7 +132,7 @@ public static Env unconfigured(Errors errors, TypeProvider typeProvider, CelOpti
148132
*/
149133
@Deprecated
150134
public static Env standard(Errors errors) {
151-
return standard(errors, new DescriptorTypeProvider());
135+
return standard(errors, new ProtoMessageTypeProvider(), LEGACY_TYPE_CHECKER_OPTIONS);
152136
}
153137

154138
/**
@@ -173,6 +157,11 @@ public static Env standard(Errors errors, TypeProvider typeProvider) {
173157
*/
174158
@Deprecated
175159
public static Env standard(Errors errors, TypeProvider typeProvider, CelOptions celOptions) {
160+
CelTypeProvider adapted = new TypeProviderLegacyImpl(typeProvider);
161+
return standard(errors, adapted, celOptions);
162+
}
163+
164+
static Env standard(Errors errors, CelTypeProvider typeProvider, CelOptions celOptions) {
176165
CelStandardDeclarations celStandardDeclaration =
177166
CelStandardDeclarations.newBuilder()
178167
.filterFunctions(
@@ -209,10 +198,10 @@ public static Env standard(Errors errors, TypeProvider typeProvider, CelOptions
209198
return standard(celStandardDeclaration, errors, typeProvider, celOptions);
210199
}
211200

212-
public static Env standard(
201+
static Env standard(
213202
CelStandardDeclarations celStandardDeclaration,
214203
Errors errors,
215-
TypeProvider typeProvider,
204+
CelTypeProvider typeProvider,
216205
CelOptions celOptions) {
217206
Env env = Env.unconfigured(errors, typeProvider, celOptions);
218207
// Isolate the standard declarations into their own scope for forward compatibility.
@@ -228,8 +217,8 @@ public Errors getErrorContext() {
228217
return errors;
229218
}
230219

231-
/** Returns the {@code TypeProvider}. */
232-
public TypeProvider getTypeProvider() {
220+
/** Returns the {@code CelTypeProvider}. */
221+
public CelTypeProvider getTypeProvider() {
233222
return typeProvider;
234223
}
235224

@@ -491,7 +480,7 @@ public Env add(String name, Type type) {
491480

492481
// Next try to import the name as a reference to a message type.
493482
// This is done via the type provider.
494-
Optional<CelType> type = typeProvider.lookupCelType(cand);
483+
Optional<CelType> type = typeProvider.findType(cand);
495484
if (type.isPresent()) {
496485
decl = CelIdentDecl.newIdentDeclaration(cand, type.get());
497486
decls.get(0).putIdent(decl);
@@ -500,7 +489,8 @@ public Env add(String name, Type type) {
500489

501490
// Next try to import this as an enum value by splitting the name in a type prefix and
502491
// the enum inside.
503-
Integer enumValue = typeProvider.lookupEnumValue(cand);
492+
// Integer enumValue = typeProvider.lookupEnumValue(cand);
493+
Integer enumValue = 0;
504494
if (enumValue != null) {
505495
decl =
506496
CelIdentDecl.newBuilder()

checker/src/main/java/dev/cel/checker/ExprChecker.java

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,15 @@
3939
import dev.cel.common.types.CelKind;
4040
import dev.cel.common.types.CelProtoTypes;
4141
import dev.cel.common.types.CelType;
42+
import dev.cel.common.types.CelTypeProvider;
4243
import dev.cel.common.types.CelTypes;
4344
import dev.cel.common.types.ListType;
4445
import dev.cel.common.types.MapType;
4546
import dev.cel.common.types.OptionalType;
47+
import dev.cel.common.types.ProtoMessageType;
48+
import dev.cel.common.types.ProtoMessageType.Extension;
4649
import dev.cel.common.types.SimpleType;
50+
import dev.cel.common.types.StructType.Field;
4751
import dev.cel.common.types.TypeType;
4852
import java.util.ArrayList;
4953
import java.util.HashSet;
@@ -143,7 +147,7 @@ public static CelAbstractSyntaxTree typecheck(
143147
}
144148

145149
private final Env env;
146-
private final TypeProvider typeProvider;
150+
private final CelTypeProvider typeProvider;
147151
private final CelContainer container;
148152
private final Map<Long, Integer> positionMap;
149153
private final InferenceContext inferenceContext;
@@ -410,7 +414,7 @@ private CelExpr visit(CelExpr expr, CelExpr.CelStruct struct) {
410414
expr = replaceStructEntryValueSubtree(expr, visitedValueExpr, i);
411415
}
412416
CelType fieldType =
413-
getFieldType(entry.id(), getPosition(entry), messageType, entry.fieldKey()).celType();
417+
getFieldType(entry.id(), getPosition(entry), messageType, entry.fieldKey()).type();
414418
CelType valueType = env.getType(visitedValueExpr);
415419
if (entry.optionalEntry()) {
416420
if (valueType instanceof OptionalType) {
@@ -716,7 +720,7 @@ private OverloadResolution resolveOverload(
716720
// Return value from visit is not needed as the subtree is not rewritten here.
717721
@SuppressWarnings("CheckReturnValue")
718722
private CelType visitSelectField(
719-
CelExpr expr, CelExpr operand, String field, boolean isOptional) {
723+
CelExpr expr, CelExpr operand, String fieldName, boolean isOptional) {
720724
CelType operandType = inferenceContext.specialize(env.getType(operand));
721725
CelType resultType = SimpleType.ERROR;
722726

@@ -727,10 +731,9 @@ private CelType visitSelectField(
727731

728732
if (!Types.isDynOrError(operandType)) {
729733
if (operandType.kind() == CelKind.STRUCT) {
730-
TypeProvider.FieldType fieldType =
731-
getFieldType(expr.id(), getPosition(expr), operandType, field);
732-
// Type of the field
733-
resultType = fieldType.celType();
734+
Field field =
735+
getFieldType(expr.id(), getPosition(expr), operandType, fieldName);
736+
resultType = field.type();
734737
} else if (operandType.kind() == CelKind.MAP) {
735738
resultType = ((MapType) operandType).valueType();
736739
} else if (operandType.kind() == CelKind.TYPE_PARAM) {
@@ -805,18 +808,25 @@ private CelExpr visitOptionalCall(CelExpr expr, CelExpr.CelCall call) {
805808
}
806809

807810
/** Returns the field type give a type instance and field name. */
808-
private TypeProvider.FieldType getFieldType(
809-
long exprId, int position, CelType type, String fieldName) {
811+
private Field getFieldType(long exprId, int position, CelType type, String fieldName) {
810812
String typeName = type.name();
811-
if (typeProvider.lookupCelType(typeName).isPresent()) {
812-
TypeProvider.FieldType fieldType = typeProvider.lookupFieldType(type, fieldName);
813-
if (fieldType != null) {
814-
return fieldType;
813+
ProtoMessageType protoMessageType =
814+
typeProvider
815+
.findType(typeName)
816+
.filter(t -> t instanceof ProtoMessageType)
817+
.map(ProtoMessageType.class::cast)
818+
.orElse(null);
819+
820+
if (protoMessageType != null) {
821+
Field field = protoMessageType.findField(fieldName).orElse(null);
822+
if (field != null) {
823+
return field;
815824
}
816-
TypeProvider.ExtensionFieldType extensionFieldType =
817-
typeProvider.lookupExtensionType(fieldName);
818-
if (extensionFieldType != null) {
819-
return extensionFieldType.fieldType();
825+
826+
Extension extensionField = protoMessageType.findExtension(fieldName).orElse(null);
827+
828+
if (extensionField != null) {
829+
return Field.of(extensionField.name(), extensionField.type());
820830
}
821831
env.reportError(exprId, position, "undefined field '%s'", fieldName);
822832
} else {
@@ -831,6 +841,7 @@ private TypeProvider.FieldType getFieldType(
831841
}
832842
env.reportError(exprId, position, errorMessage, fieldName, typeName);
833843
}
844+
834845
return ERROR;
835846
}
836847

@@ -892,8 +903,8 @@ public static OverloadResolution of(CelReference reference, CelType type) {
892903
}
893904
}
894905

895-
/** Helper object to represent a {@link TypeProvider.FieldType} lookup failure. */
896-
private static final TypeProvider.FieldType ERROR = TypeProvider.FieldType.of(Types.ERROR);
906+
/** Helper object to represent a {@link CelTypeProvider#findType(String)} lookup failure. */
907+
private static final Field ERROR = Field.of(SimpleType.ERROR.name(), SimpleType.ERROR);
897908

898909
private static CelExpr replaceIdentSubtree(CelExpr expr, String name) {
899910
CelExpr.CelIdent newIdent = CelExpr.CelIdent.newBuilder().setName(name).build();

0 commit comments

Comments
 (0)