Kotlin / kotlinx.serialization

Kotlin multiplatform / multi-format serialization
Apache License 2.0
5.36k stars 619 forks source link

Incorrect warning about lateinit when used in conjunction with @Transient #2203

Open TorRanfelt opened 1 year ago

TorRanfelt commented 1 year ago

Describe the bug

    @Transient
    lateinit var parentHandle: InputKontoHandle

This gives a warning in IntelliJ IDEA and Kotlin compiler's log about lateinit even though it is used in conjunction with @Transient: "Lateinit is unnecessary: definitely initialized in constructors"

Expected behavior No warning.

Environment

sandwwraith commented 1 year ago

Why this warning shouldn't be there though? init blocks are executed during deserialization process, so If your parentHandle is initialized in init block, there shouldn't be any problems

TorRanfelt commented 1 year ago

It is not initialized during the init block. It is initialized at a later time, but before it is used (Hence lateinit. If that wasn't permissible there would never be a use-case for lateinit on a Transient variable.).

It is because I have a bunch of serializable child objects that each belongs to a parent non-serializable object. After deserialization these child objects are given to the corresponding parent objects, and at that point the parent object initialize parentHandle of the child object. Why this is a good idea is a long story, and not relevant here. The point is that lateinit should not cause a warning because the whole idea with lateinit is that the programmer knows it will be initialized before it is used.

EDIT: The problem with the warning is that IntelliJ IDEA and the compiler thinks that parentHandle always will be initialized because it is set in the constructor by parameter (this.parentHandle = parentHandle) and hence that lateinit is unnecessary. But in case of a save/load it has been deserialized and parentHandle will originally be uninitialized; hence I need lateinit.

sandwwraith commented 1 year ago

Can you post an example code, please?

TorRanfelt commented 1 year ago
class Foo {
  private var bar: Bar? = null // Foo can create a Bar instance.
  // Lots of variables and methods.
}
@Serializable
class Bar {
    // When a bar is created by foo the constructor is ran.
    constructor(foo: Foo) {
      this.foo = foo
    }

   // After deserialization bar gets reattached to foo and finally rebootPostLoad is ran.
   fun rebootPostLoad(foo: Foo) {
      this.foo = foo
    }

    @Transient
    lateinit var foo: Foo // If a Bar instance is created it references the Foo it was created by and belongs to.

    // Lots of variables and methods.
}

Every foo is also part of a map. The map along with a lot of other stuff is serialized. When deserialized every Bar at this point in time naturally doesn't have foo initialized, but right after they are reattached to their respective Foo and gets foo initialized through rebootPostLoad(). Only after this point will Bar be used and hence lateinit is correct.

The alternative is a dummy default-value for foo that is never used and that would be not be elegant because of the complexity of Foo (was it an int or list I would just set it to 0 or emptyList()). Such a dummy default-value is what Kotlin requires if lateinit is removed. So I don't want to remove it. But then I get a warning that "Lateinit is unnecessary: definitely initialized in constructor". But that is because the compiler doesn't take into consideration that this is a serializable class and hence constructor(foo: Foo) will not be run as part of deserialization. But not to worry because rebootPostLoad(foo: Foo) will be run right after which merits a lateinit.