Skip to content

Commit d97307c

Browse files
l46kokcopybara-github
authored andcommitted
Fix two-variable comprehension cel.@mapInsert overload IDs
PiperOrigin-RevId: 999641973
1 parent ea782a3 commit d97307c

6 files changed

Lines changed: 54 additions & 8 deletions

File tree

‎checker/src/test/resources/standardEnvDump.baseline‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ Source: 'redundant expression so the env is constructed and can be printed'
55

66
Standard environment:
77
declare cel.@mapInsert {
8-
function cel_@mapInsert_map_map (map(K, V), map(K, V)) -> map(K, V)
9-
function cel_@mapInsert_map_key_value (map(K, V), K, V) -> map(K, V)
8+
function @mapInsert_map_map (map(K, V), map(K, V)) -> map(K, V)
9+
function @mapInsert_map_key_value (map(K, V), K, V) -> map(K, V)
1010
}
1111
declare !_ {
1212
function logical_not (bool) -> bool

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ public final class CelComprehensionsCompilerLibrary
4242
implements CelCompilerLibrary, CelExtensionLibrary.FeatureSet {
4343

4444
private static final String MAP_INSERT_FUNCTION = "cel.@mapInsert";
45-
private static final String MAP_INSERT_OVERLOAD_MAP_MAP = "cel_@mapInsert_map_map";
46-
private static final String MAP_INSERT_OVERLOAD_KEY_VALUE = "cel_@mapInsert_map_key_value";
45+
private static final String MAP_INSERT_OVERLOAD_MAP_MAP = "@mapInsert_map_map";
46+
private static final String MAP_INSERT_OVERLOAD_KEY_VALUE = "@mapInsert_map_key_value";
4747

4848
private static final class Types {
4949
private static final TypeParamType TYPE_PARAM_K = TypeParamType.create("K");

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@
3434
public final class CelComprehensionsRuntimeLibrary implements CelInternalLiteRuntimeLibrary {
3535

3636
private static final String MAP_INSERT_FUNCTION = "cel.@mapInsert";
37-
private static final String MAP_INSERT_OVERLOAD_MAP_MAP = "cel_@mapInsert_map_map";
38-
private static final String MAP_INSERT_OVERLOAD_KEY_VALUE = "cel_@mapInsert_map_key_value";
37+
private static final String MAP_INSERT_OVERLOAD_MAP_MAP = "@mapInsert_map_map";
38+
private static final String MAP_INSERT_OVERLOAD_KEY_VALUE = "@mapInsert_map_key_value";
39+
private static final String LEGACY_MAP_INSERT_OVERLOAD_MAP_MAP = "cel_@mapInsert_map_map";
40+
private static final String LEGACY_MAP_INSERT_OVERLOAD_KEY_VALUE = "cel_@mapInsert_map_key_value";
3941

4042
/** Enumeration of functions for Comprehensions runtime extension. */
4143
public enum Function {
@@ -126,6 +128,16 @@ public ImmutableList<CelFunctionBinding> newFunctionBindings(RuntimeEquality run
126128
CelFunctionBinding.from(
127129
MAP_INSERT_OVERLOAD_KEY_VALUE,
128130
ImmutableList.of(Map.class, Object.class, Object.class),
131+
args -> mapInsertKeyValue(args, runtimeEquality)),
132+
// Legacy overload IDs retained to support evaluation of cached ASTs (b/573697356).
133+
CelFunctionBinding.from(
134+
LEGACY_MAP_INSERT_OVERLOAD_MAP_MAP,
135+
Map.class,
136+
Map.class,
137+
(map1, map2) -> mapInsertMap(map1, map2, runtimeEquality)),
138+
CelFunctionBinding.from(
139+
LEGACY_MAP_INSERT_OVERLOAD_KEY_VALUE,
140+
ImmutableList.of(Map.class, Object.class, Object.class),
129141
args -> mapInsertKeyValue(args, runtimeEquality))));
130142
}
131143
return bindings.build();

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.common.base.Throwables;
2222
import com.google.common.collect.ImmutableMap;
2323
import com.google.common.collect.ImmutableSet;
24+
import com.google.common.collect.Maps;
2425
import com.google.testing.junit.testparameterinjector.TestParameter;
2526
import com.google.testing.junit.testparameterinjector.TestParameterInjector;
2627
import com.google.testing.junit.testparameterinjector.TestParameters;
@@ -31,6 +32,7 @@
3132
import dev.cel.common.CelOptions;
3233
import dev.cel.common.CelValidationException;
3334
import dev.cel.common.CelValidationResult;
35+
import dev.cel.common.ast.CelReference;
3436
import dev.cel.common.exceptions.CelAttributeNotFoundException;
3537
import dev.cel.common.exceptions.CelDivideByZeroException;
3638
import dev.cel.common.exceptions.CelIndexOutOfBoundsException;
@@ -552,4 +554,36 @@ public void runtimeOptions_unsupported_throws() {
552554
UnsupportedOperationException.class,
553555
() -> runtimeLibrary.setRuntimeOptions(liteRuntimeBuilder));
554556
}
557+
558+
@Test
559+
@TestParameters(
560+
"{expr: '[1, 2].transformMap(i, v, v + 1) == {0: 2, 1: 3}',"
561+
+ " canonicalOverloadId: '@mapInsert_map_key_value'}")
562+
@TestParameters(
563+
"{expr: \"{'a': 1, 'b': 2}.transformMapEntry(k, v, {k: v + 1}) == {'a': 2, 'b': 3}\","
564+
+ " canonicalOverloadId: '@mapInsert_map_map'}")
565+
public void evaluate_legacyMapInsertOverloadIds_success(String expr, String canonicalOverloadId)
566+
throws Exception {
567+
Assume.assumeFalse(isParseOnly);
568+
CelAbstractSyntaxTree ast = cel.compile(expr).getAst();
569+
CelAbstractSyntaxTree legacyAst =
570+
CelAbstractSyntaxTree.newCheckedAst(
571+
ast.getExpr(),
572+
ast.getSource(),
573+
Maps.transformValues(
574+
ast.getReferenceMap(),
575+
ref ->
576+
ref.overloadIds().contains(canonicalOverloadId)
577+
? CelReference.newBuilder()
578+
.addOverloadIds("cel_" + canonicalOverloadId)
579+
.build()
580+
: ref),
581+
ast.getTypeMap());
582+
583+
Object result = cel.createProgram(legacyAst).eval();
584+
585+
assertThat(ast.getReferenceMap().values())
586+
.contains(CelReference.newBuilder().addOverloadIds(canonicalOverloadId).build());
587+
assertThat(result).isEqualTo(true);
588+
}
555589
}

‎runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ private static DefaultDispatcher newDispatcher(CelOptions celOptions) {
255255
CelFunctionBinding.fromOverloads(
256256
"cel.@mapInsert",
257257
CelFunctionBinding.from(
258-
"cel_@mapInsert_map_key_value",
258+
"@mapInsert_map_key_value",
259259
ImmutableList.of(Map.class, Object.class, Object.class),
260260
args -> args[0])));
261261

‎verifier/src/main/java/dev/cel/verifier/axioms/MapInsertAxiom.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ final class MapInsertAxiom {
2929
static final CelZ3FunctionAxiom INSTANCE =
3030
CelZ3FunctionAxiom.newBuilder(CelComprehensionsExtensions.Function.MAP_INSERT.functionDecl())
3131
.addTernaryOverloadTranslator(
32-
"cel_@mapInsert_map_key_value",
32+
"@mapInsert_map_key_value",
3333
(ctx, typeSystem, constraintSink, mapVal, keyVal, valueVal) -> {
3434
BoolExpr isMap = typeSystem.isMap(mapVal);
3535
BoolExpr isValidKey =

0 commit comments

Comments
 (0)