JSON field name resolution fix for shadowed cases

bundle/src/test/java/dev/cel/bundle/BUILD.bazel

@@ -17,6 +17,7 @@ java_library(
     deps = [
         "//:java_truth",
         "//bundle:cel",
+        "//bundle:cel_experimental_factory",
         "//bundle:cel_impl",
         "//bundle:environment",
         "//bundle:environment_exception",
@@ -55,6 +56,7 @@ java_library(
         "//runtime:evaluation_listener",
         "//runtime:function_binding",
         "//runtime:unknown_attributes",
+        "//testing/protos:single_file_extension_java_proto",
         "//testing/protos:single_file_java_proto",
         "@cel_spec//proto/cel/expr:checked_java_proto",
         "@cel_spec//proto/cel/expr:syntax_java_proto",

bundle/src/test/java/dev/cel/bundle/CelImplTest.java

@@ -98,6 +98,7 @@
 import dev.cel.expr.conformance.proto2.Proto2ExtensionScopedMessage;
 import dev.cel.expr.conformance.proto2.TestAllTypesExtensions;
 import dev.cel.expr.conformance.proto3.TestAllTypes;
+import dev.cel.extensions.CelExtensions;
 import dev.cel.parser.CelParserImpl;
 import dev.cel.parser.CelStandardMacro;
 import dev.cel.runtime.CelAttribute;
@@ -113,7 +114,8 @@
 import dev.cel.runtime.CelUnknownSet;
 import dev.cel.runtime.CelVariableResolver;
 import dev.cel.runtime.UnknownContext;
-import dev.cel.testing.testdata.SingleFileProto.SingleFile;
+import dev.cel.testing.testdata.SingleFile;
+import dev.cel.testing.testdata.SingleFileExtensionsProto;
 import dev.cel.testing.testdata.proto3.StandaloneGlobalEnum;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -2142,20 +2144,90 @@ public void toBuilder_isImmutable() {
   }
 
   @Test
-  public void eval_withJsonFieldName() throws Exception {
-    Cel cel =
-        standardCelBuilderWithMacros()
-            .addVar("file", StructTypeReference.create(SingleFile.getDescriptor().getFullName()))
-            .addMessageTypes(SingleFile.getDescriptor())
-            .setOptions(CelOptions.current().enableJsonFieldNames(true).build())
-            .build();
-    CelAbstractSyntaxTree ast = cel.compile("file.camelCased").getAst();
+  public void eval_withJsonFieldName(@TestParameter RuntimeEnv runtimeEnv) throws Exception {
+    Cel cel = runtimeEnv.cel;
+    CelAbstractSyntaxTree ast =
+        cel.compile(
+                "file.int32_snake_case_json_name == 1 && "
+                    + "file.int64CamelCaseJsonName == 2 && "
+                    + "file.uint32DefaultJsonName == 3u && "
+                    + "file.`uint64-custom-json-name` == 4u && "
+                    + "file.single_string == 'shadows' && "
+                    + "file.singleString == 'shadowed'")
+            .getAst();
+
+    boolean result =
+        (boolean)
+            cel.createProgram(ast)
+                .eval(
+                    ImmutableMap.of(
+                        "file",
+                        SingleFile.newBuilder()
+                            .setInt32SnakeCaseJsonName(1)
+                            .setInt64CamelCaseJsonName(2L)
+                            .setUint32DefaultJsonName(3)
+                            .setUint64CustomJsonName(4)
+                            .setStringJsonNameShadows("shadows")
+                            .setSingleString("shadowed")
+                            .setExtension(SingleFileExtensionsProto.int64CamelCaseJsonName, 5L)
+                            .build()));
 
-    Object result =
-        cel.createProgram(ast)
-            .eval(ImmutableMap.of("file", SingleFile.newBuilder().setSnakeCased("foo").build()));
+    assertThat(result).isTrue();
+  }
+
+  @Test
+  public void eval_withJsonFieldName_fieldsFallBack(@TestParameter RuntimeEnv runtimeEnv) throws Exception {
+    Cel cel = runtimeEnv.cel;
+    CelAbstractSyntaxTree ast =
+        cel.compile(
+                "dyn(file).int32_snake_case_json_name == 1 && "
+                    + "dyn(file).`uint64-custom-json-name` == 4u && "
+                    + "dyn(file).single_string == 'shadows' && "
+                    + "dyn(file).string_json_name_shadows == 'shadows' && "
+                    + "dyn(file).singleString == 'shadowed'")
+            .getAst();
+
+    boolean result =
+        (boolean)
+            cel.createProgram(ast)
+                .eval(
+                    ImmutableMap.of(
+                        "file",
+                        SingleFile.newBuilder()
+                            .setInt32SnakeCaseJsonName(1)
+                            .setInt64CamelCaseJsonName(2L)
+                            .setUint32DefaultJsonName(3)
+                            .setUint64CustomJsonName(4)
+                            .setStringJsonNameShadows("shadows")
+                            .setSingleString("shadowed")
+                            .build()));
 
-    assertThat(result).isEqualTo("foo");
+    assertThat(result).isTrue();
+  }
+
+  @Test
+  public void eval_withJsonFieldName_extensionFields(@TestParameter RuntimeEnv runtimeEnv) throws Exception {
+    Cel cel = runtimeEnv.cel;
+    CelAbstractSyntaxTree ast =
+        cel.compile(
+                "proto.getExt(file, dev.cel.testing.testdata.int64CamelCaseJsonName) == 5 &&"
+                    + " proto.getExt(file, dev.cel.testing.testdata.single_string) == 'foo'")
+            .getAst();
+
+    boolean result =
+        (boolean)
+            cel.createProgram(ast)
+                .eval(
+                    ImmutableMap.of(
+                        "file",
+                        SingleFile.newBuilder()
+                            .setInt64CamelCaseJsonName(2L)
+                            .setExtension(SingleFileExtensionsProto.int64CamelCaseJsonName, 5L)
+                            .setSingleString("This should not be used")
+                            .setExtension(SingleFileExtensionsProto.singleString, "foo")
+                            .build()));
+
+    assertThat(result).isTrue();
   }
 
   @Test
@@ -2171,7 +2243,7 @@ public void eval_withJsonFieldName_runtimeOptionDisabled_throws() throws Excepti
             .addMessageTypes(SingleFile.getDescriptor())
             .setOptions(CelOptions.current().enableJsonFieldNames(false).build())
             .build();
-    CelAbstractSyntaxTree ast = celCompiler.compile("file.camelCased").getAst();
+    CelAbstractSyntaxTree ast = celCompiler.compile("file.int64CamelCaseJsonName").getAst();
 
     CelEvaluationException e =
         assertThrows(
@@ -2183,7 +2255,8 @@ public void eval_withJsonFieldName_runtimeOptionDisabled_throws() throws Excepti
     assertThat(e)
         .hasMessageThat()
         .contains(
-            "field 'camelCased' is not declared in message 'dev.cel.testing.testdata.SingleFile");
+            "field 'int64CamelCaseJsonName' is not declared in message"
+                + " 'dev.cel.testing.testdata.SingleFile");
   }
 
   @Test
@@ -2194,7 +2267,7 @@ public void compile_withJsonFieldName_astTagged() throws Exception {
             .addMessageTypes(SingleFile.getDescriptor())
             .setOptions(CelOptions.current().enableJsonFieldNames(true).build())
             .build();
-    CelAbstractSyntaxTree ast = cel.compile("file.camelCased").getAst();
+    CelAbstractSyntaxTree ast = cel.compile("file.int64CamelCaseJsonName").getAst();
 
     assertThat(ast.getSource().getExtensions())
         .contains(
@@ -2243,4 +2316,34 @@ private static TypeProvider aliasingProvider(ImmutableMap<String, Type> typeAlia
       }
     };
   }
+
+  private enum RuntimeEnv {
+    LEGACY(setupEnv(CelFactory.standardCelBuilder())),
+    PLANNER(setupEnv(CelExperimentalFactory.plannerCelBuilder()))
+    ;
+
+    private final Cel cel;
+
+    private static Cel setupEnv(CelBuilder celBuilder) {
+      ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
+      SingleFileExtensionsProto.registerAllExtensions(extensionRegistry);
+      return celBuilder
+          .addVar("file", StructTypeReference.create(SingleFile.getDescriptor().getFullName()))
+          .addMessageTypes(SingleFile.getDescriptor())
+          .addFileTypes(SingleFileExtensionsProto.getDescriptor())
+          .addCompilerLibraries(CelExtensions.protos())
+          .setExtensionRegistry(extensionRegistry)
+          .setOptions(
+              CelOptions.current()
+                  .enableJsonFieldNames(true)
+                  .enableHeterogeneousNumericComparisons(true)
+                  .enableQuotedIdentifierSyntax(true)
+                  .build())
+          .build();
+    }
+
+    RuntimeEnv(Cel cel) {
+      this.cel = cel;
+    }
+  }
 }

common/src/main/java/dev/cel/common/values/ProtoMessageValue.java

@@ -92,11 +92,6 @@ public static ProtoMessageValue create(
 
   private FieldDescriptor findField(
       CelDescriptorPool celDescriptorPool, Descriptor descriptor, String fieldName) {
-    FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
-    if (fieldDescriptor != null) {
-      return fieldDescriptor;
-    }
-
     if (enableJsonFieldNames()) {
       for (FieldDescriptor fd : descriptor.getFields()) {
         if (fd.getJsonName().equals(fieldName)) {
@@ -105,6 +100,11 @@ private FieldDescriptor findField(
       }
     }
 
+    FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
+    if (fieldDescriptor != null) {
+      return fieldDescriptor;
+    }
+
     return celDescriptorPool
         .findExtensionDescriptor(descriptor, fieldName)
         .orElseThrow(

common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java

@@ -68,11 +68,6 @@ public Optional<Object> newValue(String structType, Map<String, Object> fields)
   }
 
   private FieldDescriptor findField(Descriptor descriptor, String fieldName) {
-    FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
-    if (fieldDescriptor != null) {
-      return fieldDescriptor;
-    }
-
     if (celOptions.enableJsonFieldNames()) {
       for (FieldDescriptor fd : descriptor.getFields()) {
         if (fd.getJsonName().equals(fieldName)) {
@@ -81,6 +76,11 @@ private FieldDescriptor findField(Descriptor descriptor, String fieldName) {
       }
     }
 
+    FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
+    if (fieldDescriptor != null) {
+      return fieldDescriptor;
+    }
+
     return protoMessageFactory
         .getDescriptorPool()
         .findExtensionDescriptor(descriptor, fieldName)

common/src/test/java/dev/cel/common/internal/DynamicProtoTest.java

@@ -37,7 +37,7 @@
 import dev.cel.common.CelDescriptorUtil;
 import dev.cel.common.CelDescriptors;
 import dev.cel.testing.testdata.MultiFile;
-import dev.cel.testing.testdata.SingleFileProto.SingleFile;
+import dev.cel.testing.testdata.SingleFile;
 import java.io.IOException;
 import org.junit.Test;
 import org.junit.runner.RunWith;

common/src/test/java/dev/cel/common/types/ProtoMessageTypeProviderTest.java

@@ -23,7 +23,7 @@
 import dev.cel.common.types.StructType.Field;
 import dev.cel.expr.conformance.proto2.TestAllTypes;
 import dev.cel.expr.conformance.proto2.TestAllTypesExtensions;
-import dev.cel.testing.testdata.SingleFileProto.SingleFile;
+import dev.cel.testing.testdata.SingleFile;
 import java.util.Optional;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -269,8 +269,8 @@ public void findField_withJsonNameOption() {
         (ProtoMessageType) typeProvider.findType(SingleFile.getDescriptor().getFullName()).get();
 
     // Note that these are the same fields, with json_name option set
-    Optional<Field> snakeCasedField = msgType.findField("snake_cased");
-    Optional<Field> jsonNameField = msgType.findField("camelCased");
+    Optional<Field> snakeCasedField = msgType.findField("int64_camel_case_json_name");
+    Optional<Field> jsonNameField = msgType.findField("int64CamelCaseJsonName");
 
     assertThat(snakeCasedField).isEmpty();
     assertThat(jsonNameField).isPresent();

policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java

@@ -45,7 +45,7 @@
 import dev.cel.policy.PolicyTestHelper.TestYamlPolicy;
 import dev.cel.runtime.CelFunctionBinding;
 import dev.cel.runtime.CelLateFunctionBindings;
-import dev.cel.testing.testdata.SingleFileProto.SingleFile;
+import dev.cel.testing.testdata.SingleFile;
 import dev.cel.testing.testdata.proto3.StandaloneGlobalEnum;
 import java.io.IOException;
 import java.util.Map;

runtime/src/main/java/dev/cel/runtime/DescriptorMessageProvider.java

@@ -173,30 +173,28 @@ public Object hasField(Object message, String fieldName) {
   }
 
   private FieldDescriptor findField(Descriptor descriptor, String fieldName) {
-    FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
-    if (fieldDescriptor == null) {
-      Optional<FieldDescriptor> maybeFieldDescriptor =
-          protoMessageFactory.getDescriptorPool().findExtensionDescriptor(descriptor, fieldName);
-      if (maybeFieldDescriptor.isPresent()) {
-        fieldDescriptor = maybeFieldDescriptor.get();
-      }
-    }
-
-    if (fieldDescriptor == null && celOptions.enableJsonFieldNames()) {
+    if (celOptions.enableJsonFieldNames()) {
       for (FieldDescriptor fd : descriptor.getFields()) {
         if (fd.getJsonName().equals(fieldName)) {
-          fieldDescriptor = fd;
-          break;
+          return fd;
         }
       }
     }
 
-    if (fieldDescriptor == null) {
-      throw new IllegalArgumentException(
-          String.format(
-              "field '%s' is not declared in message '%s'", fieldName, descriptor.getFullName()));
+    FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
+    if (fieldDescriptor != null) {
+      return fieldDescriptor;
+    }
+    fieldDescriptor =
+        protoMessageFactory.getDescriptorPool().findExtensionDescriptor(descriptor, fieldName).orElse(null);
+    if (fieldDescriptor != null) {
+      return fieldDescriptor;
     }
-    return fieldDescriptor;
+
+
+    throw new IllegalArgumentException(
+        String.format(
+            "field '%s' is not declared in message '%s'", fieldName, descriptor.getFullName()));
   }
 
   private static MessageOrBuilder assertFullProtoMessage(Object candidate, String fieldName) {

runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeTest.java

@@ -59,8 +59,8 @@
 import dev.cel.testing.testdata.MultiFile;
 import dev.cel.testing.testdata.MultiFileCelDescriptor;
 import dev.cel.testing.testdata.SimpleEnum;
+import dev.cel.testing.testdata.SingleFile;
 import dev.cel.testing.testdata.SingleFileCelDescriptor;
-import dev.cel.testing.testdata.SingleFileProto.SingleFile;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;

testing/protos/BUILD.bazel

@@ -9,6 +9,11 @@ alias(
     actual = "//testing/src/test/resources/protos:single_file_java_proto",
 )
 
+alias(
+    name = "single_file_extension_java_proto",
+    actual = "//testing/src/test/resources/protos:single_file_extension_java_proto",
+)
+
 alias(
     name = "multi_file_java_proto",
     actual = "//testing/src/test/resources/protos:multi_file_java_proto",

testing/src/test/resources/protos/BUILD.bazel

@@ -25,6 +25,19 @@ java_proto_library(
     deps = [":single_file_proto"],
 )
 
+proto_library(
+    name = "single_file_extension_proto",
+    srcs = ["single_file_extensions.proto"],
+    deps = [":single_file_proto"],
+)
+
+java_proto_library(
+    name = "single_file_extension_java_proto",
+    tags = [
+    ],
+    deps = [":single_file_extension_proto"],
+)
+
 proto_library(
     name = "multi_file_proto",
     srcs = [