You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Passing a BigDecimal to JsonObjectBuilder.put currently selects the Number? overload. When the resulting element is encoded, a value such as 1.000000000000000000000000001 can become 1.0, and 1E+400 can fail the special floating-point check.
The new experimental put(key: String, value: BigDecimal?) overload delegates to JsonUnquotedLiteral(value?.toString()). This preserves the decimal representation, including scale and exponent notation, without converting through Double. A null value produces JsonNull, and the builder still returns the previous value for the key.
This is deliberately limited to JVM builder calls whose argument has type BigDecimal or BigDecimal?. It does not change JsonPrimitive(Number?), Number-typed arguments, or the serialization of arbitrary parsed JSON numbers. On recompilation, calls with a statically typed BigDecimal select the new overload and require the experimental API opt-in.
Following review feedback, cross-platform checks for the existing builder overloads and special floating-point validation live in commonTest, while BigDecimal-specific coverage remains in jvmTest. The JVM tests cover precision, large and small exponents, scale, nullable arguments, literal null overload resolution, replacement values, nested objects, pretty printing, and unchanged Number-typed behavior. The shared test helper exercises string, tree, Java stream, Okio, and kotlinx-io paths.
Validation on Windows, Kotlin 2.4.10, Gradle 8.14.5, JDK 11 toolchain:
Before the implementation, the initial focused suite had 5 failures out of 8 tests, including precision loss and exponent overflow. After the implementation and test split, the focused JVM coverage has 9 tests with no failures, errors, or skipped tests.
Full JSON JVM suite: 744 tests, 3 skipped, no failures or errors.
Cross-platform common tests: 2 tests on JS Node and 2 tests on mingwX64, all passing.
Full Wasm JS Node and Wasm WASI Node suites: 645 tests each, 1 skipped in each suite, no failures or errors.
Core JVM suite: 167 tests, 1 skipped, no failures or errors. Both IO modules: 5 tests each, all passing.
JVM API dump updated; jvmApiCheck, animalsnifferJvmMain (Java 8 / Android API 14 signatures), and jvmJar pass. The API diff only adds the new JVM facade method.
Common metadata and JS, Wasm JS, Wasm WASI, and mingwX64 test compilation pass.
API dump generation and comparison were run separately: combining them in one invocation triggered Gradle's implicit task dependency validation.
A small point, you may want to split the test suite into a common part that tests the cross-platform behaviour and the tests that require the JVM (they use bigdecimal)
Thanks for the suggestion, @pdvrieze. I split the test suite so the cross-platform JsonObjectBuilder checks now live in commonTest, while the BigDecimal-specific coverage remains in jvmTest. I also ran the common tests on JVM, JS, Wasm JS, Wasm WASI, and mingwX64, and they all pass.
In the current JVM implementation, JsonArrayBuilder.add(BigDecimal) falls back to the Number overload, which can lose precision through Double encoding. A JVM-only BigDecimal? overload would preserve the value through JsonUnquotedLiteral(value?.toString()), matching the object builder. Since this is an additive JVM-only API and does not change existing binaries or non-JVM APIs, I think the risk of a critical compatibility issue is low and the overload would be reasonable to add.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the JVM-specific builder overload suggested in the discussion on #3257.
Passing a
BigDecimaltoJsonObjectBuilder.putcurrently selects theNumber?overload. When the resulting element is encoded, a value such as1.000000000000000000000000001can become1.0, and1E+400can fail the special floating-point check.The new experimental
put(key: String, value: BigDecimal?)overload delegates toJsonUnquotedLiteral(value?.toString()). This preserves the decimal representation, including scale and exponent notation, without converting throughDouble. A null value producesJsonNull, and the builder still returns the previous value for the key.This is deliberately limited to JVM builder calls whose argument has type
BigDecimalorBigDecimal?. It does not changeJsonPrimitive(Number?),Number-typed arguments, or the serialization of arbitrary parsed JSON numbers. On recompilation, calls with a statically typedBigDecimalselect the new overload and require the experimental API opt-in.Following review feedback, cross-platform checks for the existing builder overloads and special floating-point validation live in
commonTest, whileBigDecimal-specific coverage remains injvmTest. The JVM tests cover precision, large and small exponents, scale, nullable arguments, literal null overload resolution, replacement values, nested objects, pretty printing, and unchangedNumber-typed behavior. The shared test helper exercises string, tree, Java stream, Okio, and kotlinx-io paths.Validation on Windows, Kotlin 2.4.10, Gradle 8.14.5, JDK 11 toolchain:
jvmApiCheck,animalsnifferJvmMain(Java 8 / Android API 14 signatures), andjvmJarpass. The API diff only adds the new JVM facade method.API dump generation and comparison were run separately: combining them in one invocation triggered Gradle's implicit task dependency validation.
Refs #3257.