Improve Kotlin type variable nullability consistency

[aerb]

If I have a model defined like so:

data class HasAttributes(
  val name: Attribute<String?>
)

data class Attribute<T>(
  val value: T,
  val version: Int,
)

val attributes = HasAttributes(name = Attribute(value = null, version = 1))

and serialize it using the following adapter:

val moshi = Moshi.Builder()
      .add(KotlinJsonAdapterFactory())
      .build()
  val adapter = moshi.adapter(HasAttributes::class.java)

Moshi will correctly output:

{"name":{"version":1}}

On deserialization however the following error is thrown:

Required value 'value' missing at $.name

I would expect it to correctly deserialize to the following:

HasAttributes(name=Attribute(value=null, version=1))

[aerb]

Here's a project demonstrating the failure: moshi-bug.zip

[ZacSweers]

Thanks for the repro steps. Propagating this nullability down to the used property may not be possible in Java alone, as we pass around java Type instances to the generated adapter that contain no information about nullability. Furthermore - the generated adapters themselves assume the nullability of the declared type variable.

There are two possible solutions:

1. A local workaround is to mark the property nullable if you know it may be typed as such

data class HasAttributes(
  val name: Attribute<String?>
)

data class Attribute<T>(
  val value: T?,
  val version: Int,
)

2. Another, potentially more invasive approach we could take is to be über-explicit with generic type nullability. That is to say, assume that generic types are nullable unless they extend Any.

data class HasAttributes(
  val name: Attribute<String?> // <- This is no longer possible
)

data class Attribute<T : Any>(
  val value: T,
  val version: Int,
)

This change would likely futz a lot of people's builds though, so we should take care in making any behavior change. This is also more in line with how Kotlin thinks of nullability in generics. Maybe we could add this support but offer a flag to opt in/out of this strict generic nullability checking? This could be a parameter on KotlinJsonAdapterFactory and apt option on code gen.

CC @swankjesse @JakeWharton @rharter

[zahoranp]

Is there any update on this? We are facing the exact same issue using moshi-kotlin-codegen and the local workaround suggested above is rather inconvenient as it breaks the contract when a non-nullable type parameter is passed.

Ideally

data class HasAttributes(
   val required: Attribute<String>,
   val optional: Attribute<String?>
)

should correctly deserialize

{"required":{"value":"some value","version":1},"optional":{"version":1}}

and fail deserializing

{"required":{"version":1},"optional":{"version":1}}

with error

Required value 'value' missing at $.required

[Antimonit]

It appears this problem can manifest only with generic types since the nullability of generic types can be defined both at call-site and declaration-site.

data class One<T>(val value: T)
data class Two<T>(val value: T?)

val a = One<String>("")
val b = Two<String>(null)
val c = One<String?>(null)
val d = Two<String?>(null)

val aValue: String = a.value
val bValue: String? = b.value // declaration-site nullable
val cValue: String? = c.value // call-site nullable
val dValue: String? = d.value // both sites nullable

The dValue can be thought of as being "nullable twice". That, of course, does not exist in Kotlin and double nullability "coalesces" into a single ?.

If we replaced the Kotlin's ? with an Optional type, the double nullability is explicit.

data class One<T>(val value: T)
data class Two<T>(val value: Optional<T>)

val a = One<String>("")
val b = Two<String>(Optional.empty())
val c = One<Optional<String>>(Optional.empty())
val d = Two<Optional<String>>(Optional.empty())

val aValue: String = a.value
val bValue: Optional<String> = b.value
val cValue: Optional<String> = c.value
val dValue: Optional<Optional<String>> = d.value

---

Moshi struggles with a somewhat similar problem. Moshi internally uses null to represent uninitialized values which clashes with the nullability of nullable generic types.

For a generic class

@JsonClass(generateAdapter = true)
data class Generic<out T>(
    val generic: T,
    val nullableGeneric: T?,
)

the generated parsing method will look like

  public override fun fromJson(reader: JsonReader): Generic<T> {
    var generic: T? = null
    var nullableGeneric: T? = null
    reader.beginObject()
    while (reader.hasNext()) {
      when (reader.selectName(options)) {
        0 -> generic = tNullableAnyAdapter.fromJson(reader) ?: throw Util.unexpectedNull("generic",
            "generic", reader)
        1 -> nullableGeneric = nullableTNullableAnyAdapter.fromJson(reader)
        -1 -> {
          // Unknown name, skip it.
          reader.skipName()
          reader.skipValue()
        }
      }
    }
    reader.endObject()
    return Generic<T>(
        generic = generic ?: throw Util.missingProperty("generic", "generic", reader),
        nullableGeneric = nullableGeneric
    )
  }

It only takes into account the declaration-site nullability. The use-site nullability is unknown.

This makes cases like this throw an exception:

val data = Generic<String?>(null, null)
val string = adapter.toJson(data)
val obj = adapter.fromJson(string)

---

I thought I was so clever and wanted to propose using a wrapper like

sealed interface Result<out T> {
    object Uninitialized : Result<Nothing>
    //    @JvmInline value class from Kotlin 1.8 
    data class Initialized<out T>(val value: T) : Result<T>
}

fun <T> Result<T>.valueOrThrow(propertyName: String, jsonName: String, reader: JsonReader): T =
    when (this) {
        Result.Uninitialized -> throw Util.missingProperty(propertyName, jsonName, reader)
        is Result.Initialized -> value
    }

fun <T> Result<T?>.valueOrNull(): T? =
    when (this) {
        Result.Uninitialized -> null
        is Result.Initialized -> value
    }

and

    var generic: Result<T> = Result.Uninitialized
    var nullableGeneric: Result<T?> = Result.Uninitialized

rather than

    var generic: T? = null
    var nullableGeneric: T? = null

But even with the Result wrapper I still ran into issues when constructing the object. When the generic value is Uninitialized, we want to provide a default null value but we can't do that because the T type's nullability is not known from the Generic class's declaration-site nullability alone.

---

Unfortunately, I was not able to provide a solution but it might be worth checking out how kotlinx.serialization library handles this situation. I didn't delve into the implementation but the following sample works as I would expect:

@Serializable
data class Generic<out T>(
    val generic: T,
    val nullableGeneric: T? = null,
)

fun kotlinx() {
    val json = Json { explicitNulls = false }

    val data: Generic<String?> = Generic(null, null)
    val string = json.encodeToString(data)
    println(string)

    val obj = json.decodeFromString<Generic<String?>>(string)
    println(obj)
}

{"generic":null}
Generic(generic=null, nullableGeneric=null)