From 024853ef60af08e09422c03f8494a56586c4334a Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Fri, 30 May 2025 15:18:09 -0400 Subject: [PATCH 1/3] Fix declarations for format overload so that all types are accepted --- .../buf/protovalidate/CustomDeclarations.java | 39 +++---------------- .../java/build/buf/protovalidate/Format.java | 22 +++++++---- .../build/buf/protovalidate/FormatTest.java | 18 +-------- .../string_ext_supplemental.textproto | 4 +- 4 files changed, 24 insertions(+), 59 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/CustomDeclarations.java b/src/main/java/build/buf/protovalidate/CustomDeclarations.java index 095478bd..d46d07a7 100644 --- a/src/main/java/build/buf/protovalidate/CustomDeclarations.java +++ b/src/main/java/build/buf/protovalidate/CustomDeclarations.java @@ -160,39 +160,12 @@ static List create() { // Add 'format' function declaration List formatOverloads = new ArrayList<>(); - for (com.google.api.expr.v1alpha1.Type type : - Arrays.asList( - Decls.String, - Decls.Int, - Decls.Uint, - Decls.Double, - Decls.Bytes, - Decls.Bool, - Decls.Duration, - Decls.Timestamp)) { - formatOverloads.add( - Decls.newInstanceOverload( - String.format("format_%s", Types.formatCheckedType(type).toLowerCase(Locale.US)), - Arrays.asList(Decls.String, Decls.newListType(type)), - Decls.String)); - formatOverloads.add( - Decls.newInstanceOverload( - String.format("format_list_%s", Types.formatCheckedType(type).toLowerCase(Locale.US)), - Arrays.asList(Decls.String, Decls.newListType(Decls.newListType(type))), - Decls.String)); - formatOverloads.add( - Decls.newInstanceOverload( - String.format( - "format_bytes_%s", Types.formatCheckedType(type).toLowerCase(Locale.US)), - Arrays.asList(Decls.Bytes, Decls.newListType(type)), - Decls.Bytes)); - formatOverloads.add( - Decls.newInstanceOverload( - String.format( - "format_bytes_list_%s", Types.formatCheckedType(type).toLowerCase(Locale.US)), - Arrays.asList(Decls.Bytes, Decls.newListType(Decls.newListType(type))), - Decls.Bytes)); - } + formatOverloads.add( + Decls.newInstanceOverload( + "format_list_dyn", + Arrays.asList(Decls.String, Decls.newListType(Decls.Dyn)), + Decls.String)); + decls.add(Decls.newFunction("format", formatOverloads)); return Collections.unmodifiableList(decls); diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index 38a7008a..63cbfcf7 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -22,6 +22,9 @@ import java.nio.charset.StandardCharsets; import java.text.DecimalFormat; import java.time.Instant; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Locale; import java.util.Optional; import org.projectnessie.cel.common.types.Err.ErrException; @@ -132,10 +135,11 @@ static String format(String fmtString, ListT list) { private static String formatString(Val val) { TypeEnum type = val.type().typeEnum(); switch (type) { - case Bool: - return Boolean.toString(val.booleanValue()); + case Type: case String: return val.value().toString(); + case Bool: + return Boolean.toString(val.booleanValue()); case Int: case Uint: Optional str = validateNumber(val); @@ -195,17 +199,21 @@ private static String formatMap(MapT val) { StringBuilder builder = new StringBuilder(); builder.append('{'); + List entries = new ArrayList(); + IteratorT iter = val.iterator(); 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 (iter.hasNext().booleanValue()) { - builder.append(", "); - } + entries.add(mapKey + ": " + mapVal); } - builder.append('}'); + + // The formatted string should be sorted by keys + Collections.sort(entries); + + builder.append(String.join(", ", entries)).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 88f51b39..feee7057 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -60,23 +60,7 @@ class FormatTest { private static List formatTests; private static List formatErrorTests; - 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"); + private static List SKIPPED_TESTS = Arrays.asList(); @BeforeAll private static void setUp() throws Exception { diff --git a/src/test/resources/testdata/string_ext_supplemental.textproto b/src/test/resources/testdata/string_ext_supplemental.textproto index 6f4402fc..dde7b83b 100644 --- a/src/test/resources/testdata/string_ext_supplemental.textproto +++ b/src/test/resources/testdata/string_ext_supplemental.textproto @@ -11,14 +11,14 @@ section: { name: "format" test: { name: "bytes support for string with invalid utf-8 encoding" - expr: '"%s".format([b"\xF0abc\x8C\xF0xyz"])' + expr: '"%s".format([b"\\xF0abc\\x8C\\xF0xyz"])' value: { string_value: '\ufffdabc\ufffdxyz', } } test: { name: "bytes support for string with only invalid utf-8 sequences" - expr: '"%s".format([b"\xF0\x8C\xF0"])' + expr: '"%s".format([b"\\xF0\\x8C\\xF0"])' value: { string_value: '\ufffd', } From 6425dae8a9be0182633b89b9b2deeb770e3ff398 Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Fri, 30 May 2025 15:24:04 -0400 Subject: [PATCH 2/3] Fix declarations for format overload so that all types are accepted --- .../java/build/buf/protovalidate/Format.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index 63cbfcf7..a4879447 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -267,7 +267,7 @@ private static String formatHex(Val val) { } else { throw new ErrException( "error during formatting: only integers, byte buffers, and strings can be formatted as hex, was given " - + typeToString(val)); + + val.type()); } } @@ -288,7 +288,7 @@ private static String formatDecimal(Val val) { } else { throw new ErrException( "error during formatting: decimal clause can only be used on integers, was given " - + typeToString(val)); + + val.type()); } } @@ -299,7 +299,7 @@ private static String formatOctal(Val val) { } else { throw new ErrException( "error during formatting: octal clause can only be used on integers, was given " - + typeToString(val)); + + val.type()); } } @@ -312,7 +312,7 @@ private static String formatBinary(Val val) { } else { throw new ErrException( "error during formatting: only integers and bools can be formatted as binary, was given " - + typeToString(val)); + + val.type()); } } @@ -328,7 +328,7 @@ private static String formatExponential(Val val, int precision) { } else { throw new ErrException( "error during formatting: scientific clause can only be used on doubles, was given " - + typeToString(val)); + + val.type()); } } @@ -352,7 +352,7 @@ private static String formatFloat(Val val, int precision) { } else { throw new ErrException( "error during formatting: fixed-point clause can only be used on doubles, was given " - + typeToString(val)); + + val.type()); } } @@ -364,9 +364,4 @@ private static Optional validateNumber(Val val) { } return Optional.empty(); } - - private static String typeToString(Val val) { - TypeEnum type = val.type().typeEnum(); - return type.getName(); - } } From 077aeefa7bb86bc0d22d41f70aa2d7d6726cb28c Mon Sep 17 00:00:00 2001 From: Steve Ayers Date: Fri, 30 May 2025 16:21:11 -0400 Subject: [PATCH 3/3] Feedback --- .../java/build/buf/protovalidate/Format.java | 18 +++++++++++------- .../build/buf/protovalidate/FormatTest.java | 14 ++++---------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index a4879447..9b1a8211 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -22,11 +22,13 @@ import java.nio.charset.StandardCharsets; import java.text.DecimalFormat; import java.time.Instant; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Optional; +import java.util.SortedMap; +import java.util.TreeMap; +import java.util.stream.Collectors; import org.projectnessie.cel.common.types.Err.ErrException; import org.projectnessie.cel.common.types.IntT; import org.projectnessie.cel.common.types.IteratorT; @@ -199,20 +201,22 @@ private static String formatMap(MapT val) { StringBuilder builder = new StringBuilder(); builder.append('{'); - List entries = new ArrayList(); + SortedMap sorted = new TreeMap<>(); IteratorT iter = val.iterator(); while (iter.hasNext().booleanValue()) { Val key = iter.next(); String mapKey = formatString(key); String mapVal = formatString(val.find(key)); - entries.add(mapKey + ": " + mapVal); + sorted.put(mapKey, mapVal); } - // The formatted string should be sorted by keys - Collections.sort(entries); + String result = + sorted.entrySet().stream() + .map(entry -> entry.getKey() + ": " + entry.getValue()) + .collect(Collectors.joining(", ")); - builder.append(String.join(", ", entries)).append('}'); + builder.append(result).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 feee7057..c13bb515 100644 --- a/src/test/java/build/buf/protovalidate/FormatTest.java +++ b/src/test/java/build/buf/protovalidate/FormatTest.java @@ -29,7 +29,6 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -60,8 +59,6 @@ class FormatTest { private static List formatTests; private static List formatErrorTests; - private static List SKIPPED_TESTS = Arrays.asList(); - @BeforeAll private static void setUp() throws Exception { // The test data from the cel-spec conformance tests @@ -142,11 +139,9 @@ private static Program.EvalResult evaluate(SimpleTest test) { } private static Stream getTestStream(List tests) { - List args = new ArrayList(); + List args = new ArrayList<>(); for (SimpleTest test : tests) { - if (!SKIPPED_TESTS.contains(test.getName())) { - args.add(Arguments.arguments(Named.named(test.getName(), test))); - } + args.add(Arguments.arguments(Named.named(test.getName(), test))); } return args.stream(); @@ -162,7 +157,7 @@ private static Stream getFormatErrorTests() { // Builds the variable definitions to be used during evaluation private static Map buildVariables(Map bindings) { - Map vars = new HashMap(); + Map vars = new HashMap<>(); for (Map.Entry entry : bindings.entrySet()) { ExprValue exprValue = entry.getValue(); if (exprValue.hasValue()) { @@ -192,8 +187,7 @@ private static String getExpectedResult(SimpleTest test) { // Builds the declarations for a given test private static List buildDecls(SimpleTest test) { - List decls = - new ArrayList(); + List decls = new ArrayList<>(); for (Decl decl : test.getTypeEnvList()) { if (decl.hasIdent()) { Decl.IdentDecl ident = decl.getIdent();