diff --git a/common/ast/BUILD.bazel b/common/ast/BUILD.bazel index 39d2b5fd7..91097daf4 100644 --- a/common/ast/BUILD.bazel +++ b/common/ast/BUILD.bazel @@ -22,3 +22,13 @@ java_library( name = "cel_expr_visitor", exports = ["//common/src/main/java/dev/cel/common/ast:cel_expr_visitor"], ) + +java_library( + name = "expr_factory", + exports = ["//common/src/main/java/dev/cel/common/ast:expr_factory"], +) + +java_library( + name = "expr_util", + exports = ["//common/src/main/java/dev/cel/common/ast:expr_util"], +) diff --git a/common/navigation/BUILD.bazel b/common/navigation/BUILD.bazel index 1bccf932e..f4e757ad5 100644 --- a/common/navigation/BUILD.bazel +++ b/common/navigation/BUILD.bazel @@ -1,6 +1,6 @@ package( default_applicable_licenses = ["//:license"], - default_visibility = ["//visibility:public"], # TODO: Expose when ready + default_visibility = ["//visibility:public"], ) java_library( diff --git a/common/src/main/java/dev/cel/common/CelOptions.java b/common/src/main/java/dev/cel/common/CelOptions.java index ec13b1bd5..b4a944daa 100644 --- a/common/src/main/java/dev/cel/common/CelOptions.java +++ b/common/src/main/java/dev/cel/common/CelOptions.java @@ -301,7 +301,11 @@ public abstract static class Builder { * checker will implicitly coerce them to type dyn. * *

This flag is recommended for all new uses of CEL. + * + * @deprecated Use standalone {@code dev.cel.validators.validator.HomogeneousLiteralValidator} + * instead. */ + @Deprecated public abstract Builder enableHomogeneousLiterals(boolean value); /** diff --git a/common/src/main/java/dev/cel/common/ast/BUILD.bazel b/common/src/main/java/dev/cel/common/ast/BUILD.bazel index bb93ebdfb..62af57e69 100644 --- a/common/src/main/java/dev/cel/common/ast/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/ast/BUILD.bazel @@ -73,3 +73,32 @@ java_library( "//common", ], ) + +java_library( + name = "expr_factory", + srcs = ["CelExprFactory.java"], + tags = [ + ], + deps = [ + ":ast", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven//:com_google_guava_guava", + "@maven//:com_google_protobuf_protobuf_java", + ], +) + +java_library( + name = "expr_util", + srcs = ["CelExprUtil.java"], + tags = [ + ], + deps = [ + ":ast", + "//bundle:cel", + "//common", + "//common:compiler_common", + "//runtime", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven//:com_google_guava_guava", + ], +) diff --git a/parser/src/main/java/dev/cel/parser/CelExprFactory.java b/common/src/main/java/dev/cel/common/ast/CelExprFactory.java similarity index 83% rename from parser/src/main/java/dev/cel/parser/CelExprFactory.java rename to common/src/main/java/dev/cel/common/ast/CelExprFactory.java index 433e0c2bf..a930852a7 100644 --- a/parser/src/main/java/dev/cel/parser/CelExprFactory.java +++ b/common/src/main/java/dev/cel/common/ast/CelExprFactory.java @@ -1,4 +1,4 @@ -// Copyright 2022 Google LLC +// Copyright 2023 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,44 +12,59 @@ // See the License for the specific language governing permissions and // limitations under the License. -package dev.cel.parser; +package dev.cel.common.ast; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Strings.isNullOrEmpty; +import com.google.common.base.Preconditions; import com.google.common.primitives.UnsignedLong; -import com.google.errorprone.annotations.FormatMethod; -import com.google.errorprone.annotations.FormatString; +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.errorprone.annotations.CheckReturnValue; import com.google.protobuf.ByteString; -import dev.cel.common.CelIssue; -import dev.cel.common.CelSourceLocation; -import dev.cel.common.ast.CelConstant; -import dev.cel.common.ast.CelExpr; import java.util.Arrays; -/** - * Assists with the expansion of {@link CelMacro} in a manner which is consistent with the source - * position and expression ID generation code leveraged by both the parser and type-checker. - */ -public abstract class CelExprFactory { +/** Factory for generating expression nodes. */ +public class CelExprFactory { + private long exprId = 0L; - // Package-private default constructor to prevent extensions outside of the codebase. - CelExprFactory() {} + /** Builder for configuring {@link CelExprFactory}. */ + public static final class Builder { + private long startingExprId = 0L; + + @CanIgnoreReturnValue + public Builder setStartingExpressionId(long exprId) { + Preconditions.checkArgument(exprId > 0); + startingExprId = exprId; + return this; + } + + @CheckReturnValue + public CelExprFactory build() { + return new CelExprFactory(startingExprId); + } + + private Builder() {} + } + + /** Creates a new builder to configure CelExprFactory. */ + public static CelExprFactory.Builder newBuilder() { + return new Builder(); + } + + /** Create a new constant expression. */ + public final CelExpr newConstant(CelConstant constant) { + return CelExpr.newBuilder().setId(nextExprId()).setConstant(constant).build(); + } /** Creates a new constant {@link CelExpr} for a bool value. */ public final CelExpr newBoolLiteral(boolean value) { - return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) - .setConstant(CelConstant.ofValue(value)) - .build(); + return newConstant(CelConstant.ofValue(value)); } /** Creates a new constant {@link CelExpr} for a bytes value. */ public final CelExpr newBytesLiteral(ByteString value) { - return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) - .setConstant(CelConstant.ofValue(value)) - .build(); + return newConstant(CelConstant.ofValue(value)); } /** Creates a new constant {@link CelExpr} for a bytes value. */ @@ -69,34 +84,22 @@ public final CelExpr newBytesLiteral(String value) { /** Creates a new constant {@link CelExpr} for a double value. */ public final CelExpr newDoubleLiteral(double value) { - return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) - .setConstant(CelConstant.ofValue(value)) - .build(); + return newConstant(CelConstant.ofValue(value)); } /** Creates a new constant {@link CelExpr} for an int value. */ public final CelExpr newIntLiteral(long value) { - return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) - .setConstant(CelConstant.ofValue(value)) - .build(); + return newConstant(CelConstant.ofValue(value)); } /** Creates a new constant {@link CelExpr} for a string value. */ public final CelExpr newStringLiteral(String value) { - return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) - .setConstant(CelConstant.ofValue(value)) - .build(); + return newConstant(CelConstant.ofValue(value)); } /** Creates a new constant {@link CelExpr} for a uint value. */ public final CelExpr newUintLiteral(long value) { - return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) - .setConstant(CelConstant.ofValue(UnsignedLong.fromLongBits(value))) - .build(); + return newConstant(CelConstant.ofValue(UnsignedLong.fromLongBits(value))); } /** Creates a new list {@link CelExpr} comprised of the elements. */ @@ -107,7 +110,7 @@ public final CelExpr newList(CelExpr... elements) { /** Creates a new list {@link CelExpr} comprised of the elements. */ public final CelExpr newList(Iterable elements) { return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) + .setId(nextExprId()) .setCreateList(CelExpr.CelCreateList.newBuilder().addElements(elements).build()) .build(); } @@ -120,7 +123,7 @@ public final CelExpr newMap(CelExpr.CelCreateMap.Entry... entries) { /** Creates a new map {@link CelExpr} comprised of the entries. */ public final CelExpr newMap(Iterable entries) { return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) + .setId(nextExprId()) .setCreateMap(CelExpr.CelCreateMap.newBuilder().addEntries(entries).build()) .build(); } @@ -130,7 +133,7 @@ public final CelExpr newMap(Iterable entries) { */ public final CelExpr.CelCreateMap.Entry newMapEntry(CelExpr key, CelExpr value) { return CelExpr.CelCreateMap.Entry.newBuilder() - .setId(nextExprIdForMacro()) + .setId(nextExprId()) .setKey(key) .setValue(value) .build(); @@ -145,7 +148,7 @@ public final CelExpr newMessage(String typeName, CelExpr.CelCreateStruct.Entry.. public final CelExpr newMessage(String typeName, Iterable fields) { checkArgument(!isNullOrEmpty(typeName)); return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) + .setId(nextExprId()) .setCreateStruct( CelExpr.CelCreateStruct.newBuilder() .setMessageName(typeName) @@ -161,7 +164,7 @@ public final CelExpr newMessage(String typeName, Iterable arguments) { checkArgument(!isNullOrEmpty(function)); return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) + .setId(nextExprId()) .setCall(CelExpr.CelCall.newBuilder().setFunction(function).addArgs(arguments).build()) .build(); } @@ -534,7 +537,7 @@ public final CelExpr newReceiverCall( String function, CelExpr target, Iterable arguments) { checkArgument(!isNullOrEmpty(function)); return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) + .setId(nextExprId()) .setCall( CelExpr.CelCall.newBuilder() .setFunction(function) @@ -551,7 +554,7 @@ public final CelExpr newReceiverCall( public final CelExpr newSelect(CelExpr operand, String field, boolean testOnly) { checkArgument(!isNullOrEmpty(field)); return CelExpr.newBuilder() - .setId(nextExprIdForMacro()) + .setId(nextExprId()) .setSelect( CelExpr.CelSelect.newBuilder() .setOperand(operand) @@ -561,38 +564,14 @@ public final CelExpr newSelect(CelExpr operand, String field, boolean testOnly) .build(); } - /** Retrieves the source location for the given {@link CelExpr} ID. */ - public final CelSourceLocation getSourceLocation(CelExpr expr) { - return getSourceLocation(expr.id()); + /** Returns the next unique expression ID. */ + protected long nextExprId() { + return ++exprId; } - /** Retrieves the source location for the given {@link CelExpr} ID. */ - protected abstract CelSourceLocation getSourceLocation(long exprId); - - /** - * Creates a {@link CelIssue} and reports it, returning a sentinel {@link CelExpr} that indicates - * an error. - */ - @FormatMethod - public final CelExpr reportError(@FormatString String format, Object... args) { - return reportError( - CelIssue.formatError(currentSourceLocationForMacro(), String.format(format, args))); - } + protected CelExprFactory() {} - /** - * Creates a {@link CelIssue} and reports it, returning a sentinel {@link CelExpr} that indicates - * an error. - */ - public final CelExpr reportError(String message) { - return reportError(CelIssue.formatError(currentSourceLocationForMacro(), message)); + private CelExprFactory(long exprId) { + this.exprId = exprId; } - - /** Reports a {@link CelIssue} and returns a sentinel {@link CelExpr} that indicates an error. */ - public abstract CelExpr reportError(CelIssue error); - - /** Returns the next unique expression ID. This should only be used for macros. */ - protected abstract long nextExprIdForMacro(); - - /** Returns the current (last known) source location. This should only be used for macros. */ - protected abstract CelSourceLocation currentSourceLocationForMacro(); } diff --git a/common/src/main/java/dev/cel/common/ast/CelExprUtil.java b/common/src/main/java/dev/cel/common/ast/CelExprUtil.java new file mode 100644 index 000000000..20217128e --- /dev/null +++ b/common/src/main/java/dev/cel/common/ast/CelExprUtil.java @@ -0,0 +1,71 @@ +// Copyright 2023 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.ast; + +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import dev.cel.bundle.Cel; +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelSource; +import dev.cel.common.CelValidationException; +import dev.cel.runtime.CelEvaluationException; + +/** Utility class for working with CelExpr. */ +public final class CelExprUtil { + + /** + * Type-checks and evaluates a CelExpr. This method should be used in the context of validating or + * optimizing an AST. + * + * @return Evaluated result. + * @throws CelValidationException if CelExpr fails to type-check. + * @throws CelEvaluationException if CelExpr fails to evaluate. + */ + @CanIgnoreReturnValue + public static Object evaluateExpr(Cel cel, CelExpr expr) + throws CelValidationException, CelEvaluationException { + CelAbstractSyntaxTree ast = + CelAbstractSyntaxTree.newParsedAst(expr, CelSource.newBuilder().build()); + ast = cel.check(ast).getAst(); + + return cel.createProgram(ast).eval(); + } + + /** + * Type-checks and evaluates a CelExpr. The evaluated result is then checked to see if it's the + * expected result type. + * + *

This method should be used in the context of validating or optimizing an AST. + * + * @return Evaluated result. + * @throws CelValidationException if CelExpr fails to type-check. + * @throws CelEvaluationException if CelExpr fails to evaluate. + * @throws IllegalStateException if the evaluated result is not of type {@code + * expectedResultType}. + */ + @CanIgnoreReturnValue + public static Object evaluateExpr(Cel cel, CelExpr expr, Class expectedResultType) + throws CelValidationException, CelEvaluationException { + Object result = evaluateExpr(cel, expr); + if (!expectedResultType.isInstance(result)) { + throw new IllegalStateException( + String.format( + "Expected %s type but got %s instead", + expectedResultType.getName(), result.getClass().getName())); + } + return result; + } + + private CelExprUtil() {} +} diff --git a/common/src/test/java/dev/cel/common/ast/BUILD.bazel b/common/src/test/java/dev/cel/common/ast/BUILD.bazel index a01cbc912..0e22c429f 100644 --- a/common/src/test/java/dev/cel/common/ast/BUILD.bazel +++ b/common/src/test/java/dev/cel/common/ast/BUILD.bazel @@ -15,6 +15,7 @@ java_library( "//common/ast", "//common/ast:cel_expr_visitor", "//common/ast:expr_converter", + "//common/ast:expr_factory", "//common/ast:expr_v1alpha1_converter", "//common/resources/testdata/proto3:test_all_types_java_proto", "//common/types", diff --git a/common/src/test/java/dev/cel/common/ast/CelExprFactoryTest.java b/common/src/test/java/dev/cel/common/ast/CelExprFactoryTest.java new file mode 100644 index 000000000..6bd0898e8 --- /dev/null +++ b/common/src/test/java/dev/cel/common/ast/CelExprFactoryTest.java @@ -0,0 +1,57 @@ +// Copyright 2023 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.ast; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CelExprFactoryTest { + + @Test + public void construct_success() { + CelExprFactory exprFactory = CelExprFactory.newBuilder().build(); + + assertThat(exprFactory).isNotNull(); + } + + @Test + public void nextExprId_startingDefaultIsOne() { + CelExprFactory exprFactory = CelExprFactory.newBuilder().build(); + + assertThat(exprFactory.nextExprId()).isEqualTo(1L); + assertThat(exprFactory.nextExprId()).isEqualTo(2L); + } + + @Test + public void construct_withStartingExprId_throwsIfIdIsZero() { + assertThrows( + IllegalArgumentException.class, + () -> CelExprFactory.newBuilder().setStartingExpressionId(0L)); + } + + @Test + public void construct_withStartingExprId_success() { + CelExprFactory exprFactory = CelExprFactory.newBuilder().setStartingExpressionId(3L).build(); + + assertThat(exprFactory).isNotNull(); + assertThat(exprFactory.nextExprId()).isEqualTo(4L); + assertThat(exprFactory.nextExprId()).isEqualTo(5L); + } +} diff --git a/extensions/src/main/java/dev/cel/extensions/CelBindingsExtensions.java b/extensions/src/main/java/dev/cel/extensions/CelBindingsExtensions.java index a50648798..f05c79c6c 100644 --- a/extensions/src/main/java/dev/cel/extensions/CelBindingsExtensions.java +++ b/extensions/src/main/java/dev/cel/extensions/CelBindingsExtensions.java @@ -22,8 +22,8 @@ import dev.cel.common.CelIssue; import dev.cel.common.ast.CelExpr; import dev.cel.compiler.CelCompilerLibrary; -import dev.cel.parser.CelExprFactory; import dev.cel.parser.CelMacro; +import dev.cel.parser.CelMacroExprFactory; import dev.cel.parser.CelParserBuilder; import java.util.Optional; @@ -45,7 +45,7 @@ public void setParserOptions(CelParserBuilder parserBuilder) { * variable to be used in the subsequent result expression. */ private static Optional expandBind( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(target); if (!isTargetInNamespace(target)) { // Return empty to indicate that we're not interested in expanding this macro, and diff --git a/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java b/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java index 135b2c75c..7326cabf9 100644 --- a/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java +++ b/extensions/src/main/java/dev/cel/extensions/CelMathExtensions.java @@ -34,8 +34,8 @@ import dev.cel.common.types.ListType; import dev.cel.common.types.SimpleType; import dev.cel.compiler.CelCompilerLibrary; -import dev.cel.parser.CelExprFactory; import dev.cel.parser.CelMacro; +import dev.cel.parser.CelMacroExprFactory; import dev.cel.parser.CelParserBuilder; import dev.cel.runtime.CelRuntime; import dev.cel.runtime.CelRuntimeBuilder; @@ -390,7 +390,7 @@ public void setRuntimeOptions(CelRuntimeBuilder runtimeBuilder) { } private static Optional expandGreatestMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { if (!isTargetInNamespace(target)) { // Return empty to indicate that we're not interested in expanding this macro, and // that the parser should default to a function call on the receiver. @@ -469,7 +469,7 @@ private static Comparable minList(List list) { } private static Optional expandLeastMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { if (!isTargetInNamespace(target)) { // Return empty to indicate that we're not interested in expanding this macro, and // that the parser should default to a function call on the receiver. @@ -511,7 +511,7 @@ private static boolean isTargetInNamespace(CelExpr target) { } private static Optional checkInvalidArgument( - CelExprFactory exprFactory, String functionName, List arguments) { + CelMacroExprFactory exprFactory, String functionName, List arguments) { for (CelExpr arg : arguments) { if (!isArgumentValidType(arg)) { @@ -525,7 +525,7 @@ private static Optional checkInvalidArgument( } private static Optional checkInvalidArgumentSingleArg( - CelExprFactory exprFactory, String functionName, CelExpr argument) { + CelMacroExprFactory exprFactory, String functionName, CelExpr argument) { if (argument.exprKind().getKind() == Kind.CREATE_LIST) { if (argument.createList().elements().isEmpty()) { return newError( @@ -558,7 +558,7 @@ private static boolean isArgumentValidType(CelExpr argument) { } private static Optional newError( - CelExprFactory exprFactory, String errorMessage, CelExpr argument) { + CelMacroExprFactory exprFactory, String errorMessage, CelExpr argument) { return Optional.of( exprFactory.reportError( CelIssue.formatError(exprFactory.getSourceLocation(argument), errorMessage))); diff --git a/extensions/src/main/java/dev/cel/extensions/CelOptionalLibrary.java b/extensions/src/main/java/dev/cel/extensions/CelOptionalLibrary.java index 1529d95ac..96796cde3 100644 --- a/extensions/src/main/java/dev/cel/extensions/CelOptionalLibrary.java +++ b/extensions/src/main/java/dev/cel/extensions/CelOptionalLibrary.java @@ -35,8 +35,8 @@ import dev.cel.common.types.SimpleType; import dev.cel.common.types.TypeParamType; import dev.cel.compiler.CelCompilerLibrary; -import dev.cel.parser.CelExprFactory; import dev.cel.parser.CelMacro; +import dev.cel.parser.CelMacroExprFactory; import dev.cel.parser.CelParserBuilder; import dev.cel.parser.Operator; import dev.cel.runtime.CelRuntime; @@ -199,7 +199,7 @@ private static boolean isZeroValue(Object val) { } private static Optional expandOptMap( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 2); @@ -233,7 +233,7 @@ private static Optional expandOptMap( } private static Optional expandOptFlatMap( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 2); diff --git a/extensions/src/main/java/dev/cel/extensions/CelProtoExtensions.java b/extensions/src/main/java/dev/cel/extensions/CelProtoExtensions.java index 960f36f58..bb932bc41 100644 --- a/extensions/src/main/java/dev/cel/extensions/CelProtoExtensions.java +++ b/extensions/src/main/java/dev/cel/extensions/CelProtoExtensions.java @@ -23,8 +23,8 @@ import dev.cel.common.ast.CelExpr; import dev.cel.common.internal.Constants; import dev.cel.compiler.CelCompilerLibrary; -import dev.cel.parser.CelExprFactory; import dev.cel.parser.CelMacro; +import dev.cel.parser.CelMacroExprFactory; import dev.cel.parser.CelParserBuilder; import java.util.Optional; @@ -43,17 +43,17 @@ public void setParserOptions(CelParserBuilder parserBuilder) { } private static Optional expandHasProtoExt( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { return expandProtoExt(exprFactory, target, arguments, true); } private static Optional expandGetProtoExt( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { return expandProtoExt(exprFactory, target, arguments, false); } private static Optional expandProtoExt( - CelExprFactory exprFactory, + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments, boolean testOnly) { diff --git a/parser/src/main/java/dev/cel/parser/BUILD.bazel b/parser/src/main/java/dev/cel/parser/BUILD.bazel index f4c5c70dd..d9d826c49 100644 --- a/parser/src/main/java/dev/cel/parser/BUILD.bazel +++ b/parser/src/main/java/dev/cel/parser/BUILD.bazel @@ -25,9 +25,9 @@ PARSER_BUILDER_SOURCES = [ # keep sorted MACRO_SOURCES = [ - "CelExprFactory.java", "CelMacro.java", "CelMacroExpander.java", + "CelMacroExprFactory.java", "CelStandardMacro.java", ] @@ -84,9 +84,9 @@ java_library( "//common", "//common:compiler_common", "//common/ast", + "//common/ast:expr_factory", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", - "@maven//:com_google_protobuf_protobuf_java", "@maven//:org_jspecify_jspecify", ], ) diff --git a/parser/src/main/java/dev/cel/parser/CelMacro.java b/parser/src/main/java/dev/cel/parser/CelMacro.java index bc1718a77..2e7fd1b9e 100644 --- a/parser/src/main/java/dev/cel/parser/CelMacro.java +++ b/parser/src/main/java/dev/cel/parser/CelMacro.java @@ -244,7 +244,7 @@ abstract static class Builder { // CelMacroExpander implementation for CEL's has() macro. private static Optional expandHasMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 1); @@ -257,7 +257,7 @@ private static Optional expandHasMacro( // CelMacroExpander implementation for CEL's all() macro. private static Optional expandAllMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 2); @@ -281,7 +281,7 @@ private static Optional expandAllMacro( // CelMacroExpander implementation for CEL's exists() macro. private static Optional expandExistsMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 2); @@ -307,7 +307,7 @@ private static Optional expandExistsMacro( // CelMacroExpander implementation for CEL's exists_one() macro. private static Optional expandExistsOneMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 2); @@ -337,7 +337,7 @@ private static Optional expandExistsOneMacro( // CelMacroExpander implementation for CEL's map() macro. private static Optional expandMapMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 2 || arguments.size() == 3); @@ -372,7 +372,7 @@ private static Optional expandMapMacro( // CelMacroExpander implementation for CEL's filter() macro. private static Optional expandFilterMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments) { + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments) { checkNotNull(exprFactory); checkNotNull(target); checkArgument(arguments.size() == 2); @@ -392,7 +392,7 @@ private static Optional expandFilterMacro( arg0.ident().name(), target, ACCUMULATOR_VAR, accuInit, condition, step, accuIdent)); } - private static CelExpr reportArgumentError(CelExprFactory exprFactory, CelExpr argument) { + private static CelExpr reportArgumentError(CelMacroExprFactory exprFactory, CelExpr argument) { return exprFactory.reportError( CelIssue.formatError( exprFactory.getSourceLocation(argument), "The argument must be a simple name")); diff --git a/parser/src/main/java/dev/cel/parser/CelMacroExpander.java b/parser/src/main/java/dev/cel/parser/CelMacroExpander.java index 2e0d1d833..2b41f3629 100644 --- a/parser/src/main/java/dev/cel/parser/CelMacroExpander.java +++ b/parser/src/main/java/dev/cel/parser/CelMacroExpander.java @@ -35,5 +35,5 @@ public interface CelMacroExpander { * that an expansion is not needed. */ Optional expandMacro( - CelExprFactory exprFactory, CelExpr target, ImmutableList arguments); + CelMacroExprFactory exprFactory, CelExpr target, ImmutableList arguments); } diff --git a/parser/src/main/java/dev/cel/parser/CelMacroExprFactory.java b/parser/src/main/java/dev/cel/parser/CelMacroExprFactory.java new file mode 100644 index 000000000..9a01d7968 --- /dev/null +++ b/parser/src/main/java/dev/cel/parser/CelMacroExprFactory.java @@ -0,0 +1,64 @@ +// Copyright 2022 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.parser; + +import com.google.errorprone.annotations.FormatMethod; +import com.google.errorprone.annotations.FormatString; +import dev.cel.common.CelIssue; +import dev.cel.common.CelSourceLocation; +import dev.cel.common.ast.CelExpr; +import dev.cel.common.ast.CelExprFactory; + +/** + * Assists with the expansion of {@link CelMacro} in a manner which is consistent with the source + * position and expression ID generation code leveraged by both the parser and type-checker. + */ +public abstract class CelMacroExprFactory extends CelExprFactory { + + // Package-private default constructor to prevent extensions outside of the codebase. + CelMacroExprFactory() {} + + /** + * Creates a {@link CelIssue} and reports it, returning a sentinel {@link CelExpr} that indicates + * an error. + */ + @FormatMethod + public final CelExpr reportError(@FormatString String format, Object... args) { + return reportError( + CelIssue.formatError(currentSourceLocationForMacro(), String.format(format, args))); + } + + /** + * Creates a {@link CelIssue} and reports it, returning a sentinel {@link CelExpr} that indicates + * an error. + */ + public final CelExpr reportError(String message) { + return reportError(CelIssue.formatError(currentSourceLocationForMacro(), message)); + } + + /** Reports a {@link CelIssue} and returns a sentinel {@link CelExpr} that indicates an error. */ + public abstract CelExpr reportError(CelIssue error); + + /** Retrieves the source location for the given {@link CelExpr} ID. */ + public final CelSourceLocation getSourceLocation(CelExpr expr) { + return getSourceLocation(expr.id()); + } + + /** Retrieves the source location for the given {@link CelExpr} ID. */ + protected abstract CelSourceLocation getSourceLocation(long exprId); + + /** Returns the current (last known) source location. This should only be used for macros. */ + protected abstract CelSourceLocation currentSourceLocationForMacro(); +} diff --git a/parser/src/main/java/dev/cel/parser/Parser.java b/parser/src/main/java/dev/cel/parser/Parser.java index 07512ba2f..bb1dbea9e 100644 --- a/parser/src/main/java/dev/cel/parser/Parser.java +++ b/parser/src/main/java/dev/cel/parser/Parser.java @@ -1024,21 +1024,19 @@ private ParseTree unnest(ParseTree tree) { return tree; } - /** Implementation of {@link CelExprFactory}. */ - private static final class ExprFactory extends CelExprFactory { + /** Implementation of {@link CelMacroExprFactory}. */ + private static final class ExprFactory extends CelMacroExprFactory { private final org.antlr.v4.runtime.Parser recognizer; private final CelSource.Builder sourceInfo; private final ArrayList issues; private final ArrayDeque positions; - private long exprId; private ExprFactory(org.antlr.v4.runtime.Parser recognizer, CelSource.Builder sourceInfo) { this.recognizer = recognizer; this.sourceInfo = sourceInfo; issues = new ArrayList<>(); positions = new ArrayDeque<>(1); // Currently this usually contains at most 1 position. - exprId = 0L; } // Implementation of CelExprFactory. @@ -1082,13 +1080,6 @@ private CelExpr reportError(Token token, String message) { // Implementation of CelExprFactory. - @Override - public long nextExprIdForMacro() { - checkState(!positions.isEmpty()); // Should only be called while expanding macros. - // Do not call this method directly from within the parser, use nextExprId(int). - return nextExprId(peekPosition()); - } - @Override protected CelSourceLocation currentSourceLocationForMacro() { checkState(!positions.isEmpty()); // Should only be called while expanding macros. @@ -1112,13 +1103,20 @@ private int peekPosition() { } private long nextExprId(int position) { - long exprId = ++this.exprId; + long exprId = super.nextExprId(); if (position != -1) { sourceInfo.addPositions(exprId, position); } return exprId; } + @Override + public long nextExprId() { + checkState(!positions.isEmpty()); // Should only be called while expanding macros. + // Do not call this method directly from within the parser, use nextExprId(int). + return nextExprId(peekPosition()); + } + private List getIssuesList() { return issues; } diff --git a/parser/src/test/java/dev/cel/parser/CelExprFactoryTest.java b/parser/src/test/java/dev/cel/parser/CelMacroExprFactoryTest.java similarity index 98% rename from parser/src/test/java/dev/cel/parser/CelExprFactoryTest.java rename to parser/src/test/java/dev/cel/parser/CelMacroExprFactoryTest.java index 7f24e2fc3..68ac5e662 100644 --- a/parser/src/test/java/dev/cel/parser/CelExprFactoryTest.java +++ b/parser/src/test/java/dev/cel/parser/CelMacroExprFactoryTest.java @@ -32,9 +32,9 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public final class CelExprFactoryTest { +public final class CelMacroExprFactoryTest { - private static final class TestCelExprFactory extends CelExprFactory { + private static final class TestCelExprFactory extends CelMacroExprFactory { private long exprId; @@ -43,7 +43,7 @@ private TestCelExprFactory() { } @Override - public long nextExprIdForMacro() { + public long nextExprId() { return ++exprId; } @@ -54,7 +54,7 @@ protected CelSourceLocation currentSourceLocationForMacro() { @Override public CelExpr reportError(CelIssue issue) { - return CelExpr.newBuilder().setId(nextExprIdForMacro()).setConstant(Constants.ERROR).build(); + return CelExpr.newBuilder().setId(nextExprId()).setConstant(Constants.ERROR).build(); } @Override diff --git a/validator/BUILD.bazel b/validator/BUILD.bazel index fbf29028f..a5afcbd23 100644 --- a/validator/BUILD.bazel +++ b/validator/BUILD.bazel @@ -1,6 +1,6 @@ package( default_applicable_licenses = ["//:license"], - default_visibility = ["//visibility:public"], # TODO: Expose when ready + default_visibility = ["//visibility:public"], ) java_library( diff --git a/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel b/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel index c1e8cbad0..c6e06f02f 100644 --- a/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel +++ b/validator/src/main/java/dev/cel/validator/validators/BUILD.bazel @@ -47,6 +47,25 @@ java_library( ], ) +java_library( + name = "homogeneous_literal", + srcs = [ + "HomogeneousLiteralValidator.java", + ], + tags = [ + ], + deps = [ + "//bundle:cel", + "//common", + "//common/ast", + "//common/navigation", + "//common/types:cel_types", + "//common/types:type_providers", + "//validator:ast_validator", + "@maven//:com_google_guava_guava", + ], +) + java_library( name = "literal_validator", srcs = [ @@ -57,10 +76,10 @@ java_library( visibility = ["//visibility:private"], deps = [ "//bundle:cel", - "//common", "//common/ast", + "//common/ast:expr_factory", + "//common/ast:expr_util", "//common/navigation", "//validator:ast_validator", - "@maven//:com_google_guava_guava", ], ) diff --git a/validator/src/main/java/dev/cel/validator/validators/HomogeneousLiteralValidator.java b/validator/src/main/java/dev/cel/validator/validators/HomogeneousLiteralValidator.java new file mode 100644 index 000000000..1783a9d9b --- /dev/null +++ b/validator/src/main/java/dev/cel/validator/validators/HomogeneousLiteralValidator.java @@ -0,0 +1,148 @@ +// Copyright 2023 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.validator.validators; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import dev.cel.bundle.Cel; +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.ast.CelExpr; +import dev.cel.common.ast.CelExpr.CelCreateMap; +import dev.cel.common.ast.CelExpr.ExprKind.Kind; +import dev.cel.common.navigation.CelNavigableAst; +import dev.cel.common.navigation.CelNavigableExpr; +import dev.cel.common.types.CelType; +import dev.cel.common.types.CelTypes; +import dev.cel.validator.CelAstValidator; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Optional; + +/** + * HomogeneousLiteralValidator checks that all list and map literals entries have the same types, + * i.e. no mixed list element types or mixed map key or map value types. + */ +public class HomogeneousLiteralValidator implements CelAstValidator { + private final ImmutableSet exemptFunctions; + + /** + * Construct a new instance of {@link HomogeneousLiteralValidator}. This validator will not for + * functions in {@code exemptFunctions}. + */ + public static HomogeneousLiteralValidator newInstance(Iterable exemptFunctions) { + return new HomogeneousLiteralValidator(exemptFunctions); + } + + /** + * Construct a new instance of {@link HomogeneousLiteralValidator}. This validator will not for + * functions in {@code exemptFunctions}. + */ + public static HomogeneousLiteralValidator newInstance(String... exemptFunctions) { + return newInstance(Arrays.asList(exemptFunctions)); + } + + @Override + public void validate(CelNavigableAst navigableAst, Cel cel, IssuesFactory issuesFactory) { + navigableAst + .getRoot() + .descendants() + .filter( + node -> + node.getKind().equals(Kind.CREATE_LIST) || node.getKind().equals(Kind.CREATE_MAP)) + .filter(node -> !isExemptFunction(node)) + .map(CelNavigableExpr::expr) + .forEach( + expr -> { + if (expr.exprKind().getKind().equals(Kind.CREATE_LIST)) { + validateList(navigableAst.getAst(), issuesFactory, expr); + } else if (expr.exprKind().getKind().equals(Kind.CREATE_MAP)) { + validateMap(navigableAst.getAst(), issuesFactory, expr); + } + }); + } + + private void validateList(CelAbstractSyntaxTree ast, IssuesFactory issuesFactory, CelExpr expr) { + CelType previousType = null; + HashSet optionalIndices = new HashSet<>(expr.createList().optionalIndices()); + ImmutableList elements = expr.createList().elements(); + for (int i = 0; i < elements.size(); i++) { + CelExpr element = elements.get(i); + CelType currentType = ast.getType(element.id()).get(); + if (optionalIndices.contains(i)) { + currentType = currentType.parameters().get(0); + } + + if (previousType == null) { + previousType = currentType; + continue; + } + + reportErrorIfUnassignable(issuesFactory, element.id(), previousType, currentType); + } + } + + private void validateMap(CelAbstractSyntaxTree ast, IssuesFactory issuesFactory, CelExpr expr) { + CelType previousKeyType = null; + CelType previousValueType = null; + for (CelCreateMap.Entry entry : expr.createMap().entries()) { + CelType currentKeyType = ast.getType(entry.key().id()).get(); + CelType currentValueType = ast.getType(entry.value().id()).get(); + if (entry.optionalEntry()) { + currentValueType = currentValueType.parameters().get(0); + } + + if (previousKeyType == null) { + previousKeyType = currentKeyType; + previousValueType = currentValueType; + continue; + } + + reportErrorIfUnassignable(issuesFactory, entry.id(), previousKeyType, currentKeyType); + reportErrorIfUnassignable(issuesFactory, entry.id(), previousValueType, currentValueType); + } + } + + private void reportErrorIfUnassignable( + IssuesFactory issuesFactory, long elementId, CelType previousType, CelType currentType) { + if (!previousType.isAssignableFrom(currentType)) { + issuesFactory.addError( + elementId, + String.format( + "expected type '%s' but found '%s'", + CelTypes.format(previousType), CelTypes.format(currentType))); + } + } + + private boolean isExemptFunction(CelNavigableExpr listExpr) { + Optional parent = listExpr.parent(); + while (parent.isPresent()) { + CelNavigableExpr node = parent.get(); + if (node.getKind().equals(Kind.CALL)) { + return exemptFunctions.contains(node.expr().callOrDefault().function()); + } + + if (!node.getKind().equals(Kind.CREATE_LIST) && !node.getKind().equals(Kind.CREATE_MAP)) { + break; + } + parent = node.parent(); + } + + return false; + } + + private HomogeneousLiteralValidator(Iterable exemptFunctions) { + this.exemptFunctions = ImmutableSet.copyOf(exemptFunctions); + } +} diff --git a/validator/src/main/java/dev/cel/validator/validators/LiteralValidator.java b/validator/src/main/java/dev/cel/validator/validators/LiteralValidator.java index 324483571..6183febb4 100644 --- a/validator/src/main/java/dev/cel/validator/validators/LiteralValidator.java +++ b/validator/src/main/java/dev/cel/validator/validators/LiteralValidator.java @@ -14,16 +14,14 @@ package dev.cel.validator.validators; -import com.google.common.collect.ImmutableList; import dev.cel.bundle.Cel; -import dev.cel.common.CelAbstractSyntaxTree; -import dev.cel.common.CelSource; import dev.cel.common.ast.CelExpr; import dev.cel.common.ast.CelExpr.ExprKind.Kind; +import dev.cel.common.ast.CelExprFactory; +import dev.cel.common.ast.CelExprUtil; import dev.cel.common.navigation.CelNavigableAst; import dev.cel.common.navigation.CelNavigableExpr; import dev.cel.validator.CelAstValidator; -import java.util.Optional; /** * LiteralValidator defines the common logic to handle simple validation of a literal in a function @@ -41,6 +39,8 @@ protected LiteralValidator(String functionName, Class expectedResultType) { @Override public void validate(CelNavigableAst navigableAst, Cel cel, IssuesFactory issuesFactory) { + CelExprFactory exprFactory = CelExprFactory.newBuilder().build(); + navigableAst .getRoot() .descendants() @@ -55,25 +55,9 @@ public void validate(CelNavigableAst navigableAst, Cel cel, IssuesFactory issues .forEach( expr -> { CelExpr callExpr = - CelExpr.ofCallExpr( - 1, - Optional.empty(), - functionName, - ImmutableList.of(CelExpr.ofConstantExpr(2, expr.constant()))); - - CelAbstractSyntaxTree ast = - CelAbstractSyntaxTree.newParsedAst(callExpr, CelSource.newBuilder().build()); + exprFactory.newGlobalCall(functionName, exprFactory.newConstant(expr.constant())); try { - ast = cel.check(ast).getAst(); - Object result = cel.createProgram(ast).eval(); - - if (!expectedResultType.isInstance(result)) { - throw new IllegalStateException( - String.format( - "Expected %s type but got %s instead", - expectedResultType.getName(), result.getClass().getName())); - } - + CelExprUtil.evaluateExpr(cel, callExpr, expectedResultType); } catch (Exception e) { issuesFactory.addError( expr.id(), diff --git a/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel b/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel index 321e2b759..eca252aa9 100644 --- a/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel +++ b/validator/src/test/java/dev/cel/validator/validators/BUILD.bazel @@ -13,10 +13,12 @@ java_library( "//common:compiler_common", "//common:options", "//common/types", + "//extensions:optional_library", "//runtime", "//validator", "//validator:validator_builder", "//validator/validators:duration", + "//validator/validators:homogeneous_literal", "//validator/validators:regex", "//validator/validators:timestamp", "@maven//:com_google_guava_guava", diff --git a/validator/src/test/java/dev/cel/validator/validators/HomogeneousLiteralValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/HomogeneousLiteralValidatorTest.java new file mode 100644 index 000000000..6a534097e --- /dev/null +++ b/validator/src/test/java/dev/cel/validator/validators/HomogeneousLiteralValidatorTest.java @@ -0,0 +1,257 @@ +// Copyright 2023 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.validator.validators; + +import static com.google.common.truth.Truth.assertThat; +import static dev.cel.common.CelFunctionDecl.newFunctionDeclaration; +import static dev.cel.common.CelOverloadDecl.newGlobalOverload; +import static dev.cel.common.CelOverloadDecl.newMemberOverload; + +import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import com.google.testing.junit.testparameterinjector.TestParameters; +import dev.cel.bundle.Cel; +import dev.cel.bundle.CelFactory; +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelValidationResult; +import dev.cel.common.types.SimpleType; +import dev.cel.extensions.CelOptionalLibrary; +import dev.cel.runtime.CelRuntime.CelFunctionBinding; +import dev.cel.validator.CelValidator; +import dev.cel.validator.CelValidatorFactory; +import java.util.List; +import java.util.Map; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(TestParameterInjector.class) +public class HomogeneousLiteralValidatorTest { + private static final Cel CEL = + CelFactory.standardCelBuilder() + .addCompilerLibraries(CelOptionalLibrary.INSTANCE) + .addRuntimeLibraries(CelOptionalLibrary.INSTANCE) + .build(); + + private static final CelValidator CEL_VALIDATOR = + CelValidatorFactory.standardCelValidatorBuilder(CEL) + .addAstValidators(HomogeneousLiteralValidator.newInstance()) + .build(); + + @Test + @TestParameters("{source: '[1, 2, 3]'}") + @TestParameters("{source: '[dyn(1), dyn(2), dyn(3)]'}") + @TestParameters("{source: '[''hello'', ''world'', ''test'']'}") + @TestParameters("{source: '[''hello'', ?optional.ofNonZeroValue(''''), ?optional.of('''')]'}") + @TestParameters("{source: '[?optional.ofNonZeroValue(''''), ?optional.of(''''), ''hello'']'}") + public void list_containsHomogeneousLiterals(String source) throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(source).getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isFalse(); + assertThat(result.getAllIssues()).isEmpty(); + assertThat(CEL.createProgram(ast).eval()).isInstanceOf(List.class); + } + + @Test + @TestParameters("{source: '{1: false, 2: true}'}") + @TestParameters("{source: '{''hello'': false, ''world'': true}'}") + @TestParameters("{source: '{''hello'': false, ?''world'': optional.ofNonZeroValue(true)}'}") + @TestParameters("{source: '{?''hello'': optional.ofNonZeroValue(false), ''world'': true}'}") + public void map_containsHomogeneousLiterals(String source) throws Exception { + CelAbstractSyntaxTree ast = CEL.compile(source).getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isFalse(); + assertThat(result.getAllIssues()).isEmpty(); + assertThat(CEL.createProgram(ast).eval()).isInstanceOf(Map.class); + } + + @Test + public void list_containsHeterogeneousLiterals() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("[1, 2, 'hello']").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:8: expected type 'int' but found 'string'\n" + + " | [1, 2, 'hello']\n" + + " | .......^"); + } + + @Test + public void list_containsHeterogeneousLiteralsInNestedLists() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("[[1], ['hello']]").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:7: expected type 'list(int)' but found 'list(string)'\n" + + " | [[1], ['hello']]\n" + + " | ......^"); + } + + @Test + public void list_containsHeterogeneousLiteralsInDyn() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("[1, 2, dyn(3)]").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:11: expected type 'int' but found 'dyn'\n" + + " | [1, 2, dyn(3)]\n" + + " | ..........^"); + } + + @Test + public void mapKey_containsHeterogeneousLiterals() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("{1: true, 'hello': false}").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:18: expected type 'int' but found 'string'\n" + + " | {1: true, 'hello': false}\n" + + " | .................^"); + } + + @Test + public void mapKey_containsHeterogeneousLiteralsInNestedMaps() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("{{'a': 1}: true, {'b': 'hello'}: false}").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:32: expected type 'map(string, int)' but found 'map(string," + + " string)'\n" + + " | {{'a': 1}: true, {'b': 'hello'}: false}\n" + + " | ...............................^"); + } + + @Test + public void mapKey_containsHeterogeneousLiteralsInDyn() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("{1: true, dyn(2): false}").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:17: expected type 'int' but found 'dyn'\n" + + " | {1: true, dyn(2): false}\n" + + " | ................^"); + } + + @Test + public void mapValue_containsHeterogeneousLiterals() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("{1: true, 2: 'hello'}").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:12: expected type 'bool' but found 'string'\n" + + " | {1: true, 2: 'hello'}\n" + + " | ...........^"); + } + + @Test + public void mapValue_containsHeterogeneousLiteralsInNestedMaps() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("{1: {'a': true}, 2: {'b': 'hello'}}").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:19: expected type 'map(string, bool)' but found 'map(string," + + " string)'\n" + + " | {1: {'a': true}, 2: {'b': 'hello'}}\n" + + " | ..................^"); + } + + @Test + public void mapValue_containsHeterogeneousLiteralsInDyn() throws Exception { + CelAbstractSyntaxTree ast = CEL.compile("{1: true, 2: dyn(false)}").getAst(); + + CelValidationResult result = CEL_VALIDATOR.validate(ast); + + assertThat(result.hasError()).isTrue(); + assertThat(result.getAllIssues()).hasSize(1); + assertThat(result.getErrorString()) + .contains( + "ERROR: :1:12: expected type 'bool' but found 'dyn'\n" + + " | {1: true, 2: dyn(false)}\n" + + " | ...........^"); + } + + @Test + @TestParameters("{source: 'exemptFunction([''a'', 2])'}") + @TestParameters("{source: 'exemptFunction({1: true, ''hello'': false})'}") + @TestParameters("{source: 'exemptFunction({1: {''a'': true, 2: false}})'}") + @TestParameters("{source: 'exemptFunction({{''a'': true, 2: false} : false})'}") + @TestParameters("{source: '''%s''.format([[1], [2.0]])'}") + @TestParameters("{source: '''%s''.format([[1, 2, [3.0, 4]]])'}") + public void heterogeneousLiterals_inExemptFunction(String source) throws Exception { + Cel cel = + CelFactory.standardCelBuilder() + .addFunctionDeclarations( + newFunctionDeclaration( + "exemptFunction", + newGlobalOverload("exemptFunctionOverloadId", SimpleType.BOOL, SimpleType.DYN)), + newFunctionDeclaration( + "format", + newMemberOverload( + "stringFormatOverloadId", + SimpleType.BOOL, + SimpleType.STRING, + SimpleType.DYN))) + .addFunctionBindings( + CelFunctionBinding.from("exemptFunctionOverloadId", Object.class, (arg) -> true), + CelFunctionBinding.from( + "stringFormatOverloadId", String.class, Object.class, (str, arg) -> true)) + .build(); + CelValidator validator = + CelValidatorFactory.standardCelValidatorBuilder(cel) + .addAstValidators(HomogeneousLiteralValidator.newInstance("exemptFunction", "format")) + .build(); + CelAbstractSyntaxTree ast = cel.compile(source).getAst(); + + CelValidationResult result = validator.validate(ast); + + assertThat(result.hasError()).isFalse(); + assertThat(result.getAllIssues()).isEmpty(); + assertThat(cel.createProgram(ast).eval()).isInstanceOf(Boolean.class); + } +} diff --git a/validator/validators/BUILD.bazel b/validator/validators/BUILD.bazel index 333bbc1eb..e4f9ea54b 100644 --- a/validator/validators/BUILD.bazel +++ b/validator/validators/BUILD.bazel @@ -1,6 +1,6 @@ package( default_applicable_licenses = ["//:license"], - default_visibility = ["//visibility:public"], # TODO: Expose when ready + default_visibility = ["//visibility:public"], ) java_library( @@ -17,3 +17,8 @@ java_library( name = "regex", exports = ["//validator/src/main/java/dev/cel/validator/validators:regex"], ) + +java_library( + name = "homogeneous_literal", + exports = ["//validator/src/main/java/dev/cel/validator/validators:homogeneous_literal"], +)