Fix some formatting issues (#277)

This fixes a few issues noticed from failing conformance tests: * Quotes were being escaped in string values in lists * Durations had the word `duration` added to their formatting (along with escaped quotes) * Doubles were not formatting the same as CEL-Go. Namely, insignificant digits were not being removed (`1.0` should format as `1`, not `1.0`) In addition, this adds some additional unit tests for doubles to ensure the string formatting of doubles is adhering to the same behavior that CEL-Go uses.

Author: smaye81

build.gradle.kts

@@ -207,6 +207,13 @@ allprojects {
  }
  tasks.withType<Test>().configureEach {
  useJUnitPlatform()
+ this.testLogging {
+ events("failed")
+ exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
+ showExceptions = true
+ showCauses = true
+ showStackTraces = true
+ }
  }
 }
 

conformance/expected-failures.yaml

@@ -15,14 +15,3 @@ custom_rules:
  - field_expression/map/int64/invalid
  - field_expression/map/uint64/valid
  - field_expression/map/uint64/invalid
-kitchen_sink:
- - field/embedded/invalid
- - field/transitive/invalid
- - many/all-non-message-fields/invalid
- - field/invalid
-standard_rules/repeated:
- - items/in/invalid
- - items/not_in/invalid
-standard_rules/well_known_types/duration:
- - in/invalid
- - not in/invalid

src/main/java/build/buf/protovalidate/Format.java

@@ -139,52 +139,25 @@ private static String bytesToHex(byte[] bytes, char[] digits) {
  * @param val the value to format.
  */
  private static void formatString(StringBuilder builder, Val val) {
- if (val.type().typeEnum() == TypeEnum.String) {
- builder.append(val.value());
- } else if (val.type().typeEnum() == TypeEnum.Bytes) {
- builder.append(new String((byte[]) val.value(), StandardCharsets.UTF_8));
- } else if (val.type().typeEnum() == TypeEnum.Double) {
- DecimalFormat format = new DecimalFormat();
- builder.append(format.format(val.value()));
- } else {
- formatStringSafe(builder, val, false);
- }
- }
-
- /**
- * Formats a string value safely for other value types.
- *
- * @param builder the StringBuilder to append the formatted string to.
- * @param val the value to format.
- * @param listType indicates if the value type is a list.
- */
- private static void formatStringSafe(StringBuilder builder, Val val, boolean listType) {
  TypeEnum type = val.type().typeEnum();
  if (type == TypeEnum.Bool) {
  builder.append(val.booleanValue());
- } else if (type == TypeEnum.Int || type == TypeEnum.Uint) {
- formatDecimal(builder, val);
- } else if (type == TypeEnum.Double) {
- // When a double is nested in another type (e.g. a list) it will have a minimum of 6 decimal
- // digits. This is to maintain consistency with the Go CEL runtime.
- DecimalFormat format = new DecimalFormat();
- format.setMaximumFractionDigits(Integer.MAX_VALUE);
- format.setMinimumFractionDigits(6);
- builder.append(format.format(val.value()));
- } else if (type == TypeEnum.String) {
- builder.append("\"").append(val.value().toString()).append("\"");
+ } else if (type == TypeEnum.String || type == TypeEnum.Int || type == TypeEnum.Uint) {
+ builder.append(val.value());
  } else if (type == TypeEnum.Bytes) {
- formatBytes(builder, val);
+ 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, listType);
+ 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 stringSafe map type");
+ throw new ErrException("unimplemented string map type");
  } else if (type == TypeEnum.Null) {
- throw new ErrException("unimplemented stringSafe null type");
+ throw new ErrException("unimplemented string null type");
  }
  }
 
@@ -200,7 +173,7 @@ private static void formatList(StringBuilder builder, Val val) {
  List list = val.convertToNative(List.class);
  for (int i = 0; i < list.size(); i++) {
  Object obj = list.get(i);
- formatStringSafe(builder, DefaultTypeAdapter.nativeToValue(Db.newDb(), null, obj), true);
+ formatString(builder, DefaultTypeAdapter.nativeToValue(Db.newDb(), null, obj));
  if (i != list.size() - 1) {
  builder.append(", ");
  }
@@ -225,35 +198,15 @@ private static void formatTimestamp(StringBuilder builder, Val val) {
  *
  * @param builder the StringBuilder to append the formatted duration value to.
  * @param val the value to format.
- * @param listType indicates if the value type is a list.
  */
- private static void formatDuration(StringBuilder builder, Val val, boolean listType) {
- if (listType) {
- builder.append("duration(\"");
- }
+ private static void formatDuration(StringBuilder builder, Val val) {
  Duration duration = val.convertToNative(Duration.class);
 
  double totalSeconds = duration.getSeconds() + (duration.getNanos() / 1_000_000_000.0);
 
- DecimalFormat format = new DecimalFormat("0.#########");
- builder.append(format.format(totalSeconds));
+ DecimalFormat formatter = new DecimalFormat("0.#########");
+ builder.append(formatter.format(totalSeconds));
  builder.append("s");
- if (listType) {
- builder.append("\")");
- }
- }
-
- /**
- * Formats a byte array value.
- *
- * @param builder the StringBuilder to append the formatted byte array value to.
- * @param val the value to format.
- */
- private static void formatBytes(StringBuilder builder, Val val) {
- builder
- .append("\"")
- .append(new String((byte[]) val.value(), StandardCharsets.UTF_8))
- .append("\"");
  }
 
  /**
@@ -283,9 +236,10 @@ private static void formatHex(StringBuilder builder, Val val, char[] digits) {
  * Formats a decimal value.
  *
  * @param builder the StringBuilder to append the formatted decimal value to.
- * @param arg the value to format.
+ * @param val the value to format.
  */
- private static void formatDecimal(StringBuilder builder, Val arg) {
- builder.append(arg.value());
+ private static void formatDecimal(StringBuilder builder, Val val) {
+ DecimalFormat formatter = new DecimalFormat("0.#########");
+ builder.append(formatter.format(val.value()));
  }
 }

src/test/java/build/buf/protovalidate/FormatTest.java

@@ -15,18 +15,107 @@
 package build.buf.protovalidate;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import com.google.protobuf.Duration;
 import org.junit.jupiter.api.Test;
+import org.projectnessie.cel.common.types.DoubleT;
+import org.projectnessie.cel.common.types.Err.ErrException;
 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.Val;
 
 class FormatTest {
  @Test
- void largeDecimalValuesAreProperlyFormatted() {
+ 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");
+ }
+
+ @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");
+ }
+
+ @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");
+ }
+
+ @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");
+ }
+
+ @Test
+ void testDurationSecondsOnly() {
+ Duration duration = Duration.newBuilder().setSeconds(123).build();
+
+ ListT val =
+ (ListT) ListT.newGenericArrayList(DefaultTypeAdapter.Instance, new Duration[] {duration});
+ String formatted = Format.format("%s", val);
+ assertThat(formatted).isEqualTo("123s");
+ }
+
+ @Test
+ void testDurationNanosOnly() {
+ Duration duration = Duration.newBuilder().setNanos(42).build();
+
+ ListT val =
+ (ListT) ListT.newGenericArrayList(DefaultTypeAdapter.Instance, new Duration[] {duration});
+ String formatted = Format.format("%s", val);
+ assertThat(formatted).isEqualTo("0.000000042s");
+ }
 }