Add abbreviation support to type-checker

l46kok · copybara-github · commit 712b265680be · 2025-07-28T11:58:40.000-07:00
PiperOrigin-RevId: 788101544
diff --git a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java
@@ -984,15 +984,7 @@ public void optionalErrors() throws Exception {
     CelAbstractSyntaxTree parsedAst = celCompiler.parse(source).getAst();
     CelMutableAst mutableAst = CelMutableAst.fromCelAst(parsedAst);
     mutableAst.expr().call().args().get(1).setConstant(CelConstant.ofValue(true));
-    // ParsedExpr parsedExpr =
-    //     CelProtoAbstractSyntaxTree.fromCelAst(celCompiler.parse(source).getAst()).toParsedExpr();
-    // ParsedExpr.Builder parsedExprBuilder = parsedExpr.toBuilder();
-    // parsedExprBuilder
-    //     .getExprBuilder()
-    //     .getCallExprBuilder()
-    //     .getArgsBuilder(1)
-    //     .setConstExpr(Constant.newBuilder().setBoolValue(true).build()); // Const must be a
-    // string
+
     runErroneousTest(mutableAst.toParsedAst());
   }
 
diff --git a/common/src/main/java/dev/cel/common/CelContainer.java b/common/src/main/java/dev/cel/common/CelContainer.java
@@ -15,12 +15,14 @@
 package dev.cel.common;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.annotations.CheckReturnValue;
 import com.google.errorprone.annotations.Immutable;
 import java.util.LinkedHashMap;
+import java.util.Locale;
 import java.util.Optional;
 
 /** CelContainer holds a reference to an optional qualified container name and set of aliases. */
@@ -38,20 +40,125 @@ public abstract static class Builder {
 
     private final LinkedHashMap<String, String> aliases = new LinkedHashMap<>();
 
+    abstract String name();
+
     /** Sets the fully-qualified name of the container. */
     public abstract Builder setName(String name);
 
     abstract Builder setAliases(ImmutableMap<String, String> aliases);
 
-    /** Alias associates a fully-qualified name with a user-defined alias. */
+    /** See {@link #addAbbreviations(ImmutableSet)} for documentation. */
+    @CanIgnoreReturnValue
+    public Builder addAbbreviations(String... qualifiedNames) {
+      Preconditions.checkNotNull(qualifiedNames);
+      return addAbbreviations(ImmutableSet.copyOf(qualifiedNames));
+    }
+
+    /**
+     * Configures a set of simple names as abbreviations for fully-qualified names.
+     *
+     * <p>An abbreviation is a simple name that expands to a fully-qualified name. Abbreviations can
+     * be useful when working with variables, functions, and especially types from multiple
+     * namespaces:
+     *
+     * <pre>{@code
+     * // CEL object construction
+     * qual.pkg.version.ObjTypeName{
+     *   field: alt.container.ver.FieldTypeName{value: ...}
+     * }
+     * }</pre>
+     *
+     * <p>Only one the qualified names above may be used as the CEL container, so at least one of
+     * these references must be a long qualified name within an otherwise short CEL program. Using
+     * the following abbreviations, the program becomes much simpler:
+     *
+     * <pre>{@code
+     * // CEL Java option
+     * CelContainer.newBuilder().addAbbreviations("qual.pkg.version.ObjTypeName", "alt.container.ver.FieldTypeName").build()
+     * }
+     * {@code
+     * // Simplified Object construction
+     * ObjTypeName{field: FieldTypeName{value: ...}}
+     * }</pre>
+     *
+     * <p>There are a few rules for the qualified names and the simple abbreviations generated from
+     * them:
+     *
+     * <ul>
+     *   <li>Qualified names must be dot-delimited, e.g. `package.subpkg.name`.
+     *   <li>The last element in the qualified name is the abbreviation.
+     *   <li>Abbreviations must not collide with each other.
+     *   <li>The abbreviation must not collide with unqualified names in use.
+     * </ul>
+     *
+     * <p>Abbreviations are distinct from container-based references in the following important
+     * ways:
+     *
+     * <ul>
+     *   <li>Abbreviations must expand to a fully-qualified name.
+     *   <li>Expanded abbreviations do not participate in namespace resolution.
+     *   <li>Abbreviation expansion is done instead of the container search for a matching
+     *       identifier.
+     *   <li>Containers follow C++ namespace resolution rules with searches from the most qualified
+     *       name to the least qualified name.
+     *   <li>Container references within the CEL program may be relative, and are resolved to fully
+     *       qualified names at either type-check time or program plan time, whichever comes first.
+     * </ul>
+     *
+     * <p>If there is ever a case where an identifier could be in both the container and as an
+     * abbreviation, the abbreviation wins as this will ensure that the meaning of a program is
+     * preserved between compilations even as the container evolves.
+     */
+    @CanIgnoreReturnValue
+    public Builder addAbbreviations(ImmutableSet<String> qualifiedNames) {
+      for (String qualifiedName : qualifiedNames) {
+        qualifiedName = qualifiedName.trim();
+        for (int i = 0; i < qualifiedName.length(); i++) {
+          if (!isIdentifierChar(qualifiedName.charAt(i))) {
+            throw new IllegalArgumentException(
+                String.format(
+                    "invalid qualified name: %s, wanted name of the form 'qualified.name'",
+                    qualifiedName));
+          }
+        }
+
+        int index = qualifiedName.lastIndexOf(".");
+        if (index <= 0 || index >= qualifiedName.length() - 1) {
+          throw new IllegalArgumentException(
+              String.format(
+                  "invalid qualified name: %s, wanted name of the form 'qualified.name'",
+                  qualifiedName));
+        }
+
+        String alias = qualifiedName.substring(index + 1);
+        aliasAs(AliasKind.ABBREVIATION, qualifiedName, alias);
+      }
+
+      return this;
+    }
+
+    /**
+     * Alias associates a fully-qualified name with a user-defined alias.
+     *
+     * <p>In general, {@link #addAbbreviations} is preferred to aliasing since the names generated
+     * from the Abbrevs option are more easily traced back to source code. Aliasing is useful for
+     * propagating alias configuration from one container instance to another, and may also be
+     * useful for remapping poorly chosen protobuf message / package names.
+     *
+     * <p>Note: all the rules that apply to abbreviations also apply to aliasing.
+     */
     @CanIgnoreReturnValue
     public Builder addAlias(String alias, String qualifiedName) {
-      validateAliasOrThrow("alias", qualifiedName, alias);
-      aliases.put(alias, qualifiedName);
+      aliasAs(AliasKind.ALIAS, qualifiedName, alias);
       return this;
     }
 
-    private void validateAliasOrThrow(String kind, String qualifiedName, String alias) {
+    private void aliasAs(AliasKind kind, String qualifiedName, String alias) {
+      validateAliasOrThrow(kind, qualifiedName, alias);
+      aliases.put(alias, qualifiedName);
+    }
+
+    private void validateAliasOrThrow(AliasKind kind, String qualifiedName, String alias) {
       if (alias.isEmpty() || alias.contains(".")) {
         throw new IllegalArgumentException(
             String.format(
@@ -76,6 +183,14 @@ private void validateAliasOrThrow(String kind, String qualifiedName, String alia
                 "%s collides with existing reference: name=%s, %s=%s, existing=%s",
                 kind, qualifiedName, kind, alias, aliasRef));
       }
+
+      String containerName = name();
+      if (containerName.startsWith(alias + ".") || containerName.equals(alias)) {
+        throw new IllegalArgumentException(
+            String.format(
+                "%s collides with container name: name=%s, %s=%s, container=%s",
+                kind, qualifiedName, kind, alias, containerName));
+      }
     }
 
     abstract CelContainer autoBuild();
@@ -135,6 +250,14 @@ public ImmutableSet<String> resolveCandidateNames(String typeName) {
     return candidates.add(typeName).build();
   }
 
+  public static Builder newBuilder() {
+    return new AutoValue_CelContainer.Builder().setName("");
+  }
+
+  public static CelContainer ofName(String containerName) {
+    return newBuilder().setName(containerName).build();
+  }
+
   private Optional<String> findAlias(String name) {
     // If an alias exists for the name, ensure it is searched last.
     String simple = name;
@@ -152,11 +275,22 @@ private Optional<String> findAlias(String name) {
     return Optional.of(alias + qualifier);
   }
 
-  public static Builder newBuilder() {
-    return new AutoValue_CelContainer.Builder().setName("");
+  private static boolean isIdentifierChar(int r) {
+    if (r > 127) {
+      // Not ASCII
+      return false;
+    }
+
+    return r == '.' || r == '_' || Character.isLetter(r) || Character.isDigit(r);
   }
 
-  public static CelContainer ofName(String containerName) {
-    return newBuilder().setName(containerName).build();
+  private enum AliasKind {
+    ALIAS,
+    ABBREVIATION;
+
+    @Override
+    public String toString() {
+      return this.name().toLowerCase(Locale.getDefault());
+    }
   }
 }
diff --git a/common/src/test/java/dev/cel/common/CelContainerTest.java b/common/src/test/java/dev/cel/common/CelContainerTest.java
@@ -66,7 +66,30 @@ public void resolveCandidateName_withAlias_resolvesSingleCandidate(
   }
 
   @Test
-  public void containerBuilder_aliasCollides_throws() {
+  @TestParameters("{typeName: R, resolved: my.alias.R}")
+  @TestParameters("{typeName: R.S.T, resolved: my.alias.R.S.T}")
+  public void resolveCandidateName_withMatchingAbbreviation_resolvesSingleCandidate(
+      String typeName, String resolved) {
+    CelContainer container =
+        CelContainer.newBuilder().setName("a.b.c").addAbbreviations("my.alias.R").build();
+
+    ImmutableSet<String> resolvedNames = container.resolveCandidateNames(typeName);
+
+    assertThat(resolvedNames).containsExactly(resolved);
+  }
+
+  @Test
+  public void resolveCandidateName_withUnmatchedAbbreviation_resolvesMultipleCandidates() {
+    CelContainer container =
+        CelContainer.newBuilder().setName("a.b.c").addAbbreviations("my.alias.R").build();
+
+    ImmutableSet<String> resolvedNames = container.resolveCandidateNames("S");
+
+    assertThat(resolvedNames).containsExactly("a.b.c.S", "a.b.S", "a.S", "S").inOrder();
+  }
+
+  @Test
+  public void containerBuilder_duplicateAliases_throws() {
     IllegalArgumentException e =
         assertThrows(
             IllegalArgumentException.class,
@@ -76,6 +99,17 @@ public void containerBuilder_aliasCollides_throws() {
         .contains("alias collides with existing reference: name=b.c, alias=foo, existing=a.b");
   }
 
+  @Test
+  public void containerBuilder_aliasCollidesWithContainer_throws() {
+    IllegalArgumentException e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> CelContainer.newBuilder().setName("foo").addAlias("foo", "a.b"));
+    assertThat(e)
+        .hasMessageThat()
+        .contains("alias collides with container name: name=a.b, alias=foo, container=foo");
+  }
+
   @Test
   public void containerBuilder_addAliasError_throws(@TestParameter AliasingErrorTestCase testCase) {
     IllegalArgumentException e =
@@ -106,4 +140,62 @@ private enum AliasingErrorTestCase {
       this.errorMessage = errorMessage;
     }
   }
+
+  @Test
+  public void containerBuilder_addAbbreviationsError_throws(
+      @TestParameter AbbreviationErrorTestCase testCase) {
+    IllegalArgumentException e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> CelContainer.newBuilder().addAbbreviations(testCase.qualifiedNames));
+    assertThat(e).hasMessageThat().contains(testCase.errorMessage);
+  }
+
+  private enum AbbreviationErrorTestCase {
+    ABBREVIATION_COLLISION(
+        ImmutableSet.of("my.alias.R", "yer.other.R"),
+        "abbreviation collides with existing reference: name=yer.other.R, abbreviation=R,"
+            + " existing=my.alias.R"),
+    INVALID_DOT_PREFIX(
+        ".bad", "invalid qualified name: .bad, wanted name of the form 'qualified.name'"),
+    INVALID_DOT_SUFFIX(
+        "bad.alias.",
+        "invalid qualified name: bad.alias., wanted name of the form 'qualified.name'"),
+    NO_QUALIFIER(
+        "  bad_alias1  ",
+        "invalid qualified name: bad_alias1, wanted name of the form 'qualified.name'"),
+    INVALID_IDENTIFIER(
+        "  bad.alias!",
+        "invalid qualified name: bad.alias!, wanted name of the form 'qualified.name'"),
+    ;
+
+    private final ImmutableSet<String> qualifiedNames;
+    private final String errorMessage;
+
+    AbbreviationErrorTestCase(String qualifiedNames, String errorMessage) {
+      this(ImmutableSet.of(qualifiedNames), errorMessage);
+    }
+
+    AbbreviationErrorTestCase(ImmutableSet<String> qualifiedNames, String errorMessage) {
+      this.qualifiedNames = qualifiedNames;
+      this.errorMessage = errorMessage;
+    }
+  }
+
+  @Test
+  public void containerBuilder_addAbbreviationsCollidesWithContainer_throws() {
+    IllegalArgumentException e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                CelContainer.newBuilder()
+                    .setName("a.b.c.M.N")
+                    .addAbbreviations("my.alias.a", "yer.other.b"));
+
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "abbreviation collides with container name: name=my.alias.a, abbreviation=a,"
+                + " container=a.b.c.M.N");
+  }
 }
diff --git a/runtime/src/test/java/dev/cel/runtime/BUILD.bazel b/runtime/src/test/java/dev/cel/runtime/BUILD.bazel
@@ -173,6 +173,7 @@ java_library(
         "//extensions:optional_library",
         "//runtime",
         "//testing:base_interpreter_test",
+        "//testing/protos:test_all_types_cel_java_proto2",
         "//testing/protos:test_all_types_cel_java_proto3",
         "@maven//:com_google_testparameterinjector_test_parameter_injector",
         "@maven//:junit_junit",
diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteInterpreterTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteInterpreterTest.java
@@ -32,6 +32,7 @@ public CelLiteInterpreterTest() {
         CelRuntimeFactory.standardCelRuntimeBuilder()
             .setValueProvider(
                 ProtoMessageLiteValueProvider.newInstance(
+                    dev.cel.expr.conformance.proto2.TestAllTypesCelDescriptor.getDescriptor(),
                     TestAllTypesCelDescriptor.getDescriptor()))
             .addLibraries(CelOptionalLibrary.INSTANCE)
             .setOptions(newBaseCelOptions().toBuilder().enableCelValue(true).build())
diff --git a/runtime/src/test/resources/containers.baseline b/runtime/src/test/resources/containers.baseline
@@ -2,3 +2,18 @@ Source: test_alias.TestAllTypes{} == cel.expr.conformance.proto3.TestAllTypes{}
 =====>
 bindings: {}
 result:   true
+
+Source: proto2.TestAllTypes{} == cel.expr.conformance.proto2.TestAllTypes{}
+=====>
+bindings: {}
+result:   true
+
+Source: proto3.TestAllTypes{} == cel.expr.conformance.proto3.TestAllTypes{}
+=====>
+bindings: {}
+result:   true
+
+Source: SGAR
+=====>
+bindings: {}
+result:   1
diff --git a/testing/src/main/java/dev/cel/testing/BUILD.bazel b/testing/src/main/java/dev/cel/testing/BUILD.bazel
@@ -90,6 +90,7 @@ java_library(
         "//runtime:late_function_binding",
         "//runtime:unknown_attributes",
         "@cel_spec//proto/cel/expr:checked_java_proto",
+        "@cel_spec//proto/cel/expr/conformance/proto2:test_all_types_java_proto",
         "@cel_spec//proto/cel/expr/conformance/proto3:test_all_types_java_proto",
         "@maven//:com_google_errorprone_error_prone_annotations",
         "@maven//:com_google_guava_guava",
diff --git a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java
