Skip to content

Commit

Permalink
Remove modifier create from CelCreateList, CelCreateMap and CelCreate…
Browse files Browse the repository at this point in the history
…Struct

PiperOrigin-RevId: 625741276
  • Loading branch information
l46kok authored and copybara-github committed Apr 17, 2024
1 parent 141dda8 commit 0b81d13
Show file tree
Hide file tree
Showing 70 changed files with 2,420 additions and 2,287 deletions.
6 changes: 3 additions & 3 deletions bundle/src/test/java/dev/cel/bundle/CelImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
import dev.cel.common.CelValidationResult;
import dev.cel.common.CelVarDecl;
import dev.cel.common.ast.CelExpr;
import dev.cel.common.ast.CelExpr.CelCreateList;
import dev.cel.common.ast.CelExpr.CelList;
import dev.cel.common.testing.RepeatedTestProvider;
import dev.cel.common.types.CelKind;
import dev.cel.common.types.CelType;
Expand Down Expand Up @@ -483,9 +483,9 @@ public void compile_withOptionalTypes() throws Exception {

CelAbstractSyntaxTree ast = cel.compile("[?a]").getAst();

CelCreateList createList = ast.getExpr().createList();
CelList createList = ast.getExpr().createList();
assertThat(createList.optionalIndices()).containsExactly(0);
assertThat(createList.elements()).containsExactly(CelExpr.ofIdentExpr(2, "a"));
assertThat(createList.elements()).containsExactly(CelExpr.ofIdent(2, "a"));
}

@Test
Expand Down
37 changes: 18 additions & 19 deletions checker/src/main/java/dev/cel/checker/ExprChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,11 @@ public CelExpr visit(CelExpr expr) {
return visit(expr, expr.select());
case CALL:
return visit(expr, expr.call());
case CREATE_LIST:
case LIST:
return visit(expr, expr.createList());
case CREATE_STRUCT:
case STRUCT:
return visit(expr, expr.createStruct());
case CREATE_MAP:
case MAP:
return visit(expr, expr.createMap());
case COMPREHENSION:
return visit(expr, expr.comprehension());
Expand Down Expand Up @@ -355,7 +355,7 @@ private CelExpr visit(CelExpr expr, CelExpr.CelCall call) {
}

@CheckReturnValue
private CelExpr visit(CelExpr expr, CelExpr.CelCreateStruct createStruct) {
private CelExpr visit(CelExpr expr, CelExpr.CelStruct createStruct) {
// Determine the type of the message.
CelType messageType = SimpleType.ERROR;
CelIdentDecl decl = env.lookupIdent(getPosition(expr), inContainer, createStruct.messageName());
Expand Down Expand Up @@ -383,9 +383,9 @@ private CelExpr visit(CelExpr expr, CelExpr.CelCreateStruct createStruct) {
}

// Check the field initializers.
ImmutableList<CelExpr.CelCreateStruct.Entry> entriesList = createStruct.entries();
ImmutableList<CelExpr.CelStruct.Entry> entriesList = createStruct.entries();
for (int i = 0; i < entriesList.size(); i++) {
CelExpr.CelCreateStruct.Entry entry = entriesList.get(i);
CelExpr.CelStruct.Entry entry = entriesList.get(i);
CelExpr visitedValueExpr = visit(entry.value());
if (namespacedDeclarations && !visitedValueExpr.equals(entry.value())) {
// Subtree has been rewritten. Replace the struct value.
Expand Down Expand Up @@ -414,12 +414,12 @@ private CelExpr visit(CelExpr expr, CelExpr.CelCreateStruct createStruct) {
}

@CheckReturnValue
private CelExpr visit(CelExpr expr, CelExpr.CelCreateMap createMap) {
private CelExpr visit(CelExpr expr, CelExpr.CelMap createMap) {
CelType mapKeyType = null;
CelType mapValueType = null;
ImmutableList<CelExpr.CelCreateMap.Entry> entriesList = createMap.entries();
ImmutableList<CelExpr.CelMap.Entry> entriesList = createMap.entries();
for (int i = 0; i < entriesList.size(); i++) {
CelExpr.CelCreateMap.Entry entry = entriesList.get(i);
CelExpr.CelMap.Entry entry = entriesList.get(i);
CelExpr visitedMapKeyExpr = visit(entry.key());
if (namespacedDeclarations && !visitedMapKeyExpr.equals(entry.key())) {
// Subtree has been rewritten. Replace the map key.
Expand Down Expand Up @@ -455,7 +455,7 @@ private CelExpr visit(CelExpr expr, CelExpr.CelCreateMap createMap) {
}

@CheckReturnValue
private CelExpr visit(CelExpr expr, CelExpr.CelCreateList createList) {
private CelExpr visit(CelExpr expr, CelExpr.CelList createList) {
CelType elemsType = null;
ImmutableList<CelExpr> elementsList = createList.elements();
HashSet<Integer> optionalIndices = new HashSet<>(createList.optionalIndices());
Expand Down Expand Up @@ -797,7 +797,7 @@ private int getPosition(CelExpr expr) {
return pos == null ? 0 : pos;
}

private int getPosition(CelExpr.CelCreateStruct.Entry entry) {
private int getPosition(CelExpr.CelStruct.Entry entry) {
Integer pos = positionMap.get(entry.id());
return pos == null ? 0 : pos;
}
Expand Down Expand Up @@ -848,30 +848,29 @@ private static CelExpr replaceCallSubtree(CelExpr expr, CelExpr target) {
}

private static CelExpr replaceListElementSubtree(CelExpr expr, CelExpr element, int index) {
CelExpr.CelCreateList newList =
expr.createList().toBuilder().setElement(index, element).build();
CelExpr.CelList newList = expr.createList().toBuilder().setElement(index, element).build();
return expr.toBuilder().setCreateList(newList).build();
}

private static CelExpr replaceStructEntryValueSubtree(CelExpr expr, CelExpr newValue, int index) {
CelExpr.CelCreateStruct createStruct = expr.createStruct();
CelExpr.CelCreateStruct.Entry newEntry =
CelExpr.CelStruct createStruct = expr.createStruct();
CelExpr.CelStruct.Entry newEntry =
createStruct.entries().get(index).toBuilder().setValue(newValue).build();
createStruct = createStruct.toBuilder().setEntry(index, newEntry).build();
return expr.toBuilder().setCreateStruct(createStruct).build();
}

private static CelExpr replaceMapEntryKeySubtree(CelExpr expr, CelExpr newKey, int index) {
CelExpr.CelCreateMap createMap = expr.createMap();
CelExpr.CelCreateMap.Entry newEntry =
CelExpr.CelMap createMap = expr.createMap();
CelExpr.CelMap.Entry newEntry =
createMap.entries().get(index).toBuilder().setKey(newKey).build();
createMap = createMap.toBuilder().setEntry(index, newEntry).build();
return expr.toBuilder().setCreateMap(createMap).build();
}

private static CelExpr replaceMapEntryValueSubtree(CelExpr expr, CelExpr newValue, int index) {
CelExpr.CelCreateMap createMap = expr.createMap();
CelExpr.CelCreateMap.Entry newEntry =
CelExpr.CelMap createMap = expr.createMap();
CelExpr.CelMap.Entry newEntry =
createMap.entries().get(index).toBuilder().setValue(newValue).build();
createMap = createMap.toBuilder().setEntry(index, newEntry).build();
return expr.toBuilder().setCreateMap(createMap).build();
Expand Down
10 changes: 10 additions & 0 deletions common/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ java_library(
exports = ["//common/src/main/java/dev/cel/common:error_codes"],
)

java_library(
name = "mutable_ast",
exports = ["//common/src/main/java/dev/cel/common:mutable_ast"],
)

java_library(
name = "mutable_source",
exports = ["//common/src/main/java/dev/cel/common:mutable_source"],
)

java_library(
name = "runtime_exception",
visibility = ["//visibility:public"],
Expand Down
4 changes: 2 additions & 2 deletions common/ast/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ java_library(
)

java_library(
name = "mutable_ast",
exports = ["//common/src/main/java/dev/cel/common/ast:mutable_ast"],
name = "mutable_expr",
exports = ["//common/src/main/java/dev/cel/common/ast:mutable_expr"],
)
28 changes: 28 additions & 0 deletions common/src/main/java/dev/cel/common/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,31 @@ java_library(
"//common/annotations",
],
)

java_library(
name = "mutable_ast",
srcs = ["CelMutableAst.java"],
tags = [
],
deps = [
":mutable_source",
"//common",
"//common/ast",
"//common/ast:mutable_expr",
"//common/types:type_providers",
],
)

java_library(
name = "mutable_source",
srcs = ["CelMutableSource.java"],
tags = [
],
deps = [
":common",
"//:auto_value",
"//common/ast:mutable_expr",
"@maven//:com_google_errorprone_error_prone_annotations",
"@maven//:com_google_guava_guava",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package dev.cel.common.ast;
package dev.cel.common;

import dev.cel.common.CelAbstractSyntaxTree;
import dev.cel.common.CelSource;
import dev.cel.common.ast.CelMutableExpr;
import dev.cel.common.ast.CelMutableExprConverter;
import dev.cel.common.ast.CelReference;
import dev.cel.common.types.CelType;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -30,7 +31,7 @@
*/
public final class CelMutableAst {
private final CelMutableExpr mutatedExpr;
private final CelSource.Builder source;
private final CelMutableSource source;
private final Map<Long, CelReference> references;
private final Map<Long, CelType> types;

Expand All @@ -40,9 +41,10 @@ public CelMutableExpr expr() {
}

/**
* Returns the {@link CelSource} that was used during construction of the abstract syntax tree.
* Returns the {@link CelMutableSource} that was used during construction of the abstract syntax
* tree.
*/
public CelSource.Builder source() {
public CelMutableSource source() {
return source;
}

Expand All @@ -69,7 +71,7 @@ public Optional<CelType> getType(long exprId) {
/** Converts this mutable AST into a parsed {@link CelAbstractSyntaxTree}. */
public CelAbstractSyntaxTree toParsedAst() {
return CelAbstractSyntaxTree.newParsedAst(
CelMutableExprConverter.fromMutableExpr(mutatedExpr), source.build());
CelMutableExprConverter.fromMutableExpr(mutatedExpr), source.toCelSource());
}

/**
Expand All @@ -79,7 +81,7 @@ public CelAbstractSyntaxTree toParsedAst() {
public static CelMutableAst fromCelAst(CelAbstractSyntaxTree ast) {
return new CelMutableAst(
CelMutableExprConverter.fromCelExpr(ast.getExpr()),
ast.getSource().toBuilder(),
CelMutableSource.fromCelSource(ast.getSource()),
ast.getReferenceMap(),
ast.getTypeMap());
}
Expand All @@ -88,21 +90,21 @@ public static CelMutableAst fromCelAst(CelAbstractSyntaxTree ast) {
* Constructs an instance of {@link CelMutableAst} with the mutable expression and its source
* builder.
*/
public static CelMutableAst of(CelMutableExpr mutableExpr, CelSource.Builder sourceBuilder) {
return new CelMutableAst(mutableExpr, sourceBuilder);
public static CelMutableAst of(CelMutableExpr mutableExpr, CelMutableSource mutableSource) {
return new CelMutableAst(mutableExpr, mutableSource);
}

private CelMutableAst(CelMutableExpr mutatedExpr, CelSource.Builder source) {
this(mutatedExpr, source, new HashMap<>(), new HashMap<>());
private CelMutableAst(CelMutableExpr mutatedExpr, CelMutableSource mutableSource) {
this(mutatedExpr, mutableSource, new HashMap<>(), new HashMap<>());
}

private CelMutableAst(
CelMutableExpr mutatedExpr,
CelSource.Builder source,
CelMutableSource mutableSource,
Map<Long, CelReference> references,
Map<Long, CelType> types) {
this.mutatedExpr = mutatedExpr;
this.source = source;
this.source = mutableSource;
this.references = new HashMap<>(references);
this.types = new HashMap<>(types);
}
Expand Down
114 changes: 114 additions & 0 deletions common/src/main/java/dev/cel/common/CelMutableSource.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// Copyright 2024 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;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.ImmutableMap.toImmutableMap;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import dev.cel.common.CelSource.Extension;
import dev.cel.common.ast.CelMutableExpr;
import dev.cel.common.ast.CelMutableExprConverter;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Represents the mutable portion of the {@link CelSource}. This is intended for the purposes of
* augmenting an AST through CEL optimizers.
*/
public final class CelMutableSource {

final Map<Long, CelMutableExpr> macroCalls;
final Set<Extension> extensions;

@CanIgnoreReturnValue
public CelMutableSource addMacroCalls(long exprId, CelMutableExpr expr) {
this.macroCalls.put(exprId, checkNotNull(CelMutableExpr.newInstance(expr)));
return this;
}

@CanIgnoreReturnValue
public CelMutableSource addAllMacroCalls(Map<Long, CelMutableExpr> macroCalls) {
this.macroCalls.putAll(macroCalls);
return this;
}

@CanIgnoreReturnValue
public CelMutableSource addAllExtensions(Collection<? extends Extension> extensions) {
checkNotNull(extensions);
this.extensions.addAll(extensions);
return this;
}

@CanIgnoreReturnValue
public CelMutableSource clearMacroCall(long exprId) {
this.macroCalls.remove(exprId);
return this;
}

@CanIgnoreReturnValue
public CelMutableSource clearMacroCalls() {
this.macroCalls.clear();
return this;
}

public Map<Long, CelMutableExpr> getMacroCalls() {
return macroCalls;
}

public Set<Extension> getExtensions() {
return extensions;
}

public CelSource toCelSource() {
return CelSource.newBuilder()
.addAllExtensions(extensions)
.addAllMacroCalls(
macroCalls.entrySet().stream()
.collect(
toImmutableMap(
Entry::getKey, v -> CelMutableExprConverter.fromMutableExpr(v.getValue()))))
.build();
}

public static CelMutableSource newInstance() {
return new CelMutableSource(new HashMap<>(), new HashSet<>());
}

public static CelMutableSource fromCelSource(CelSource source) {
return new CelMutableSource(
source.getMacroCalls().entrySet().stream()
.collect(
Collectors.toMap(
Entry::getKey,
v -> CelMutableExprConverter.fromCelExpr(v.getValue()),
(prev, next) -> {
throw new IllegalStateException(
"Unexpected source collision at ID: " + prev.id());
},
HashMap::new)),
source.getExtensions());
}

CelMutableSource(Map<Long, CelMutableExpr> macroCalls, Set<Extension> extensions) {
this.macroCalls = checkNotNull(macroCalls);
this.extensions = checkNotNull(extensions);
}
}
4 changes: 4 additions & 0 deletions common/src/main/java/dev/cel/common/CelSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,10 @@ public Builder clearMacroCall(long exprId) {
return this;
}

public ImmutableSet<Extension> getExtensions() {
return extensions.build();
}

/**
* Adds one or more {@link Extension}s to the source information. Extensions implement set
* semantics and deduped if same ones are provided.
Expand Down
Loading

0 comments on commit 0b81d13

Please sign in to comment.