diff --git a/common/navigation/BUILD.bazel b/common/navigation/BUILD.bazel index 0c03596f9..8da2514b8 100644 --- a/common/navigation/BUILD.bazel +++ b/common/navigation/BUILD.bazel @@ -25,3 +25,13 @@ java_library( name = "mutable_navigation", exports = ["//common/src/main/java/dev/cel/common/navigation:mutable_navigation"], ) + +java_library( + name = "expr_util", + exports = ["//common/src/main/java/dev/cel/common/navigation:expr_util"], +) + +cel_android_library( + name = "expr_util_android", + exports = ["//common/src/main/java/dev/cel/common/navigation:expr_util_android"], +) diff --git a/common/src/main/java/dev/cel/common/navigation/BUILD.bazel b/common/src/main/java/dev/cel/common/navigation/BUILD.bazel index 3c2eaad62..4ae2908bc 100644 --- a/common/src/main/java/dev/cel/common/navigation/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/navigation/BUILD.bazel @@ -48,6 +48,36 @@ cel_android_library( ], ) +java_library( + name = "expr_util", + srcs = [ + "CelNavigableExprUtil.java", + ], + tags = [ + ], + deps = [ + ":common", + "//common/ast", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven//:com_google_guava_guava", + ], +) + +cel_android_library( + name = "expr_util_android", + srcs = [ + "CelNavigableExprUtil.java", + ], + tags = [ + ], + deps = [ + ":common_android", + "//common/ast:ast_android", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", + ], +) + java_library( name = "navigation", srcs = [ diff --git a/common/src/main/java/dev/cel/common/navigation/BaseNavigableExpr.java b/common/src/main/java/dev/cel/common/navigation/BaseNavigableExpr.java index 1699b4a96..dabcac3a2 100644 --- a/common/src/main/java/dev/cel/common/navigation/BaseNavigableExpr.java +++ b/common/src/main/java/dev/cel/common/navigation/BaseNavigableExpr.java @@ -16,6 +16,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.CheckReturnValue; +import com.google.errorprone.annotations.DoNotMock; import dev.cel.common.ast.CelExpr; import dev.cel.common.ast.CelExpr.ExprKind; import dev.cel.common.ast.Expression; @@ -25,9 +26,15 @@ /** * BaseNavigableExpr represents the base navigable expression value with methods to inspect the * parent and child expressions. + * + *

This class is intentionally non-extensible outside of the {@code dev.cel.common.navigation} + * package. */ +@DoNotMock("Use CelNavigableExpr or CelNavigableMutableExpr") @SuppressWarnings("unchecked") // Generic types are properly bound to Expression -abstract class BaseNavigableExpr { +public abstract class BaseNavigableExpr { + + BaseNavigableExpr() {} public abstract E expr(); diff --git a/common/src/main/java/dev/cel/common/navigation/CelNavigableExprUtil.java b/common/src/main/java/dev/cel/common/navigation/CelNavigableExprUtil.java new file mode 100644 index 000000000..9229214eb --- /dev/null +++ b/common/src/main/java/dev/cel/common/navigation/CelNavigableExprUtil.java @@ -0,0 +1,156 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dev.cel.common.navigation; + +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.errorprone.annotations.CheckReturnValue; +import dev.cel.common.ast.CelExpr.ExprKind.Kind; +import dev.cel.common.ast.Expression; +import java.util.Collection; +import java.util.Collections; +import java.util.Optional; + +/** Utility class for common AST navigation and scoping inspections on {@link BaseNavigableExpr}. */ +@CheckReturnValue +public final class CelNavigableExprUtil { + + /** + * Returns true if {@code variableName} is in scope and shadowed by an enclosing comprehension + * above {@code expr}. + * + *

A variable is shadowed at {@code expr} if an ancestor comprehension declares it as an + * iteration variable ({@code iterVar}, {@code iterVar2}) or accumulator variable ({@code + * accuVar}) and {@code expr} resides within a branch where that variable is active: + * + *

+ * + *

For example, in the expression: + * + * 

{@code
+   * [1, 2].all(x, x > 0)
+   * }
+ * + * + */ + public static boolean isVariableShadowed(BaseNavigableExpr expr, String variableName) { + return areVariablesShadowed(expr, Collections.singleton(variableName)); + } + + /** + * Returns true if any of {@code variableNames} is in scope and shadowed by an enclosing + * comprehension above {@code expr}. + * + *

For example, in the nested comprehension expression: + * + * 

{@code
+   * [1, 2].all(x, [3, 4].all(y, x > 0 && y > 0))
+   * }
+ * + * At {@code y > 0}, {@code areVariablesShadowed(node, ImmutableSet.of("x", "z"))} is {@code true} + * because {@code x} is in scope from the outer comprehension. + */ + @SuppressWarnings("ReferenceEquality") // Required to disambiguate child branches + public static boolean areVariablesShadowed( + BaseNavigableExpr expr, Collection variableNames) { + checkNotNull(expr); + checkNotNull(variableNames); + if (variableNames.isEmpty()) { + return false; + } + BaseNavigableExpr curr = expr; + Optional> maybeParent = curr.parent(); + while (maybeParent.isPresent()) { + BaseNavigableExpr parent = maybeParent.get(); + if (parent.getKind() == Kind.COMPREHENSION) { + Expression.Comprehension comp = parent.expr().comprehension(); + Expression currExpr = curr.expr(); + + if (currExpr != comp.iterRange() && currExpr != comp.accuInit()) { + if (currExpr == comp.result()) { + if (variableNames.contains(comp.accuVar())) { + return true; + } + } else { + if (variableNames.contains(comp.iterVar()) + || variableNames.contains(comp.iterVar2()) + || variableNames.contains(comp.accuVar())) { + return true; + } + } + } + } + curr = parent; + maybeParent = parent.parent(); + } + return false; + } + + /** + * Returns true if {@code expr} is an {@code IDENT} node that references a variable declared by an + * enclosing comprehension. + * + *

For example, in the expression: + * + * 

{@code
+   * [a].all(x, x > a)
+   * }
+ * + * + */ + public static boolean isComprehensionVariable(BaseNavigableExpr expr) { + checkNotNull(expr); + return expr.getKind() == Kind.IDENT + && areVariablesShadowed(expr, Collections.singleton(expr.expr().ident().name())); + } + + /** + * Returns true if {@code expr} or any identifier within {@code expr} references a variable + * declared by an enclosing comprehension. + * + *

For example, in the expression: + * + * 

{@code
+   * [a].all(x, x > a)
+   * }
+ * + * + */ + public static boolean hasComprehensionVariable(BaseNavigableExpr expr) { + checkNotNull(expr); + return expr.allNodes() + .filter(node -> node.getKind() == Kind.IDENT) + .anyMatch(CelNavigableExprUtil::isComprehensionVariable); + } + + private CelNavigableExprUtil() {} +} diff --git a/common/src/test/java/dev/cel/common/navigation/BUILD.bazel b/common/src/test/java/dev/cel/common/navigation/BUILD.bazel index f8b2b988b..0a29dfe8a 100644 --- a/common/src/test/java/dev/cel/common/navigation/BUILD.bazel +++ b/common/src/test/java/dev/cel/common/navigation/BUILD.bazel @@ -21,10 +21,12 @@ java_library( "//common/ast:mutable_expr", "//common/navigation", "//common/navigation:common", + "//common/navigation:expr_util", "//common/navigation:mutable_navigation", "//common/types", "//compiler", "//compiler:compiler_builder", + "//extensions", "//parser:macro", "@cel_spec//proto/cel/expr/conformance/proto3:test_all_types_java_proto", "@maven//:com_google_guava_guava", diff --git a/common/src/test/java/dev/cel/common/navigation/CelNavigableExprUtilTest.java b/common/src/test/java/dev/cel/common/navigation/CelNavigableExprUtilTest.java new file mode 100644 index 000000000..56e06d187 --- /dev/null +++ b/common/src/test/java/dev/cel/common/navigation/CelNavigableExprUtilTest.java @@ -0,0 +1,348 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dev.cel.common.navigation; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelMutableAst; +import dev.cel.common.ast.CelConstant; +import dev.cel.common.ast.CelExpr.ExprKind.Kind; +import dev.cel.common.ast.CelMutableExpr; +import dev.cel.common.ast.CelMutableExpr.CelMutableCall; +import dev.cel.common.ast.CelMutableExpr.CelMutableComprehension; +import dev.cel.common.ast.CelMutableExpr.CelMutableList; +import dev.cel.common.types.SimpleType; +import dev.cel.compiler.CelCompiler; +import dev.cel.compiler.CelCompilerFactory; +import dev.cel.extensions.CelExtensions; +import dev.cel.parser.CelStandardMacro; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class CelNavigableExprUtilTest { + + private static final CelCompiler COMPILER = + CelCompilerFactory.standardCelCompilerBuilder() + .setStandardMacros(CelStandardMacro.STANDARD_MACROS) + .addLibraries(CelExtensions.comprehensions()) + .addVar("a", SimpleType.INT) + .addVar("b", SimpleType.INT) + .build(); + + @Test + public void isVariableShadowed_singleVarComprehension_loopStep() throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[1, 2].all(x, x > 0)").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr identX = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("x")) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.isVariableShadowed(identX, "x")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(identX, "y")).isFalse(); + } + + @Test + public void isVariableShadowed_twoVarComprehension_loopStep() throws Exception { + CelAbstractSyntaxTree ast = + COMPILER.compile("{'k1': 1, 'k2': 2}.all(k, v, k != '' && v > 0)").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr identK = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("k")) + .findFirst() + .get(); + CelNavigableExpr identV = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("v")) + .findFirst() + .get(); + CelNavigableExpr iterRangeMap = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.getKind() == Kind.MAP) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.isVariableShadowed(identK, "k")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(identK, "v")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(identV, "k")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(identV, "v")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(identK, "other")).isFalse(); + + assertThat(CelNavigableExprUtil.isVariableShadowed(iterRangeMap, "k")).isFalse(); + assertThat(CelNavigableExprUtil.isVariableShadowed(iterRangeMap, "v")).isFalse(); + } + + @Test + public void isVariableShadowed_iterRange_notShadowed() throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[a].all(x, x > 0)").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr identA = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("a")) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.isVariableShadowed(identA, "x")).isFalse(); + assertThat(CelNavigableExprUtil.isVariableShadowed(identA, "a")).isFalse(); + } + + @Test + public void isVariableShadowed_nestedComprehension_scopedCorrectly() throws Exception { + CelAbstractSyntaxTree ast = + COMPILER.compile("[1, 2].all(x, [3, 4].all(y, x > 0 && y > 0))").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr innerIdentX = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("x")) + .findFirst() + .get(); + CelNavigableExpr innerIdentY = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("y")) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.isVariableShadowed(innerIdentX, "x")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(innerIdentX, "y")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(innerIdentY, "x")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(innerIdentY, "y")).isTrue(); + } + + @Test + public void isVariableShadowed_nestedComprehension_innerIterRangeShadowsOuterOnly() + throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[1, 2].all(x, [x].all(y, y > 0))").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr innerIterRangeIdentX = + navigableAst + .getRoot() + .allNodes() + .filter( + node -> + node.expr().identOrDefault().name().equals("x") + && node.parent().isPresent() + && node.parent().get().getKind() == Kind.LIST) + .findFirst() + .get(); + + // In the inner comprehension's iterRange, outer 'x' IS in scope, but inner 'y' is NOT in scope + assertThat(CelNavigableExprUtil.isVariableShadowed(innerIterRangeIdentX, "x")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(innerIterRangeIdentX, "y")).isFalse(); + } + + @Test + public void isVariableShadowed_comprehensionResultBranch() throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[1, 2].all(x, x > 0)").getAst(); + CelNavigableMutableAst navigableAst = + CelNavigableMutableAst.fromAst(CelMutableAst.fromCelAst(ast)); + + CelNavigableMutableExpr comprehensionNode = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.getKind() == Kind.COMPREHENSION) + .findFirst() + .get(); + + CelMutableComprehension comprehension = comprehensionNode.expr().comprehension(); + long resultId = comprehension.result().id(); + + CelNavigableMutableExpr resultNode = + comprehensionNode.allNodes().filter(node -> node.id() == resultId).findFirst().get(); + + // In result branch, accuVar is in scope, but iterVar is not + assertThat(CelNavigableExprUtil.isVariableShadowed(resultNode, comprehension.accuVar())) + .isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(resultNode, comprehension.iterVar())) + .isFalse(); + } + + @Test + public void areVariablesShadowed_multipleVariables() throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[1, 2].all(x, x > 0)").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr identX = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("x")) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.areVariablesShadowed(identX, ImmutableSet.of("y", "z", "x"))) + .isTrue(); + assertThat(CelNavigableExprUtil.areVariablesShadowed(identX, ImmutableSet.of("y", "z"))) + .isFalse(); + assertThat(CelNavigableExprUtil.areVariablesShadowed(identX, ImmutableList.of())).isFalse(); + } + + @Test + public void isComprehensionVariable_identNode() throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[a].all(x, x > a)").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr identX = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("x")) + .findFirst() + .get(); + CelNavigableExpr identAInLoop = + navigableAst + .getRoot() + .allNodes() + .filter( + node -> + node.expr().identOrDefault().name().equals("a") + && node.parent().isPresent() + && node.parent().get().getKind() == Kind.CALL) + .findFirst() + .get(); + CelNavigableExpr constNode = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.getKind() == Kind.CONSTANT) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.isComprehensionVariable(identX)).isTrue(); + assertThat(CelNavigableExprUtil.isComprehensionVariable(identAInLoop)).isFalse(); + assertThat(CelNavigableExprUtil.isComprehensionVariable(constNode)).isFalse(); + } + + @Test + public void hasComprehensionVariable_subtreeCheck() throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[a].all(x, x > a)").getAst(); + CelNavigableAst navigableAst = CelNavigableAst.fromAst(ast); + + CelNavigableExpr root = navigableAst.getRoot(); + CelNavigableExpr iterRange = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.getKind() == Kind.LIST) + .findFirst() + .get(); + CelNavigableExpr loopStepCall = + navigableAst + .getRoot() + .allNodes() + .filter(node -> node.expr().callOrDefault().function().equals("@not_strictly_false")) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.hasComprehensionVariable(root)).isTrue(); + assertThat(CelNavigableExprUtil.hasComprehensionVariable(loopStepCall)).isTrue(); + assertThat(CelNavigableExprUtil.hasComprehensionVariable(iterRange)).isFalse(); + } + + @Test + public void mutableAst_parityWithImmutableAst() throws Exception { + CelAbstractSyntaxTree ast = COMPILER.compile("[1, 2].all(x, x > 0)").getAst(); + CelNavigableAst immutableNavAst = CelNavigableAst.fromAst(ast); + CelNavigableMutableAst mutableNavAst = + CelNavigableMutableAst.fromAst(CelMutableAst.fromCelAst(ast)); + + CelNavigableExpr immutableIdentX = + immutableNavAst + .getRoot() + .allNodes() + .filter(node -> node.expr().identOrDefault().name().equals("x")) + .findFirst() + .get(); + CelNavigableMutableExpr mutableIdentX = + mutableNavAst + .getRoot() + .allNodes() + .filter(node -> node.getKind() == Kind.IDENT && node.expr().ident().name().equals("x")) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.isVariableShadowed(mutableIdentX, "x")) + .isEqualTo(CelNavigableExprUtil.isVariableShadowed(immutableIdentX, "x")); + assertThat(CelNavigableExprUtil.isComprehensionVariable(mutableIdentX)) + .isEqualTo(CelNavigableExprUtil.isComprehensionVariable(immutableIdentX)); + assertThat(CelNavigableExprUtil.hasComprehensionVariable(mutableNavAst.getRoot())) + .isEqualTo(CelNavigableExprUtil.hasComprehensionVariable(immutableNavAst.getRoot())); + } + + @Test + public void isVariableShadowed_zeroedOutIds_scopedCorrectly() { + // Construct a mutable comprehension where ALL expression IDs are 0 (e.g. freshly minted AST) + CelMutableExpr iterRange = CelMutableExpr.ofList(0, CelMutableList.create()); + CelMutableExpr accuInit = CelMutableExpr.ofConstant(0, CelConstant.ofValue(true)); + CelMutableExpr loopCond = CelMutableExpr.ofConstant(0, CelConstant.ofValue(true)); + CelMutableExpr identX = CelMutableExpr.ofIdent(0, "x"); + CelMutableExpr loopStep = CelMutableExpr.ofCall(0, CelMutableCall.create("!_", identX)); + CelMutableExpr result = CelMutableExpr.ofIdent(0, "accu"); + + CelMutableExpr comp = + CelMutableExpr.ofComprehension( + 0, + CelMutableComprehension.create( + "x", iterRange, "accu", accuInit, loopCond, loopStep, result)); + + CelNavigableMutableExpr root = CelNavigableMutableExpr.fromExpr(comp); + + CelNavigableMutableExpr navIdentX = + root.allNodes() + .filter(node -> node.getKind() == Kind.IDENT && node.expr().ident().name().equals("x")) + .findFirst() + .get(); + CelNavigableMutableExpr navIterRange = + root.allNodes().filter(node -> node.getKind() == Kind.LIST).findFirst().get(); + CelNavigableMutableExpr navResult = + root.allNodes() + .filter( + node -> node.getKind() == Kind.IDENT && node.expr().ident().name().equals("accu")) + .findFirst() + .get(); + + assertThat(CelNavigableExprUtil.isVariableShadowed(navIdentX, "x")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(navIdentX, "accu")).isTrue(); + assertThat(CelNavigableExprUtil.isVariableShadowed(navIterRange, "x")).isFalse(); + assertThat(CelNavigableExprUtil.isVariableShadowed(navResult, "x")).isFalse(); + assertThat(CelNavigableExprUtil.isVariableShadowed(navResult, "accu")).isTrue(); + } +} diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel b/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel index da722d521..0c4b78826 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/BUILD.bazel @@ -29,6 +29,7 @@ java_library( "//common/ast:mutable_expr", "//common/internal:date_time_helpers", "//common/navigation:common", + "//common/navigation:expr_util", "//common/navigation:mutable_navigation", "//common/types", "//common/types:type_providers", @@ -95,6 +96,7 @@ java_library( "//common:operator", "//common/ast", "//common/ast:mutable_expr", + "//common/navigation:expr_util", "//common/navigation:mutable_navigation", "//common/types", "//common/types:type_providers", diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java b/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java index 1cf52bcbe..266059426 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/ConstantFoldingOptimizer.java @@ -36,12 +36,12 @@ import dev.cel.common.ast.CelExpr.ExprKind.Kind; import dev.cel.common.ast.CelMutableExpr; import dev.cel.common.ast.CelMutableExpr.CelMutableCall; -import dev.cel.common.ast.CelMutableExpr.CelMutableComprehension; import dev.cel.common.ast.CelMutableExpr.CelMutableList; import dev.cel.common.ast.CelMutableExpr.CelMutableMap; import dev.cel.common.ast.CelMutableExpr.CelMutableStruct; import dev.cel.common.ast.CelMutableExprConverter; import dev.cel.common.internal.DateTimeHelpers; +import dev.cel.common.navigation.CelNavigableExprUtil; import dev.cel.common.navigation.CelNavigableMutableAst; import dev.cel.common.navigation.CelNavigableMutableExpr; import dev.cel.common.navigation.TraversalOrder; @@ -247,7 +247,7 @@ private boolean canFold(CelNavigableMutableExpr navigableExpr) { if (functionName.equals(Operator.EQUALS.getFunction()) || functionName.equals(Operator.NOT_EQUALS.getFunction())) { - if (hasComprehensionVar(navigableExpr)) { + if (CelNavigableExprUtil.hasComprehensionVariable(navigableExpr)) { return false; } if (mutableCall.args().stream() @@ -259,7 +259,7 @@ private boolean canFold(CelNavigableMutableExpr navigableExpr) { } if (functionName.equals(Operator.IN.getFunction())) { - return !hasComprehensionVar(navigableExpr); + return !CelNavigableExprUtil.hasComprehensionVariable(navigableExpr); } // Default case: all call arguments must be constants. If the argument is a container (ex: @@ -288,33 +288,6 @@ private static boolean isCallTimestampOrDuration(CelMutableCall call) { || call.function().equals(DURATION.functionName()); } - private static boolean hasComprehensionVar(CelNavigableMutableExpr expr) { - return expr.allNodes() - .filter(node -> node.getKind().equals(Kind.IDENT)) - .anyMatch( - identNode -> { - String identName = identNode.expr().ident().name(); - CelNavigableMutableExpr curr = identNode; - Optional maybeParent = curr.parent(); - while (maybeParent.isPresent()) { - CelNavigableMutableExpr parent = maybeParent.get(); - if (parent.getKind().equals(Kind.COMPREHENSION)) { - CelMutableComprehension compre = parent.expr().comprehension(); - if ((compre.accuVar().equals(identName) - || compre.iterVar().equals(identName) - || compre.iterVar2().equals(identName)) - && curr.id() != compre.iterRange().id() - && curr.id() != compre.accuInit().id()) { - return true; - } - } - curr = parent; - maybeParent = parent.parent(); - } - return false; - }); - } - private static boolean areChildrenArgConstant(CelNavigableMutableExpr expr) { if (expr.getKind().equals(Kind.CONSTANT)) { return true; @@ -350,7 +323,8 @@ private Optional maybeFold( CelMutableAst mutableAst, CelNavigableMutableExpr node) throws CelOptimizationException, CelEvaluationException { - if (!node.getKind().equals(Kind.COMPREHENSION) && hasComprehensionVar(node)) { + if (!node.getKind().equals(Kind.COMPREHENSION) + && CelNavigableExprUtil.hasComprehensionVariable(node)) { return Optional.empty(); } Object result; diff --git a/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java b/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java index e4051f82f..147673e47 100644 --- a/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java +++ b/optimizer/src/main/java/dev/cel/optimizer/optimizers/InliningOptimizer.java @@ -27,8 +27,8 @@ import dev.cel.common.ast.CelExpr.ExprKind.Kind; import dev.cel.common.ast.CelMutableExpr; import dev.cel.common.ast.CelMutableExpr.CelMutableCall; -import dev.cel.common.ast.CelMutableExpr.CelMutableComprehension; import dev.cel.common.ast.CelMutableExpr.CelMutableStruct; +import dev.cel.common.navigation.CelNavigableExprUtil; import dev.cel.common.navigation.CelNavigableMutableAst; import dev.cel.common.navigation.CelNavigableMutableExpr; import dev.cel.common.types.CelKind; @@ -41,7 +41,6 @@ import java.util.List; import java.util.NoSuchElementException; import java.util.Optional; -import java.util.stream.Stream; /** * Performs optimization for inlining variables within function calls and select statements with @@ -222,23 +221,7 @@ private static boolean canInline(CelNavigableMutableExpr node, String identifier return false; } - for (CelNavigableMutableExpr p = node.parent().orElse(null); - p != null; - p = p.parent().orElse(null)) { - if (p.getKind() != Kind.COMPREHENSION) { - continue; - } - - CelMutableComprehension comp = p.expr().comprehension(); - boolean shadows = - Stream.of(comp.iterVar(), comp.iterVar2(), comp.accuVar()).anyMatch(identifier::equals); - - if (shadows) { - return false; - } - } - - return true; + return !CelNavigableExprUtil.isVariableShadowed(node, identifier); } private static Optional maybeToQualifiedName(CelNavigableMutableExpr node) {