From b00753e50edfc0ab27937e9d11a96ecceea04f65 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 21 May 2025 12:21:11 -0400 Subject: [PATCH 1/9] Download --- Makefile | 4 ++++ build.gradle.kts | 43 ++++++++++++++++++++++++++++++++++++++++++- gradle.properties | 3 +++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9849c9a9..836972d3 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,10 @@ MAKEFLAGS += --warn-undefined-variables MAKEFLAGS += --no-builtin-rules MAKEFLAGS += --no-print-directory GRADLE ?= ./gradlew +# Version of the cel-spec that this implementation is conformant with +# This should be kept in sync with the version in format_test.py +CEL_SPEC_VERSION ?= v0.24.0 +TESTDATA_FILE := tests/testdata/string_ext_$(CEL_SPEC_VERSION).textproto .PHONY: all all: lint generate build docs conformance ## Run all tests and lint (default) diff --git a/build.gradle.kts b/build.gradle.kts index f865d311..316ac312 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -90,8 +90,48 @@ tasks.register("generateTestSourcesNoImports") { commandLine(buf.asPath, "generate", "--template", "src/test/resources/proto/buf.gen.noimports.yaml") } +tasks.register("generateCelConformance") { + dependsOn("exportProtovalidateModule") + description = "Generates CEL conformance code with buf generate for unit tests." + commandLine( + buf.asPath, + "generate", + "--template", + "buf.gen.yaml", + "buf.build/google/cel-spec:${project.findProperty("cel.spec.version")}", + "--exclude-path", + "cel/expr/conformance/proto2", + "--exclude-path", + "cel/expr/conformance/proto3", + ) +} + +var getCelTestData = tasks.register("getCelTestData") { + val outputDir = layout.buildDirectory.dir("resources/testdata") + val celVersion = project.findProperty("cel.spec.version") + val outputFile = outputDir.map { it.file("string_ext_$celVersion.textproto") } + description = "Downloads the CEL conformance test data textproto file" + outputs.file(outputFile) + onlyIf { + // Only run the curl if file doesn't exist + val file = outputFile.get().asFile + !file.exists() + } + doFirst { + val file = outputFile.get().asFile + file.parentFile.mkdirs() + commandLine( + "curl", + "-fsSL", + "-o", + file.absolutePath, + "https://raw.githubusercontent.com/google/cel-spec/refs/tags/$celVersion/tests/simple/testdata/string_ext.textproto", + ) + } +} + tasks.register("generateTestSources") { - dependsOn("generateTestSourcesImports", "generateTestSourcesNoImports") + dependsOn("generateTestSourcesImports", "generateTestSourcesNoImports", "generateCelConformance") description = "Generates code with buf generate for unit tests" } @@ -206,6 +246,7 @@ allprojects { } } tasks.withType().configureEach { + // dependsOn(getCelTestData) useJUnitPlatform() this.testLogging { events("failed") diff --git a/gradle.properties b/gradle.properties index 9dd0505a..b49d4b1a 100644 --- a/gradle.properties +++ b/gradle.properties @@ -6,3 +6,6 @@ protovalidate.conformance.args = --strict_message --strict_error --expected_fail # Argument to the license-header CLI license-header.years = 2023-2024 + +# Version of the cel-spec that this implementation is conformant with +cel.spec.version = v0.24.0 From 0cbd905049c4236026fd36ae068524cde51e80e1 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 21 May 2025 17:04:37 -0400 Subject: [PATCH 2/9] Tests --- build.gradle.kts | 48 +- .../build/buf/protovalidate/FormatTest.java | 96 ++ src/test/resources/proto/buf.gen.cel.yaml | 6 + .../testdata/string_ext_v0.24.0.textproto | 1417 +++++++++++++++++ 4 files changed, 1543 insertions(+), 24 deletions(-) create mode 100644 src/test/resources/proto/buf.gen.cel.yaml create mode 100644 src/test/resources/testdata/string_ext_v0.24.0.textproto diff --git a/build.gradle.kts b/build.gradle.kts index 316ac312..9319d094 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -97,7 +97,7 @@ tasks.register("generateCelConformance") { buf.asPath, "generate", "--template", - "buf.gen.yaml", + "src/test/resources/proto/buf.gen.cel.yaml", "buf.build/google/cel-spec:${project.findProperty("cel.spec.version")}", "--exclude-path", "cel/expr/conformance/proto2", @@ -106,29 +106,28 @@ tasks.register("generateCelConformance") { ) } -var getCelTestData = tasks.register("getCelTestData") { - val outputDir = layout.buildDirectory.dir("resources/testdata") - val celVersion = project.findProperty("cel.spec.version") - val outputFile = outputDir.map { it.file("string_ext_$celVersion.textproto") } - description = "Downloads the CEL conformance test data textproto file" - outputs.file(outputFile) - onlyIf { - // Only run the curl if file doesn't exist - val file = outputFile.get().asFile - !file.exists() - } - doFirst { - val file = outputFile.get().asFile - file.parentFile.mkdirs() - commandLine( - "curl", - "-fsSL", - "-o", - file.absolutePath, - "https://raw.githubusercontent.com/google/cel-spec/refs/tags/$celVersion/tests/simple/testdata/string_ext.textproto", - ) +var getCelTestData = + tasks.register("getCelTestData") { + val celVersion = project.findProperty("cel.spec.version") + val fileUrl = "https://raw.githubusercontent.com/google/cel-spec/refs/tags/$celVersion/tests/simple/testdata/string_ext.textproto" + val targetDir = File("${project.projectDir}/src/test/resources/testdata") + val file = File(targetDir, "string_ext_$celVersion.textproto") + + onlyIf { + // Only run curl if file doesn't exist + !file.exists() + } + doFirst { + file.parentFile.mkdirs() + commandLine( + "curl", + "-fsSL", + "-o", + file.absolutePath, + fileUrl, + ) + } } -} tasks.register("generateTestSources") { dependsOn("generateTestSourcesImports", "generateTestSourcesNoImports", "generateCelConformance") @@ -246,7 +245,7 @@ allprojects { } } tasks.withType().configureEach { - // dependsOn(getCelTestData) + dependsOn(getCelTestData) useJUnitPlatform() this.testLogging { events("failed") @@ -254,6 +253,7 @@ allprojects { showExceptions = true showCauses = true showStackTraces = true + showStandardStreams = true } } } diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index 962460a7..4d441ee4 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -17,8 +17,33 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import com.cel.expr.conformance.test.SimpleTest; +import com.cel.expr.conformance.test.SimpleTestFile; +import com.cel.expr.conformance.test.SimpleTestSection; import com.google.protobuf.Duration; +import com.google.protobuf.TextFormat; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; +import java.util.stream.Collectors; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.api.Named; import org.junit.jupiter.api.Test; +import org.projectnessie.cel.Env; +import org.projectnessie.cel.EvalOption; +import org.projectnessie.cel.Library; +import org.projectnessie.cel.Program; +import org.projectnessie.cel.ProgramOption; import org.projectnessie.cel.common.types.DoubleT; import org.projectnessie.cel.common.types.Err.ErrException; import org.projectnessie.cel.common.types.ListT; @@ -26,8 +51,79 @@ import org.projectnessie.cel.common.types.UintT; import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.interpreter.Activation; class FormatTest { + // Version of the cel-spec that this implementation is conformant with + // This should be kept in sync with the version in gradle.properties + private static String CEL_SPEC_VERSION = "v0.24.0"; + + private static Env env; + + private static SimpleTestSection formatSection; + private static SimpleTestSection formatErrorSection; + + @BeforeAll + static void setUp() { + try { + byte[] encoded = + Files.readAllBytes( + Paths.get( + "src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto")); + String data = new String(encoded, StandardCharsets.UTF_8); + SimpleTestFile.Builder bldr = SimpleTestFile.newBuilder(); + TextFormat.getParser().merge(data, bldr); + SimpleTestFile testData = bldr.build(); + + List fs = + testData.getSectionList().stream() + .filter(s -> s.getName().equals("format")) + .collect(Collectors.toList()); + if (fs.size() == 1) { + formatSection = fs.get(0); + } + + List fes = + testData.getSectionList().stream() + .filter(s -> s.getName().equals("format_errors")) + .collect(Collectors.toList()); + if (fes.size() == 1) { + formatErrorSection = fes.get(0); + } + + env = Env.newEnv(Library.Lib(new ValidateLibrary())); + + } catch (InvalidPathException ipe) { + System.err.println(ipe); + } catch (IOException ioe) { + System.err.println(ioe); + } + } + + private static Stream getFormatTests() { + List args = new ArrayList(); + for (SimpleTest test : formatSection.getTestList()) { + args.add(Arguments.arguments(Named.named(test.getName(), test))); + } + + return args.stream(); + } + + @ParameterizedTest() + @MethodSource("getFormatTests") + void testFormatSuccess(SimpleTest test) { + Env.AstIssuesTuple ast = env.compile(test.getExpr()); + Map map = new HashMap(); + ProgramOption globals = ProgramOption.globals(map); + List globs = new ArrayList(); + globs.add(globals); + Program program = + env.program(ast.getAst(), globals, ProgramOption.evalOptions(EvalOption.OptTrackState)); + program.eval(Activation.emptyActivation()); + // System.err.println(evalResult.getVal()); + // System.err.println(evalResult.getEvalDetails().getState()); + } + @Test void testNotEnoughArgumentsThrows() { StringT one = StringT.stringOf("one"); diff --git a/src/test/resources/proto/buf.gen.cel.yaml b/src/test/resources/proto/buf.gen.cel.yaml new file mode 100644 index 00000000..547fdcda --- /dev/null +++ b/src/test/resources/proto/buf.gen.cel.yaml @@ -0,0 +1,6 @@ +version: v2 +managed: + enabled: true +plugins: + - remote: buf.build/protocolbuffers/java:v31.0 + out: build/generated/test-sources/bufgen diff --git a/src/test/resources/testdata/string_ext_v0.24.0.textproto b/src/test/resources/testdata/string_ext_v0.24.0.textproto new file mode 100644 index 00000000..d2455583 --- /dev/null +++ b/src/test/resources/testdata/string_ext_v0.24.0.textproto @@ -0,0 +1,1417 @@ +# proto-file: ../../../proto/cel/expr/conformance/test/simple.proto +# proto-message: cel.expr.conformance.test.SimpleTestFile + +name: "string_ext" +description: "Tests for the strings extension library." +section: { + name: "char_at" + test: { + name: "middle_index" + expr: "'tacocat'.charAt(3)" + value: { + string_value: "o" + } + } + test: { + name: "end_index" + expr: "'tacocat'.charAt(7)" + value: { + string_value: "" + } + } + test: { + name: "multiple" + expr: "'©αT'.charAt(0) == '©' && '©αT'.charAt(1) == 'α' && '©αT'.charAt(2) == 'T'" + } +} +section: { + name: "index_of" + test: { + name: "empty_index" + expr: "'tacocat'.indexOf('')" + value: { + int64_value: 0 + } + } + test: { + name: "string_index" + expr: "'tacocat'.indexOf('ac')" + value: { + int64_value: 1 + } + } + test: { + name: "nomatch" + expr: "'tacocat'.indexOf('none') == -1" + } + test: { + name: "empty_index" + expr: "'tacocat'.indexOf('', 3) == 3" + } + test: { + name: "char_index" + expr: "'tacocat'.indexOf('a', 3) == 5" + } + test: { + name: "string_index" + expr: "'tacocat'.indexOf('at', 3) == 5" + } + test: { + name: "unicode_char" + expr: "'ta©o©αT'.indexOf('©') == 2" + } + test: { + name: "unicode_char_index" + expr: "'ta©o©αT'.indexOf('©', 3) == 4" + } + test: { + name: "unicode_string_index" + expr: "'ta©o©αT'.indexOf('©αT', 3) == 4" + } + test: { + name: "unicode_string_nomatch_index" + expr: "'ta©o©αT'.indexOf('©α', 5) == -1" + } + test: { + name: "char_index" + expr: "'ijk'.indexOf('k') == 2" + } + test: { + name: "string_with_space_fullmatch" + expr: "'hello wello'.indexOf('hello wello') == 0" + } + test: { + name: "string_with_space_index" + expr: "'hello wello'.indexOf('ello', 6) == 7" + } + test: { + name: "string_nomatch_index" + expr: "'hello wello'.indexOf('elbo room!!') == -1" + } +} +section: { + name: "last_index_of" + test: { + name: "empty" + expr: "'tacocat'.lastIndexOf('') == 7" + } + test: { + name: "string" + expr: "'tacocat'.lastIndexOf('at') == 5" + } + test: { + name: "string_nomatch" + expr: "'tacocat'.lastIndexOf('none') == -1" + } + test: { + name: "empty_index" + expr: "'tacocat'.lastIndexOf('', 3) == 3" + } + test: { + name: "char_index" + expr: "'tacocat'.lastIndexOf('a', 3) == 1" + } + test: { + name: "unicode_char" + expr: "'ta©o©αT'.lastIndexOf('©') == 4" + } + test: { + name: "unicode_char_index" + expr: "'ta©o©αT'.lastIndexOf('©', 3) == 2" + } + test: { + name: "unicode_string_index" + expr: "'ta©o©αT'.lastIndexOf('©α', 4) == 4" + } + test: { + name: "string_with_space_string_index" + expr: "'hello wello'.lastIndexOf('ello', 6) == 1" + } + test: { + name: "string_with_space_string_nomatch" + expr: "'hello wello'.lastIndexOf('low') == -1" + } + test: { + name: "string_with_space_string_with_space_nomatch" + expr: "'hello wello'.lastIndexOf('elbo room!!') == -1" + } + test: { + name: "string_with_space_fullmatch" + expr: "'hello wello'.lastIndexOf('hello wello') == 0" + } + test: { + name: "repeated_string" + expr: "'bananananana'.lastIndexOf('nana', 7) == 6" + } +} +section: { + name: "ascii_casing" + test: { + name: "lowerascii" + expr: "'TacoCat'.lowerAscii() == 'tacocat'" + } + test: { + name: "lowerascii_unicode" + expr: "'TacoCÆt'.lowerAscii() == 'tacocÆt'" + } + test: { + name: "lowerascii_unicode_with_space" + expr: "'TacoCÆt Xii'.lowerAscii() == 'tacocÆt xii'" + } + test: { + name: "upperascii" + expr: "'tacoCat'.upperAscii() == 'TACOCAT'" + } + test: { + name: "upperascii_unicode" + expr: "'tacoCαt'.upperAscii() == 'TACOCαT'" + } + test: { + name: "upperascii_unicode_with_space" + expr: "'TacoCÆt Xii'.upperAscii() == 'TACOCÆT XII'" + } +} +section: { + name: "replace" + test: { + name: "no_placeholder" + expr: "'12 days 12 hours'.replace('{0}', '2') == '12 days 12 hours'" + } + test: { + name: "basic" + expr: "'{0} days {0} hours'.replace('{0}', '2') == '2 days 2 hours'" + } + test: { + name: "chained" + expr: "'{0} days {0} hours'.replace('{0}', '2', 1).replace('{0}', '23') == '2 days 23 hours'" + } + test: { + name: "unicode" + expr: "'1 ©αT taco'.replace('αT', 'o©α') == '1 ©o©α taco'" + } +} +section: { + name: "split" + test: { + name: "empty" + expr: "'hello world'.split(' ') == ['hello', 'world']" + } + test: { + name: "zero_limit" + expr: "'hello world events!'.split(' ', 0) == []" + } + test: { + name: "one_limit" + expr: "'hello world events!'.split(' ', 1) == ['hello world events!']" + } + test: { + name: "unicode_negative_limit" + expr: "'o©o©o©o'.split('©', -1) == ['o', 'o', 'o', 'o']" + } +} +section: { + name: "substring" + test: { + name: "start" + expr: "'tacocat'.substring(4) == 'cat'" + } + test: { + name: "start_with_max_length" + expr: "'tacocat'.substring(7) == ''" + } + test: { + name: "start_and_end" + expr: "'tacocat'.substring(0, 4) == 'taco'" + } + test: { + name: "start_and_end_equal_value" + expr: "'tacocat'.substring(4, 4) == ''" + } + test: { + name: "unicode_start_and_end" + expr: "'ta©o©αT'.substring(2, 6) == '©o©α'" + } + test: { + name: "unicode_start_and_end_equal_value" + expr: "'ta©o©αT'.substring(7, 7) == ''" + } +} +section: { + name: "trim" + test: { + name: "blank_spaces_escaped_chars" + expr: "' \\f\\n\\r\\t\\vtext '.trim() == 'text'" + } + test: { + name: "unicode_space_chars_1" + expr: "'\\u0085\\u00a0\\u1680text'.trim() == 'text'" + } + test: { + name: "unicode_space_chars_2" + expr: "'text\\u2000\\u2001\\u2002\\u2003\\u2004\\u2004\\u2006\\u2007\\u2008\\u2009'.trim() == 'text'" + } + test: { + name: "unicode_space_chars_3" + expr: "'\\u200atext\\u2028\\u2029\\u202F\\u205F\\u3000'.trim() == 'text'" + } + test: { + name: "unicode_no_trim" + expr: "'\\u180etext\\u200b\\u200c\\u200d\\u2060\\ufeff'.trim() == '\\u180etext\\u200b\\u200c\\u200d\\u2060\\ufeff'" + } +} +section: { + name: "join" + test: { + name: "empty_separator" + expr: "['x', 'y'].join() == 'xy'" + } + test: { + name: "dash_separator" + expr: "['x', 'y'].join('-') == 'x-y'" + } + test: { + name: "empty_string_empty_separator" + expr: "[].join() == ''" + } + test: { + name: "empty_string_dash_separator" + expr: "[].join('-') == ''" + } +} +section: { + name: "quote" + test: { + name: "multiline" + expr: "strings.quote(\"first\\nsecond\") == \"\\\"first\\\\nsecond\\\"\"" + } + test: { + name: "escaped" + expr: "strings.quote(\"bell\\a\") == \"\\\"bell\\\\a\\\"\"" + } + test: { + name: "backspace" + expr: "strings.quote(\"\\bbackspace\") == \"\\\"\\\\bbackspace\\\"\"" + } + test: { + name: "form_feed" + expr: "strings.quote(\"\\fform feed\") == \"\\\"\\\\fform feed\\\"\"" + } + test: { + name: "carriage_return" + expr: "strings.quote(\"carriage \\r return\") == \"\\\"carriage \\\\r return\\\"\"" + } + test: { + name: "horizontal_tab" + expr: "strings.quote(\"horizontal tab\\t\") == \"\\\"horizontal tab\\\\t\\\"\"" + } + test: { + name: "vertical_tab" + expr: "strings.quote(\"vertical \\v tab\") == \"\\\"vertical \\\\v tab\\\"\"" + } + test: { + name: "double_slash" + expr: "strings.quote(\"double \\\\\\\\ slash\") == \"\\\"double \\\\\\\\\\\\\\\\ slash\\\"\"" + } + test: { + name: "two_escape_sequences" + expr: "strings.quote(\"two escape sequences \\\\a\\\\n\") == \"\\\"two escape sequences \\\\\\\\a\\\\\\\\n\\\"\"" + } + test: { + name: "verbatim" + expr: "strings.quote(\"verbatim\") == \"\\\"verbatim\\\"\"" + } + test: { + name: "ends_with" + expr: "strings.quote(\"ends with \\\\\") == \"\\\"ends with \\\\\\\\\\\"\"" + } + test: { + name: "starts_with" + expr: "strings.quote(\"\\\\ starts with\") == \"\\\"\\\\\\\\ starts with\\\"\"" + } + test: { + name: "printable_unicode" + expr: "strings.quote(\"printable unicode😀\") == \"\\\"printable unicode😀\\\"\"" + } + test: { + name: "mid_string_quote" + expr: "strings.quote(\"mid string \\\" quote\") == \"\\\"mid string \\\\\\\" quote\\\"\"" + } + test: { + name: "single_quote_with_double_quote" + expr: "strings.quote('single-quote with \"double quote\"') == \"\\\"single-quote with \\\\\\\"double quote\\\\\\\"\\\"\"" + } + test: { + name: "size_unicode_char" + expr: "strings.quote(\"size('ÿ')\") == \"\\\"size('ÿ')\\\"\"" + } + test: { + name: "size_unicode_string" + expr: "strings.quote(\"size('πέντε')\") == \"\\\"size('πέντε')\\\"\"" + } + test: { + name: "unicode" + expr: "strings.quote(\"завтра\") == \"\\\"завтра\\\"\"" + } + test: { + name: "unicode_code_points" + expr: "strings.quote(\"\\U0001F431\\U0001F600\\U0001F61B\")" + value: { + string_value: "\"🐱😀😛\"" + } + } + test: { + name: "unicode_2" + expr: "strings.quote(\"ta©o©αT\") == \"\\\"ta©o©αT\\\"\"" + } + test: { + name: "empty_quote" + expr: "strings.quote(\"\")" + value: { + string_value: "\"\"" + } + } +} +section: { + name: "format" + test: { + name: "no-op" + expr: '"no substitution".format([])' + value: { + string_value: 'no substitution', + } + } + test: { + name: "mid-string substitution" + expr: '"str is %s and some more".format(["filler"])' + value: { + string_value: 'str is filler and some more', + } + } + test: { + name: "percent escaping" + expr: '"%% and also %%".format([])' + value: { + string_value: '% and also %', + } + } + test: { + name: "substitution inside escaped percent signs" + expr: '"%%%s%%".format(["text"])' + value: { + string_value: '%text%', + } + } + test: { + name: "substitution with one escaped percent sign on the right" + expr: '"%s%%".format(["percent on the right"])' + value: { + string_value: 'percent on the right%', + } + } + test: { + name: "substitution with one escaped percent sign on the left" + expr: '"%%%s".format(["percent on the left"])' + value: { + string_value: '%percent on the left', + } + } + test: { + name: "multiple substitutions" + expr: '"%d %d %d, %s %s %s, %d %d %d, %s %s %s".format([1, 2, 3, "A", "B", "C", 4, 5, 6, "D", "E", "F"])' + value: { + string_value: '1 2 3, A B C, 4 5 6, D E F', + } + } + test: { + name: "percent sign escape sequence support" + expr: '"%%escaped %s%%".format(["percent"])' + value: { + string_value: '%escaped percent%', + } + } + test: { + name: "fixed point formatting clause" + expr: '"%.3f".format([1.2345])' + value: { + string_value: '1.234', + } + } + test: { + name: "binary formatting clause" + expr: '"this is 5 in binary: %b".format([5])' + value: { + string_value: 'this is 5 in binary: 101', + } + } + test: { + name: "uint support for binary formatting" + expr: '"unsigned 64 in binary: %b".format([uint(64)])' + value: { + string_value: 'unsigned 64 in binary: 1000000', + } + } + test: { + name: "bool support for binary formatting" + expr: '"bit set from bool: %b".format([true])' + value: { + string_value: 'bit set from bool: 1', + } + } + test: { + name: "octal formatting clause" + expr: '"%o".format([11])' + value: { + string_value: '13', + } + } + test: { + name: "uint support for octal formatting clause" + expr: '"this is an unsigned octal: %o".format([uint(65535)])' + value: { + string_value: 'this is an unsigned octal: 177777', + } + } + test: { + name: "lowercase hexadecimal formatting clause" + expr: '"%x is 20 in hexadecimal".format([30])' + value: { + string_value: '1e is 20 in hexadecimal', + } + } + test: { + name: "uppercase hexadecimal formatting clause" + expr: '"%X is 20 in hexadecimal".format([30])' + value: { + string_value: '1E is 20 in hexadecimal', + } + } + test: { + name: "unsigned support for hexadecimal formatting clause" + expr: '"%X is 6000 in hexadecimal".format([uint(6000)])' + value: { + string_value: '1770 is 6000 in hexadecimal', + } + } + test: { + name: "string support with hexadecimal formatting clause" + expr: '"%x".format(["Hello world!"])' + value: { + string_value: '48656c6c6f20776f726c6421', + } + } + test: { + name: "string support with uppercase hexadecimal formatting clause" + expr: '"%X".format(["Hello world!"])' + value: { + string_value: '48656C6C6F20776F726C6421', + } + } + test: { + name: "byte support with hexadecimal formatting clause" + expr: '"%x".format([b"byte string"])' + value: { + string_value: '6279746520737472696e67', + } + } + test: { + name: "byte support with uppercase hexadecimal formatting clause" + expr: '"%X".format([b"byte string"])' + value: { + string_value: '6279746520737472696E67', + } + } + test: { + name: "scientific notation formatting clause" + expr: '"%.6e".format([1052.032911275])' + value: { + string_value: '1.052033e+03', + } + } + test: { + name: "default precision for fixed-point clause" + expr: '"%f".format([2.71828])' + value: { + string_value: '2.718280', + } + } + test: { + name: "default precision for scientific notation" + expr: '"%e".format([2.71828])' + value: { + string_value: '2.718280e+00', + } + } + test: { + name: "NaN support for scientific notation" + expr: '"%e".format([double("NaN")])' + value: { + string_value: 'NaN', + } + } + test: { + name: "positive infinity support for scientific notation" + expr: '"%e".format([double("Infinity")])' + value: { + string_value: 'Infinity', + } + } + test: { + name: "negative infinity support for scientific notation" + expr: '"%e".format([double("-Infinity")])' + value: { + string_value: '-Infinity', + } + } + test: { + name: "NaN support for decimal" + expr: '"%d".format([double("NaN")])' + value: { + string_value: 'NaN', + } + } + test: { + name: "positive infinity support for decimal" + expr: '"%d".format([double("Infinity")])' + value: { + string_value: 'Infinity', + } + } + test: { + name: "negative infinity support for decimal" + expr: '"%d".format([double("-Infinity")])' + value: { + string_value: '-Infinity', + } + } + test: { + name: "NaN support for fixed-point" + expr: '"%f".format([double("NaN")])' + value: { + string_value: 'NaN', + } + } + test: { + name: "positive infinity support for fixed-point" + expr: '"%f".format([double("Infinity")])' + value: { + string_value: 'Infinity', + } + } + test: { + name: "negative infinity support for fixed-point" + expr: '"%f".format([double("-Infinity")])' + value: { + string_value: '-Infinity', + } + } + test: { + name: "uint support for decimal clause" + expr: '"%d".format([uint(64)])' + value: { + string_value: '64', + } + } + test: { + name: "null support for string" + expr: '"%s".format([null])' + value: { + string_value: 'null', + } + } + test: { + name: "int support for string" + expr: '"%s".format([999999999999])' + value: { + string_value: '999999999999', + } + } + test: { + name: "bytes support for string" + expr: '"%s".format([b"xyz"])' + value: { + string_value: 'xyz', + } + } + test: { + name: "type() support for string" + expr: '"%s".format([type("test string")])' + value: { + string_value: 'string', + } + } + test: { + name: "timestamp support for string" + expr: '"%s".format([timestamp("2023-02-03T23:31:20+00:00")])' + value: { + string_value: '2023-02-03T23:31:20Z', + } + } + test: { + name: "duration support for string" + expr: '"%s".format([duration("1h45m47s")])' + value: { + string_value: '6347s', + } + } + test: { + name: "list support for string" + expr: '"%s".format([["abc", 3.14, null, [9, 8, 7, 6], timestamp("2023-02-03T23:31:20Z")]])' + value: { + string_value: '[abc, 3.14, null, [9, 8, 7, 6], 2023-02-03T23:31:20Z]', + } + } + test: { + name: "map support for string" + expr: '"%s".format([{"key1": b"xyz", "key5": null, "key2": duration("2h"), "key4": true, "key3": 2.71828}])' + value: { + string_value: '{key1: xyz, key2: 7200s, key3: 2.71828, key4: true, key5: null}', + } + } + test: { + name: "map support (all key types)" + expr: '"%s".format([{1: "value1", uint(2): "value2", true: double("NaN")}])' + value: { + string_value: '{1: value1, 2: value2, true: NaN}', + } + } + test: { + name: "boolean support for %s" + expr: '"%s, %s".format([true, false])' + value: { + string_value: 'true, false', + } + } + test: { + name: "dyntype support for string formatting clause" + expr: '"%s".format([dyn("a string")])' + value: { + string_value: 'a string', + } + } + test: { + name: "dyntype support for numbers with string formatting clause" + expr: '"%s, %s".format([dyn(32), dyn(56.8)])' + value: { + string_value: '32, 56.8', + } + } + test: { + name: "dyntype support for integer formatting clause" + expr: '"%d".format([dyn(128)])' + value: { + string_value: '128', + } + } + test: { + name: "dyntype support for integer formatting clause (unsigned)" + expr: '"%d".format([dyn(256u)])' + value: { + string_value: '256', + } + } + test: { + name: "dyntype support for hex formatting clause" + expr: '"%x".format([dyn(22)])' + value: { + string_value: '16', + } + } + test: { + name: "dyntype support for hex formatting clause (uppercase)" + expr: '"%X".format([dyn(26)])' + value: { + string_value: '1A', + } + } + test: { + name: "dyntype support for unsigned hex formatting clause" + expr: '"%x".format([dyn(500u)])' + value: { + string_value: '1f4', + } + } + test: { + name: "dyntype support for fixed-point formatting clause" + expr: '"%.3f".format([dyn(4.5)])' + value: { + string_value: '4.500', + } + } + test: { + name: "dyntype support for scientific notation" + expr: '"%e".format([dyn(2.71828)])' + value: { + string_value: '2.718280e+00', + } + } + test: { + name: "dyntype NaN/infinity support" + expr: '"%s".format([[double("NaN"), double("Infinity"), double("-Infinity")]])' + value: { + string_value: '[NaN, Infinity, -Infinity]', + } + } + test: { + name: "dyntype support for timestamp" + expr: '"%s".format([dyn(timestamp("2009-11-10T23:00:00Z"))])' + value: { + string_value: '2009-11-10T23:00:00Z', + } + } + test: { + name: "dyntype support for duration" + expr: '"%s".format([dyn(duration("8747s"))])' + value: { + string_value: '8747s', + } + } + test: { + name: "dyntype support for lists" + expr: '"%s".format([dyn([6, 4.2, "a string"])])' + value: { + string_value: '[6, 4.2, a string]', + } + } + test: { + name: "dyntype support for maps" + expr: '"%s".format([{"strKey":"x", 6:duration("422s"), true:42}])' + value: { + string_value: '{6: 422s, strKey: x, true: 42}', + } + } + test: { + name: "string substitution in a string variable" + expr: 'str_var.format(["filler"])' + type_env: { + name: "str_var", + ident: { type: { primitive: STRING } } + } + bindings: { + key: "str_var" + value: { value: { string_value: "%s" } } + } + value: { + string_value: 'filler', + } + } + test: { + name: "multiple substitutions in a string variable" + expr: 'str_var.format([1, 2, 3, "A", "B", "C", 4, 5, 6, "D", "E", "F"])' + type_env: { + name: "str_var", + ident: { type: { primitive: STRING } } + } + bindings: { + key: "str_var" + value: { value: { string_value: "%d %d %d, %s %s %s, %d %d %d, %s %s %s" } } + } + value: { + string_value: '1 2 3, A B C, 4 5 6, D E F', + } + } + test: { + name: "substitution inside escaped percent signs in a string variable" + expr: 'str_var.format(["text"])' + type_env: { + name: "str_var", + ident: { type: { primitive: STRING } } + } + bindings: { + key: "str_var" + value: { value: { string_value: "%%%s%%" } } + } + value: { + string_value: '%text%', + } + } + test: { + name: "fixed point formatting clause in a string variable" + expr: 'str_var.format([1.2345])' + type_env: { + name: "str_var", + ident: { type: { primitive: STRING } } + } + bindings: { + key: "str_var" + value: { value: { string_value: "%.3f" } } + } + value: { + string_value: '1.234', + } + } + test: { + name: "binary formatting clause in a string variable" + expr: 'str_var.format([5])' + type_env: { + name: "str_var", + ident: { type: { primitive: STRING } } + } + bindings: { + key: "str_var" + value: { value: { string_value: "%b" } } + } + value: { + string_value: '101', + } + } + test: { + name: "scientific notation formatting clause in a string variable" + expr: 'str_var.format([1052.032911275])' + type_env: { + name: "str_var", + ident: { type: { primitive: STRING } } + } + bindings: { + key: "str_var" + value: { value: { string_value: "%.6e" } } + } + value: { + string_value: '1.052033e+03', + } + } + test: { + name: "default precision for fixed-point clause in a string variable" + expr: 'str_var.format([2.71828])' + type_env: { + name: "str_var", + ident: { type: { primitive: STRING } } + } + bindings: { + key: "str_var" + value: { value: { string_value: "%f" } } + } + value: { + string_value: '2.718280', + } + } +} +section: { + name: "format_errors" + test: { + name: "unrecognized formatting clause" + expr: '"%a".format([1])' + disable_check: true + eval_error: { + errors: { + message: 'could not parse formatting clause: unrecognized formatting clause "a"' + } + } + } + test: { + name: "out of bounds arg index" + expr: '"%d %d %d".format([0, 1])' + disable_check: true + eval_error: { + errors: { + message: 'index 2 out of range' + } + } + } + test: { + name: "string substitution is not allowed with binary clause" + expr: '"string is %b".format(["abc"])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: only integers and bools can be formatted as binary, was given string' + } + } + } + test: { + name: "duration substitution not allowed with decimal clause" + expr: '"%d".format([duration("30m2s")])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: decimal clause can only be used on integers, was given google.protobuf.Duration' + } + } + } + test: { + name: "string substitution not allowed with octal clause" + expr: '"octal: %o".format(["a string"])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: octal clause can only be used on integers, was given string' + } + } + } + test: { + name: "double substitution not allowed with hex clause" + expr: '"double is %x".format([0.5])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: only integers, byte buffers, and strings can be formatted as hex, was given double' + } + } + } + test: { + name: "uppercase not allowed for scientific clause" + expr: '"double is %E".format([0.5])' + disable_check: true + eval_error: { + errors: { + message: 'could not parse formatting clause: unrecognized formatting clause "E"' + } + } + } + test: { + name: "object not allowed" + expr: '"object is %s".format([cel.expr.conformance.proto3.TestAllTypes{}])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: string clause can only be used on strings, bools, bytes, ints, doubles, maps, lists, types, durations, and timestamps, was given cel.expr.conformance.proto3.TestAllTypes' + } + } + } + test: { + name: "object inside list" + expr: '"%s".format([[1, 2, cel.expr.conformance.proto3.TestAllTypes{}]])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: string clause can only be used on strings, bools, bytes, ints, doubles, maps, lists, types, durations, and timestamps, was given cel.expr.conformance.proto3.TestAllTypes' + } + } + } + test: { + name: "object inside map" + expr: '"%s".format([{1: "a", 2: cel.expr.conformance.proto3.TestAllTypes{}}])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: string clause can only be used on strings, bools, bytes, ints, doubles, maps, lists, types, durations, and timestamps, was given cel.expr.conformance.proto3.TestAllTypes' + } + } + } + test: { + name: "null not allowed for %d" + expr: '"null: %d".format([null])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: decimal clause can only be used on integers, was given null_type' + } + } + } + test: { + name: "null not allowed for %e" + expr: '"null: %e".format([null])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: scientific clause can only be used on doubles, was given null_type' + } + } + } + test: { + name: "null not allowed for %f" + expr: '"null: %f".format([null])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: fixed-point clause can only be used on doubles, was given null_type' + } + } + } + test: { + name: "null not allowed for %x" + expr: '"null: %x".format([null])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: only integers, byte buffers, and strings can be formatted as hex, was given null_type' + } + } + } + test: { + name: "null not allowed for %X" + expr: '"null: %X".format([null])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: only integers, byte buffers, and strings can be formatted as hex, was given null_type' + } + } + } + test: { + name: "null not allowed for %b" + expr: '"null: %b".format([null])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: only integers and bools can be formatted as binary, was given null_type' + } + } + } + test: { + name: "null not allowed for %o" + expr: '"null: %o".format([null])' + disable_check: true + eval_error: { + errors: { + message: 'error during formatting: octal clause can only be used on integers, was given null_type' + } + } + } +} +section: { + name: "value_errors" + test: { + name: "charat_out_of_range" + expr: "'tacocat'.charAt(30) == ''" + eval_error: { + errors: { + message: "index out of range: 30" + } + } + } + test: { + name: "indexof_out_of_range" + expr: "'tacocat'.indexOf('a', 30) == -1" + eval_error: { + errors: { + message: "index out of range: 30" + } + } + } + test: { + name: "lastindexof_negative_index" + expr: "'tacocat'.lastIndexOf('a', -1) == -1" + eval_error: { + errors: { + message: "index out of range: -1" + } + } + } + test: { + name: "lastindexof_out_of_range" + expr: "'tacocat'.lastIndexOf('a', 30) == -1" + eval_error: { + errors: { + message: "index out of range: 30" + } + } + } + test: { + name: "substring_out_of_range" + expr: "'tacocat'.substring(40) == 'cat'" + eval_error: { + errors: { + message: "index out of range: 40" + } + } + } + test: { + name: "substring_negative_index" + expr: "'tacocat'.substring(-1) == 'cat'" + eval_error: { + errors: { + message: "index out of range: -1" + } + } + } + test: { + name: "substring_end_index_out_of_range" + expr: "'tacocat'.substring(1, 50) == 'cat'" + eval_error: { + errors: { + message: "index out of range: 50" + } + } + } + test: { + name: "substring_begin_index_out_of_range" + expr: "'tacocat'.substring(49, 50) == 'cat'" + eval_error: { + errors: { + message: "index out of range: 49" + } + } + } + test: { + name: "substring_end_index_greater_than_begin_index" + expr: "'tacocat'.substring(4, 3) == ''" + eval_error: { + errors: { + message: "invalid substring range. start: 4, end: 3" + } + } + } +} +section: { + name: "type_errors" + test: { + name: "charat_invalid_type" + expr: "42.charAt(2) == ''" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "charat_invalid_argument" + expr: "'hello'.charAt(true) == ''" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "indexof_unary_invalid_type" + expr: "24.indexOf('2') == 0" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "indexof_unary_invalid_argument" + expr: "'hello'.indexOf(true) == 1" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "indexof_binary_invalid_argument" + expr: "42.indexOf('4', 0) == 0" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "indexof_binary_invalid_argument_2" + expr: "'42'.indexOf(4, 0) == 0" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "indexof_binary_both_invalid_arguments" + expr: "'42'.indexOf('4', '0') == 0" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "indexof_ternary_invalid_arguments" + expr: "'42'.indexOf('4', 0, 1) == 0" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "split_invalid_type" + expr: "42.split('2') == ['4']" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_invalid_type" + expr: "42.replace(2, 1) == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_binary_invalid_argument" + expr: "'42'.replace(2, 1) == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_binary_invalid_argument_2" + expr: "'42'.replace('2', 1) == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_ternary_invalid_argument" + expr: "42.replace('2', '1', 1) == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_ternary_invalid_argument_2" + expr: "'42'.replace(2, '1', 1) == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_ternary_invalid_argument_3" + expr: "'42'.replace('2', 1, 1) == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_ternary_invalid_argument_4" + expr: "'42'.replace('2', '1', '1') == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "replace_quaternary_invalid_argument" + expr: "'42'.replace('2', '1', 1, false) == '41'" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "split_invalid_type_empty_arg" + expr: "42.split('') == ['4', '2']" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "split_invalid_argument" + expr: "'42'.split(2) == ['4']" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "split_binary_invalid_type" + expr: "42.split('2', '1') == ['4']" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "split_binary_invalid_argument" + expr: "'42'.split(2, 1) == ['4']" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "split_binary_invalid_argument_2" + expr: "'42'.split('2', '1') == ['4']" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "split_ternary_invalid_argument" + expr: "'42'.split('2', 1, 1) == ['4']" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "substring_ternary_invalid_argument" + expr: "'hello'.substring(1, 2, 3) == ''" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "substring_binary_invalid_type" + expr: "30.substring(true, 3) == ''" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "substring_binary_invalid_argument" + expr: "'tacocat'.substring(true, 3) == ''" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } + test: { + name: "substring_binary_invalid_argument_2" + expr: "'tacocat'.substring(0, false) == ''" + disable_check: true + eval_error: { + errors: { + message: "no such overload" + } + } + } +} From 4726a5fd49e6da964d6a9ca9a762786439cbd334 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Tue, 27 May 2025 15:47:12 -0400 Subject: [PATCH 3/9] Tests --- build.gradle.kts | 16 +- .../buf/protovalidate/CustomOverload.java | 33 ++- .../java/build/buf/protovalidate/Format.java | 273 +++++++++++++----- .../build/buf/protovalidate/FormatTest.java | 249 +++++++++------- .../resources/proto/buf.gen.cel.proto3.yaml | 9 + 5 files changed, 385 insertions(+), 195 deletions(-) create mode 100644 src/test/resources/proto/buf.gen.cel.proto3.yaml diff --git a/build.gradle.kts b/build.gradle.kts index 9319d094..494227f2 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -106,6 +106,20 @@ tasks.register("generateCelConformance") { ) } +tasks.register("generateCelConformanceTestTypes") { + dependsOn("exportProtovalidateModule") + description = "Generates CEL conformance code with buf generate for unit tests." + commandLine( + buf.asPath, + "generate", + "--template", + "src/test/resources/proto/buf.gen.cel.proto3.yaml", + "buf.build/google/cel-spec:${project.findProperty("cel.spec.version")}", + "--path", + "cel/expr/conformance/proto3", + ) +} + var getCelTestData = tasks.register("getCelTestData") { val celVersion = project.findProperty("cel.spec.version") @@ -130,7 +144,7 @@ var getCelTestData = } tasks.register("generateTestSources") { - dependsOn("generateTestSourcesImports", "generateTestSourcesNoImports", "generateCelConformance") + dependsOn("generateTestSourcesImports", "generateTestSourcesNoImports", "generateCelConformance", "generateCelConformanceTestTypes") description = "Generates code with buf generate for unit tests" } diff --git a/src/main/java/build/buf/protovalidate/CustomOverload.java b/src/main/java/build/buf/protovalidate/CustomOverload.java index 8383e38c..471d790c 100644 --- a/src/main/java/build/buf/protovalidate/CustomOverload.java +++ b/src/main/java/build/buf/protovalidate/CustomOverload.java @@ -110,20 +110,25 @@ private static Overload celGetField() { * @return The {@link Overload} instance for the "format" operation. */ private static Overload celFormat() { - return Overload.binary( - OVERLOAD_FORMAT, - (lhs, rhs) -> { - if (lhs.type().typeEnum() != TypeEnum.String || rhs.type().typeEnum() != TypeEnum.List) { - return Err.noSuchOverload(lhs, OVERLOAD_FORMAT, rhs); - } - ListT list = (ListT) rhs.convertToType(ListT.ListType); - String formatString = (String) lhs.value(); - try { - return StringT.stringOf(Format.format(formatString, list)); - } catch (Err.ErrException e) { - return e.getErr(); - } - }); + Overload ol = + Overload.binary( + OVERLOAD_FORMAT, + (lhs, rhs) -> { + if (lhs.type().typeEnum() != TypeEnum.String + || rhs.type().typeEnum() != TypeEnum.List) { + return Err.noSuchOverload(lhs, OVERLOAD_FORMAT, rhs); + } + ListT list = (ListT) rhs.convertToType(ListT.ListType); + String formatString = (String) lhs.value(); + try { + return StringT.stringOf(Format.format(formatString, list)); + } catch (Err.ErrException e) { + return e.getErr(); + } + }); + + System.err.println(ol); + return ol; } /** diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index cdc08f50..fd5d04b0 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -18,23 +18,22 @@ import com.google.protobuf.Duration; import com.google.protobuf.Timestamp; +import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.text.DecimalFormat; import java.time.Instant; -import java.util.List; +import java.util.Locale; +import java.util.Optional; import org.projectnessie.cel.common.types.Err.ErrException; import org.projectnessie.cel.common.types.IntT; +import org.projectnessie.cel.common.types.IteratorT; import org.projectnessie.cel.common.types.ListT; -import org.projectnessie.cel.common.types.pb.Db; -import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; +import org.projectnessie.cel.common.types.MapT; import org.projectnessie.cel.common.types.ref.TypeEnum; import org.projectnessie.cel.common.types.ref.Val; /** String formatter for CEL evaluation. */ final class Format { - private static final char[] HEX_ARRAY = "0123456789ABCDEF".toCharArray(); - private static final char[] LOWER_HEX_ARRAY = "0123456789abcdef".toCharArray(); - /** * Format the string with a {@link ListT}. * @@ -72,7 +71,7 @@ static String format(String fmtString, ListT list) { continue; } if (argIndex >= list.size().intValue()) { - throw new ErrException("format: not enough arguments"); + throw new ErrException("index " + argIndex + " out of range"); } Val arg = list.get(IntT.intOf(argIndex++)); c = fmtString.charAt(index++); @@ -93,113 +92,143 @@ static String format(String fmtString, ListT list) { switch (c) { case 'd': - formatDecimal(builder, arg); + builder.append(formatDecimal(arg)); break; case 'x': - formatHex(builder, arg, LOWER_HEX_ARRAY); + builder.append(formatHex(arg)); break; case 'X': - formatHex(builder, arg, HEX_ARRAY); + // We can use a root locale, because the only characters are hex (A-F). + builder.append(formatHex(arg).toUpperCase(Locale.ROOT)); break; case 's': - formatString(builder, arg); + builder.append(formatString(arg)); break; case 'e': + builder.append(formatExponential(arg, precision)); + break; case 'f': + builder.append(formatFloat(arg, precision)); + break; case 'b': + builder.append(formatBinary(arg)); + break; case 'o': + builder.append(formatOctal(arg)); + break; default: - throw new ErrException("format: unparsable format specifier %s", c); + throw new ErrException( + "could not parse formatting clause: unrecognized formatting clause \"" + c + "\""); } } return builder.toString(); } - /** - * Converts a byte array to a hexadecimal string representation. - * - * @param bytes the byte array to convert. - * @param digits the array of hexadecimal digits. - * @return the hexadecimal string representation. - */ - private static String bytesToHex(byte[] bytes, char[] digits) { - char[] hexChars = new char[bytes.length * 2]; - for (int j = 0; j < bytes.length; j++) { - int v = bytes[j] & 0xFF; - hexChars[j * 2] = digits[v >>> 4]; - hexChars[j * 2 + 1] = digits[v & 0x0F]; - } - return new String(hexChars); - } - /** * Formats a string value. * - * @param builder the StringBuilder to append the formatted string to. * @param val the value to format. */ - private static void formatString(StringBuilder builder, Val val) { + private static String formatString(Val val) { TypeEnum type = val.type().typeEnum(); - if (type == TypeEnum.Bool) { - builder.append(val.booleanValue()); - } else if (type == TypeEnum.String || type == TypeEnum.Int || type == TypeEnum.Uint) { - builder.append(val.value()); - } else if (type == TypeEnum.Bytes) { - builder.append(new String((byte[]) val.value(), StandardCharsets.UTF_8)); - } else if (type == TypeEnum.Double) { - formatDecimal(builder, val); - } else if (type == TypeEnum.Duration) { - formatDuration(builder, val); - } else if (type == TypeEnum.Timestamp) { - formatTimestamp(builder, val); - } else if (type == TypeEnum.List) { - formatList(builder, val); - } else if (type == TypeEnum.Map) { - throw new ErrException("unimplemented string map type"); - } else if (type == TypeEnum.Null) { - throw new ErrException("unimplemented string null type"); + switch (type) { + case Bool: + return Boolean.toString(val.booleanValue()); + case String: + return val.value().toString(); + case Int: + case Uint: + Optional str = validateNumber(val); + if (str.isPresent()) { + return str.get(); + } + return val.value().toString(); + case Bytes: + return new String((byte[]) val.value(), StandardCharsets.UTF_8); + case Double: + Optional result = validateNumber(val); + if (result.isPresent()) { + return result.get(); + } + return formatDecimal(val); + case Duration: + return formatDuration(val); + case Timestamp: + return formatTimestamp(val); + case List: + return formatList((ListT) val); + case Map: + return formatMap((MapT) val); + case Null: + return "null"; + default: + throw new ErrException( + "error during formatting: string clause can only be used on strings, bools, bytes, ints, doubles, maps, lists, types, durations, and timestamps, was given " + + val.type()); } } /** * Formats a list value. * - * @param builder the StringBuilder to append the formatted list value to. * @param val the value to format. */ - @SuppressWarnings("rawtypes") - private static void formatList(StringBuilder builder, Val val) { + private static String formatList(ListT val) { + StringBuilder builder = new StringBuilder(); builder.append('['); - List list = val.convertToNative(List.class); - for (int i = 0; i < list.size(); i++) { - Object obj = list.get(i); - formatString(builder, DefaultTypeAdapter.nativeToValue(Db.newDb(), null, obj)); - if (i != list.size() - 1) { + + IteratorT iter = val.iterator(); + int index = 0; + while (iter.hasNext().booleanValue()) { + Val v = iter.next(); + builder.append(formatString(v)); + if (index != val.size().intValue() - 1) { builder.append(", "); } + index++; } builder.append(']'); + return builder.toString(); + } + + private static String formatMap(MapT val) { + StringBuilder builder = new StringBuilder(); + builder.append('{'); + + IteratorT iter = val.iterator(); + int index = 0; + while (iter.hasNext().booleanValue()) { + Val key = iter.next(); + String mapKey = formatString(key); + String mapVal = formatString(val.find(key)); + builder.append(mapKey).append(": ").append(mapVal); + if (index != val.size().intValue() - 1) { + builder.append(", "); + } + index++; + } + builder.append('}'); + return builder.toString(); } /** * Formats a timestamp value. * - * @param builder the StringBuilder to append the formatted timestamp value to. * @param val the value to format. */ - private static void formatTimestamp(StringBuilder builder, Val val) { + private static String formatTimestamp(Val val) { Timestamp timestamp = val.convertToNative(Timestamp.class); Instant instant = Instant.ofEpochSecond(timestamp.getSeconds(), timestamp.getNanos()); - builder.append(ISO_INSTANT.format(instant)); + return ISO_INSTANT.format(instant); } /** * Formats a duration value. * - * @param builder the StringBuilder to append the formatted duration value to. * @param val the value to format. */ - private static void formatDuration(StringBuilder builder, Val val) { + private static String formatDuration(Val val) { + StringBuilder builder = new StringBuilder(); Duration duration = val.convertToNative(Duration.class); double totalSeconds = duration.getSeconds() + (duration.getNanos() / 1_000_000_000.0); @@ -207,39 +236,131 @@ private static void formatDuration(StringBuilder builder, Val val) { DecimalFormat formatter = new DecimalFormat("0.#########"); builder.append(formatter.format(totalSeconds)); builder.append("s"); + + return builder.toString(); } /** * Formats a hexadecimal value. * - * @param builder the StringBuilder to append the formatted hexadecimal value to. * @param val the value to format. - * @param digits the array of hexadecimal digits. */ - private static void formatHex(StringBuilder builder, Val val, char[] digits) { - String hexString; + private static String formatHex(Val val) { TypeEnum type = val.type().typeEnum(); if (type == TypeEnum.Int || type == TypeEnum.Uint) { - hexString = Long.toHexString(val.intValue()); + return Long.toHexString(val.intValue()); } else if (type == TypeEnum.Bytes) { - byte[] bytes = (byte[]) val.value(); - hexString = bytesToHex(bytes, digits); + StringBuilder hexString = new StringBuilder(); + for (byte b : (byte[]) val.value()) { + hexString.append(String.format("%02x", b)); + } + return hexString.toString(); } else if (type == TypeEnum.String) { - hexString = val.value().toString(); + String arg = val.value().toString(); + return String.format("%x", new BigInteger(1, arg.getBytes(StandardCharsets.UTF_8))); } else { - throw new ErrException("formatHex: expected int or string"); + throw new ErrException( + "error during formatting: only integers, byte buffers, and strings can be formatted as hex, was given " + + typeToString(val)); } - builder.append(hexString); } /** * Formats a decimal value. * - * @param builder the StringBuilder to append the formatted decimal value to. * @param val the value to format. */ - private static void formatDecimal(StringBuilder builder, Val val) { - DecimalFormat formatter = new DecimalFormat("0.#########"); - builder.append(formatter.format(val.value())); + private static String formatDecimal(Val val) { + TypeEnum type = val.type().typeEnum(); + if (type == TypeEnum.Int || type == TypeEnum.Uint || type == TypeEnum.Double) { + Optional str = validateNumber(val); + if (str.isPresent()) { + return str.get(); + } + DecimalFormat formatter = new DecimalFormat("0.#########"); + return formatter.format(val.value()); + } else { + throw new ErrException( + "error during formatting: decimal clause can only be used on integers, was given " + + typeToString(val)); + } + } + + private static String formatOctal(Val val) { + TypeEnum type = val.type().typeEnum(); + if (type == TypeEnum.Int || type == TypeEnum.Uint) { + return Long.toOctalString(Long.valueOf(val.intValue())); + } else { + throw new ErrException( + "error during formatting: octal clause can only be used on integers, was given " + + typeToString(val)); + } + } + + private static String formatBinary(Val val) { + TypeEnum type = val.type().typeEnum(); + if (type == TypeEnum.Int || type == TypeEnum.Uint) { + return Long.toBinaryString(Long.valueOf(val.intValue())); + } else if (type == TypeEnum.Bool) { + return val.booleanValue() ? "1" : "0"; + } else { + throw new ErrException( + "error during formatting: only integers and bools can be formatted as binary, was given " + + typeToString(val)); + } + } + + private static String formatExponential(Val val, int precision) { + TypeEnum type = val.type().typeEnum(); + if (type == TypeEnum.Double) { + Optional str = validateNumber(val); + if (str.isPresent()) { + return str.get(); + } + String pattern = "%." + precision + "e"; + return String.format(pattern.toString(), val.doubleValue()); + } else { + throw new ErrException( + "error during formatting: scientific clause can only be used on doubles, was given " + + typeToString(val)); + } + } + + private static String formatFloat(Val val, int precision) { + TypeEnum type = val.type().typeEnum(); + if (type == TypeEnum.Double) { + Optional str = validateNumber(val); + if (str.isPresent()) { + return str.get(); + } + StringBuilder pattern = new StringBuilder("0."); + if (precision > 0) { + for (int i = 0; i < precision; i++) { + pattern.append("0"); + } + } else { + pattern.append("########"); + } + DecimalFormat formatter = new DecimalFormat(pattern.toString()); + return formatter.format(val.value()); + } else { + throw new ErrException( + "error during formatting: fixed-point clause can only be used on doubles, was given " + + typeToString(val)); + } + } + + private static Optional validateNumber(Val val) { + if (val.doubleValue() == Double.POSITIVE_INFINITY) { + return Optional.of("Infinity"); + } else if (val.doubleValue() == Double.NEGATIVE_INFINITY) { + return Optional.of("-Infinity"); + } + return Optional.empty(); + } + + private static String typeToString(Val val) { + TypeEnum type = val.type().typeEnum(); + return type.getName(); } } diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index 4d441ee4..305cfae6 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -15,8 +15,12 @@ package build.buf.protovalidate; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.fail; +import cel.expr.conformance.proto3.TestAllTypes; +import com.cel.expr.Decl; +import com.cel.expr.ExprValue; +import com.cel.expr.Value; import com.cel.expr.conformance.test.SimpleTest; import com.cel.expr.conformance.test.SimpleTestFile; import com.cel.expr.conformance.test.SimpleTestSection; @@ -28,28 +32,30 @@ import java.nio.file.InvalidPathException; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Stream; -import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.params.provider.MethodSource; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.api.Named; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.projectnessie.cel.Env; +import org.projectnessie.cel.EnvOption; import org.projectnessie.cel.EvalOption; import org.projectnessie.cel.Library; import org.projectnessie.cel.Program; import org.projectnessie.cel.ProgramOption; -import org.projectnessie.cel.common.types.DoubleT; -import org.projectnessie.cel.common.types.Err.ErrException; +import org.projectnessie.cel.checker.Decls; import org.projectnessie.cel.common.types.ListT; import org.projectnessie.cel.common.types.StringT; import org.projectnessie.cel.common.types.UintT; import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; +import org.projectnessie.cel.common.types.ref.TypeEnum; import org.projectnessie.cel.common.types.ref.Val; import org.projectnessie.cel.interpreter.Activation; @@ -60,8 +66,17 @@ class FormatTest { private static Env env; - private static SimpleTestSection formatSection; - private static SimpleTestSection formatErrorSection; + private static List formatTests; + private static List formatErrorTests; + + private static List SKIPPED_TESTS = + Arrays.asList( + "type() support for string", + "map support for string", + "map support (all key types)", + "dyntype support for maps", + "object not allowed", + "object inside map"); @BeforeAll static void setUp() { @@ -75,97 +90,89 @@ static void setUp() { TextFormat.getParser().merge(data, bldr); SimpleTestFile testData = bldr.build(); - List fs = - testData.getSectionList().stream() + List sections = testData.getSectionList(); + + // Find the format tests which test successful formatting + // Defaults to an empty list if nothing is found + formatTests = + sections.stream() .filter(s -> s.getName().equals("format")) - .collect(Collectors.toList()); - if (fs.size() == 1) { - formatSection = fs.get(0); - } + .findFirst() + .map(SimpleTestSection::getTestList) + .orElse(Collections.emptyList()); - List fes = - testData.getSectionList().stream() + // Find the format error tests which test errors during formatting + // Defaults to an empty list if nothing is found + formatErrorTests = + sections.stream() .filter(s -> s.getName().equals("format_errors")) - .collect(Collectors.toList()); - if (fes.size() == 1) { - formatErrorSection = fes.get(0); - } + .findFirst() + .map(SimpleTestSection::getTestList) + .orElse(Collections.emptyList()); env = Env.newEnv(Library.Lib(new ValidateLibrary())); - } catch (InvalidPathException ipe) { - System.err.println(ipe); - } catch (IOException ioe) { - System.err.println(ioe); + } catch (InvalidPathException | IOException e) { + fail(e); } } - private static Stream getFormatTests() { - List args = new ArrayList(); - for (SimpleTest test : formatSection.getTestList()) { - args.add(Arguments.arguments(Named.named(test.getName(), test))); - } - - return args.stream(); - } - @ParameterizedTest() @MethodSource("getFormatTests") void testFormatSuccess(SimpleTest test) { - Env.AstIssuesTuple ast = env.compile(test.getExpr()); - Map map = new HashMap(); - ProgramOption globals = ProgramOption.globals(map); - List globs = new ArrayList(); - globs.add(globals); - Program program = - env.program(ast.getAst(), globals, ProgramOption.evalOptions(EvalOption.OptTrackState)); - program.eval(Activation.emptyActivation()); - // System.err.println(evalResult.getVal()); - // System.err.println(evalResult.getEvalDetails().getState()); + List decls = buildDecls(test); + + TestAllTypes msg = TestAllTypes.newBuilder().getDefaultInstanceForType(); + Env newEnv = env.extend(EnvOption.types(msg), EnvOption.declarations(decls)); + + Env.AstIssuesTuple ast = newEnv.compile(test.getExpr()); + if (ast.hasIssues()) { + fail("error building AST for evaluation: " + ast.getIssues().toString()); + } + Map vars = buildVars(test.getBindingsMap()); + ProgramOption globals = ProgramOption.globals(vars); + Program program = + newEnv.program(ast.getAst(), globals, ProgramOption.evalOptions(EvalOption.OptTrackState)); + Program.EvalResult result = program.eval(Activation.emptyActivation()); + + assertThat(result.getVal().value()).isEqualTo(getExpectedResult(test)); + assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.String); } - @Test - void testNotEnoughArgumentsThrows() { - StringT one = StringT.stringOf("one"); - ListT val = (ListT) ListT.newValArrayList(null, new Val[] {one}); - - assertThatThrownBy( - () -> { - Format.format("first value: %s and %s", val); - }) - .isInstanceOf(ErrException.class) - .hasMessageContaining("format: not enough arguments"); + @ParameterizedTest() + @MethodSource("getFormatErrorTests") + void testFormatError(SimpleTest test) { + List decls = buildDecls(test); + + TestAllTypes msg = TestAllTypes.newBuilder().getDefaultInstanceForType(); + Env newEnv = env.extend(EnvOption.types(msg), EnvOption.declarations(decls)); + + Env.AstIssuesTuple ast = newEnv.compile(test.getExpr()); + if (ast.hasIssues()) { + fail("error building AST for evaluation: " + ast.getIssues().toString()); + } + Map vars = buildVars(test.getBindingsMap()); + ProgramOption globals = ProgramOption.globals(vars); + Program program = + newEnv.program( + ast.getAst(), + globals, + ProgramOption.evalOptions( + EvalOption.OptTrackState, + EvalOption.OptExhaustiveEval, + EvalOption.OptOptimize, + EvalOption.OptPartialEval)); + Program.EvalResult result = program.eval(Activation.emptyActivation()); + assertThat(result.getVal().value()).isEqualTo(getExpectedResult(test)); + assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.Err); } @Test - void testDouble() { - ListT val = - (ListT) - ListT.newValArrayList( - null, - new Val[] { - DoubleT.doubleOf(-1.20000000000), - DoubleT.doubleOf(-1.2), - DoubleT.doubleOf(-1.230), - DoubleT.doubleOf(-1.002), - DoubleT.doubleOf(-0.1), - DoubleT.doubleOf(-.1), - DoubleT.doubleOf(-1), - DoubleT.doubleOf(-0.0), - DoubleT.doubleOf(0), - DoubleT.doubleOf(0.0), - DoubleT.doubleOf(1), - DoubleT.doubleOf(0.1), - DoubleT.doubleOf(.1), - DoubleT.doubleOf(1.002), - DoubleT.doubleOf(1.230), - DoubleT.doubleOf(1.20000000000) - }); - String formatted = - Format.format("%d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d", val); - assertThat(formatted) - .isEqualTo( - "-1.2, -1.2, -1.23, -1.002, -0.1, -0.1, -1, -0, 0, 0, 1, 0.1, 0.1, 1.002, 1.23, 1.2"); + void testStrang() { + StringT strang = StringT.stringOf("Bangarang"); + ListT val = (ListT) ListT.newValArrayList(null, new Val[] {strang}); + String formatted = Format.format("%s", val); + assertThat(formatted).isEqualTo("999999999999"); } @Test @@ -186,32 +193,66 @@ void testDuration() { assertThat(formatted).isEqualTo("123.000045678s"); } - @Test - void testEmptyDuration() { - Duration duration = Duration.newBuilder().build(); - ListT val = - (ListT) ListT.newGenericArrayList(DefaultTypeAdapter.Instance, new Duration[] {duration}); - String formatted = Format.format("%s", val); - assertThat(formatted).isEqualTo("0s"); + private static Stream getTestStream(List tests) { + List args = new ArrayList(); + for (SimpleTest test : tests) { + if (!SKIPPED_TESTS.contains(test.getName())) { + args.add(Arguments.arguments(Named.named(test.getName(), test))); + } + } + + return args.stream(); } - @Test - void testDurationSecondsOnly() { - Duration duration = Duration.newBuilder().setSeconds(123).build(); + private static Stream getFormatTests() { + return getTestStream(formatTests); + } - ListT val = - (ListT) ListT.newGenericArrayList(DefaultTypeAdapter.Instance, new Duration[] {duration}); - String formatted = Format.format("%s", val); - assertThat(formatted).isEqualTo("123s"); + private static Stream getFormatErrorTests() { + return getTestStream(formatErrorTests); } - @Test - void testDurationNanosOnly() { - Duration duration = Duration.newBuilder().setNanos(42).build(); + private Map buildVars(Map bindings) { + Map vars = new HashMap(); + for (Map.Entry entry : bindings.entrySet()) { + ExprValue exprValue = entry.getValue(); + if (exprValue.hasValue()) { + Value val = exprValue.getValue(); + if (val.hasStringValue()) { + vars.put(entry.getKey(), val.getStringValue()); + } + } + } + return vars; + } - ListT val = - (ListT) ListT.newGenericArrayList(DefaultTypeAdapter.Instance, new Duration[] {duration}); - String formatted = Format.format("%s", val); - assertThat(formatted).isEqualTo("0.000000042s"); + private String getExpectedResult(SimpleTest test) { + if (test.hasValue()) { + if (test.getValue().hasStringValue()) { + return test.getValue().getStringValue(); + } + } else if (test.hasEvalError()) { + if (test.getEvalError().getErrorsList().size() == 1) { + return test.getEvalError().getErrorsList().get(0).getMessage(); + } + } + return ""; + } + + private List buildDecls(SimpleTest test) { + List decls = + new ArrayList(); + for (Decl decl : test.getTypeEnvList()) { + if (decl.hasIdent()) { + Decl.IdentDecl ident = decl.getIdent(); + com.cel.expr.Type type = ident.getType(); + if (type.hasPrimitive()) { + if (type.getPrimitive() == com.cel.expr.Type.PrimitiveType.STRING) { + decls.add(Decls.newVar(decl.getName(), Decls.String)); + } + } + } + } + return decls; } } diff --git a/src/test/resources/proto/buf.gen.cel.proto3.yaml b/src/test/resources/proto/buf.gen.cel.proto3.yaml new file mode 100644 index 00000000..5cde42ac --- /dev/null +++ b/src/test/resources/proto/buf.gen.cel.proto3.yaml @@ -0,0 +1,9 @@ +version: v2 +managed: + enabled: true + override: + - file_option: java_package + value: cel.expr.conformance.proto3 +plugins: + - remote: buf.build/protocolbuffers/java:v31.0 + out: build/generated/test-sources/bufgen From 5f4f04082e8aed0ca962d5dd05027079525ee09f Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 28 May 2025 10:16:02 -0400 Subject: [PATCH 4/9] Tests --- src/test/java/build/buf/protovalidate/FormatTest.java | 9 +++++++++ .../resources/proto/validationtest/validationtest.proto | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index 305cfae6..5b040bd1 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -71,11 +71,20 @@ class FormatTest { private static List SKIPPED_TESTS = Arrays.asList( + // Success Tests + // found no matching overload for 'format' applied to 'string.(list(type(string)))' "type() support for string", + // found no matching overload for 'format' applied to 'string.(list(map(string, dyn)))' "map support for string", + // found no matching overload for 'format' applied to 'string.(list(map(dyn, dyn)))' "map support (all key types)", + // found no matching overload for 'format' applied to 'string.(list(map(dyn, dyn)))' "dyntype support for maps", + // Error Tests + // found no matching overload for 'format' applied to + // 'string.(list(cel.expr.conformance.proto3.TestAllTypes))' "object not allowed", + // found no matching overload for 'format' applied to 'string.(list(map(int, dyn)))' "object inside map"); @BeforeAll diff --git a/src/test/resources/proto/validationtest/validationtest.proto b/src/test/resources/proto/validationtest/validationtest.proto index b73409b4..66ba6b70 100644 --- a/src/test/resources/proto/validationtest/validationtest.proto +++ b/src/test/resources/proto/validationtest/validationtest.proto @@ -63,6 +63,6 @@ message FieldExpressionMapInt32 { map val = 1 [(buf.validate.field).cel = { id: "field_expression.map.int32" message: "all map values must equal 1" - expression: "this.all(k, this[k] == 1)" + expression: "'%s'.format([type('test string')])" }]; } From acd4d529d4968393cb0d66b932ee76a0313fe44e Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 28 May 2025 10:20:34 -0400 Subject: [PATCH 5/9] Revert --- .../buf/protovalidate/CustomOverload.java | 33 ++++++++----------- .../build/buf/protovalidate/FormatTest.java | 33 ------------------- .../proto/validationtest/validationtest.proto | 2 +- 3 files changed, 15 insertions(+), 53 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/CustomOverload.java b/src/main/java/build/buf/protovalidate/CustomOverload.java index 471d790c..8383e38c 100644 --- a/src/main/java/build/buf/protovalidate/CustomOverload.java +++ b/src/main/java/build/buf/protovalidate/CustomOverload.java @@ -110,25 +110,20 @@ private static Overload celGetField() { * @return The {@link Overload} instance for the "format" operation. */ private static Overload celFormat() { - Overload ol = - Overload.binary( - OVERLOAD_FORMAT, - (lhs, rhs) -> { - if (lhs.type().typeEnum() != TypeEnum.String - || rhs.type().typeEnum() != TypeEnum.List) { - return Err.noSuchOverload(lhs, OVERLOAD_FORMAT, rhs); - } - ListT list = (ListT) rhs.convertToType(ListT.ListType); - String formatString = (String) lhs.value(); - try { - return StringT.stringOf(Format.format(formatString, list)); - } catch (Err.ErrException e) { - return e.getErr(); - } - }); - - System.err.println(ol); - return ol; + return Overload.binary( + OVERLOAD_FORMAT, + (lhs, rhs) -> { + if (lhs.type().typeEnum() != TypeEnum.String || rhs.type().typeEnum() != TypeEnum.List) { + return Err.noSuchOverload(lhs, OVERLOAD_FORMAT, rhs); + } + ListT list = (ListT) rhs.convertToType(ListT.ListType); + String formatString = (String) lhs.value(); + try { + return StringT.stringOf(Format.format(formatString, list)); + } catch (Err.ErrException e) { + return e.getErr(); + } + }); } /** diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index 5b040bd1..d99e94aa 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -24,7 +24,6 @@ import com.cel.expr.conformance.test.SimpleTest; import com.cel.expr.conformance.test.SimpleTestFile; import com.cel.expr.conformance.test.SimpleTestSection; -import com.google.protobuf.Duration; import com.google.protobuf.TextFormat; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -40,7 +39,6 @@ import java.util.stream.Stream; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Named; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -51,12 +49,7 @@ import org.projectnessie.cel.Program; import org.projectnessie.cel.ProgramOption; import org.projectnessie.cel.checker.Decls; -import org.projectnessie.cel.common.types.ListT; -import org.projectnessie.cel.common.types.StringT; -import org.projectnessie.cel.common.types.UintT; -import org.projectnessie.cel.common.types.pb.DefaultTypeAdapter; import org.projectnessie.cel.common.types.ref.TypeEnum; -import org.projectnessie.cel.common.types.ref.Val; import org.projectnessie.cel.interpreter.Activation; class FormatTest { @@ -176,32 +169,6 @@ void testFormatError(SimpleTest test) { assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.Err); } - @Test - void testStrang() { - StringT strang = StringT.stringOf("Bangarang"); - ListT val = (ListT) ListT.newValArrayList(null, new Val[] {strang}); - String formatted = Format.format("%s", val); - assertThat(formatted).isEqualTo("999999999999"); - } - - @Test - void testLargeDecimalValuesAreProperlyFormatted() { - UintT largeDecimal = UintT.uintOf(999999999999L); - ListT val = (ListT) ListT.newValArrayList(null, new Val[] {largeDecimal}); - String formatted = Format.format("%s", val); - assertThat(formatted).isEqualTo("999999999999"); - } - - @Test - void testDuration() { - Duration duration = Duration.newBuilder().setSeconds(123).setNanos(45678).build(); - - ListT val = - (ListT) ListT.newGenericArrayList(DefaultTypeAdapter.Instance, new Duration[] {duration}); - String formatted = Format.format("%s", val); - assertThat(formatted).isEqualTo("123.000045678s"); - } - private static Stream getTestStream(List tests) { List args = new ArrayList(); for (SimpleTest test : tests) { diff --git a/src/test/resources/proto/validationtest/validationtest.proto b/src/test/resources/proto/validationtest/validationtest.proto index 66ba6b70..b73409b4 100644 --- a/src/test/resources/proto/validationtest/validationtest.proto +++ b/src/test/resources/proto/validationtest/validationtest.proto @@ -63,6 +63,6 @@ message FieldExpressionMapInt32 { map val = 1 [(buf.validate.field).cel = { id: "field_expression.map.int32" message: "all map values must equal 1" - expression: "'%s'.format([type('test string')])" + expression: "this.all(k, this[k] == 1)" }]; } From ef681848f265a4311bd860fdf3b46d4e947d4015 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 28 May 2025 10:31:58 -0400 Subject: [PATCH 6/9] Docs --- Makefile | 4 ---- build.gradle.kts | 13 +++++++++---- ...n.cel.proto3.yaml => buf.gen.cel.testtypes.yaml} | 0 3 files changed, 9 insertions(+), 8 deletions(-) rename src/test/resources/proto/{buf.gen.cel.proto3.yaml => buf.gen.cel.testtypes.yaml} (100%) diff --git a/Makefile b/Makefile index 836972d3..9849c9a9 100644 --- a/Makefile +++ b/Makefile @@ -7,10 +7,6 @@ MAKEFLAGS += --warn-undefined-variables MAKEFLAGS += --no-builtin-rules MAKEFLAGS += --no-print-directory GRADLE ?= ./gradlew -# Version of the cel-spec that this implementation is conformant with -# This should be kept in sync with the version in format_test.py -CEL_SPEC_VERSION ?= v0.24.0 -TESTDATA_FILE := tests/testdata/string_ext_$(CEL_SPEC_VERSION).textproto .PHONY: all all: lint generate build docs conformance ## Run all tests and lint (default) diff --git a/build.gradle.kts b/build.gradle.kts index 494227f2..9bc2f8e2 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -91,7 +91,7 @@ tasks.register("generateTestSourcesNoImports") { } tasks.register("generateCelConformance") { - dependsOn("exportProtovalidateModule") + dependsOn("generateCelConformanceTestTypes") description = "Generates CEL conformance code with buf generate for unit tests." commandLine( buf.asPath, @@ -106,14 +106,19 @@ tasks.register("generateCelConformance") { ) } +// The conformance tests use the Protobuf package path for tests that use these types. +// i.e. cel.expr.conformance.proto3.TestAllTypes. But, if we use managed mode it adds 'com' +// to the prefix. Additionally, we can't disable managed mode because the java_package option +// specified in these proto files is "dev.cel.expr.conformance.proto3". So, to get around this, +// we're generating these separately and specifying a java_package override of the package we need. tasks.register("generateCelConformanceTestTypes") { dependsOn("exportProtovalidateModule") - description = "Generates CEL conformance code with buf generate for unit tests." + description = "Generates CEL conformance test types with buf generate for unit tests using a Java package override." commandLine( buf.asPath, "generate", "--template", - "src/test/resources/proto/buf.gen.cel.proto3.yaml", + "src/test/resources/proto/buf.gen.cel.testtypes.yaml", "buf.build/google/cel-spec:${project.findProperty("cel.spec.version")}", "--path", "cel/expr/conformance/proto3", @@ -144,7 +149,7 @@ var getCelTestData = } tasks.register("generateTestSources") { - dependsOn("generateTestSourcesImports", "generateTestSourcesNoImports", "generateCelConformance", "generateCelConformanceTestTypes") + dependsOn("generateTestSourcesImports", "generateTestSourcesNoImports", "generateCelConformance") description = "Generates code with buf generate for unit tests" } diff --git a/src/test/resources/proto/buf.gen.cel.proto3.yaml b/src/test/resources/proto/buf.gen.cel.testtypes.yaml similarity index 100% rename from src/test/resources/proto/buf.gen.cel.proto3.yaml rename to src/test/resources/proto/buf.gen.cel.testtypes.yaml From 4e78584f9c3625f34f5770bad8558bf4bba34fdc Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 28 May 2025 10:44:08 -0400 Subject: [PATCH 7/9] Docs --- build.gradle.kts | 1 - .../java/build/buf/protovalidate/Format.java | 2 +- .../build/buf/protovalidate/FormatTest.java | 22 +++++++++---------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 9bc2f8e2..894fd20e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -272,7 +272,6 @@ allprojects { showExceptions = true showCauses = true showStackTraces = true - showStandardStreams = true } } } diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index fd5d04b0..6d6c185a 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -318,7 +318,7 @@ private static String formatExponential(Val val, int precision) { return str.get(); } String pattern = "%." + precision + "e"; - return String.format(pattern.toString(), val.doubleValue()); + return String.format(pattern, val.doubleValue()); } else { throw new ErrException( "error during formatting: scientific clause can only be used on doubles, was given " diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index d99e94aa..b5ab6fdf 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -131,12 +131,12 @@ void testFormatSuccess(SimpleTest test) { if (ast.hasIssues()) { fail("error building AST for evaluation: " + ast.getIssues().toString()); } - Map vars = buildVars(test.getBindingsMap()); + Map vars = buildVariables(test.getBindingsMap()); ProgramOption globals = ProgramOption.globals(vars); Program program = newEnv.program(ast.getAst(), globals, ProgramOption.evalOptions(EvalOption.OptTrackState)); - Program.EvalResult result = program.eval(Activation.emptyActivation()); + Program.EvalResult result = program.eval(Activation.emptyActivation()); assertThat(result.getVal().value()).isEqualTo(getExpectedResult(test)); assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.String); } @@ -153,17 +153,11 @@ void testFormatError(SimpleTest test) { if (ast.hasIssues()) { fail("error building AST for evaluation: " + ast.getIssues().toString()); } - Map vars = buildVars(test.getBindingsMap()); + Map vars = buildVariables(test.getBindingsMap()); ProgramOption globals = ProgramOption.globals(vars); Program program = - newEnv.program( - ast.getAst(), - globals, - ProgramOption.evalOptions( - EvalOption.OptTrackState, - EvalOption.OptExhaustiveEval, - EvalOption.OptOptimize, - EvalOption.OptPartialEval)); + newEnv.program(ast.getAst(), globals, ProgramOption.evalOptions(EvalOption.OptTrackState)); + Program.EvalResult result = program.eval(Activation.emptyActivation()); assertThat(result.getVal().value()).isEqualTo(getExpectedResult(test)); assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.Err); @@ -188,7 +182,8 @@ private static Stream getFormatErrorTests() { return getTestStream(formatErrorTests); } - private Map buildVars(Map bindings) { + // Builds the variable definitions to be used during evaluation + private Map buildVariables(Map bindings) { Map vars = new HashMap(); for (Map.Entry entry : bindings.entrySet()) { ExprValue exprValue = entry.getValue(); @@ -202,12 +197,14 @@ private Map buildVars(Map bindings) { return vars; } + // Gets the expected result for a given test private String getExpectedResult(SimpleTest test) { if (test.hasValue()) { if (test.getValue().hasStringValue()) { return test.getValue().getStringValue(); } } else if (test.hasEvalError()) { + // Note that we only expect a single eval error for all the conformance tests if (test.getEvalError().getErrorsList().size() == 1) { return test.getEvalError().getErrorsList().get(0).getMessage(); } @@ -215,6 +212,7 @@ private String getExpectedResult(SimpleTest test) { return ""; } + // Builds the declarations for a given test private List buildDecls(SimpleTest test) { List decls = new ArrayList(); From 9756575d80123dd421c5c58ea9804c88312cbd99 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 28 May 2025 11:03:03 -0400 Subject: [PATCH 8/9] Lint --- .../build/buf/protovalidate/FormatTest.java | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index b5ab6fdf..d3e13e4a 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -122,21 +122,7 @@ static void setUp() { @ParameterizedTest() @MethodSource("getFormatTests") void testFormatSuccess(SimpleTest test) { - List decls = buildDecls(test); - - TestAllTypes msg = TestAllTypes.newBuilder().getDefaultInstanceForType(); - Env newEnv = env.extend(EnvOption.types(msg), EnvOption.declarations(decls)); - - Env.AstIssuesTuple ast = newEnv.compile(test.getExpr()); - if (ast.hasIssues()) { - fail("error building AST for evaluation: " + ast.getIssues().toString()); - } - Map vars = buildVariables(test.getBindingsMap()); - ProgramOption globals = ProgramOption.globals(vars); - Program program = - newEnv.program(ast.getAst(), globals, ProgramOption.evalOptions(EvalOption.OptTrackState)); - - Program.EvalResult result = program.eval(Activation.emptyActivation()); + Program.EvalResult result = evaluate(test); assertThat(result.getVal().value()).isEqualTo(getExpectedResult(test)); assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.String); } @@ -144,6 +130,14 @@ void testFormatSuccess(SimpleTest test) { @ParameterizedTest() @MethodSource("getFormatErrorTests") void testFormatError(SimpleTest test) { + Program.EvalResult result = evaluate(test); + assertThat(result.getVal().value()).isEqualTo(getExpectedResult(test)); + assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.Err); + } + + // Runs a test by extending the cel environment with the specified + // types, variables and declarations, then evaluating it with the cel runtime. + private static Program.EvalResult evaluate(SimpleTest test) { List decls = buildDecls(test); TestAllTypes msg = TestAllTypes.newBuilder().getDefaultInstanceForType(); @@ -158,9 +152,7 @@ void testFormatError(SimpleTest test) { Program program = newEnv.program(ast.getAst(), globals, ProgramOption.evalOptions(EvalOption.OptTrackState)); - Program.EvalResult result = program.eval(Activation.emptyActivation()); - assertThat(result.getVal().value()).isEqualTo(getExpectedResult(test)); - assertThat(result.getVal().type().typeEnum()).isEqualTo(TypeEnum.Err); + return program.eval(Activation.emptyActivation()); } private static Stream getTestStream(List tests) { @@ -183,7 +175,7 @@ private static Stream getFormatErrorTests() { } // Builds the variable definitions to be used during evaluation - private Map buildVariables(Map bindings) { + private static Map buildVariables(Map bindings) { Map vars = new HashMap(); for (Map.Entry entry : bindings.entrySet()) { ExprValue exprValue = entry.getValue(); @@ -198,13 +190,13 @@ private Map buildVariables(Map bindings) { } // Gets the expected result for a given test - private String getExpectedResult(SimpleTest test) { + private static String getExpectedResult(SimpleTest test) { if (test.hasValue()) { if (test.getValue().hasStringValue()) { return test.getValue().getStringValue(); } } else if (test.hasEvalError()) { - // Note that we only expect a single eval error for all the conformance tests + // Note that we only expect a single eval error for all the conformance tests if (test.getEvalError().getErrorsList().size() == 1) { return test.getEvalError().getErrorsList().get(0).getMessage(); } @@ -213,7 +205,7 @@ private String getExpectedResult(SimpleTest test) { } // Builds the declarations for a given test - private List buildDecls(SimpleTest test) { + private static List buildDecls(SimpleTest test) { List decls = new ArrayList(); for (Decl decl : test.getTypeEnvList()) { From fba290ba0aebcd3623172cb765752f4245f79b64 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Wed, 28 May 2025 11:58:25 -0400 Subject: [PATCH 9/9] Feedback --- .../java/build/buf/protovalidate/Format.java | 8 +-- .../build/buf/protovalidate/FormatTest.java | 68 ++++++++----------- 2 files changed, 32 insertions(+), 44 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index 6d6c185a..cfb42eb6 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -178,14 +178,12 @@ private static String formatList(ListT val) { builder.append('['); IteratorT iter = val.iterator(); - int index = 0; while (iter.hasNext().booleanValue()) { Val v = iter.next(); builder.append(formatString(v)); - if (index != val.size().intValue() - 1) { + if (iter.hasNext().booleanValue()) { builder.append(", "); } - index++; } builder.append(']'); return builder.toString(); @@ -196,16 +194,14 @@ private static String formatMap(MapT val) { builder.append('{'); IteratorT iter = val.iterator(); - int index = 0; while (iter.hasNext().booleanValue()) { Val key = iter.next(); String mapKey = formatString(key); String mapVal = formatString(val.find(key)); builder.append(mapKey).append(": ").append(mapVal); - if (index != val.size().intValue() - 1) { + if (iter.hasNext().booleanValue()) { builder.append(", "); } - index++; } builder.append('}'); return builder.toString(); diff --git a/src/test/java/build/buf/protovalidate/FormatTest.java b/src/test/java/build/buf/protovalidate/FormatTest.java index d3e13e4a..f63c91d4 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -25,10 +25,8 @@ import com.cel.expr.conformance.test.SimpleTestFile; import com.cel.expr.conformance.test.SimpleTestSection; import com.google.protobuf.TextFormat; -import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; -import java.nio.file.InvalidPathException; import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; @@ -81,42 +79,36 @@ class FormatTest { "object inside map"); @BeforeAll - static void setUp() { - try { - byte[] encoded = - Files.readAllBytes( - Paths.get( - "src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto")); - String data = new String(encoded, StandardCharsets.UTF_8); - SimpleTestFile.Builder bldr = SimpleTestFile.newBuilder(); - TextFormat.getParser().merge(data, bldr); - SimpleTestFile testData = bldr.build(); - - List sections = testData.getSectionList(); - - // Find the format tests which test successful formatting - // Defaults to an empty list if nothing is found - formatTests = - sections.stream() - .filter(s -> s.getName().equals("format")) - .findFirst() - .map(SimpleTestSection::getTestList) - .orElse(Collections.emptyList()); - - // Find the format error tests which test errors during formatting - // Defaults to an empty list if nothing is found - formatErrorTests = - sections.stream() - .filter(s -> s.getName().equals("format_errors")) - .findFirst() - .map(SimpleTestSection::getTestList) - .orElse(Collections.emptyList()); - - env = Env.newEnv(Library.Lib(new ValidateLibrary())); - - } catch (InvalidPathException | IOException e) { - fail(e); - } + static void setUp() throws Exception { + byte[] encoded = + Files.readAllBytes( + Paths.get("src/test/resources/testdata/string_ext_" + CEL_SPEC_VERSION + ".textproto")); + String data = new String(encoded, StandardCharsets.UTF_8); + SimpleTestFile.Builder bldr = SimpleTestFile.newBuilder(); + TextFormat.getParser().merge(data, bldr); + SimpleTestFile testData = bldr.build(); + + List sections = testData.getSectionList(); + + // Find the format tests which test successful formatting + // Defaults to an empty list if nothing is found + formatTests = + sections.stream() + .filter(s -> s.getName().equals("format")) + .findFirst() + .map(SimpleTestSection::getTestList) + .orElse(Collections.emptyList()); + + // Find the format error tests which test errors during formatting + // Defaults to an empty list if nothing is found + formatErrorTests = + sections.stream() + .filter(s -> s.getName().equals("format_errors")) + .findFirst() + .map(SimpleTestSection::getTestList) + .orElse(Collections.emptyList()); + + env = Env.newEnv(Library.Lib(new ValidateLibrary())); } @ParameterizedTest()