Skip to content
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.

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

Construct JsonPrimitives from unsigned types #2130

Closed
rhdunn opened this issue Dec 19, 2022 · 3 comments
Closed

Construct JsonPrimitives from unsigned types #2130

rhdunn opened this issue Dec 19, 2022 · 3 comments
Labels
feature json-builders Suggested enhancements for JsonElements builders up for grabs A good issue for external contribution

Comments

@rhdunn
Copy link

rhdunn commented Dec 19, 2022

What is your use-case and why do you need this feature?
Various JSON protocols (e.g. Language Server Protocol) make use of unsigned types.

Currently, code like JsonPrimitive(7u) does not work as UInt does not implement the Number interface -- you need to do e.g. JsonPrimitive(value.toLong()) instead.

Note: This means that for ULong types, the max value will be capped at Int.MAX_INT due to the conversion to a signed type.

Describe the solution you'd like
Implement overloads for JsonPrimitive that take the unsigned integer types: UByte, UShort, UInt, and ULong.

@rhdunn rhdunn added the feature label Dec 19, 2022
@sandwwraith
Copy link
Member

As a workaround, it would be possible to use JsonUnquotedLiteral(uLong.toString()) once it is released in the next version.

@sandwwraith sandwwraith added up for grabs A good issue for external contribution json-builders Suggested enhancements for JsonElements builders labels Dec 19, 2022
@aSemy
Copy link
Contributor

aSemy commented Jan 17, 2023

I've taken a look into this, but I've come across a weird issue, and I'm not sure how to proceed.

I created functions that can accept unsigned numbers, and will use JsonUnquotedLiteral. For convenience, I made the values nullable.

public fun JsonPrimitive(value: UByte?): JsonPrimitive = 
    if (value == null) JsonNull else JsonUnquotedLiteral(value.toString())
public fun JsonPrimitive(value: UShort?): JsonPrimitive = 
    if (value == null) JsonNull else JsonUnquotedLiteral(value.toString())
public fun JsonPrimitive(value: UInt?): JsonPrimitive = 
    if (value == null) JsonNull else JsonUnquotedLiteral(value.toString())
public fun JsonPrimitive(value: ULong?): JsonPrimitive = 
    if (value == null) JsonNull else JsonUnquotedLiteral(value.toString())

This works correctly when the type is explicitly specified

val maxL: ULong = 18446744073709551615u
assertEquals("18446744073709551615", JsonPrimitive(maxL).toString())

It also works when Kotlin can figure out the exact type, for example if the number is so large it must be a ULong

assertEquals("18446744073709551615", JsonPrimitive(18446744073709551615u).toString())

However, Kotlin can't figure out what to do when it's ambiguous which unsigned number to use:

JsonPrimitive(0u)
Overload resolution ambiguity. All these functions match.
public fun JsonPrimitive(value: UByte?): JsonPrimitive defined in kotlinx.serialization.json in file JsonElement.kt
public fun JsonPrimitive(value: UInt?): JsonPrimitive defined in kotlinx.serialization.json in file JsonElement.kt
public fun JsonPrimitive(value: ULong?): JsonPrimitive defined in kotlinx.serialization.json in file JsonElement.kt
public fun JsonPrimitive(value: UShort?): JsonPrimitive defined in kotlinx.serialization.json in file JsonElement.kt

Options

Here's a few options I can think of

  1. Don't accept nullable unsigned numbers.

    fun JsonPrimitive(value: UByte): JsonPrimitive = ...
    fun JsonPrimitive(value: UInt): JsonPrimitive = ...
    fun JsonPrimitive(value: ULong): JsonPrimitive = ...
    fun JsonPrimitive(value: UShort): JsonPrimitive = ...

    Kotlin can then correctly deduce the correct function to use, but of course nullable values won't work.

    JsonPrimitive(0u) // works
    val ub: UByte? = 0u
    JsonPrimitive(ub) // fails
  2. Only accept ULong? - users must convert to ULong

    JsonPrimitive(0u) // works
    val ub: UByte? = 0u
    JsonPrimitive(ub) // fails
    JsonPrimitive(ub?.toULong) // works
  3. use different parameter names for each function

    fun operation(uByte: UByte?): Unit = operation(uByte.toString())

    while there’s still an error with operation(0u), at least it can be specified with operation(uLong = 0u)

Update: I've decided to go with option 1. It's the least intrusive, and it's possible to expand the behaviour later to be more convenient.

@aSemy
Copy link
Contributor

aSemy commented May 29, 2023

hi @rhdunn & @sandwwraith, this issue can be closed as done now https://github.com/Kotlin/kotlinx.serialization/releases/tag/v1.5.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature json-builders Suggested enhancements for JsonElements builders up for grabs A good issue for external contribution
Projects
None yet
Development

No branches or pull requests

4 participants