Fix type-checker to always rewrite resolved identifiers and functions through container or alias resolutions

l46kok · copybara-github · commit d877a05ba690 · 2025-10-07T13:57:53.000-07:00
PiperOrigin-RevId: 816019607
diff --git a/checker/src/main/java/dev/cel/checker/ExprChecker.java b/checker/src/main/java/dev/cel/checker/ExprChecker.java
@@ -362,6 +362,13 @@ private CelExpr visit(CelExpr expr, CelExpr.CelStruct struct) {
     CelType messageType = SimpleType.ERROR;
     CelIdentDecl decl =
         env.lookupIdent(expr.id(), getPosition(expr), container, struct.messageName());
+    if (!struct.messageName().equals(decl.name())) {
+      expr =
+          expr.toBuilder()
+              .setStruct(struct.toBuilder().setMessageName(decl.name()).build())
+              .build();
+    }
+
     env.setRef(expr, CelReference.newBuilder().setName(decl.name()).build());
     CelType type = decl.type();
     if (type.kind() != CelKind.ERROR) {
diff --git a/checker/src/test/java/dev/cel/checker/CelProtoExprVisitorTest.java b/checker/src/test/java/dev/cel/checker/CelProtoExprVisitorTest.java
@@ -175,15 +175,17 @@ public void visitSelect() throws Exception {
         .isEqualTo(
             VisitedReference.newBuilder()
                 .setCreateStruct(
-                    Expr.CreateStruct.newBuilder().setMessageName("TestAllTypes").build())
+                    Expr.CreateStruct.newBuilder()
+                        .setMessageName("cel.expr.conformance.proto3.TestAllTypes")
+                        .build())
                 .setSelect(
                     Expr.Select.newBuilder()
                         .setOperand(
                             Expr.newBuilder()
                                 .setId(1)
                                 .setStructExpr(
                                     Expr.CreateStruct.newBuilder()
-                                        .setMessageName("TestAllTypes")
+                                        .setMessageName("cel.expr.conformance.proto3.TestAllTypes")
                                         .build()))
                         .setField("single_int64")
                         .build())
@@ -227,7 +229,9 @@ public void visitCreateStruct() throws Exception {
         .isEqualTo(
             VisitedReference.newBuilder()
                 .setCreateStruct(
-                    Expr.CreateStruct.newBuilder().setMessageName("TestAllTypes").build())
+                    Expr.CreateStruct.newBuilder()
+                        .setMessageName("cel.expr.conformance.proto3.TestAllTypes")
+                        .build())
                 .build());
   }
 
diff --git a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java
@@ -73,8 +73,9 @@ private void runTest() throws Exception {
     CelAbstractSyntaxTree ast =
         prepareTest(
             Arrays.asList(
-                TestAllTypes.getDescriptor(),
-                dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor()));
+                StandaloneGlobalEnum.getDescriptor().getFile(),
+                TestAllTypes.getDescriptor().getFile(),
+                dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor().getFile()));
     if (ast != null) {
       testOutput()
           .println(
@@ -239,6 +240,45 @@ public void messageFieldSelect() throws Exception {
     runTest();
   }
 
+  @Test
+  public void containers() throws Exception {
+    container =
+        CelContainer.newBuilder()
+            .setName("dev.cel.testing.testdata.proto3.StandaloneGlobalEnum")
+            .addAlias("p3_alias", "cel.expr.conformance.proto3")
+            .addAlias("foo_bar_alias", "foo.bar")
+            .addAlias("foo_bar_baz_alias", "foo.bar.baz")
+            .addAbbreviations("cel.expr.conformance.proto2", "cel.expr.conformance.proto3")
+            .build();
+    source = "p3_alias.TestAllTypes{}";
+    runTest();
+
+    source = "proto2.TestAllTypes{}";
+    runTest();
+
+    source = "proto3.TestAllTypes{}";
+    runTest();
+
+    source = "SGAR"; // From StandaloneGlobalEnum
+    runTest();
+
+    declareVariable("foo.bar", SimpleType.STRING);
+    declareFunction(
+        "baz",
+        memberOverload(
+            "foo_bar_baz_overload", ImmutableList.of(SimpleType.STRING), SimpleType.DYN));
+    // Member call of "baz()" on "foo.bar" identifier
+    source = "foo_bar_alias.baz()";
+    runTest();
+
+    declareFunction(
+        "foo.bar.baz.qux",
+        globalOverload("foo_bar_baz_qux_overload", ImmutableList.of(), SimpleType.DYN));
+    // Global call of "foo.bar.baz.qux" as a fully qualified name
+    source = "foo_bar_baz_alias.qux()";
+    runTest();
+  }
+
   @Test
   public void messageCreationError() throws Exception {
     declareVariable("x", SimpleType.INT);
@@ -851,17 +891,16 @@ public void twoVarComprehensions_incorrectIterVars() throws Exception {
   public void twoVarComprehensions_duplicateIterVars() throws Exception {
     CelType messageType = StructTypeReference.create("cel.expr.conformance.proto3.TestAllTypes");
     declareVariable("x", messageType);
-    source =
-        "x.repeated_int64.exists(i, i, i < v)";
+    source = "x.repeated_int64.exists(i, i, i < v)";
     runTest();
   }
-  
+
   @Test
   public void twoVarComprehensions_incorrectNumberOfArgs() throws Exception {
     CelType messageType = StructTypeReference.create("cel.expr.conformance.proto3.TestAllTypes");
     declareVariable("x", messageType);
     source =
-            "[1, 2, 3, 4].exists_one(i, v, i < v, v)"
+        "[1, 2, 3, 4].exists_one(i, v, i < v, v)"
             + "&& x.map_string_string.transformList(i, i < v) "
             + "&& [1, 2, 3].transformList(i, v, i > 0 && x < 3, (i * v) + v) == [4]";
     runTest();
diff --git a/checker/src/test/resources/aggregateMessage.baseline b/checker/src/test/resources/aggregateMessage.baseline
@@ -1,7 +1,6 @@
 Source: TestAllTypes{single_int32: 1, single_int64: 2}
 =====>
-TestAllTypes{
+cel.expr.conformance.proto3.TestAllTypes{
   single_int32:1~int,
   single_int64:2~int
-}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes
-
+}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes
diff --git a/checker/src/test/resources/containers.baseline b/checker/src/test/resources/containers.baseline
@@ -0,0 +1,38 @@
+Source: p3_alias.TestAllTypes{}
+=====>
+cel.expr.conformance.proto3.TestAllTypes{}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes
+
+Source: proto2.TestAllTypes{}
+=====>
+cel.expr.conformance.proto2.TestAllTypes{}~cel.expr.conformance.proto2.TestAllTypes^cel.expr.conformance.proto2.TestAllTypes
+
+Source: proto3.TestAllTypes{}
+=====>
+cel.expr.conformance.proto3.TestAllTypes{}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes
+
+Source: SGAR
+=====>
+dev.cel.testing.testdata.proto3.StandaloneGlobalEnum.SGAR~int^dev.cel.testing.testdata.proto3.StandaloneGlobalEnum.SGAR
+
+Source: foo_bar_alias.baz()
+declare foo.bar {
+  value string
+}
+declare baz {
+  function foo_bar_baz_overload string.() -> dyn
+}
+=====>
+foo.bar~string^foo.bar.baz()~dyn^foo_bar_baz_overload
+
+Source: foo_bar_baz_alias.qux()
+declare foo.bar {
+  value string
+}
+declare baz {
+  function foo_bar_baz_overload string.() -> dyn
+}
+declare foo.bar.baz.qux {
+  function foo_bar_baz_qux_overload () -> dyn
+}
+=====>
+foo.bar.baz.qux()~dyn^foo_bar_baz_qux_overload
diff --git a/checker/src/test/resources/nestedEnums.baseline b/checker/src/test/resources/nestedEnums.baseline
@@ -30,8 +30,8 @@ declare single_nested_enum {
 }
 =====>
 _==_(
-  TestAllTypes{
+  cel.expr.conformance.proto3.TestAllTypes{
     single_nested_enum:cel.expr.conformance.proto3.TestAllTypes.NestedEnum.BAR~int^cel.expr.conformance.proto3.TestAllTypes.NestedEnum.BAR
   }~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes.single_nested_enum~int,
   1~int
-)~bool^equals
+)~bool^equals
diff --git a/checker/src/test/resources/nullableMessage.baseline b/checker/src/test/resources/nullableMessage.baseline
@@ -16,10 +16,10 @@ declare x {
 _||_(
   _==_(
     null~null,
-    TestAllTypes{}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes
+    cel.expr.conformance.proto3.TestAllTypes{}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes
   )~bool^equals,
   _==_(
-    TestAllTypes{}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes,
+    cel.expr.conformance.proto3.TestAllTypes{}~cel.expr.conformance.proto3.TestAllTypes^cel.expr.conformance.proto3.TestAllTypes,
     null~null
   )~bool^equals
-)~bool^logical_or
+)~bool^logical_or
diff --git a/checker/src/test/resources/optionals.baseline b/checker/src/test/resources/optionals.baseline
@@ -72,7 +72,7 @@ Source: {?'key': {'a': 'b'}.?value}.key
 
 Source: TestAllTypes{?single_int32: {}.?i}
 =====>
-TestAllTypes{
+cel.expr.conformance.proto3.TestAllTypes{
   ?single_int32:_?._(
     {}~map(dyn, int),
     "i"
@@ -118,4 +118,4 @@ declare b {
 {
   ?"str"~string:a~optional_type(string)^a,
   2~int:3~int
-}~map(dyn, dyn)
+}~map(dyn, dyn)
diff --git a/common/src/test/java/dev/cel/common/ast/CelExprFormatterTest.java b/common/src/test/java/dev/cel/common/ast/CelExprFormatterTest.java
@@ -191,7 +191,7 @@ public void struct() throws Exception {
     assertThat(formattedExpr)
         .isEqualTo(
             "STRUCT [1] {\n"
-                + "  name: TestAllTypes\n"
+                + "  name: cel.expr.conformance.proto3.TestAllTypes\n"
                 + "  entries: {\n"
                 + "    ENTRY [2] {\n"
                 + "      field_key: single_int64\n"
diff --git a/common/src/test/java/dev/cel/common/ast/CelExprVisitorTest.java b/common/src/test/java/dev/cel/common/ast/CelExprVisitorTest.java
@@ -206,14 +206,19 @@ public void visitSelect() throws Exception {
     assertThat(visited)
         .isEqualTo(
             VisitedReference.newBuilder()
-                .setStruct(CelStruct.newBuilder().setMessageName("TestAllTypes").build())
+                .setStruct(
+                    CelStruct.newBuilder()
+                        .setMessageName("cel.expr.conformance.proto3.TestAllTypes")
+                        .build())
                 .setSelect(
                     CelSelect.newBuilder()
                         .setOperand(
                             CelExpr.newBuilder()
                                 .setId(1)
                                 .setStruct(
-                                    CelStruct.newBuilder().setMessageName("TestAllTypes").build())
+                                    CelStruct.newBuilder()
+                                        .setMessageName("cel.expr.conformance.proto3.TestAllTypes")
+                                        .build())
                                 .build())
                         .setField("single_int64")
                         .build())
@@ -268,7 +273,7 @@ public void visitStruct_fieldkey() throws Exception {
                                 .setFieldKey("single_int64")
                                 .setValue(CelExpr.ofConstant(3, longConstant))
                                 .build())
-                        .setMessageName("TestAllTypes")
+                        .setMessageName("cel.expr.conformance.proto3.TestAllTypes")
                         .build())
                 .build());
   }
diff --git a/common/src/test/java/dev/cel/common/navigation/CelNavigableExprVisitorTest.java b/common/src/test/java/dev/cel/common/navigation/CelNavigableExprVisitorTest.java
@@ -595,7 +595,7 @@ public void messageConstruction_allNodesReturned() throws Exception {
             constExpr,
             CelExpr.ofStruct(
                 1,
-                "TestAllTypes",
+                "cel.expr.conformance.proto3.TestAllTypes",
                 ImmutableList.of(CelExpr.ofStructEntry(2, "single_int64", constExpr, false))));
   }
 
@@ -621,7 +621,7 @@ public void messageConstruction_filterStruct_allNodesReturned() throws Exception
         .isEqualTo(
             CelExpr.ofStruct(
                 1,
-                "TestAllTypes",
+                "cel.expr.conformance.proto3.TestAllTypes",
                 ImmutableList.of(
                     CelExpr.ofStructEntry(
                         2, "single_int64", CelExpr.ofConstant(3, CelConstant.ofValue(1)), false))));
diff --git a/optimizer/src/test/java/dev/cel/optimizer/AstMutatorTest.java b/optimizer/src/test/java/dev/cel/optimizer/AstMutatorTest.java
@@ -584,7 +584,7 @@ public void struct_replaceValue() throws Exception {
     CelMutableAst result = AST_MUTATOR.replaceSubtree(mutableAst, newExpr, 3);
 
     assertThat(CEL_UNPARSER.unparse(result.toParsedAst()))
-        .isEqualTo("TestAllTypes{single_int64: 5}");
+        .isEqualTo("cel.expr.conformance.proto3.TestAllTypes{single_int64: 5}");
   }
 
   @Test
diff --git a/optimizer/src/test/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizerTest.java b/optimizer/src/test/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizerTest.java
@@ -159,18 +159,18 @@ public class ConstantFoldingOptimizerTest {
           + " optional.of(1), ?y: optional.none()}'}")
   @TestParameters(
       "{source: 'TestAllTypes{single_int64: 1 + 2 + 3 + x}', "
-          + " expected: 'TestAllTypes{single_int64: 6 + x}'}")
+          + " expected: 'cel.expr.conformance.proto3.TestAllTypes{single_int64: 6 + x}'}")
   @TestParameters(
       "{source: 'TestAllTypes{?single_int64: optional.ofNonZeroValue(1)}', "
-          + " expected: 'TestAllTypes{single_int64: 1}'}")
+          + " expected: 'cel.expr.conformance.proto3.TestAllTypes{single_int64: 1}'}")
   @TestParameters(
       "{source: 'TestAllTypes{?single_int64: optional.ofNonZeroValue(0)}', "
-          + " expected: 'TestAllTypes{}'}")
+          + " expected: 'cel.expr.conformance.proto3.TestAllTypes{}'}")
   @TestParameters(
       "{source: 'TestAllTypes{?single_int64: optional.ofNonZeroValue(1), ?single_int32:"
           + " optional.of(4), ?single_uint64: optional.ofNonZeroValue(x)}', expected:"
-          + " 'TestAllTypes{single_int64: 1, single_int32: 4, ?single_uint64:"
-          + " optional.ofNonZeroValue(x)}'}")
+          + " 'cel.expr.conformance.proto3.TestAllTypes{single_int64: 1, single_int32: 4,"
+          + " ?single_uint64: optional.ofNonZeroValue(x)}'}")
   @TestParameters("{source: '{\"hello\": \"world\"}.hello == x', expected: '\"world\" == x'}")
   @TestParameters("{source: '{\"hello\": \"world\"}[\"hello\"] == x', expected: '\"world\" == x'}")
   @TestParameters("{source: '{\"hello\": \"world\"}.?hello', expected: 'optional.of(\"world\")'}")
@@ -358,7 +358,9 @@ public void constantFold_macros_withoutMacroCallMetadata(String source) throws E
   @TestParameters("{source: 'optional.none()'}")
   @TestParameters("{source: '[optional.none()]'}")
   @TestParameters("{source: '[?x.?y]'}")
-  @TestParameters("{source: 'TestAllTypes{single_int32: x, repeated_int32: [1, 2, 3]}'}")
+  @TestParameters(
+      "{source: 'cel.expr.conformance.proto3.TestAllTypes{"
+          + "single_int32: x, repeated_int32: [1, 2, 3]}'}")
   @TestParameters("{source: 'get_true() == get_true()'}")
   @TestParameters("{source: 'get_true() == true'}")
   @TestParameters("{source: 'x == x'}")
diff --git a/optimizer/src/test/resources/subexpression_ast_block_common_subexpr_only.baseline b/optimizer/src/test/resources/subexpression_ast_block_common_subexpr_only.baseline
@@ -4242,7 +4242,7 @@ CALL [1] {
     LIST [2] {
       elements: {
         STRUCT [3] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [4] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_1.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_1.baseline
@@ -5426,7 +5426,7 @@ CALL [1] {
           }
         }
         STRUCT [7] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [8] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_2.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_2.baseline
@@ -5155,7 +5155,7 @@ CALL [1] {
     LIST [2] {
       elements: {
         STRUCT [3] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [4] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_3.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_3.baseline
@@ -4830,7 +4830,7 @@ CALL [1] {
     LIST [2] {
       elements: {
         STRUCT [3] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [4] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_4.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_4.baseline
@@ -4401,7 +4401,7 @@ CALL [1] {
     LIST [2] {
       elements: {
         STRUCT [3] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [4] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_5.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_5.baseline
@@ -4362,7 +4362,7 @@ CALL [1] {
     LIST [2] {
       elements: {
         STRUCT [3] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [4] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_6.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_6.baseline
@@ -4338,7 +4338,7 @@ CALL [1] {
     LIST [2] {
       elements: {
         STRUCT [3] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [4] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_7.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_7.baseline
@@ -4299,7 +4299,7 @@ CALL [1] {
     LIST [2] {
       elements: {
         STRUCT [3] {
-          name: TestAllTypes
+          name: cel.expr.conformance.proto3.TestAllTypes
           entries: {
             ENTRY [4] {
               field_key: single_int64
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_8.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_8.baseline
diff --git a/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_9.baseline b/optimizer/src/test/resources/subexpression_ast_block_recursion_depth_9.baseline
diff --git a/optimizer/src/test/resources/subexpression_unparsed.baseline b/optimizer/src/test/resources/subexpression_unparsed.baseline
diff --git a/runtime/src/test/java/dev/cel/runtime/CelRuntimeTest.java b/runtime/src/test/java/dev/cel/runtime/CelRuntimeTest.java
diff --git a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java
