Skip to content

Commit 3a4e33a

Browse files
committed
Namespace resolution fix
1 parent 9ec3ba1 commit 3a4e33a

7 files changed

Lines changed: 165 additions & 19 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
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.runtime.planner;
16+
17+
import dev.cel.runtime.GlobalResolver;
18+
19+
/** Identifies a resolver that can be unwrapped to bypass local variable state. */
20+
public interface ActivationWrapper extends GlobalResolver {
21+
GlobalResolver unwrap();
22+
}

runtime/src/main/java/dev/cel/runtime/planner/AttributeFactory.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,23 @@ final class AttributeFactory {
2929
private final CelValueConverter celValueConverter;
3030

3131
NamespacedAttribute newAbsoluteAttribute(String... names) {
32-
return new NamespacedAttribute(typeProvider, celValueConverter, ImmutableSet.copyOf(names));
32+
return NamespacedAttribute.create(typeProvider, celValueConverter, ImmutableSet.copyOf(names));
3333
}
3434

3535
RelativeAttribute newRelativeAttribute(PlannedInterpretable operand) {
3636
return new RelativeAttribute(operand, celValueConverter);
3737
}
3838

3939
MaybeAttribute newMaybeAttribute(String name) {
40+
// When there's a single name with a dot prefix, it indicates that the 'maybe' attribute is a
41+
// globally namespaced identifier.
42+
// Otherwise, the candidate names resolved from the container should be inferred.
43+
ImmutableSet<String> names =
44+
name.startsWith(".") ? ImmutableSet.of(name) : container.resolveCandidateNames(name);
45+
4046
return new MaybeAttribute(
4147
this,
42-
ImmutableList.of(
43-
new NamespacedAttribute(
44-
typeProvider, celValueConverter, container.resolveCandidateNames(name))));
48+
ImmutableList.of(NamespacedAttribute.create(typeProvider, celValueConverter, names)));
4549
}
4650

4751
static AttributeFactory newAttributeFactory(

runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ java_library(
114114
"RelativeAttribute.java",
115115
],
116116
deps = [
117+
":activation_wrapper",
117118
":eval_helpers",
118119
":execution_frame",
119120
":planned_interpretable",
@@ -127,6 +128,16 @@ java_library(
127128
"//runtime:interpretable",
128129
"@maven//:com_google_errorprone_error_prone_annotations",
129130
"@maven//:com_google_guava_guava",
131+
"@maven//:org_jspecify_jspecify",
132+
],
133+
)
134+
135+
java_library(
136+
name = "activation_wrapper",
137+
srcs = ["ActivationWrapper.java"],
138+
deps = [
139+
"//runtime:activation",
140+
"//runtime:interpretable",
130141
],
131142
)
132143

@@ -333,6 +344,7 @@ java_library(
333344
name = "eval_fold",
334345
srcs = ["EvalFold.java"],
335346
deps = [
347+
":activation_wrapper",
336348
":execution_frame",
337349
":planned_interpretable",
338350
"//runtime:concatenated_list_view",

runtime/src/main/java/dev/cel/runtime/planner/EvalFold.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ private Object evalMap(Map<?, ?> iterRange, Folder folder, ExecutionFrame frame)
9797
if (!iterVar2.isEmpty()) {
9898
folder.iterVar2Val = entry.getValue();
9999
}
100-
100+
101101
boolean cond = (boolean) condition.eval(folder, frame);
102102
if (!cond) {
103103
return result.eval(folder, frame);
@@ -149,7 +149,7 @@ private static Object maybeUnwrapAccumulator(Object val) {
149149
return val;
150150
}
151151

152-
private static class Folder implements GlobalResolver {
152+
private static class Folder implements ActivationWrapper {
153153
private final GlobalResolver resolver;
154154
private final String accuVar;
155155
private final String iterVar;
@@ -166,6 +166,11 @@ private Folder(GlobalResolver resolver, String accuVar, String iterVar, String i
166166
this.iterVar2 = iterVar2;
167167
}
168168

169+
@Override
170+
public GlobalResolver unwrap() {
171+
return resolver;
172+
}
173+
169174
@Override
170175
public @Nullable Object resolve(String name) {
171176
if (name.equals(accuVar)) {

runtime/src/main/java/dev/cel/runtime/planner/NamespacedAttribute.java

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,30 @@
2828

2929
@Immutable
3030
final class NamespacedAttribute implements Attribute {
31+
private final ImmutableSet<Integer> disambiguateNames;
3132
private final ImmutableSet<String> namespacedNames;
3233
private final ImmutableList<Qualifier> qualifiers;
3334
private final CelValueConverter celValueConverter;
3435
private final CelTypeProvider typeProvider;
3536

3637
@Override
3738
public Object resolve(GlobalResolver ctx, ExecutionFrame frame) {
39+
GlobalResolver inputVars = ctx;
40+
// Unwrap any local activations to ensure that we reach the variables provided as input
41+
// to the expression in the event that we need to disambiguate between global and local
42+
// variables.
43+
if (!disambiguateNames.isEmpty()) {
44+
inputVars = unwrapToRoot(ctx);
45+
}
46+
47+
int i = 0;
3848
for (String name : namespacedNames) {
39-
Object value = ctx.resolve(name);
49+
GlobalResolver resolver = ctx;
50+
if (disambiguateNames.contains(i)) {
51+
resolver = inputVars;
52+
}
53+
54+
Object value = resolver.resolve(name);
4055
if (value != null) {
4156
if (!qualifiers.isEmpty()) {
4257
return applyQualifiers(value, celValueConverter, qualifiers);
@@ -69,6 +84,7 @@ public Object resolve(GlobalResolver ctx, ExecutionFrame frame) {
6984
throw new IllegalStateException(
7085
"Unexpected type resolution when there were remaining qualifiers: " + type.name());
7186
}
87+
i++;
7288
}
7389

7490
return MissingAttribute.newMissingAttribute(namespacedNames);
@@ -82,12 +98,20 @@ ImmutableSet<String> candidateVariableNames() {
8298
return namespacedNames;
8399
}
84100

101+
private GlobalResolver unwrapToRoot(GlobalResolver resolver) {
102+
while (resolver instanceof ActivationWrapper) {
103+
resolver = ((ActivationWrapper) resolver).unwrap();
104+
}
105+
return resolver;
106+
}
107+
85108
@Override
86109
public NamespacedAttribute addQualifier(Qualifier qualifier) {
87110
return new NamespacedAttribute(
88111
typeProvider,
89112
celValueConverter,
90113
namespacedNames,
114+
disambiguateNames,
91115
ImmutableList.<Qualifier>builder().addAll(qualifiers).add(qualifier).build());
92116
}
93117

@@ -106,21 +130,40 @@ private static Object applyQualifiers(
106130
return obj;
107131
}
108132

109-
NamespacedAttribute(
133+
static NamespacedAttribute create(
110134
CelTypeProvider typeProvider,
111135
CelValueConverter celValueConverter,
112136
ImmutableSet<String> namespacedNames) {
113-
this(typeProvider, celValueConverter, namespacedNames, ImmutableList.of());
137+
ImmutableSet.Builder<String> namesBuilder = ImmutableSet.builder();
138+
ImmutableSet.Builder<Integer> indicesBuilder = ImmutableSet.builder();
139+
int i = 0;
140+
for (String name : namespacedNames) {
141+
if (name.startsWith(".")) {
142+
indicesBuilder.add(i);
143+
namesBuilder.add(name.substring(1));
144+
} else {
145+
namesBuilder.add(name);
146+
}
147+
i++;
148+
}
149+
return new NamespacedAttribute(
150+
typeProvider,
151+
celValueConverter,
152+
namesBuilder.build(),
153+
indicesBuilder.build(),
154+
ImmutableList.of());
114155
}
115156

116-
private NamespacedAttribute(
157+
NamespacedAttribute(
117158
CelTypeProvider typeProvider,
118159
CelValueConverter celValueConverter,
119160
ImmutableSet<String> namespacedNames,
161+
ImmutableSet<Integer> disambiguateNames,
120162
ImmutableList<Qualifier> qualifiers) {
121163
this.typeProvider = typeProvider;
122164
this.celValueConverter = celValueConverter;
123165
this.namespacedNames = namespacedNames;
166+
this.disambiguateNames = disambiguateNames;
124167
this.qualifiers = qualifiers;
125168
}
126169
}

runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package dev.cel.runtime.planner;
1616

1717
import com.google.auto.value.AutoValue;
18+
import com.google.common.base.Strings;
1819
import com.google.common.collect.ImmutableList;
1920
import com.google.common.collect.ImmutableMap;
2021
import com.google.common.collect.ImmutableSet;
@@ -48,6 +49,9 @@
4849
import dev.cel.runtime.CelResolvedOverload;
4950
import dev.cel.runtime.DefaultDispatcher;
5051
import dev.cel.runtime.Program;
52+
53+
import java.util.HashMap;
54+
import java.util.Map;
5155
import java.util.NoSuchElementException;
5256
import java.util.Optional;
5357

@@ -161,8 +165,14 @@ private PlannedInterpretable planIdent(CelExpr celExpr, PlannerContext ctx) {
161165
return planCheckedIdent(celExpr.id(), ref, ctx.typeMap());
162166
}
163167

168+
String identName = celExpr.ident().name();
169+
if (ctx.isLocalVar(identName)) {
170+
return EvalAttribute.create(
171+
celExpr.id(), attributeFactory.newAbsoluteAttribute(identName));
172+
}
173+
164174
return EvalAttribute.create(
165-
celExpr.id(), attributeFactory.newMaybeAttribute(celExpr.ident().name()));
175+
celExpr.id(), attributeFactory.newMaybeAttribute(identName));
166176
}
167177

168178
private PlannedInterpretable planCheckedIdent(
@@ -314,10 +324,18 @@ private PlannedInterpretable planComprehension(CelExpr expr, PlannerContext ctx)
314324

315325
PlannedInterpretable accuInit = plan(comprehension.accuInit(), ctx);
316326
PlannedInterpretable iterRange = plan(comprehension.iterRange(), ctx);
327+
328+
ctx.pushLocalVars(comprehension.accuVar(), comprehension.iterVar(), comprehension.iterVar2());
329+
317330
PlannedInterpretable loopCondition = plan(comprehension.loopCondition(), ctx);
318331
PlannedInterpretable loopStep = plan(comprehension.loopStep(), ctx);
332+
333+
ctx.popLocalVars(comprehension.iterVar(), comprehension.iterVar2());
334+
319335
PlannedInterpretable result = plan(comprehension.result(), ctx);
320336

337+
ctx.popLocalVars(comprehension.accuVar());
338+
321339
return EvalFold.create(
322340
expr.id(),
323341
comprehension.accuVar(),
@@ -460,15 +478,57 @@ private static Builder newBuilder() {
460478
}
461479
}
462480

463-
@AutoValue
464-
abstract static class PlannerContext {
481+
static final class PlannerContext {
482+
private final ImmutableMap<Long, CelReference> referenceMap;
483+
private final ImmutableMap<Long, CelType> typeMap;
484+
private final Map<String, Integer> localVars = new HashMap<>();
485+
486+
ImmutableMap<Long, CelReference> referenceMap() {
487+
return referenceMap;
488+
}
489+
490+
ImmutableMap<Long, CelType> typeMap() {
491+
return typeMap;
492+
}
493+
494+
private void pushLocalVars(String... names) {
495+
for (String name : names) {
496+
if (Strings.isNullOrEmpty(name)) {
497+
continue;
498+
}
499+
localVars.merge(name, 1, Integer::sum);
500+
}
501+
}
502+
503+
private void popLocalVars(String... names) {
504+
for (String name : names) {
505+
if (Strings.isNullOrEmpty(name)) {
506+
continue;
507+
}
508+
Integer count = localVars.get(name);
509+
if (count != null) {
510+
if (count == 1) {
511+
localVars.remove(name);
512+
} else {
513+
localVars.put(name, count - 1);
514+
}
515+
}
516+
}
517+
}
465518

466-
abstract ImmutableMap<Long, CelReference> referenceMap();
519+
/** Checks if the given name is a local variable in the current scope. */
520+
private boolean isLocalVar(String name) {
521+
return localVars.containsKey(name);
522+
}
467523

468-
abstract ImmutableMap<Long, CelType> typeMap();
524+
private PlannerContext(
525+
ImmutableMap<Long, CelReference> referenceMap, ImmutableMap<Long, CelType> typeMap) {
526+
this.referenceMap = referenceMap;
527+
this.typeMap = typeMap;
528+
}
469529

470-
private static PlannerContext create(CelAbstractSyntaxTree ast) {
471-
return new AutoValue_ProgramPlanner_PlannerContext(ast.getReferenceMap(), ast.getTypeMap());
530+
static PlannerContext create(CelAbstractSyntaxTree ast) {
531+
return new PlannerContext(ast.getReferenceMap(), ast.getTypeMap());
472532
}
473533
}
474534

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ public void plan_comprehension_iterationLimit_success() throws Exception {
824824
CEL_VALUE_CONVERTER,
825825
CEL_CONTAINER,
826826
options,
827-
/* lateBoundFunctionNames= */ ImmutableSet.of());
827+
/* lateBoundFunctionNames= */ ImmutableSet.of());
828828
CelAbstractSyntaxTree ast = compile("[1, 2, 3].map(x, [1, 2].map(y, x + y))");
829829

830830
Program program = planner.plan(ast);
@@ -938,7 +938,7 @@ private CelAbstractSyntaxTree compile(CelCompiler compiler, String expression) t
938938
return ast;
939939
}
940940

941-
return CEL_COMPILER.check(ast).getAst();
941+
return compiler.check(ast).getAst();
942942
}
943943

944944
private static CelByteString concatenateByteArrays(CelByteString bytes1, CelByteString bytes2) {

0 commit comments

Comments
 (0)