Skip to content

Commit b84982e

Browse files
l46kokcopybara-github
authored andcommitted
Fix type-checker to search local scopes for identifiers before container resolution
PiperOrigin-RevId: 853502183
1 parent 233430f commit b84982e

File tree

3 files changed

+66
-22
lines changed

3 files changed

+66
-22
lines changed

checker/src/main/java/dev/cel/checker/Env.java

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -454,35 +454,66 @@ public Env add(String name, Type type) {
454454
* <p>Returns {@code null} if the function cannot be found.
455455
*/
456456
public @Nullable CelIdentDecl tryLookupCelIdent(CelContainer container, String name) {
457+
// Attempt to find the decl with just the ident name to account for shadowed variables
458+
CelIdentDecl decl = tryLookupCelIdentFromLocalScopes(name);
459+
if (decl != null) {
460+
return decl;
461+
}
462+
457463
for (String cand : container.resolveCandidateNames(name)) {
458-
// First determine whether we know this name already.
459-
CelIdentDecl decl = findIdentDecl(cand);
464+
decl = tryLookupCelIdent(cand);
460465
if (decl != null) {
461466
return decl;
462467
}
468+
}
463469

464-
// Next try to import the name as a reference to a message type.
465-
// This is done via the type provider.
466-
Optional<CelType> type = typeProvider.lookupCelType(cand);
467-
if (type.isPresent()) {
468-
decl = CelIdentDecl.newIdentDeclaration(cand, type.get());
469-
decls.get(0).putIdent(decl);
470-
return decl;
471-
}
470+
return null;
471+
}
472472

473-
// Next try to import this as an enum value by splitting the name in a type prefix and
474-
// the enum inside.
475-
Integer enumValue = typeProvider.lookupEnumValue(cand);
476-
if (enumValue != null) {
477-
decl =
478-
CelIdentDecl.newBuilder()
479-
.setName(cand)
480-
.setType(SimpleType.INT)
481-
.setConstant(CelConstant.ofValue(enumValue))
482-
.build();
473+
private @Nullable CelIdentDecl tryLookupCelIdent(String cand) {
474+
// First determine whether we know this name already.
475+
CelIdentDecl decl = findIdentDecl(cand);
476+
if (decl != null) {
477+
return decl;
478+
}
483479

484-
decls.get(0).putIdent(decl);
485-
return decl;
480+
// Next try to import the name as a reference to a message type.
481+
// This is done via the type provider.
482+
Optional<CelType> type = typeProvider.lookupCelType(cand);
483+
if (type.isPresent()) {
484+
decl = CelIdentDecl.newIdentDeclaration(cand, type.get());
485+
decls.get(0).putIdent(decl);
486+
return decl;
487+
}
488+
489+
// Next try to import this as an enum value by splitting the name in a type prefix and
490+
// the enum inside.
491+
Integer enumValue = typeProvider.lookupEnumValue(cand);
492+
if (enumValue != null) {
493+
decl =
494+
CelIdentDecl.newBuilder()
495+
.setName(cand)
496+
.setType(SimpleType.INT)
497+
.setConstant(CelConstant.ofValue(enumValue))
498+
.build();
499+
500+
decls.get(0).putIdent(decl);
501+
return decl;
502+
}
503+
return null;
504+
}
505+
506+
private @Nullable CelIdentDecl tryLookupCelIdentFromLocalScopes(String name) {
507+
int firstUserSpaceScope = 2;
508+
// Iterate from the top of the stack down to the first local scope.
509+
// Note that:
510+
// Scope 0: Standard environment
511+
// Scope 1: User defined environment
512+
// Scope 2 and onwards: comprehension scopes
513+
for (int i = decls.size() - 1; i >= firstUserSpaceScope; i--) {
514+
CelIdentDecl ident = decls.get(i).getIdent(name);
515+
if (ident != null) {
516+
return ident;
486517
}
487518
}
488519
return null;

runtime/src/test/resources/comprehension.baseline

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,11 @@ Source: [0, 1, 2].exists(x, x > 2)
1212
=====>
1313
bindings: {}
1414
result: false
15+
16+
Source: [0].exists(x, x == 0)
17+
declare com.x {
18+
value int
19+
}
20+
=====>
21+
bindings: {com.x=1}
22+
result: true

testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,11 @@ public void comprehension() throws Exception {
858858

859859
source = "[0, 1, 2].exists(x, x > 2)";
860860
runTest();
861+
862+
declareVariable("com.x", SimpleType.INT);
863+
container = CelContainer.ofName("com");
864+
source = "[0].exists(x, x == 0)";
865+
runTest(ImmutableMap.of("com.x", 1));
861866
}
862867

863868
@Test

0 commit comments

Comments
 (0)