Skip to content

add functions for creating JsonPrimitives from unsigned numbers #2160

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

Merged
shanshin merged 3 commits into from
Mar 1, 2023
Merged

add functions for creating JsonPrimitives from unsigned numbers #2160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6dd4dac
add unsigned-number functions for creating JsonPrimitives
aSemy Jan 17, 2023
e668b06
update api file
aSemy Jan 19, 2023
79dce8e
update unsigned number tests
aSemy Mar 1, 2023
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
80 changes: 80 additions & 0 deletions ...ests/commonTest/src/kotlinx/serialization/json/serializers/JsonPrimitiveSerializerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,84 @@ class JsonPrimitiveSerializerTest : JsonTestBase() {
assertEquals(string, jvmExpectedString)
}
}

/**
* Helper function for [testJsonPrimitiveUnsignedNumbers]
*
* Asserts that an [unsigned number][actual] can be used to create a [JsonPrimitive][actualPrimitive],
* which can be decoded correctly.
*
* @param expected the expected string value of [actual]
* @param actual the unsigned number
* @param T should be an unsigned number
*/
private inline fun <reified T> assertUnsignedNumberEncoding(
expected: String,
actual: T,
actualPrimitive: JsonPrimitive,
) {
assertEquals(
expected,
actualPrimitive.toString(),
"expect ${T::class.simpleName} $actual can be used to create a JsonPrimitive"
)

parametrizedTest { mode ->
assertEquals(
expected,
default.encodeToString(JsonElement.serializer(), actualPrimitive, mode),
"expect ${T::class.simpleName} primitive can be decoded",
)
}
}

@Test
Copy link
Contributor

@shanshin shanshin Mar 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add serialization tests, like JsonUnquotedLiteralTest

Copy link
Contributor Author

@aSemy aSemy Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added parametrizedTest {}, like in JsonUnquotedLiteralTest

fun testJsonPrimitiveUnsignedNumbers() {

val expectedActualUBytes: Map<String, UByte> = mapOf(
"0" to 0u,
"1" to 1u,
"255" to UByte.MAX_VALUE,
)

expectedActualUBytes.forEach { (expected, actual) ->
assertUnsignedNumberEncoding(expected, actual, JsonPrimitive(actual))
}

val expectedActualUShorts: Map<String, UShort> = mapOf(
"0" to 0u,
"1" to 1u,
"255" to UByte.MAX_VALUE.toUShort(),
"65535" to UShort.MAX_VALUE,
)

expectedActualUShorts.forEach { (expected, actual) ->
assertUnsignedNumberEncoding(expected, actual, JsonPrimitive(actual))
}

val expectedActualUInts: Map<String, UInt> = mapOf(
"0" to 0u,
"1" to 1u,
"255" to UByte.MAX_VALUE.toUInt(),
"65535" to UShort.MAX_VALUE.toUInt(),
"4294967295" to UInt.MAX_VALUE,
)

expectedActualUInts.forEach { (expected, actual) ->
assertUnsignedNumberEncoding(expected, actual, JsonPrimitive(actual))
}

val expectedActualULongs: Map<String, ULong> = mapOf(
"0" to 0u,
"1" to 1u,
"255" to UByte.MAX_VALUE.toULong(),
"65535" to UShort.MAX_VALUE.toULong(),
"4294967295" to UInt.MAX_VALUE.toULong(),
"18446744073709551615" to ULong.MAX_VALUE,
)

expectedActualULongs.forEach { (expected, actual) ->
assertUnsignedNumberEncoding(expected, actual, JsonPrimitive(actual))
}
}
}
4 changes: 4 additions & 0 deletions formats/json/api/kotlinx-serialization-json.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ public final class kotlinx/serialization/json/JsonElementKt {
public static final fun JsonPrimitive (Ljava/lang/Number;)Lkotlinx/serialization/json/JsonPrimitive;
public static final fun JsonPrimitive (Ljava/lang/String;)Lkotlinx/serialization/json/JsonPrimitive;
public static final fun JsonPrimitive (Ljava/lang/Void;)Lkotlinx/serialization/json/JsonNull;
public static final fun JsonPrimitive-7apg3OU (B)Lkotlinx/serialization/json/JsonPrimitive;
public static final fun JsonPrimitive-VKZWuLQ (J)Lkotlinx/serialization/json/JsonPrimitive;
public static final fun JsonPrimitive-WZ4Q5Ns (I)Lkotlinx/serialization/json/JsonPrimitive;
public static final fun JsonPrimitive-xj2QHRw (S)Lkotlinx/serialization/json/JsonPrimitive;
public static final fun JsonUnquotedLiteral (Ljava/lang/String;)Lkotlinx/serialization/json/JsonPrimitive;
public static final fun getBoolean (Lkotlinx/serialization/json/JsonPrimitive;)Z
public static final fun getBooleanOrNull (Lkotlinx/serialization/json/JsonPrimitive;)Ljava/lang/Boolean;
Expand Down
44 changes: 34 additions & 10 deletions formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,33 +49,57 @@ public sealed class JsonPrimitive : JsonElement() {
public override fun toString(): String = content
}

/**
* Creates [JsonPrimitive] from the given boolean.
*/
/** Creates a [JsonPrimitive] from the given boolean. */
public fun JsonPrimitive(value: Boolean?): JsonPrimitive {
if (value == null) return JsonNull
return JsonLiteral(value, isString = false)
}

/**
* Creates [JsonPrimitive] from the given number.
*/
/** Creates a [JsonPrimitive] from the given number. */
public fun JsonPrimitive(value: Number?): JsonPrimitive {
if (value == null) return JsonNull
return JsonLiteral(value, isString = false)
}

/**
* Creates [JsonPrimitive] from the given string.
* Creates a numeric [JsonPrimitive] from the given [UByte].
*
* The value will be encoded as a JSON number.
*/
@ExperimentalSerializationApi
public fun JsonPrimitive(value: UByte): JsonPrimitive = JsonPrimitive(value.toULong())
Copy link
Contributor

@shanshin shanshin Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other functions use nullable parameters. I think for unsigned types we should do the same.

Copy link
Member

@qwwdfsad qwwdfsad Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we shouldn't, and nullability for other functions is rather a mistake.
It will cause unnecessary boxing, while bringing no benefit whatsoever: for nullable types, Nothing? overload will be chosen

Copy link
Contributor Author

@aSemy aSemy Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried adding nullability, but I ran into issues: #2130 (comment)

Copy link
Contributor

@shanshin shanshin Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qwwdfsad, shouldn't we leave an explanatory comment so that in the future there will be no questions why a different approach is used?

Copy link
Member

@qwwdfsad qwwdfsad Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have


/**
* Creates a numeric [JsonPrimitive] from the given [UShort].
*
* The value will be encoded as a JSON number.
*/
@ExperimentalSerializationApi
public fun JsonPrimitive(value: UShort): JsonPrimitive = JsonPrimitive(value.toULong())

/**
* Creates a numeric [JsonPrimitive] from the given [UInt].
*
* The value will be encoded as a JSON number.
*/
@ExperimentalSerializationApi
public fun JsonPrimitive(value: UInt): JsonPrimitive = JsonPrimitive(value.toULong())

/**
* Creates a numeric [JsonPrimitive] from the given [ULong].
*
* The value will be encoded as a JSON number.
*/
@ExperimentalSerializationApi
public fun JsonPrimitive(value: ULong): JsonPrimitive = JsonUnquotedLiteral(value.toString())

/** Creates a [JsonPrimitive] from the given string. */
public fun JsonPrimitive(value: String?): JsonPrimitive {
if (value == null) return JsonNull
return JsonLiteral(value, isString = true)
}

/**
* Creates [JsonNull].
*/
/** Creates [JsonNull]. */
@ExperimentalSerializationApi
@Suppress("FunctionName", "UNUSED_PARAMETER") // allows to call `JsonPrimitive(null)`
public fun JsonPrimitive(value: Nothing?): JsonNull = JsonNull
Expand Down