Kotlin / kotlinx.serialization

Kotlin multiplatform / multi-format serialization
Apache License 2.0
5.37k stars 620 forks source link

Transient lambdas capture wrong parameters #2492

Open adrian-shape opened 11 months ago

adrian-shape commented 11 months ago

Describe the bug

When you have a @Serializable class with a @Transient lambda, the lambda captures the wrong parameters. The code compiles just fine, but crashes during runtime.

To Reproduce

If we have the following class, and call StringWrapper("1.0").double

data class DoubleWrapper(val value: Double)
interface DoubleWrapperBuilder {
    fun build(value: Double): DoubleWrapper = DoubleWrapper(value)
}

@Serializable
data class StringWrapper(
    val value: String,
    @Transient val doubleWrapperBuilder: DoubleWrapperBuilder = object : DoubleWrapperBuilder {
        override fun build(value: Double): DoubleWrapper = DoubleWrapper(value)
    }
) {
    val double: DoubleWrapper
        get() {
            val doubleValue: Double = value.toDouble()
            Timber.d("Value as double: $doubleValue")
            return doubleWrapperBuilder.build(doubleValue)
        }
}

Then this is the result:

Value as double: 1.0
java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String com.example.foo.StringWrapper.getValue()' on a null object reference
  at com.example.foo.StringWrapper$2.build(Foo.kt:36)
  at com.example.foo.StringWrapper.getDouble(Foo.kt:46)

If I remove the annotations, the code behaves as expected.

If I change the argument names inside DoubleWrapperBuilder to not clash, then the code behaves as expected.

I have also in some other iteration of this bug with different usage gotten a ClassCastException from String to Number while having Java code involved.

Expected behavior

Calling the .double function as specified should not crash.

Environment

pdvrieze commented 11 months ago

This bug seems to do with name resolution by the compiler. The implementation of the build function in the anonymous object somehow resolves value to the string property, rather than to the double parameter. Some questions:

adrian-shape commented 11 months ago

Is this problem the same if you just create the object without using serialization (but it being serializable)

I just created the object manually with StringWrapper("1.0").double, not using serialization, but it still crashes. If I remove both annotations, the issue no longer appears.

What happens if you don't make the class a data class

With both data classes being converted to regular classes, same crash appears.

What happens if you move the doubleWrapperBuilder to the companion object

In both scenarios if I move the entire doubleWrapperBuilder value to the companion object everything works fine. If I move the lambda to the companion object and assign it in the constructor, it works fine too.

I guess could have something to do with creating the lambda in the constructor with @Transient