Skip to content

Inconsistencies with nullability design #3087

@BenWoodworth

Description

@BenWoodworth

There are some pain points I run into with nullability, and I'm curious where the nullable APIs stand and what the thoughts currently are around them since some are still marked as experimental. Nullability is something I brush up against often enough, so I'm also interested in helping round off some rough edges if it means working with nullable types would be more resilient.

With the current design, there are a few things on my mind:

  • The purpose of descriptor.isNullable, and inconsistencies with how it's being used
  • Correctness of generated serializer for generic elements
    • Defensive isNullable checks leaking into the runtime, sometimes justifying impossible casts
    • Reliability of encode/decodeNullableSerializableValue, and whether it provides any value

I'm going to mainly focus on the first point, though, since it bleeds into the rest.


For SerialDescriptor.isNullable, the docs aren't very clear about what the contract specifically is. I'd assume isNullable means either:

  1. whether the serializer can accept null Kotlin values,
  2. whether the serializer can handle null in the serialized form,
  3. or both, with the serialized form's nullability being tied to the Kotlin value's nullability

Poking through some of its uses in the library, it looks like it's interpreted inconsistently in some places. For example:

  • ProtoBuf's schema generator uses isNullable to determine the shape of the serialized form (as optional in the schema, and omitted when encodeNull() is used)
    • So, this usage assumes either (2.) or (3.) is the case.
  • But, JsonNullSerializer (implicitly) has isNullable = false. It doesn't accept null Kotlin values, but does encode null in the serialized form.
    • This means the code assumes only (1.) is the case, but that's contradicting ProtoBuf's usage.

Here's an overview of nullability usage I put together to guide my thought process and help bring things into focus:

Usages of isNullable inside the library

K: Uses isNullable to assume that the Kotlin type can be null
S: Uses isNullable to assume that the serialized form can be null

Serializers with isNullable outside the library

From this GitHub search (228 files), these are all the KSerializer implementations I found where descriptor.isNullable is true (or potentially true, e.g. delegating to a constructor parameter).

Here's the complete table, including the serializers that didn't actually have isNullable: potentially-nullable-serializers-on-github.md

Nullable Kotlin Type Nullable Serialized Form Project Serializer
true true BetrixDev/ssm-brawl object NullableInstantIsoSerializer
: KSerializer<Instant?>
true true Bruce0203/Mintlin object NullableFloatSerializer
: KSerializer<Float?>
false true Bundesdruckerei-GmbH/pid-issuer object ArrayElementSelectorSerializer
: KSerializer<ArrayElementSelector>
true true ChrisKruegerDev/tmdb-kotlin class LocalDateSerializer
: KSerializer<LocalDate?>
true true Hansanto/kault object OptionalInstantSerializer
: KSerializer<Instant?>
true true RevenueCat/purchases-android object EmptyStringToNullSerializer
: KSerializer<String?>
false true* Snd-R/komf class KomfProvidersSerializer
: KSerializer<KomfProviders>
false false Snd-R/komf class SubjectTypeSerializer
: KSerializer<SubjectType>
false true* Snd-R/komf class MalMediaTypeSerializer
: KSerializer<MalMediaType>
false true* Snd-R/komf class MangaBakaStatusSerializer
: KSerializer<MangaBakaStatus>
false false Snd-R/komf class KavitaAgeRatingSerializer
: KSerializer<KavitaAgeRating>
false false Snd-R/komf class KavitaPublicationStatusSerializer
: KSerializer<KavitaPublicationStatus>
true true Snd-R/komga-client class KomgaReadingDirectionSerializer
: KSerializer<KomgaReadingDirection?>
true true Sottti/RollerCoasters object PictureSerializer
: KSerializer<PictureApiModel?>
false true amostyaev/MyVelobike object object NumNullableSerializer
: KSerializer<Int>
false true arrow-kt/arrow class OptionSerializer<T>
: KSerializer<Option<T>>
false true avro-kotlin/avro4k-arrow class OptionSerializer<A : Any>
: KSerializer<Option<A>>
false true charleskorn/kaml object YamlNodeSerializer
: KSerializer<YamlNode>
true true denyshorman/experimental-asset-trader object NullableBigDecimalAsStringSerializer
: KSerializer<BigDecimal?>
false true dimensional-fun/nats.kt class OptionalSerializer<T>
: KSerializer<Optional<T>>
true true evanc577/Somnia class NullableRedditResponseSerializer
: KSerializer<RedditResponse?>
false true hank9999/kook-kt class OptionalSerializer<T>
: KSerializer<Optional<T>>
true true ivsokol/policy-engine object ArrayNodeSerializer
: KSerializer<ArrayNode?>
true true ivsokol/policy-engine object BigDecimalSerializer
: KSerializer<BigDecimal?>
true true ivsokol/policy-engine object JsonNodeSerializer
: KSerializer<JsonNode?>
true true ivsokol/policy-engine object ObjectNodeSerializer
: KSerializer<ObjectNode?>
true true ivsokol/policy-engine object OffsetDateTimeSerializer
: KSerializer<OffsetDateTime?>
true true ivsokol/policy-engine object PeriodSerializer
: KSerializer<Period?>
false true jombidev/discoin class OptionalSerializer<T>
: KSerializer<Optional<T>>
false true jombidev/kord-selfbot class OptionalSerializer<T>
: KSerializer<Optional<T>>
false true kernel-panic-codecave/Archie object Serializer
: KSerializer<ArchieItemSlot>
false true kordlib/kord object Serializer
: KSerializer<Heartbeat>
false true lightningkite/lightning-server class PartialSerializer<T>
: KSerializer<Partial<T>>
false true meetacy/sdk class Serializer<T>
: KSerializer<OptionalSerializable<T>>
false true mixtape-oss/kord-voice class Serializer<T>
: KSerializer<Optional<T>>
false true pavelannin/Monadic-Kotlin class OptionalCatchingSerializer<Value>
: KSerializer<Optional<Value>>
true false pdvrieze/xmlutil class OptionalDateSerializer
: KSerializer<LocalDate?>
true false pdvrieze/xmlutil companion object
: KSerializer<SchemaVersion?>
false true project-eCSB/ecsb-backend class OptSerializer<T : Any>
: KSerializer<Option<T>>
true true pschichtel/idl object NullAsJsonNullJsonElementSerializer
: KSerializer<JsonElement?>
false true sugarmanz/package-json object Serializer
: KSerializer<Bugs.Simple>
true true tasgon/observable class EntitySerializer
: KSerializer<Entity?>
true true terra-money/terra.kt class EmptyToNullSerializer<T : Any>
: KSerializer<T?>
false true tudo-aqua/rereso class NullifyEmptySetSerializer<T>
: KSerializer<Set<T>>
true true twofas/2fas-pass-android object EncryptedBytesSerializer
: KSerializer<EncryptedBytes?>
true true wezuwiusz/neowulkanowy object LocalDateSerializer
: KSerializer<LocalDate?>
false true wireapp/kalium object CallQualityAsIntSerializer
: KSerializer<CallQuality>
false true wireapp/kalium object ReceiptModeAsIntSerializer
: KSerializer<ReceiptMode>
false true wireapp/wire-apps-jvm-sdk object ReceiptModeAsIntSerializer
: KSerializer<ReceiptMode>
true true xephosbot/AnilibriaRefresh class EnumSerializer<T : Enum<T>>
: KSerializer<T?>
true false yschimke/rememberwear class InstantTypeConverter
: KSerializer<Instant?>
true false zly2006/zhihu-plus-plus object LegacyAuthorSerCompat
: KSerializer<Person?>

Focusing on the 3rd-party serializers that have isNullable set, here's a summary of their nullabilities:

Nullable Kotlin Type Nullable Serialized Form # Serializers
false true 24
true true 21
true false 4
false false 3

Going off of these:

  • If code uses isNullable to assume the Kotlin type nullability, it'll be correct 48% of the time.
  • If code uses isNullable to assume the serial form nullability, it'll be correct 87% of the time.

Or, ignoring serializers where the nullability is symmetric between the Kotlin type and serial form:

  • assuming Kotlin type nullability is correct 14% of the time
  • assuming Serial form nullability is correct 86% of the time

Based on this, it seems like isNullable is largely used to represent only the serialized form's nullability, but that there's also some general inconsistency with how it's used. This does also match how isNullable is used within the library, with most isNullable checks being used to check whether the serialized form can be null, and only a couple checks being there to check the runtime Kotlin type (in KSerializer<T>.nullable and Encoder.encodeNullableSerializableValue()).

This is important to note because of those two runtime checks, since as it stands now, using isNullable to check the Kotlin type's nullability is worse than a coin flip (though not by much). My impression is that some of this is a symptom of how generated serializers are implemented when nullable generic elements are involved. For example, generated serializers can make illegal calls to KSerializer<T>.nullable with nullable T, despite T being non-nullable (and the isNullable check then justifying a cast from KSerializer<T> to KSerializer<T?>, even though that's impossible since KSerializer is invariant in T).


As far as solutions for the issue, I think I'd like to see isNullable changed to strictly reflect the serialized form's nullability, clarifying the documentation and changing the two places that use as Kotlin type checks. Intuitively I think that would makes sense, with most of the serial descriptor already being used to mirror the serialized form. It also plays more nicely when wrapping nullable serializers, compared to it representing the Kotlin type where you'd need to be mindful about updating the nullability when delegating a non-nullable serializer to one for a nullable Kotlin type. There's also the value of introspecting the serialized form, since otherwise there isn't a way to tell if a serializer can encode null.

That said, though, I do know it's a complex issue, and I understand why there's the need for Kotlin type nullability checks. The Kotlin types aren't always runtime-available, and there's a need to add nullability support to elements with non-nullable serializers, but we also don't want to intercept null Kotlin values from serializers that can handle them. That trickiness is why I'm curious about the state and current thoughts around nullability. It's something I'd be interested in contributing to, and I can go into more detail about any of this.

Also, just for some context, before writing up this issue I didn't have a great idea how isNullable really should be used. Just that I had a mental model which occasionally got subtly challenged, leaving me second-guessing whether I actually had it straight in my head. Seeing everything laid out helped clear a lot of it up for me.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions