Skip to content

Commit 5f465e7

Browse files
CEL Dev Teamcopybara-github
authored andcommitted
Port CEL-Go comprehension loop variable shadowing to CEL-Java
PiperOrigin-RevId: 944969366
1 parent 7d6d21e commit 5f465e7

2 files changed

Lines changed: 50 additions & 4 deletions

File tree

optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,37 @@ private static boolean areChildrenArgConstant(CelNavigableMutableExpr expr) {
276276
return false;
277277
}
278278

279+
/**
280+
* Checks whether the provided identifier node is a loop variable bound by a surrounding
281+
* comprehension. Identifiers existing strictly within loop initialization bounds (e.g. iterRange)
282+
* are not considered loop variables.
283+
*/
284+
private static boolean isComprehensionBoundLoopVariable(CelNavigableMutableExpr identNode) {
285+
CelNavigableMutableExpr child = identNode;
286+
CelNavigableMutableExpr parent = identNode.parent().orElse(null);
287+
288+
while (parent != null) {
289+
if (parent.getKind().equals(Kind.COMPREHENSION)) {
290+
CelMutableComprehension comprehension = parent.expr().comprehension();
291+
boolean isInitializationNode =
292+
child.expr().equals(comprehension.iterRange())
293+
|| child.expr().equals(comprehension.accuInit());
294+
295+
if (!isInitializationNode) {
296+
String identName = identNode.expr().ident().name();
297+
if (identName.equals(comprehension.accuVar())
298+
|| identName.equals(comprehension.iterVar())
299+
|| identName.equals(comprehension.iterVar2())) {
300+
return true;
301+
}
302+
}
303+
}
304+
child = parent;
305+
parent = parent.parent().orElse(null);
306+
}
307+
return false;
308+
}
309+
279310
private static boolean isNestedComprehension(CelNavigableMutableExpr expr) {
280311
Optional<CelNavigableMutableExpr> maybeParent = expr.parent();
281312
while (maybeParent.isPresent()) {
@@ -733,6 +764,7 @@ private static Object evaluateExpr(Cel cel, CelNavigableMutableExpr navigableMut
733764
navigableMutableExpr
734765
.allNodes()
735766
.filter(node -> node.getKind().equals(Kind.IDENT))
767+
.filter(node -> !isComprehensionBoundLoopVariable(node))
736768
.map(node -> node.expr().ident().name())
737769
.map(CelAttributePattern::fromQualifiedIdentifier)
738770
.collect(toImmutableList());

optimizer/src/test/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizerTest.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ private static Cel setupEnv(CelBuilder celBuilder) {
9494
.setContainer(CelContainer.ofName("cel.expr.conformance.proto3"))
9595
.setOptions(CEL_OPTIONS)
9696
.addCompilerLibraries(
97+
CelExtensions.comprehensions(),
9798
CelExtensions.bindings(),
9899
CelOptionalLibrary.INSTANCE,
99100
CelExtensions.math(CEL_OPTIONS),
@@ -228,6 +229,22 @@ private static Cel setupEnv(CelBuilder celBuilder) {
228229
@TestParameters("{source: 'true == false', expected: 'false'}")
229230
@TestParameters("{source: 'true == true', expected: 'true'}")
230231
@TestParameters("{source: 'false == true', expected: 'false'}")
232+
@TestParameters("{source: '[1, 2, 3].map(item, item * 2)', expected: '[2, 4, 6]'}")
233+
@TestParameters("{source: '[1, 2, 3].filter(item, item > 1)', expected: '[2, 3]'}")
234+
@TestParameters("{source: '[1, 2, 3].exists(item, item > 1)', expected: 'true'}")
235+
@TestParameters("{source: '[1, 2, 3].all(item, item > 1)', expected: 'false'}")
236+
@TestParameters("{source: '{\"a\": 1}.all(k, v, k == \"a\" || v > 0)', expected: 'true'}")
237+
@TestParameters(
238+
"{source: '{\"a\": 1, \"b\": x}.all(k, v, k == \"a\" || v > 0)', expected: '{\"a\": 1, \"b\":"
239+
+ " x}.all(k, v, k == \"a\" || v > 0)'}")
240+
@TestParameters("{source: '[1, 2].map(x, [3, 4].map(y, x + y))', expected: '[[4, 5], [5, 6]]'}")
241+
@TestParameters(
242+
"{source: '[1, 2, x].map(item, item * 2)', expected: '[1, 2, x].map(item, item * 2)'}")
243+
@TestParameters("{source: '[[1, 2], [3, 4]].map(x, x[0] == 1)', expected: '[true, false]'}")
244+
@TestParameters("{source: '[[1, 2]].map(list_var, list_var[0])', expected: '[1]'}")
245+
@TestParameters("{source: '[{\"a\": 1}].map(x, x.a == 1)', expected: '[true]'}")
246+
@TestParameters(
247+
"{source: '[1, 2, x].map(item, item * 2)', expected: '[1, 2, x].map(item, item * 2)'}")
231248
@TestParameters("{source: 'false == false', expected: 'true'}")
232249
@TestParameters("{source: '10 == 42', expected: 'false'}")
233250
@TestParameters("{source: '42 == 42', expected: 'true'}")
@@ -561,10 +578,7 @@ public void iterationLimitReached_throws() throws Exception {
561578
Cel cel =
562579
runtimeFlavor
563580
.builder()
564-
.setOptions(
565-
CelOptions.current()
566-
.enableHeterogeneousNumericComparisons(true)
567-
.build())
581+
.setOptions(CelOptions.current().enableHeterogeneousNumericComparisons(true).build())
568582
.build();
569583
CelAbstractSyntaxTree ast = cel.compile("1 + 1").getAst();
570584
CelOptimizer optimizer =

0 commit comments

Comments
 (0)