Change string.split to return an immutable list. Add parsed-only evaluation coverage to CelStringExtensions

extensions/src/main/java/dev/cel/extensions/CelStringExtensions.java

@@ -23,7 +23,6 @@
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
 import com.google.errorprone.annotations.Immutable;
 import dev.cel.checker.CelCheckerBuilder;
 import dev.cel.common.CelFunctionDecl;
@@ -37,7 +36,6 @@
 import dev.cel.runtime.CelFunctionBinding;
 import dev.cel.runtime.CelRuntimeBuilder;
 import dev.cel.runtime.CelRuntimeLibrary;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
@@ -475,7 +473,7 @@ private static String quote(String s) {
     sb.append('"');
     for (int i = 0; i < s.length(); ) {
       int codePoint = s.codePointAt(i);
-      if (isMalformedUtf16(s, i, codePoint)) {
+      if (isMalformedUtf16(s, i)) {
         sb.append('\uFFFD');
         i++;
         continue;
@@ -518,7 +516,7 @@ private static String quote(String s) {
     return sb.toString();
   }
 
-  private static boolean isMalformedUtf16(String s, int index, int codePoint) {
+  private static boolean isMalformedUtf16(String s, int index) {
     char currentChar = s.charAt(index);
     if (Character.isLowSurrogate(currentChar)) {
       return true;
@@ -587,14 +585,14 @@ private static String reverse(String s) {
     return new StringBuilder(s).reverse().toString();
   }
 
-  private static List<String> split(String str, String separator) {
+  private static ImmutableList<String> split(String str, String separator) {
     return split(str, separator, Integer.MAX_VALUE);
   }
 
   /**
    * @param args Object array with indices of: [0: string], [1: separator], [2: limit]
    */
-  private static List<String> split(Object[] args) throws CelEvaluationException {
+  private static ImmutableList<String> split(Object[] args) throws CelEvaluationException {
     long limitInLong = (Long) args[2];
     int limit;
     try {
@@ -609,16 +607,14 @@ private static List<String> split(Object[] args) throws CelEvaluationException {
     return split((String) args[0], (String) args[1], limit);
   }
 
-  /** Returns a **mutable** list of strings split on the separator */
-  private static List<String> split(String str, String separator, int limit) {
+  /** Returns an immutable list of strings split on the separator */
+  private static ImmutableList<String> split(String str, String separator, int limit) {
     if (limit == 0) {
-      return new ArrayList<>();
+      return ImmutableList.of();
     }
 
     if (limit == 1) {
-      List<String> singleElementList = new ArrayList<>();
-      singleElementList.add(str);
-      return singleElementList;
+      return ImmutableList.of(str);
     }
 
     if (limit < 0) {
@@ -630,7 +626,7 @@ private static List<String> split(String str, String separator, int limit) {
     }
 
     Iterable<String> splitString = Splitter.on(separator).limit(limit).split(str);
-    return Lists.newArrayList(splitString);
+    return ImmutableList.copyOf(splitString);
   }
 
   /**
@@ -643,8 +639,8 @@ private static List<String> split(String str, String separator, int limit) {
    * <p>This exists because neither the built-in String.split nor Guava's splitter is able to deal
    * with separating single printable characters.
    */
-  private static List<String> explode(String str, int limit) {
-    List<String> exploded = new ArrayList<>();
+  private static ImmutableList<String> explode(String str, int limit) {
+    ImmutableList.Builder<String> exploded = ImmutableList.builder();
     CelCodePointArray codePointArray = CelCodePointArray.fromString(str);
     if (limit > 0) {
       limit -= 1;
@@ -656,7 +652,7 @@ private static List<String> explode(String str, int limit) {
     if (codePointArray.length() > limit) {
       exploded.add(codePointArray.slice(limit, codePointArray.length()).toString());
     }
-    return exploded;
+    return exploded.build();
   }
 
   private static Object substring(String s, long i) throws CelEvaluationException {

extensions/src/main/java/dev/cel/extensions/README.md

@@ -522,7 +522,7 @@ Examples:
 
 ### Split
 
-Returns a mutable list of strings split from the input by the given separator. The
+Returns a list of strings split from the input by the given separator. The
 function accepts an optional argument specifying a limit on the number of
 substrings produced by the split.
 
@@ -1069,4 +1069,4 @@ Examples:
       {valueVar: indexVar}) // returns {1:0, 2:1, 3:2}
 
     {'greeting': 'aloha', 'farewell': 'aloha'}
-      .transformMapEntry(k, v, {v: k}) // error, duplicate key
+      .transformMapEntry(k, v, {v: k}) // error, duplicate key

extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java

@@ -28,6 +28,7 @@
 import dev.cel.common.CelFunctionDecl;
 import dev.cel.common.CelOptions;
 import dev.cel.common.CelValidationException;
+import dev.cel.common.CelValidationResult;
 import dev.cel.common.exceptions.CelAttributeNotFoundException;
 import dev.cel.common.exceptions.CelDivideByZeroException;
 import dev.cel.common.exceptions.CelIndexOutOfBoundsException;
@@ -312,8 +313,8 @@ public void unparseAST_twoVarComprehension(
           + " err: 'no matching overload'}")
   public void twoVarComprehension_compilerErrors(String expr, String err) throws Exception {
     Assume.assumeFalse(isParseOnly);
-    CelValidationException e =
-        assertThrows(CelValidationException.class, () -> cel.compile(expr).getAst());
+    CelValidationResult result = cel.compile(expr);
+    CelValidationException e = assertThrows(CelValidationException.class, () -> result.getAst());
 
     assertThat(e).hasMessageThat().contains(err);
   }

extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java

@@ -27,6 +27,7 @@
 import dev.cel.common.CelAbstractSyntaxTree;
 import dev.cel.common.CelContainer;
 import dev.cel.common.CelValidationException;
+import dev.cel.common.CelValidationResult;
 import dev.cel.common.types.SimpleType;
 import dev.cel.expr.conformance.test.SimpleTest;
 import dev.cel.parser.CelStandardMacro;
@@ -300,10 +301,8 @@ public void sortBy_success(String expression, String expected) throws Exception
           + "expectedError: 'variable name must be a simple identifier'}")
   public void sortBy_throws_validationException(String expression, String expectedError)
       throws Exception {
-    assertThat(
-            assertThrows(
-                CelValidationException.class,
-                () -> cel.createProgram(cel.compile(expression).getAst()).eval()))
+    CelValidationResult result = cel.compile(expression);
+    assertThat(assertThrows(CelValidationException.class, () -> result.getAst()))
         .hasMessageThat()
         .contains(expectedError);
   }