-
Notifications
You must be signed in to change notification settings - Fork 665
Allow Kotlin's null literal in JSON DSL #1907
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
Conversation
|
I haven't added |
I added the annotations now. |
sandwwraith
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long delay, your PR slipped out of my attention. You were completely right with @ExperimentalSerializationApi. I think we can merge this after you address the comments
| return JsonLiteral(value, isString = true) | ||
| } | ||
|
|
||
| /** Returns [JsonNull]. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that this function is needed? It's much easier here to use JsonNull instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not strictly needed, but it feels more consistent to me.
But it's fine for me if you think this one is unnecessary and should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, maybe it would be easier to discover. Then it's better to use doc style as in the other functions:
/**
* Creates [JsonNull].
*/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I adjusted the doc style 👍
formats/json/commonMain/src/kotlinx/serialization/json/JsonElementBuilders.kt
Outdated
Show resolved
Hide resolved
formats/json/commonMain/src/kotlinx/serialization/json/JsonElementBuilders.kt
Outdated
Show resolved
Hide resolved
This reverts commit d862c54.
|
Thank you for your efforts! |
Right now you can not really use Kotlin's
nullliteral with the JSON DSL:All of these errors could be resolved by using a cast like
null as String?or by usingJsonNulldirectly.However, this is somewhat irritating as other primitives that are supported by JSON (numbers, strings,
trueandfalse) can be used directly whilenullcan't.To resolve the overload resolution ambiguity and allow use of the
nullliteral in the JSON DSL, this PR adds overloads for theJsonObjectBuilder.put(),JsonArrayBuilder.add()andJsonPrimitve()functions that take an argument of typeNothing?.