Skip to content

Commit 62b0d22

Browse files
l46kokcopybara-github
authored andcommitted
Retain source description post AST optimization
PiperOrigin-RevId: 631847830
1 parent 2a8789f commit 62b0d22

File tree

3 files changed

+31
-8
lines changed

3 files changed

+31
-8
lines changed

common/src/main/java/dev/cel/common/CelMutableSource.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@
3535
*/
3636
public final class CelMutableSource {
3737

38-
final Map<Long, CelMutableExpr> macroCalls;
39-
final Set<Extension> extensions;
38+
private String description;
39+
private final Map<Long, CelMutableExpr> macroCalls;
40+
private final Set<Extension> extensions;
4041

4142
@CanIgnoreReturnValue
4243
public CelMutableSource addMacroCalls(long exprId, CelMutableExpr expr) {
@@ -57,6 +58,12 @@ public CelMutableSource addAllExtensions(Collection<? extends Extension> extensi
5758
return this;
5859
}
5960

61+
@CanIgnoreReturnValue
62+
public CelMutableSource setDescription(String description) {
63+
this.description = checkNotNull(description);
64+
return this;
65+
}
66+
6067
@CanIgnoreReturnValue
6168
public CelMutableSource clearMacroCall(long exprId) {
6269
this.macroCalls.remove(exprId);
@@ -69,6 +76,10 @@ public CelMutableSource clearMacroCalls() {
6976
return this;
7077
}
7178

79+
public String getDescription() {
80+
return description;
81+
}
82+
7283
public Map<Long, CelMutableExpr> getMacroCalls() {
7384
return macroCalls;
7485
}
@@ -79,6 +90,7 @@ public Set<Extension> getExtensions() {
7990

8091
public CelSource toCelSource() {
8192
return CelSource.newBuilder()
93+
.setDescription(description)
8294
.addAllExtensions(extensions)
8395
.addAllMacroCalls(
8496
macroCalls.entrySet().stream()
@@ -89,11 +101,12 @@ public CelSource toCelSource() {
89101
}
90102

91103
public static CelMutableSource newInstance() {
92-
return new CelMutableSource(new HashMap<>(), new HashSet<>());
104+
return new CelMutableSource("", new HashMap<>(), new HashSet<>());
93105
}
94106

95107
public static CelMutableSource fromCelSource(CelSource source) {
96108
return new CelMutableSource(
109+
source.getDescription(),
97110
source.getMacroCalls().entrySet().stream()
98111
.collect(
99112
Collectors.toMap(
@@ -107,7 +120,9 @@ public static CelMutableSource fromCelSource(CelSource source) {
107120
source.getExtensions());
108121
}
109122

110-
CelMutableSource(Map<Long, CelMutableExpr> macroCalls, Set<Extension> extensions) {
123+
CelMutableSource(
124+
String description, Map<Long, CelMutableExpr> macroCalls, Set<Extension> extensions) {
125+
this.description = checkNotNull(description);
111126
this.macroCalls = checkNotNull(macroCalls);
112127
this.extensions = checkNotNull(extensions);
113128
}

optimizer/src/main/java/dev/cel/optimizer/AstMutator.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.google.auto.value.AutoValue;
2222
import com.google.common.base.Preconditions;
23+
import com.google.common.base.Strings;
2324
import com.google.common.collect.ImmutableMap;
2425
import com.google.errorprone.annotations.Immutable;
2526
import dev.cel.common.CelAbstractSyntaxTree;
@@ -418,7 +419,8 @@ public CelMutableAst replaceSubtree(
418419

419420
CelMutableExpr mutatedRoot =
420421
mutateExpr(stableIdGenerator::renumberId, ast.expr(), newAst.expr(), exprIdToReplace);
421-
CelMutableSource newAstSource = CelMutableSource.newInstance();
422+
CelMutableSource newAstSource =
423+
CelMutableSource.newInstance().setDescription(ast.source().getDescription());
422424
if (!ast.source().getMacroCalls().isEmpty()) {
423425
newAstSource = combine(newAstSource, ast.source());
424426
}
@@ -570,6 +572,10 @@ private CelMutableExpr newBindMacroSourceExpr(
570572
private static CelMutableSource combine(
571573
CelMutableSource celSource1, CelMutableSource celSource2) {
572574
return CelMutableSource.newInstance()
575+
.setDescription(
576+
Strings.isNullOrEmpty(celSource1.getDescription())
577+
? celSource2.getDescription()
578+
: celSource1.getDescription())
573579
.addAllExtensions(celSource1.getExtensions())
574580
.addAllExtensions(celSource2.getExtensions())
575581
.addAllMacroCalls(celSource1.getMacroCalls())
@@ -635,7 +641,9 @@ private CelMutableSource normalizeMacroSource(
635641
}));
636642

637643
CelMutableSource newMacroSource =
638-
CelMutableSource.newInstance().addAllExtensions(source.getExtensions());
644+
CelMutableSource.newInstance()
645+
.setDescription(source.getDescription())
646+
.addAllExtensions(source.getExtensions());
639647
// Update the macro call IDs and their call references
640648
for (Entry<Long, CelMutableExpr> existingMacroCall : source.getMacroCalls().entrySet()) {
641649
long macroId = existingMacroCall.getKey();

optimizer/src/test/java/dev/cel/optimizer/AstMutatorTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,11 @@ public void astMutator_nonMacro_sourceCleared() throws Exception {
109109
.replaceSubtree(mutableAst, newBooleanConst, mutableAst.expr().id())
110110
.toParsedAst();
111111

112-
assertThat(mutatedAst.getSource().getDescription()).isEmpty();
113112
assertThat(mutatedAst.getSource().getLineOffsets()).isEmpty();
114113
assertThat(mutatedAst.getSource().getPositionsMap()).isEmpty();
115114
assertThat(mutatedAst.getSource().getExtensions()).isEmpty();
116115
assertThat(mutatedAst.getSource().getMacroCalls()).isEmpty();
116+
assertThat(mutatedAst.getSource().getDescription()).isEqualTo(ast.getSource().getDescription());
117117
}
118118

119119
@Test
@@ -125,11 +125,11 @@ public void astMutator_macro_sourceMacroCallsPopulated() throws Exception {
125125
CelAbstractSyntaxTree mutatedAst =
126126
AST_MUTATOR.replaceSubtree(mutableAst, newBooleanConst, 1).toParsedAst(); // no_op
127127

128-
assertThat(mutatedAst.getSource().getDescription()).isEmpty();
129128
assertThat(mutatedAst.getSource().getLineOffsets()).isEmpty();
130129
assertThat(mutatedAst.getSource().getPositionsMap()).isEmpty();
131130
assertThat(mutatedAst.getSource().getExtensions()).isEmpty();
132131
assertThat(mutatedAst.getSource().getMacroCalls()).isNotEmpty();
132+
assertThat(mutatedAst.getSource().getDescription()).isEqualTo(ast.getSource().getDescription());
133133
}
134134

135135
@Test

0 commit comments

Comments
 (0)