Skip to content

Commit e97bcaf

Browse files
l46kokcopybara-github
authored andcommitted
Fix type-checker to respect the ordering of type providers (protobuf then user provided)
PiperOrigin-RevId: 992491032
1 parent 14b79d6 commit e97bcaf

3 files changed

Lines changed: 44 additions & 4 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ public CelCheckerLegacyImpl build() {
446446
} else if (celTypeProvider != null) {
447447
messageTypeProvider =
448448
new CelTypeProvider.CombinedCelTypeProvider(
449-
ImmutableList.of(celTypeProvider, messageTypeProvider));
449+
ImmutableList.of(messageTypeProvider, celTypeProvider));
450450
}
451451

452452
// Configure the declaration set, and possibly alter the type provider if ProtoDecl values

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import com.google.common.collect.ImmutableList;
2121
import com.google.common.collect.ImmutableMap;
22+
import com.google.common.collect.ImmutableSet;
2223
import com.google.protobuf.Duration;
2324
import com.google.protobuf.FieldMask;
2425
import com.google.protobuf.Timestamp;
@@ -40,6 +41,7 @@
4041
import dev.cel.common.types.ListType;
4142
import dev.cel.common.types.MapType;
4243
import dev.cel.common.types.SimpleType;
44+
import dev.cel.common.types.StructType;
4345
import dev.cel.common.types.StructTypeReference;
4446
import dev.cel.common.types.TypeType;
4547
import dev.cel.compiler.CelCompiler;
@@ -409,6 +411,47 @@ public Optional<CelType> findType(String typeName) {
409411
assertThat(ast.getResultType()).isEqualTo(preWrappedType);
410412
}
411413

414+
@Test
415+
public void check_combinedTypeProviders_protoMessageTakesPrecedenceOverCustom() throws Exception {
416+
StructType shadowingStructType =
417+
StructType.create(
418+
TestAllTypes.getDescriptor().getFullName(),
419+
ImmutableSet.of("single_int64"),
420+
fieldName -> Optional.of(SimpleType.STRING));
421+
StructType customOnlyStructType =
422+
StructType.create(
423+
"custom.CustomStruct",
424+
ImmutableSet.of("custom_field"),
425+
fieldName -> Optional.of(SimpleType.STRING));
426+
CelTypeProvider customTypeProvider =
427+
new CelTypeProvider() {
428+
@Override
429+
public ImmutableList<CelType> types() {
430+
return ImmutableList.of(shadowingStructType, customOnlyStructType);
431+
}
432+
433+
@Override
434+
public Optional<CelType> findType(String typeName) {
435+
return types().stream().filter(t -> t.name().equals(typeName)).findFirst();
436+
}
437+
};
438+
CelCompiler celCompiler =
439+
CelCompilerFactory.standardCelCompilerBuilder()
440+
.addMessageTypes(TestAllTypes.getDescriptor())
441+
.setTypeProvider(customTypeProvider)
442+
.build();
443+
444+
CelAbstractSyntaxTree protoAst =
445+
celCompiler
446+
.compile("cel.expr.conformance.proto3.TestAllTypes{single_int64: 1}.single_int64")
447+
.getAst();
448+
CelAbstractSyntaxTree customAst =
449+
celCompiler.compile("custom.CustomStruct{custom_field: 'hello'}.custom_field").getAst();
450+
451+
assertThat(protoAst.getResultType()).isEqualTo(SimpleType.INT);
452+
assertThat(customAst.getResultType()).isEqualTo(SimpleType.STRING);
453+
}
454+
412455
private enum FieldTypeTestCase {
413456
REPEATED_PRIMITIVE("msg.repeated_int64", ListType.create(SimpleType.INT)),
414457
MAP_PRIMITIVE("msg.map_string_string", MapType.create(SimpleType.STRING, SimpleType.STRING)),

‎runtime/src/test/java/dev/cel/runtime/DescriptorTypeResolverTest.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@ public class DescriptorTypeResolverTest {
4545
private static final Cel CEL =
4646
CelFactory.plannerCelBuilder()
4747
.setTypeProvider(PROTO_MESSAGE_TYPE_PROVIDER)
48-
// TODO: Replace setValueProvider with
49-
// addMessageTypes(TestAllTypes.getDescriptor()) once CelRuntimeImpl prioritizes custom
50-
// CelTypeProvider over its internal messageTypeProvider.
5148
.setValueProvider(
5249
(structType, fields) ->
5350
structType.equals(TestAllTypes.getDescriptor().getFullName())

0 commit comments

Comments
 (0)