Skip to content

Commit fe86128

Browse files
committed
fix: align format numeric edge cases with Python rules
Motivation: Jsonnet documents std.format and the % string operator as following Python-style formatting rules. sjsonnet diverged on several numeric edge cases: floating-point formats did not preserve the IEEE -0.0 sign bit, dynamic * width/precision accepted fractional values via truncation, and %g exponent selection was sensitive to floating-point log10 roundoff. Modification: Normalize dynamic * width and precision through a shared Python-style integer conversion path, including negative width and negative precision handling. Preserve -0.0 signs for floating-point conversions, keep integral conversions unchanged, and stabilize %g exponent calculation around powers of ten. Add focused regression coverage for negative zero, dynamic * width/precision, fractional * errors, and %g boundary values. Result: sjsonnet follows the official std.format Python-style contract for these numeric formatting edge cases. The PR is format-only and no longer includes unrelated manifestXmlJsonml changes. References: https://jsonnet.org/ref/stdlib.html#std-format
1 parent e686d89 commit fe86128

8 files changed

Lines changed: 140 additions & 18 deletions

sjsonnet/src/sjsonnet/Format.scala

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,28 @@ object Format {
9393
conversion == 's' && !hasWidth && !hasPrecision &&
9494
(flags & SimpleStringDisqualifierFlags) == 0
9595

96-
def withStarWidth(newWidth: Int): FormatSpec =
97-
new FormatSpec((bits & ~(NumberMask << WidthShift)) | (encodeNumber(newWidth) << WidthShift))
96+
// Jsonnet documents std.format as Python-style formatting. Python treats a negative
97+
// dynamic width as left adjustment with the absolute width.
98+
def withStarWidth(newWidth: Int): FormatSpec = {
99+
val normalizedWidth =
100+
if (newWidth < 0) {
101+
if (newWidth == Int.MinValue)
102+
throw new Exception("Format width/precision is too large: " + newWidth)
103+
-newWidth
104+
} else newWidth
105+
val normalizedFlags = if (newWidth < 0) flags | LeftAdjustedFlag else flags
106+
new FormatSpec(
107+
(bits & ~(NumberMask << WidthShift) & ~(FlagsMask << FlagsShift)) |
108+
(normalizedFlags.toLong << FlagsShift) |
109+
(encodeNumber(normalizedWidth) << WidthShift)
110+
)
111+
}
98112

113+
// Python treats a negative dynamic precision as zero; %g later maps precision 0 to 1.
99114
def withStarPrecision(newPrecision: Int): FormatSpec =
100115
new FormatSpec(
101-
(bits & ~(NumberMask << PrecisionShift)) | (encodeNumber(newPrecision) << PrecisionShift)
116+
(bits & ~(NumberMask << PrecisionShift)) |
117+
(encodeNumber(math.max(newPrecision, 0)) << PrecisionShift)
102118
)
103119

104120
def withStarValues(newWidth: Int, newPrecision: Int): FormatSpec =
@@ -645,7 +661,7 @@ object Format {
645661
)
646662
}
647663
i += 1
648-
formatted = formatted.withStarWidth(width.asInt)
664+
formatted = withStarWidth(formatted, width, idx)
649665
valuesArr.value(i)
650666
case (false, true) =>
651667
val precision = valuesArr.value(i)
@@ -655,7 +671,7 @@ object Format {
655671
)
656672
}
657673
i += 1
658-
formatted = formatted.withStarPrecision(precision.asInt)
674+
formatted = withStarPrecision(formatted, precision, idx)
659675
valuesArr.value(i)
660676
case (true, true) =>
661677
val width = valuesArr.value(i)
@@ -672,7 +688,8 @@ object Format {
672688
)
673689
}
674690
i += 1
675-
formatted = formatted.withStarValues(width.asInt, precision.asInt)
691+
formatted =
692+
withStarPrecision(withStarWidth(formatted, width, idx), precision, idx)
676693
valuesArr.value(i)
677694
}
678695
} else {
@@ -984,14 +1001,47 @@ object Format {
9841001
case _ => false
9851002
}
9861003

1004+
private def withStarWidth(formatted: FormatSpec, width: Val, idx: Int): FormatSpec =
1005+
try formatted.withStarWidth(starInteger(width, "width", idx))
1006+
catch {
1007+
case e: Error => throw e
1008+
case e: Exception =>
1009+
Error.fail(e.getMessage)
1010+
}
1011+
1012+
private def withStarPrecision(formatted: FormatSpec, precision: Val, idx: Int): FormatSpec =
1013+
try formatted.withStarPrecision(starInteger(precision, "precision", idx))
1014+
catch {
1015+
case e: Error => throw e
1016+
case e: Exception =>
1017+
Error.fail(e.getMessage)
1018+
}
1019+
1020+
private def starInteger(value: Val, name: String, idx: Int): Int =
1021+
value match {
1022+
case n: Val.Num =>
1023+
val d = n.asDouble
1024+
// Jsonnet has one number type, so Python's "integer required for *" rule maps to
1025+
// finite whole-number values. Fractional star arguments must not be rounded.
1026+
if (!d.isWhole || !d.isValidInt)
1027+
Error.fail("* %s at position %d requires an integer".format(name, idx))
1028+
d.toInt
1029+
case _ =>
1030+
Error.fail("* %s at position %d requires an integer".format(name, idx))
1031+
}
1032+
1033+
// std.format follows Python's % formatting. Jsonnet numbers have IEEE double semantics, so
1034+
// floating-point formats preserve the sign bit of -0.0 even though -0.0 == 0.0.
1035+
private def isNegative(s: Double): Boolean = s < 0 || (s == 0.0 && 1.0 / s < 0)
1036+
9871037
private def formatInteger(formatted: FormatSpec, s: Double): String = {
9881038
formatIntegralRadix(formatted, s, 10, _ => "")
9891039
}
9901040

9911041
private def formatFloat(formatted: FormatSpec, s: Double): String = {
9921042
widen(
9931043
formatted,
994-
if (s < 0) "-" else "",
1044+
if (isNegative(s)) "-" else "",
9951045
"",
9961046
sjsonnet.DecimalFormat
9971047
.format(
@@ -1002,7 +1052,7 @@ object Format {
10021052
math.abs(s)
10031053
),
10041054
numeric = true,
1005-
signedConversion = s >= 0
1055+
signedConversion = !isNegative(s)
10061056
)
10071057

10081058
}
@@ -1055,10 +1105,10 @@ object Format {
10551105
if (s == 0) 0
10561106
else {
10571107
val abs = math.abs(s)
1058-
val rawExponent = math.floor(math.log10(abs)).toInt
1108+
val rawExponent = math.floor(math.log10(abs) + 1e-10).toInt
10591109
val scale = math.pow(10, rawExponent - precision + 1)
10601110
val rounded = Math.round(abs / scale) * scale
1061-
if (rounded == 0) 0 else math.floor(math.log10(rounded)).toInt
1111+
if (rounded == 0) 0 else math.floor(math.log10(rounded) + 1e-10).toInt
10621112
}
10631113
}
10641114

@@ -1069,7 +1119,7 @@ object Format {
10691119
if (exponent < -4 || exponent >= precision) {
10701120
widen(
10711121
formatted,
1072-
if (s < 0) "-" else "",
1122+
if (isNegative(s)) "-" else "",
10731123
"",
10741124
sjsonnet.DecimalFormat
10751125
.format(
@@ -1080,13 +1130,13 @@ object Format {
10801130
math.abs(s)
10811131
),
10821132
numeric = true,
1083-
signedConversion = s >= 0
1133+
signedConversion = !isNegative(s)
10841134
)
10851135
} else {
1086-
val fractionalPrecision = math.max(0, precision - exponent - 1)
1136+
val fractionalPrecision = math.max(0, precision - 1 - exponent)
10871137
widen(
10881138
formatted,
1089-
if (s < 0) "-" else "",
1139+
if (isNegative(s)) "-" else "",
10901140
"",
10911141
sjsonnet.DecimalFormat
10921142
.format(
@@ -1097,7 +1147,7 @@ object Format {
10971147
math.abs(s)
10981148
),
10991149
numeric = true,
1100-
signedConversion = s >= 0
1150+
signedConversion = !isNegative(s)
11011151
)
11021152
}
11031153

@@ -1106,7 +1156,7 @@ object Format {
11061156
private def formatExponent(formatted: FormatSpec, s: Double): String = {
11071157
widen(
11081158
formatted,
1109-
if (s < 0) "-" else "",
1159+
if (isNegative(s)) "-" else "",
11101160
"",
11111161
sjsonnet.DecimalFormat
11121162
.format(
@@ -1117,7 +1167,7 @@ object Format {
11171167
math.abs(s)
11181168
),
11191169
numeric = true,
1120-
signedConversion = s >= 0
1170+
signedConversion = !isNegative(s)
11211171
)
11221172
}
11231173

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"%.*g" % [2.7, 1.23456]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
sjsonnet.Error: [std.format] * precision at position 0 requires an integer
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"%*g" % [3.7, 1]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
sjsonnet.Error: [std.format] * width at position 0 requires an integer
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Tests for %g and %#g format fixes
2+
// Verifies floating-point precision fixes in roundedGenericExponent and formatGeneric
3+
4+
// Basic %g tests with various exponents
5+
std.assertEqual('%g' % 0.1, '0.1') &&
6+
std.assertEqual('%g' % 0.0001, '0.0001') &&
7+
std.assertEqual('%g' % 1.0, '1') &&
8+
std.assertEqual('%g' % 100.0, '100') &&
9+
std.assertEqual('%g' % 1234567.0, '1.23457e+06') &&
10+
11+
// %#g tests (alternate flag preserves trailing zeros)
12+
std.assertEqual('%#g' % 0.1, '0.100000') &&
13+
std.assertEqual('%#g' % 0.0001, '0.000100000') &&
14+
std.assertEqual('%#g' % 1.0, '1.00000') &&
15+
std.assertEqual('%#g' % 100.0, '100.000') &&
16+
std.assertEqual('%#g' % 1234567.0, '1.23457e+06') &&
17+
18+
// Precision-specific tests
19+
std.assertEqual('%.3g' % 0.0001, '0.0001') &&
20+
std.assertEqual('%.3g' % 123.456, '123') &&
21+
std.assertEqual('%.10g' % 0.1, '0.1') &&
22+
23+
// Zero handling
24+
std.assertEqual('%g' % 0.0, '0') &&
25+
std.assertEqual('%#g' % 0.0, '0.00000') &&
26+
27+
// Negative values
28+
std.assertEqual('%g' % -0.1, '-0.1') &&
29+
std.assertEqual('%#g' % -0.1, '-0.100000') &&
30+
31+
// Very small exponents
32+
std.assertEqual('%g' % 0.00001, '1e-05') &&
33+
std.assertEqual('%#g' % 0.00001, '1.00000e-05') &&
34+
35+
// Precision 0 (treated as precision 1)
36+
std.assertEqual('%.0g' % 0.0, '0') &&
37+
std.assertEqual('%.0g' % 1.0, '1') &&
38+
std.assertEqual('%.0g' % 0.1, '0.1') &&
39+
std.assertEqual('%#.0g' % 1.0, '1.') &&
40+
41+
// Star width/precision follows Python-style integer semantics
42+
std.assertEqual('%*g' % [10, 1.0], ' 1') &&
43+
std.assertEqual('%*g' % [10, 0.1], ' 0.1') &&
44+
std.assertEqual('%*g' % [4, 1.0], ' 1') &&
45+
std.assertEqual('%*g' % [-4, 1.0], '1 ') &&
46+
std.assertEqual('%.*g' % [3, 1.23456], '1.23') &&
47+
std.assertEqual('%.*g' % [-3, 1.23456], '1') &&
48+
std.assertEqual('%*.*g' % [6, 3, 1.23456], ' 1.23') &&
49+
std.assertEqual('%*.*g' % [-6, 3, 1.23456], '1.23 ') &&
50+
51+
// Regression tests: values close to powers of 10
52+
std.assertEqual('%g' % 99.9999, '99.9999') &&
53+
std.assertEqual('%g' % 9.99999, '9.99999') &&
54+
std.assertEqual('%g' % 999.999, '999.999') &&
55+
std.assertEqual('%g' % 0.999999, '0.999999') &&
56+
57+
true
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
true

sjsonnet/test/resources/new_test_suite/format_zero_sign_flags.jsonnet

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,14 @@ std.assertEqual("%+E" % 0.0, "+0.000000E+00") &&
77
std.assertEqual("%+g" % 0.0, "+0") &&
88
std.assertEqual("%+G" % 0.0, "+0") &&
99
std.assertEqual("%+010f" % 0.0, "+00.000000") &&
10-
std.assertEqual("%+f" % (-0.0), "+0.000000")
10+
std.assertEqual("%+f" % (-0.0), "-0.000000") &&
11+
std.assertEqual("%f" % (-0.0), "-0.000000") &&
12+
std.assertEqual("%e" % (-0.0), "-0.000000e+00") &&
13+
std.assertEqual("%E" % (-0.0), "-0.000000E+00") &&
14+
std.assertEqual("%g" % (-0.0), "-0") &&
15+
std.assertEqual("%G" % (-0.0), "-0") &&
16+
std.assertEqual("% f" % (-0.0), "-0.000000") &&
17+
std.assertEqual("%010f" % (-0.0), "-00.000000") &&
18+
std.assertEqual("%d" % (-0.0), "0") &&
19+
std.assertEqual("%o" % (-0.0), "0") &&
20+
std.assertEqual("%x" % (-0.0), "0")

0 commit comments

Comments
 (0)