Skip to content

Commit 1ec8965

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
Change string.split to return an immutable list. Add parsed-only evaluation coverage to CelStringExtensions
PiperOrigin-RevId: 900202732
1 parent 2203ac8 commit 1ec8965

File tree

16 files changed

+559
-404
lines changed

16 files changed

+559
-404
lines changed

‎common/src/main/java/dev/cel/common/values/BUILD.bazel‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,38 @@ java_library(
118118
],
119119
)
120120

121+
java_library(
122+
name = "mutable_map_value",
123+
srcs = ["MutableMapValue.java"],
124+
tags = [
125+
],
126+
deps = [
127+
"//common/annotations",
128+
"//common/exceptions:attribute_not_found",
129+
"//common/types",
130+
"//common/types:type_providers",
131+
"//common/values",
132+
"//common/values:cel_value",
133+
"@maven//:com_google_errorprone_error_prone_annotations",
134+
],
135+
)
136+
137+
cel_android_library(
138+
name = "mutable_map_value_android",
139+
srcs = ["MutableMapValue.java"],
140+
tags = [
141+
],
142+
deps = [
143+
":cel_value_android",
144+
"//common/annotations",
145+
"//common/exceptions:attribute_not_found",
146+
"//common/types:type_providers_android",
147+
"//common/types:types_android",
148+
"//common/values:values_android",
149+
"@maven//:com_google_errorprone_error_prone_annotations",
150+
],
151+
)
152+
121153
cel_android_library(
122154
name = "values_android",
123155
srcs = CEL_VALUES_SOURCES,

‎common/src/main/java/dev/cel/common/values/MutableMapValue.java‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package dev.cel.common.values;
16+
17+
import com.google.errorprone.annotations.Immutable;
18+
import dev.cel.common.annotations.Internal;
19+
import dev.cel.common.exceptions.CelAttributeNotFoundException;
20+
import dev.cel.common.types.CelType;
21+
import dev.cel.common.types.MapType;
22+
import dev.cel.common.types.SimpleType;
23+
import java.util.Collection;
24+
import java.util.LinkedHashMap;
25+
import java.util.Map;
26+
import java.util.Optional;
27+
import java.util.Set;
28+
29+
/**
30+
* A custom CelValue implementation that allows O(1) insertions for maps during comprehension.
31+
*
32+
* <p>CEL Library Internals. Do Not Use.
33+
*/
34+
@Internal
35+
@Immutable
36+
@SuppressWarnings("Immutable") // Intentionally mutable for performance reasons
37+
public final class MutableMapValue extends CelValue
38+
implements SelectableValue<Object>, Map<Object, Object> {
39+
private final Map<Object, Object> internalMap;
40+
private final CelType celType;
41+
42+
public static MutableMapValue create(Map<?, ?> map) {
43+
return new MutableMapValue(map);
44+
}
45+
46+
@Override
47+
public int size() {
48+
return internalMap.size();
49+
}
50+
51+
@Override
52+
public boolean isEmpty() {
53+
return internalMap.isEmpty();
54+
}
55+
56+
@Override
57+
public boolean containsKey(Object key) {
58+
return internalMap.containsKey(key);
59+
}
60+
61+
@Override
62+
public boolean containsValue(Object value) {
63+
return internalMap.containsValue(value);
64+
}
65+
66+
@Override
67+
public Object get(Object key) {
68+
return internalMap.get(key);
69+
}
70+
71+
@Override
72+
public Object put(Object key, Object value) {
73+
return internalMap.put(key, value);
74+
}
75+
76+
@Override
77+
public Object remove(Object key) {
78+
return internalMap.remove(key);
79+
}
80+
81+
@Override
82+
public void putAll(Map<?, ?> m) {
83+
internalMap.putAll(m);
84+
}
85+
86+
@Override
87+
public void clear() {
88+
internalMap.clear();
89+
}
90+
91+
@Override
92+
public Set<Object> keySet() {
93+
return internalMap.keySet();
94+
}
95+
96+
@Override
97+
public Collection<Object> values() {
98+
return internalMap.values();
99+
}
100+
101+
@Override
102+
public Set<Entry<Object, Object>> entrySet() {
103+
return internalMap.entrySet();
104+
}
105+
106+
@Override
107+
public Object select(Object field) {
108+
Object val = internalMap.get(field);
109+
if (val != null) {
110+
return val;
111+
}
112+
if (!internalMap.containsKey(field)) {
113+
throw CelAttributeNotFoundException.forMissingMapKey(field.toString());
114+
}
115+
throw CelAttributeNotFoundException.of(
116+
String.format("Map value cannot be null for key: %s", field));
117+
}
118+
119+
@Override
120+
public Optional<?> find(Object field) {
121+
if (internalMap.containsKey(field)) {
122+
return Optional.ofNullable(internalMap.get(field));
123+
}
124+
return Optional.empty();
125+
}
126+
127+
@Override
128+
public Object value() {
129+
return this;
130+
}
131+
132+
@Override
133+
public boolean isZeroValue() {
134+
return internalMap.isEmpty();
135+
}
136+
137+
@Override
138+
public CelType celType() {
139+
return celType;
140+
}
141+
142+
private MutableMapValue(Map<?, ?> map) {
143+
this.internalMap = new LinkedHashMap<>(map);
144+
this.celType = MapType.create(SimpleType.DYN, SimpleType.DYN);
145+
}
146+
}

‎common/values/BUILD.bazel‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ cel_android_library(
4747
exports = ["//common/src/main/java/dev/cel/common/values:values_android"],
4848
)
4949

50+
java_library(
51+
name = "mutable_map_value",
52+
visibility = ["//:internal"],
53+
exports = ["//common/src/main/java/dev/cel/common/values:mutable_map_value"],
54+
)
55+
56+
cel_android_library(
57+
name = "mutable_map_value_android",
58+
visibility = ["//:internal"],
59+
exports = ["//common/src/main/java/dev/cel/common/values:mutable_map_value_android"],
60+
)
61+
5062
java_library(
5163
name = "base_proto_cel_value_converter",
5264
exports = ["//common/src/main/java/dev/cel/common/values:base_proto_cel_value_converter"],

‎extensions/src/main/java/dev/cel/extensions/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ java_library(
307307
"//common:options",
308308
"//common/ast",
309309
"//common/types",
310+
"//common/values:mutable_map_value",
310311
"//compiler:compiler_builder",
311312
"//extensions:extension_library",
312313
"//parser:macro",

‎extensions/src/main/java/dev/cel/extensions/CelComprehensionsExtensions.java‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import dev.cel.common.ast.CelExpr;
3030
import dev.cel.common.types.MapType;
3131
import dev.cel.common.types.TypeParamType;
32+
import dev.cel.common.values.MutableMapValue;
3233
import dev.cel.compiler.CelCompilerLibrary;
3334
import dev.cel.parser.CelMacro;
3435
import dev.cel.parser.CelMacroExprFactory;
@@ -171,38 +172,46 @@ public void setParserOptions(CelParserBuilder parserBuilder) {
171172
parserBuilder.addMacros(macros());
172173
}
173174

174-
// TODO: Implement a more efficient map insertion based on mutability once mutable
175-
// maps are supported in Java stack.
176-
private static ImmutableMap<Object, Object> mapInsertMap(
175+
private static Map<Object, Object> mapInsertMap(
177176
Map<?, ?> targetMap, Map<?, ?> mapToMerge, RuntimeEquality equality) {
178-
ImmutableMap.Builder<Object, Object> resultBuilder =
179-
ImmutableMap.builderWithExpectedSize(targetMap.size() + mapToMerge.size());
180-
181-
for (Map.Entry<?, ?> entry : mapToMerge.entrySet()) {
182-
if (equality.findInMap(targetMap, entry.getKey()).isPresent()) {
177+
for (Object key : mapToMerge.keySet()) {
178+
if (equality.findInMap(targetMap, key).isPresent()) {
183179
throw new IllegalArgumentException(
184-
String.format("insert failed: key '%s' already exists", entry.getKey()));
185-
} else {
186-
resultBuilder.put(entry.getKey(), entry.getValue());
180+
String.format("insert failed: key '%s' already exists", key));
187181
}
188182
}
189-
return resultBuilder.putAll(targetMap).buildOrThrow();
183+
184+
if (targetMap instanceof MutableMapValue) {
185+
MutableMapValue wrapper = (MutableMapValue) targetMap;
186+
wrapper.putAll(mapToMerge);
187+
return wrapper;
188+
}
189+
190+
return ImmutableMap.builderWithExpectedSize(targetMap.size() + mapToMerge.size())
191+
.putAll(targetMap)
192+
.putAll(mapToMerge)
193+
.buildOrThrow();
190194
}
191195

192-
private static ImmutableMap<Object, Object> mapInsertKeyValue(
193-
Object[] args, RuntimeEquality equality) {
194-
Map<?, ?> map = (Map<?, ?>) args[0];
196+
private static Map<Object, Object> mapInsertKeyValue(Object[] args, RuntimeEquality equality) {
197+
Map<?, ?> mapArg = (Map<?, ?>) args[0];
195198
Object key = args[1];
196199
Object value = args[2];
197200

198-
if (equality.findInMap(map, key).isPresent()) {
201+
if (equality.findInMap(mapArg, key).isPresent()) {
199202
throw new IllegalArgumentException(
200203
String.format("insert failed: key '%s' already exists", key));
201204
}
202205

206+
if (mapArg instanceof MutableMapValue) {
207+
MutableMapValue mutableMap = (MutableMapValue) mapArg;
208+
mutableMap.put(key, value);
209+
return mutableMap;
210+
}
211+
203212
ImmutableMap.Builder<Object, Object> builder =
204-
ImmutableMap.builderWithExpectedSize(map.size() + 1);
205-
return builder.put(key, value).putAll(map).buildOrThrow();
213+
ImmutableMap.builderWithExpectedSize(mapArg.size() + 1);
214+
return builder.put(key, value).putAll(mapArg).buildOrThrow();
206215
}
207216

208217
private static Optional<CelExpr> expandAllMacro(

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

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import com.google.common.base.Splitter;
2424
import com.google.common.collect.ImmutableList;
2525
import com.google.common.collect.ImmutableSet;
26-
import com.google.common.collect.Lists;
2726
import com.google.errorprone.annotations.Immutable;
2827
import dev.cel.checker.CelCheckerBuilder;
2928
import dev.cel.common.CelFunctionDecl;
@@ -37,7 +36,6 @@
3736
import dev.cel.runtime.CelFunctionBinding;
3837
import dev.cel.runtime.CelRuntimeBuilder;
3938
import dev.cel.runtime.CelRuntimeLibrary;
40-
import java.util.ArrayList;
4139
import java.util.List;
4240
import java.util.Set;
4341

@@ -587,14 +585,14 @@ private static String reverse(String s) {
587585
return new StringBuilder(s).reverse().toString();
588586
}
589587

590-
private static List<String> split(String str, String separator) {
588+
private static ImmutableList<String> split(String str, String separator) {
591589
return split(str, separator, Integer.MAX_VALUE);
592590
}
593591

594592
/**
595593
* @param args Object array with indices of: [0: string], [1: separator], [2: limit]
596594
*/
597-
private static List<String> split(Object[] args) throws CelEvaluationException {
595+
private static ImmutableList<String> split(Object[] args) throws CelEvaluationException {
598596
long limitInLong = (Long) args[2];
599597
int limit;
600598
try {
@@ -609,16 +607,14 @@ private static List<String> split(Object[] args) throws CelEvaluationException {
609607
return split((String) args[0], (String) args[1], limit);
610608
}
611609

612-
/** Returns a **mutable** list of strings split on the separator */
613-
private static List<String> split(String str, String separator, int limit) {
610+
/** Returns an immutable list of strings split on the separator */
611+
private static ImmutableList<String> split(String str, String separator, int limit) {
614612
if (limit == 0) {
615-
return new ArrayList<>();
613+
return ImmutableList.of();
616614
}
617615

618616
if (limit == 1) {
619-
List<String> singleElementList = new ArrayList<>();
620-
singleElementList.add(str);
621-
return singleElementList;
617+
return ImmutableList.of(str);
622618
}
623619

624620
if (limit < 0) {
@@ -630,7 +626,7 @@ private static List<String> split(String str, String separator, int limit) {
630626
}
631627

632628
Iterable<String> splitString = Splitter.on(separator).limit(limit).split(str);
633-
return Lists.newArrayList(splitString);
629+
return ImmutableList.copyOf(splitString);
634630
}
635631

636632
/**
@@ -643,8 +639,8 @@ private static List<String> split(String str, String separator, int limit) {
643639
* <p>This exists because neither the built-in String.split nor Guava's splitter is able to deal
644640
* with separating single printable characters.
645641
*/
646-
private static List<String> explode(String str, int limit) {
647-
List<String> exploded = new ArrayList<>();
642+
private static ImmutableList<String> explode(String str, int limit) {
643+
ImmutableList.Builder<String> exploded = ImmutableList.builder();
648644
CelCodePointArray codePointArray = CelCodePointArray.fromString(str);
649645
if (limit > 0) {
650646
limit -= 1;
@@ -656,7 +652,7 @@ private static List<String> explode(String str, int limit) {
656652
if (codePointArray.length() > limit) {
657653
exploded.add(codePointArray.slice(limit, codePointArray.length()).toString());
658654
}
659-
return exploded;
655+
return exploded.build();
660656
}
661657

662658
private static Object substring(String s, long i) throws CelEvaluationException {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ Examples:
522522

523523
### Split
524524

525-
Returns a mutable list of strings split from the input by the given separator. The
525+
Returns a list of strings split from the input by the given separator. The
526526
function accepts an optional argument specifying a limit on the number of
527527
substrings produced by the split.
528528

@@ -1069,4 +1069,4 @@ Examples:
10691069
{valueVar: indexVar}) // returns {1:0, 2:1, 3:2}
10701070

10711071
{'greeting': 'aloha', 'farewell': 'aloha'}
1072-
.transformMapEntry(k, v, {v: k}) // error, duplicate key
1072+
.transformMapEntry(k, v, {v: k}) // error, duplicate key

‎extensions/src/test/java/dev/cel/extensions/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ java_library(
1515
"//common:compiler_common",
1616
"//common:container",
1717
"//common:options",
18+
"//common/exceptions:attribute_not_found",
1819
"//common/exceptions:divide_by_zero",
1920
"//common/exceptions:index_out_of_bounds",
2021
"//common/types",

0 commit comments

Comments
 (0)