Compare JSON literals in Strict mode

[Kantis]

Aim is to fix #2458

It got a bit more complicated than simply checking exact equal string representations in Strict mode. Given that 1.0E2 is the same number as 100.0, 100 or +1E+2 I think they should be considered equal. What do you think?

The JSON standard seems to not make any distinction between integer and floating point numbers.

TODO:

• Changelog?
• Update relevant docs

Side-note: Do you have a good way of writing tests that targets both JVM and JS platforms? I didn't like copying the JsonLiteralTest class to both targets. I tried adding a test factory to commonTest and including that from both platforms, but then it was hard to run a specific test within the spec.

[jschneidereit]

This is awesome work!

[sksamuel]

I think if you activate strict mode, then "100.0" does not equal "1.0E2".
Because if we consider those equal, where do we draw the line? Is 00001 the same as 1 ?
Unless the json spec says that numbers in strings should be treated as special numbers, then I think we just consider them strings.
In lenient mode, I think we can do cleverer things.

[sksamuel]

Let me know when this is ready again for a review.

[Kantis]

I think if you activate strict mode, then "100.0" does not equal "1.0E2".
Because if we consider those equal, where do we draw the line? Is 00001 the same as 1 ?
Unless the json spec says that numbers in strings should be treated as special numbers, then I think we just consider them strings.
In lenient mode, I think we can do cleverer things.

I think we should draw a line between literals (100) and strings ("100").

When working with literals, IMO we should treat every number that is same number but different format (not type) as the same.
(1.0 == 1 != "1" != 01 (invalid number)).

But it's only my opinion 😄 I can revert to exact literal comparison, e.g. value must be identical (100.0 != 1.0E2) if you wish

I added test showing that we don't do type-conversion for quoted numbers in strict mode.

[sksamuel]

As discussed on chat, I would update compare mode to use this:

enum class CompareMode {

   /**
    * Types and formats must be identical.
    *
    * For example, "true" and true will not match because one is a string, and the other is a boolean.
    * Similarly, 2.99E9 and 299000000 would not match because despite both being doubles, and the same value,
    * they are not using the same representation.
    */
   Exact,

   /**
    * Types must be identical and compare by value.
    *
    * For example, "true" and true will not match because one is a string, and the other is a boolean.
    * But 2.99E9 and 299000000 are considered equal as they are both doubles, and both represent the same number.
    * "100" and 100 would not match as they are different types.
    */
   Strict,

   /**
    * Compare by value, coercing if possible.
    *
    * For example, "true" and true will match because the string value can be coerced into a valid boolean.
    * Similarly, "100" and 100 will match as the former can be coerced into an int.
    */
   Lenient,
}

[sksamuel]

Please do.

…

On Sat, 11 Sep 2021, 06:09 Emil Kantis, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In kotest-assertions/kotest-assertions-json/src/jsTest/kotlin/io.kotest.assertions.json/JsonLiteralsTest.kt <#2464 (comment)>: > + The top level expected 1e1 but was 10.0 + + expected: + 10.0 + + actual: + 10.0 This looks/feels a bit odd. What happens is the json document is parsed to JsonElement and then encoded back to string again. (JsonMatchers.kt:73-77). I think it would be worthwhile to roll our own prettification of JSON-document that prevents parsing of the values.. Shall I file an issue for it? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2464 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFVSGV7RLEWQ5Q6YXJW5TTUBM2HTANCNFSM5DRBOEDQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[sksamuel]

Let me know when you want a re-review.

[Kantis]

@sksamuel I think this is ready for final review. I haven't come up with any better names for CompareMode.Exact ☹️

[sksamuel]

Nice job :)