CEL spec 0.25.2 conformance

README.md

@@ -22,6 +22,7 @@ The CEL specification can be found [here](https://github.com/google/cel-spec).
   - [Motivation](#motivation)
   - [Arbitrary Java classes](#arbitrary-java-classes)
   - [Unsigned 64-bit `uint`](#unsigned-64-bit-uint)
+  - [Protobuf enum semantics](#protobuf-enum-semantics)
   - [Native image and package verification](#native-image-and-package-verification)
   - [Not yet implemented](#not-yet-implemented)
   - [Unclear double-to-int rounding behavior](#unclear-double-to-int-rounding-behavior)
@@ -394,6 +395,24 @@ but `123u == 123u` and `123 == 123` are.
 If you have a `uint32` or `uint64` in protobuf objects, or use `uint`s in CEL expressions, wrap those
 values with `org.projectnessie.cel.common.ULong`.
 
+### Protobuf enum semantics
+
+CEL-Java follows the CEL-Spec v0.25.2 language definition for protobuf enum values: protobuf enum
+constants and enum fields are represented as CEL `int` values.
+
+The upstream CEL-Spec conformance testdata also contains strong-enum cases where enum values
+preserve their enum type. CEL-Java does not currently enable those strong-enum conformance cases.
+They are not part of the v0.25.2 language-definition baseline and are mutually incompatible with the
+legacy enum-as-int conformance sections in a single-mode runtime.
+
+Use numeric enum values directly, or use `int(...)` when writing expressions that should remain clear
+if strong enum support is added in the future:
+
+```cel
+TestAllTypes.NestedEnum.BAR == 1
+int(TestAllTypes.NestedEnum.BAR) == 1
+```
+
 ### Native image and package verification
 
 Native-image and package behavior must be verified in the consuming application's exact build.

core/src/main/java/org/projectnessie/cel/checker/Checker.java

@@ -48,8 +48,10 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.projectnessie.cel.checker.Types.Kind;
 import org.projectnessie.cel.common.Location;
 import org.projectnessie.cel.common.Source;
@@ -228,9 +230,11 @@ void checkIdent(Expr.Builder e) {
     Decl ident = env.lookupIdent(identExpr.getName());
     if (ident != null) {
       setType(e, ident.getIdent().getType());
-      setReference(e, newIdentReference(ident.getName(), ident.getIdent().getValue()));
+      String identName =
+          identExpr.getName().startsWith(".") ? "." + ident.getName() : ident.getName();
+      setReference(e, newIdentReference(identName, ident.getIdent().getValue()));
       // Overwrite the identifier with its fully qualified name.
-      identExpr.setName(ident.getName());
+      identExpr.setName(identName);
       return;
     }
 
@@ -242,7 +246,7 @@ void checkSelect(Expr.Builder e) {
     Select.Builder sel = e.getSelectExprBuilder();
     // Before traversing down the tree, try to interpret as qualified name.
     String qname = Container.toQualifiedName(e.build());
-    if (qname != null) {
+    if (qname != null && !isQualifiedLocalVariableSelection(sel.getOperandBuilder())) {
       Decl ident = env.lookupIdent(qname);
       if (ident != null) {
         if (sel.getTestOnly()) {
@@ -305,6 +309,16 @@ void checkSelect(Expr.Builder e) {
     setType(e, resultType);
   }
 
+  private boolean isQualifiedLocalVariableSelection(Expr.Builder e) {
+    if (e.getExprKindCase() == Expr.ExprKindCase.IDENT_EXPR) {
+      return env.hasLocalIdent(e.getIdentExpr().getName());
+    }
+    if (e.getExprKindCase() == Expr.ExprKindCase.SELECT_EXPR) {
+      return isQualifiedLocalVariableSelection(e.getSelectExprBuilder().getOperandBuilder());
+    }
+    return false;
+  }
+
   void checkCall(Expr.Builder e) {
     // Note: similar logic exists within the `interpreter/planner.go`. If making changes here
     // please consider the impact on planner.go and consolidate implementations or mirror code
@@ -415,10 +429,11 @@ OverloadResolution resolveOverload(
       }
 
       Type overloadType = Decls.newFunctionType(overload.getResultType(), overload.getParamsList());
-      if (overload.getTypeParamsCount() > 0) {
+      Set<String> typeParams = collectOverloadTypeParams(overload);
+      if (!typeParams.isEmpty()) {
         // Instantiate overload's type with fresh type variables.
         Mapping substitutions = newMapping();
-        for (String typePar : overload.getTypeParamsList()) {
+        for (String typePar : typeParams) {
           substitutions.add(Decls.newTypeParamType(typePar), newTypeVar());
         }
         overloadType = substitute(substitutions, overloadType, false);
@@ -551,14 +566,23 @@ void checkComprehension(Expr.Builder e) {
     Type accuType = getType(comp.getAccuInitBuilder());
     Type rangeType = getType(comp.getIterRangeBuilder());
     Type varType;
+    Type var2Type = null;
 
     switch (kindOf(rangeType)) {
       case kindList:
-        varType = rangeType.getListType().getElemType();
+        if (comp.getIterVar2().isEmpty()) {
+          varType = rangeType.getListType().getElemType();
+        } else {
+          varType = Decls.Int;
+          var2Type = rangeType.getListType().getElemType();
+        }
         break;
       case kindMap:
         // Ranges over the keys.
         varType = rangeType.getMapType().getKeyType();
+        if (!comp.getIterVar2().isEmpty()) {
+          var2Type = rangeType.getMapType().getValueType();
+        }
         break;
       case kindDyn:
       case kindError:
@@ -569,10 +593,16 @@ void checkComprehension(Expr.Builder e) {
         isAssignable(Decls.Dyn, rangeType);
         // Set the range iteration variable to type DYN as well.
         varType = Decls.Dyn;
+        if (!comp.getIterVar2().isEmpty()) {
+          var2Type = Decls.Dyn;
+        }
         break;
       default:
         errors.notAComprehensionRange(location(comp.getIterRangeBuilder()), rangeType);
         varType = Decls.Error;
+        if (!comp.getIterVar2().isEmpty()) {
+          var2Type = Decls.Error;
+        }
         break;
     }
 
@@ -583,6 +613,9 @@ void checkComprehension(Expr.Builder e) {
     // Create a block scope for the loop.
     env = env.enterScope();
     env.add(Decls.newVar(comp.getIterVar(), varType));
+    if (!comp.getIterVar2().isEmpty()) {
+      env.add(Decls.newVar(comp.getIterVar2(), var2Type));
+    }
     // Check the variable references in the condition and step.
     check(comp.getLoopConditionBuilder());
     assertType(comp.getLoopConditionBuilder(), Decls.Bool);
@@ -682,7 +715,7 @@ void setType(Expr.Builder e, Type t) {
   }
 
   Type getType(Expr.Builder e) {
-    return types.get(e.getId());
+    return substitute(mappings, types.get(e.getId()), false);
   }
 
   void setReference(Expr.Builder e, Reference r) {
@@ -716,6 +749,44 @@ static OverloadResolution newResolution(Reference checkedRef, Type t) {
     return new OverloadResolution(checkedRef, t);
   }
 
+  private static Set<String> collectOverloadTypeParams(Overload overload) {
+    Set<String> typeParams = new LinkedHashSet<>(overload.getTypeParamsList());
+    overload.getParamsList().forEach(type -> collectTypeParams(type, typeParams));
+    collectTypeParams(overload.getResultType(), typeParams);
+    return typeParams;
+  }
+
+  private static void collectTypeParams(Type type, Set<String> typeParams) {
+    switch (kindOf(type)) {
+      case kindTypeParam:
+        typeParams.add(type.getTypeParam());
+        return;
+      case kindAbstract:
+        type.getAbstractType()
+            .getParameterTypesList()
+            .forEach(t -> collectTypeParams(t, typeParams));
+        return;
+      case kindFunction:
+        type.getFunction().getArgTypesList().forEach(t -> collectTypeParams(t, typeParams));
+        collectTypeParams(type.getFunction().getResultType(), typeParams);
+        return;
+      case kindList:
+        collectTypeParams(type.getListType().getElemType(), typeParams);
+        return;
+      case kindMap:
+        MapType mapType = type.getMapType();
+        collectTypeParams(mapType.getKeyType(), typeParams);
+        collectTypeParams(mapType.getValueType(), typeParams);
+        return;
+      case kindType:
+        if (type.getType() != Type.getDefaultInstance()) {
+          collectTypeParams(type.getType(), typeParams);
+        }
+        return;
+      default:
+    }
+  }
+
   Location location(Expr.Builder e) {
     return locationByID(e.getId());
   }

core/src/main/java/org/projectnessie/cel/checker/CheckerEnv.java

@@ -36,6 +36,7 @@
 import java.util.Collections;
 import java.util.List;
 import org.projectnessie.cel.common.containers.Container;
+import org.projectnessie.cel.common.types.ref.TypeEnum;
 import org.projectnessie.cel.common.types.ref.TypeProvider;
 import org.projectnessie.cel.common.types.ref.Val;
 import org.projectnessie.cel.parser.Macro;
@@ -118,6 +119,12 @@ public void add(List<Decl> decls) {
    * such identifier is found in the Env.
    */
   public Decl lookupIdent(String name) {
+    if (!name.startsWith(".") && hasLocalIdent(name)) {
+      Decl ident = declarations.findIdentInScope(name);
+      if (ident != null) {
+        return ident;
+      }
+    }
     for (String candidate : container.resolveCandidateNames(name)) {
       Decl ident = declarations.findIdent(candidate);
       if (ident != null) {
@@ -146,10 +153,25 @@ public Decl lookupIdent(String name) {
         declarations.addIdent(decl);
         return decl;
       }
+
+      Val identValue = provider.findIdent(candidate);
+      if (identValue != null && identValue.type().typeEnum() == TypeEnum.String) {
+        Decl decl =
+            Decls.newIdent(
+                candidate,
+                Decls.String,
+                Constant.newBuilder().setStringValue(identValue.value().toString()).build());
+        declarations.addIdent(decl);
+        return decl;
+      }
     }
     return null;
   }
 
+  boolean hasLocalIdent(String name) {
+    return declarations.hasParent() && declarations.findIdentInScope(name) != null;
+  }
+
   /**
    * LookupFunction returns a Decl proto for typeName as a function in env. Returns nil if no such
    * function is found in env.

core/src/main/java/org/projectnessie/cel/checker/Scopes.java

@@ -59,6 +59,10 @@ public Scopes pop() {
     return this;
   }
 
+  boolean hasParent() {
+    return parent != null;
+  }
+
   /**
    * AddIdent adds the ident Decl in the current scope. Note: If the name collides with an existing
    * identifier in the scope, the Decl is overwritten.

core/src/main/java/org/projectnessie/cel/checker/Types.java

@@ -337,6 +337,9 @@ static boolean internalIsAssignable(Mapping m, Type t1, Type t2) {
     if (isDynOrError(t1) || isDynOrError(t2)) {
       return true;
     }
+    if (kind2 == Kind.kindNull) {
+      return internalIsAssignableNull(t1);
+    }
 
     // Test for when the types do not need to agree, but are more specific than dyn.
     switch (kind1) {
@@ -491,6 +494,8 @@ static Kind kindOf(Type t) {
         return Kind.kindWrapper;
       case NULL:
         return Kind.kindNull;
+      case ABSTRACT_TYPE:
+        return Kind.kindAbstract;
       case TYPE:
         return Kind.kindType;
       case LIST_TYPE:
@@ -507,6 +512,24 @@ static Kind kindOf(Type t) {
 
   /** mostGeneral returns the more general of two types which are known to unify. */
   static Type mostGeneral(Type t1, Type t2) {
+    Kind kind1 = kindOf(t1);
+    Kind kind2 = kindOf(t2);
+    if (kind1 == Kind.kindNull && internalIsAssignableNull(t2)) {
+      return t2;
+    }
+    if (kind2 == Kind.kindNull && internalIsAssignableNull(t1)) {
+      return t1;
+    }
+    if (kind1 == Kind.kindPrimitive && kind2 == Kind.kindWrapper) {
+      if (t1.getPrimitive() == t2.getWrapper()) {
+        return t2;
+      }
+    }
+    if (kind1 == Kind.kindWrapper && kind2 == Kind.kindPrimitive) {
+      if (t1.getWrapper() == t2.getPrimitive()) {
+        return t1;
+      }
+    }
     if (isEqualOrLessSpecific(t1, t2)) {
       return t1;
     }

core/src/main/java/org/projectnessie/cel/common/types/IntT.java

@@ -139,6 +139,9 @@ public <T> T convertToNative(Class<T> typeDesc) {
       return (T) Int64Value.of(i);
     }
     if (typeDesc == Int32Value.class) {
+      if (i < Integer.MIN_VALUE || i > Integer.MAX_VALUE) {
+        Err.throwErrorAsIllegalStateException(rangeError(i, "Java int"));
+      }
       return (T) Int32Value.of((int) i);
     }
     if (typeDesc == Val.class || typeDesc == IntT.class) {

core/src/main/java/org/projectnessie/cel/common/types/MapT.java

@@ -20,7 +20,6 @@
 import static org.projectnessie.cel.common.types.Err.isError;
 import static org.projectnessie.cel.common.types.Err.newErr;
 import static org.projectnessie.cel.common.types.Err.newTypeConversionError;
-import static org.projectnessie.cel.common.types.StringT.StringType;
 import static org.projectnessie.cel.common.types.TypeT.TypeType;
 import static org.projectnessie.cel.common.types.Types.boolOf;
 
@@ -87,6 +86,18 @@ public static Val newMaybeWrappedMap(TypeAdapter adapter, Map<?, ?> value) {
     return newWrappedMap(adapter, newMap);
   }
 
+  public static boolean isSupportedLiteralKeyType(Val key) {
+    switch (key.type().typeEnum()) {
+      case Bool:
+      case Int:
+      case String:
+      case Uint:
+        return true;
+      default:
+        return false;
+    }
+  }
+
   @Override
   public Type type() {
     return MapType;
@@ -136,9 +147,12 @@ private Value toPbValue() {
     private Struct toPbStruct() {
       Struct.Builder struct = Struct.newBuilder();
       map.forEach(
-          (k, v) ->
-              struct.putFields(
-                  k.convertToType(StringType).value().toString(), v.convertToNative(Value.class)));
+          (k, v) -> {
+            if (k.type().typeEnum() != TypeEnum.String) {
+              throw new IllegalArgumentException("bad key type");
+            }
+            struct.putFields(k.value().toString(), v.convertToNative(Value.class));
+          });
       return struct.build();
     }
 

core/src/main/java/org/projectnessie/cel/common/types/StringT.java

@@ -128,11 +128,19 @@ public Val convertToType(Type typeVal) {
         case Double:
           return doubleOf(Double.parseDouble(s));
         case Bool:
-          if ("true".equalsIgnoreCase(s)) {
-            return True;
-          }
-          if ("false".equalsIgnoreCase(s)) {
-            return False;
+          switch (s) {
+            case "1":
+            case "t":
+            case "true":
+            case "TRUE":
+            case "True":
+              return True;
+            case "0":
+            case "f":
+            case "false":
+            case "FALSE":
+            case "False":
+              return False;
           }
           break;
         case Bytes: