Skip to content

fix: align std.format edge cases with Python #1078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
He-Pin wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: align std.format edge cases with Python #1078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 109 additions & 27 deletions sjsonnet/src/sjsonnet/Format.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package sjsonnet

/**
* Minimal re-implementation of Python's `%` formatting logic, since Jsonnet's `%` formatter is
* basically "do whatever python does", with a link to:
* Minimal re-implementation of Python's `%` formatting logic. Jsonnet documents `std.format` and
* the `%` operator as following Python formatting rules:
*
* - https://docs.python.org/2/library/stdtypes.html#string-formatting
* - https://jsonnet.org/ref/stdlib.html#std-format
* - https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting
*
* Parses the formatted strings into a sequence of literal strings separated by `%` interpolations
* modelled as structured [[Format.FormatSpec]]s, and use those to decide how to inteprolate the
Expand Down Expand Up @@ -93,12 +94,28 @@ object Format {
conversion == 's' && !hasWidth && !hasPrecision &&
(flags & SimpleStringDisqualifierFlags) == 0

def withStarWidth(newWidth: Int): FormatSpec =
new FormatSpec((bits & ~(NumberMask << WidthShift)) | (encodeNumber(newWidth) << WidthShift))
// Jsonnet documents std.format as Python-style formatting. Python treats a negative
// dynamic width as left adjustment with the absolute width.
def withStarWidth(newWidth: Int): FormatSpec = {
val normalizedWidth =
if (newWidth < 0) {
if (newWidth == Int.MinValue)
throw new Exception("Format width/precision is too large: " + newWidth)
-newWidth
} else newWidth
val normalizedFlags = if (newWidth < 0) flags | LeftAdjustedFlag else flags
new FormatSpec(
(bits & ~(NumberMask << WidthShift) & ~(FlagsMask << FlagsShift)) |
(normalizedFlags.toLong << FlagsShift) |
(encodeNumber(normalizedWidth) << WidthShift)
)
}

// Python treats a negative dynamic precision as zero; %g later maps precision 0 to 1.
def withStarPrecision(newPrecision: Int): FormatSpec =
new FormatSpec(
(bits & ~(NumberMask << PrecisionShift)) | (encodeNumber(newPrecision) << PrecisionShift)
(bits & ~(NumberMask << PrecisionShift)) |
(encodeNumber(math.max(newPrecision, 0)) << PrecisionShift)
)

def withStarValues(newWidth: Int, newPrecision: Int): FormatSpec =
Expand Down Expand Up @@ -241,6 +258,19 @@ object Format {
def widenRaw(formatted: FormatSpec, txt: String): String =
if (!formatted.hasWidth) txt // fast path: no width/padding needed
else widen(formatted, "", "", txt, numeric = false, signedConversion = false)

private def formatString(formatted: FormatSpec, txt: String): String = {
val truncated =
if (!formatted.hasPrecision) txt
else {
val precision = formatted.precisionValue
if (precision == 0) ""
else if (txt.codePointCount(0, txt.length) <= precision) txt
else txt.substring(0, txt.offsetByCodePoints(0, precision))
}
widenRaw(formatted, truncated)
}

def widen(
formatted: FormatSpec,
lhs: String,
Expand Down Expand Up @@ -645,7 +675,7 @@ object Format {
)
}
i += 1
formatted = formatted.withStarWidth(width.asInt)
formatted = withStarWidth(formatted, width, idx)
valuesArr.value(i)
case (false, true) =>
val precision = valuesArr.value(i)
Expand All @@ -655,7 +685,7 @@ object Format {
)
}
i += 1
formatted = formatted.withStarPrecision(precision.asInt)
formatted = withStarPrecision(formatted, precision, idx)
valuesArr.value(i)
case (true, true) =>
val width = valuesArr.value(i)
Expand All @@ -672,7 +702,8 @@ object Format {
)
}
i += 1
formatted = formatted.withStarValues(width.asInt, precision.asInt)
formatted =
withStarPrecision(withStarWidth(formatted, width, idx), precision, idx)
valuesArr.value(i)
}
} else {
Expand Down Expand Up @@ -706,7 +737,8 @@ object Format {
pos
)
}
widenRaw(formatted, vs.str)
if (formatted.conversion == 's') formatString(formatted, vs.str)
else widenRaw(formatted, vs.str)
case vn: Val.Num =>
val s = vn.asDouble
formatted.conversion match {
Expand All @@ -728,7 +760,7 @@ object Format {
val c = if (codePoint >= 0xd800 && codePoint <= 0xdfff) 0xfffd else codePoint
widenRaw(formatted, Character.toString(c))
case 's' =>
widenRaw(formatted, RenderUtils.renderDouble(s))
formatString(formatted, RenderUtils.renderDouble(s))
case _ =>
Error.fail(
"unsupported format conversion at position %d, got number".format(i)
Expand All @@ -752,7 +784,7 @@ object Format {
)
case _: Val.Null =>
formatted.conversion match {
case 's' => widenRaw(formatted, "null")
case 's' => formatString(formatted, "null")
case 'c' => Error.fail("%c expected number or string, got null")
case _ =>
Error.fail(
Expand All @@ -766,7 +798,7 @@ object Format {
case r: Val.Obj => Materializer.apply0(r, new Renderer(indent = -1))
case _ => Materializer(rawVal)
}
widenRaw(formatted, value.toString)
formatString(formatted, value.toString)
}
i += 1
formattedValue
Expand Down Expand Up @@ -807,7 +839,7 @@ object Format {
case 'g' => formatGeneric(formatted, numericValue).toLowerCase
case 'G' => formatGeneric(formatted, numericValue)
case 'c' => Error.fail("%c expected number or string, got boolean")
case 's' => widenRaw(formatted, textValue)
case 's' => formatString(formatted, textValue)
case _ =>
Error.fail(
"expected number or string at position %d, got boolean".format(index)
Expand All @@ -816,7 +848,7 @@ object Format {

private def formatStrictBoolean(formatted: FormatSpec, index: Int, value: String): String =
formatted.conversion match {
case 's' => widenRaw(formatted, value)
case 's' => formatString(formatted, value)
case 'c' => Error.fail("%c expected number or string, got boolean")
case _ =>
Error.fail(
Expand Down Expand Up @@ -984,14 +1016,47 @@ object Format {
case _ => false
}

private def withStarWidth(formatted: FormatSpec, width: Val, idx: Int): FormatSpec =
try formatted.withStarWidth(starInteger(width, "width", idx))
catch {
case e: Error => throw e
case e: Exception =>
Error.fail(e.getMessage)
}

private def withStarPrecision(formatted: FormatSpec, precision: Val, idx: Int): FormatSpec =
try formatted.withStarPrecision(starInteger(precision, "precision", idx))
catch {
case e: Error => throw e
case e: Exception =>
Error.fail(e.getMessage)
}

private def starInteger(value: Val, name: String, idx: Int): Int =
value match {
case n: Val.Num =>
val d = n.asDouble
// Jsonnet has one number type, so Python's "integer required for *" rule maps to
// finite whole-number values. Fractional star arguments must not be rounded.
if (!d.isWhole || !d.isValidInt)
Error.fail("* %s at position %d requires an integer".format(name, idx))
d.toInt
case _ =>
Error.fail("* %s at position %d requires an integer".format(name, idx))
}

// std.format follows Python's % formatting. Jsonnet numbers have IEEE double semantics, so
// floating-point formats preserve the sign bit of -0.0 even though -0.0 == 0.0.
private def isNegative(s: Double): Boolean = s < 0 || (s == 0.0 && 1.0 / s < 0)

private def formatInteger(formatted: FormatSpec, s: Double): String = {
formatIntegralRadix(formatted, s, 10, _ => "")
}

private def formatFloat(formatted: FormatSpec, s: Double): String = {
widen(
formatted,
if (s < 0) "-" else "",
if (isNegative(s)) "-" else "",
"",
sjsonnet.DecimalFormat
.format(
Expand All @@ -1002,7 +1067,7 @@ object Format {
math.abs(s)
),
numeric = true,
signedConversion = s >= 0
signedConversion = !isNegative(s)
)

}
Expand Down Expand Up @@ -1051,14 +1116,31 @@ object Format {
}
}

private def decimalExponent(value: Double): Int = {
if (value == 0 || value.isNaN || value.isInfinity) 0
else {
val exact = new java.math.BigDecimal(value)
var exponent = math.floor(math.log10(value)).toInt
while (exact.compareTo(java.math.BigDecimal.ONE.scaleByPowerOfTen(exponent + 1)) >= 0) {
exponent += 1
}
while (exact.compareTo(java.math.BigDecimal.ONE.scaleByPowerOfTen(exponent)) < 0) {
exponent -= 1
}
exponent
}
}

private def roundedGenericExponent(s: Double, precision: Int): Int = {
if (s == 0) 0
else {
val abs = math.abs(s)
val rawExponent = math.floor(math.log10(abs)).toInt
val rawExponent = decimalExponent(abs)
val scale = math.pow(10, rawExponent - precision + 1)
val rounded = Math.round(abs / scale) * scale
if (rounded == 0) 0 else math.floor(math.log10(rounded)).toInt
val roundedDigits = Math.round(abs / scale)
if (roundedDigits == 0) 0
else if (roundedDigits.toDouble >= math.pow(10, precision)) rawExponent + 1
else rawExponent
}
}

Expand All @@ -1069,7 +1151,7 @@ object Format {
if (exponent < -4 || exponent >= precision) {
widen(
formatted,
if (s < 0) "-" else "",
if (isNegative(s)) "-" else "",
"",
sjsonnet.DecimalFormat
.format(
Expand All @@ -1080,13 +1162,13 @@ object Format {
math.abs(s)
),
numeric = true,
signedConversion = s >= 0
signedConversion = !isNegative(s)
)
} else {
val fractionalPrecision = math.max(0, precision - exponent - 1)
val fractionalPrecision = math.max(0, precision - 1 - exponent)
widen(
formatted,
if (s < 0) "-" else "",
if (isNegative(s)) "-" else "",
"",
sjsonnet.DecimalFormat
.format(
Expand All @@ -1097,7 +1179,7 @@ object Format {
math.abs(s)
),
numeric = true,
signedConversion = s >= 0
signedConversion = !isNegative(s)
)
}

Expand All @@ -1106,7 +1188,7 @@ object Format {
private def formatExponent(formatted: FormatSpec, s: Double): String = {
widen(
formatted,
if (s < 0) "-" else "",
if (isNegative(s)) "-" else "",
"",
sjsonnet.DecimalFormat
.format(
Expand All @@ -1117,7 +1199,7 @@ object Format {
math.abs(s)
),
numeric = true,
signedConversion = s >= 0
signedConversion = !isNegative(s)
)
}

Expand Down
1 change: 1 addition & 0 deletions sjsonnet/test/resources/new_test_suite/error.format_fractional_star_precision.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"%.*g" % [2.7, 1.23456]
1 change: 1 addition & 0 deletions sjsonnet/test/resources/new_test_suite/error.format_fractional_star_precision.jsonnet.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sjsonnet.Error: [std.format] * precision at position 0 requires an integer
1 change: 1 addition & 0 deletions sjsonnet/test/resources/new_test_suite/error.format_fractional_star_width.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"%*g" % [3.7, 1]
1 change: 1 addition & 0 deletions sjsonnet/test/resources/new_test_suite/error.format_fractional_star_width.jsonnet.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sjsonnet.Error: [std.format] * width at position 0 requires an integer
60 changes: 60 additions & 0 deletions sjsonnet/test/resources/new_test_suite/format_g_precision_fix.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Tests for %g and %#g format fixes
// Verifies floating-point precision fixes in roundedGenericExponent and formatGeneric

// Basic %g tests with various exponents
std.assertEqual('%g' % 0.1, '0.1') &&
std.assertEqual('%g' % 0.0001, '0.0001') &&
std.assertEqual('%g' % 1.0, '1') &&
std.assertEqual('%g' % 100.0, '100') &&
std.assertEqual('%g' % 1234567.0, '1.23457e+06') &&

// %#g tests (alternate flag preserves trailing zeros)
std.assertEqual('%#g' % 0.1, '0.100000') &&
std.assertEqual('%#g' % 0.0001, '0.000100000') &&
std.assertEqual('%#g' % 1.0, '1.00000') &&
std.assertEqual('%#g' % 100.0, '100.000') &&
std.assertEqual('%#g' % 1234567.0, '1.23457e+06') &&

// Precision-specific tests
std.assertEqual('%.3g' % 0.0001, '0.0001') &&
std.assertEqual('%.3g' % 123.456, '123') &&
std.assertEqual('%.10g' % 0.1, '0.1') &&

// Zero handling
std.assertEqual('%g' % 0.0, '0') &&
std.assertEqual('%#g' % 0.0, '0.00000') &&

// Negative values
std.assertEqual('%g' % -0.1, '-0.1') &&
std.assertEqual('%#g' % -0.1, '-0.100000') &&

// Very small exponents
std.assertEqual('%g' % 0.00001, '1e-05') &&
std.assertEqual('%#g' % 0.00001, '1.00000e-05') &&

// Precision 0 (treated as precision 1)
std.assertEqual('%.0g' % 0.0, '0') &&
std.assertEqual('%.0g' % 1.0, '1') &&
std.assertEqual('%.0g' % 0.1, '0.1') &&
std.assertEqual('%#.0g' % 1.0, '1.') &&

// Star width/precision follows Python-style integer semantics
std.assertEqual('%*g' % [10, 1.0], ' 1') &&
std.assertEqual('%*g' % [10, 0.1], ' 0.1') &&
std.assertEqual('%*g' % [4, 1.0], ' 1') &&
std.assertEqual('%*g' % [-4, 1.0], '1 ') &&
std.assertEqual('%.*g' % [3, 1.23456], '1.23') &&
std.assertEqual('%.*g' % [-3, 1.23456], '1') &&
std.assertEqual('%*.*g' % [6, 3, 1.23456], ' 1.23') &&
std.assertEqual('%*.*g' % [-6, 3, 1.23456], '1.23 ') &&

// Regression tests: values close to powers of 10
std.assertEqual('%g' % 99.9999, '99.9999') &&
std.assertEqual('%g' % 9.99999, '9.99999') &&
std.assertEqual('%g' % 999.999, '999.999') &&
std.assertEqual('%g' % 0.999999, '0.999999') &&
std.assertEqual('%.12g' % 9.99999999876, '9.99999999876') &&
std.assertEqual('%.12g' % 99.9999999876, '99.9999999876') &&
std.assertEqual('%.12g' % 0.999999999876, '0.999999999876') &&

true
1 change: 1 addition & 0 deletions sjsonnet/test/resources/new_test_suite/format_g_precision_fix.jsonnet.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
true
17 changes: 17 additions & 0 deletions sjsonnet/test/resources/new_test_suite/format_string_precision.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Python-style %s precision limits the formatted string by codepoint.

std.assertEqual("%.3s" % "abcdef", "abc") &&
std.assertEqual("%.0s" % "abcdef", "") &&
std.assertEqual("%5.3s" % "abcdef", " abc") &&
std.assertEqual("%-5.3s" % "abcdef", "abc ") &&
std.assertEqual("%.*s" % [3, "abcdef"], "abc") &&
std.assertEqual("%.*s" % [0, "abcdef"], "") &&
std.assertEqual("%.*s" % [-3, "abcdef"], "") &&
std.assertEqual("%*.*s" % [6, 3, "abcdef"], " abc") &&
std.assertEqual("%-*.*s" % [6, 3, "abcdef"], "abc ") &&
std.assertEqual("%.1s" % "😀x", "😀") &&
std.assertEqual("%.*s" % [1, "éx"], "é") &&
std.assertEqual("%.2s" % 123.456, "12") &&
std.assertEqual("%.2s" % true, "tr") &&
std.assertEqual("%.2s" % null, "nu") &&
true
1 change: 1 addition & 0 deletions sjsonnet/test/resources/new_test_suite/format_string_precision.jsonnet.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
true
Loading
Loading