Sneaky delegated properties fooling smart casts


#1

Obviously one of the greatest things we love about kotlin is the type safety (including distinction of nullable types) along with very smart type inference and automatic casting.

Another thing a lot of us like is the advantage of effortless final variables using the ‘val’ keyword. By assigning a value to a val, you are (hypothetically) ‘guaranteed’ the value won’t change.

However, customizable getters on vals kind of shakes things up. And since Kotlin 1.1, delegated properties even shake up local values with local delegated properties.

When creating a val using a custom getter or a delegate, there is no actual guarantee that the val will be immutable, or even retain it’s inferenced type.

From this we see that vals do NOT mean immutability. vals only mean there is no setter, and even that is loose because the getter can return whatever it wants (including variable values).

Now vals that you write yourself with no delegate or custom getter are completely safe, of course. But we see everywhere vals described as ‘immutable’ and even treated that way by the compiler, when in fact they are not.

See two examples:

import kotlin.reflect.KProperty

class IncrementOnAccess(private var value: Int) {
    operator fun getValue(thisRef: Any?, property: KProperty<*>): Int? {
        return ++value
    }
}

class EvilDelegate(val value: Int) {
    private var mutableInt: Int = 0

    operator fun getValue(thisRef: Any?, property: KProperty<*>): Int? {
        return if((mutableInt++) < 2) value else null
    }
}

fun main(vararg args: String) {

    // Example 1:
    println("Example 1:")

    val immutableIntNeverChanges by IncrementOnAccess(0)
    for (i in 0..3)
        // 'Immutable' val changes on every access
        println(immutableIntNeverChanges)


    // Example 2:
    println("Example 2:")

    val innocentLookingVal by EvilDelegate(7)

    println(innocentLookingVal)
    if (innocentLookingVal != null) {

        // Note the smart cast to non-null kotlin.Int
        // Compiler is not just completely oblivious to the risk, but misleading too.
        val fail = innocentLookingVal.plus(2)

        println("you do not reach this statement because NullPointerException")
    }
}

Now, the above example code clearly is a demonstration of BAD programming intentions. However, this topic isn’t about the implementation of EvilDelegate being evil or not, this topic is about the nature of the symptom being described, and to raise discussion for if there is anything that should/could be done to fix/address/improve it.

…because unless a defense against this symptom is built into the language design or compiler, there is no guarantee that this won’t be encountered on more innocent and less explicit terms. For example, an interface could declare a val, and an obscure implementation could fall into this trap, and then people who inadvertently use that type by handling the any subclass of the interface will pay the price. This doesn’t just have to happen with evil delegates, or bugged delegates, but might could even happen if someone misused/misunderstood a working delegate that was intended to return varying values. It isn’t up to Kotlin to figure out if you chose the wrong delegate for implementation, however, it does kind of step into Kotlin’s responsibility depending on what ‘guarantees’ are made for ‘immutable’ vals and more importantly, the mislead compiler giving you smart casts that actually turn out to fail during runtime.

(I’m not saying Jetbrains/Kotlin makes guarantees about val immutability unless they do, but in either case it is a strong idea in the userbase, and at its basic not really true.)

So with the explanation of the issue, onto the discussion.

Obviously we want to recognize immutability when possible, and smart cast when possible, and also allow customizing properties and using delegates on local values. Each of these is great. It’s just that when you combine them all, the compiler fails to catch some issues that we can all agree under normal circumstances we want it to catch.

I decided to create a discussion instead of a bug report, because I can hardly see this as a vast and pressing issue. However it’s been pressing on my mind as ‘kind of an issue’, and I want to hear the thoughts of others.

So some questions to kick off:

  • thoughts?
  • Is this severe?
  • Could it be severe?
  • In an ideal world with an ideal Kotlin, would the compiler or language design prevent misleading code and type inference like this?
    – (would and should we fix it if we assume we could?)
  • If so, what possible approaches are there?
    – (could we fix it, and how?) Maybe some kind of marking on delegates (annotation, keyword, or something)? Or maybe something “stronger than a val”, since vals apparently only mean having no setter, maybe the thing “stronger than a val” can have no custom getter, and must be a true immutable value. Or other ideas.

#2

Regarding the case with EvilDelegate, i.e. the incorrect smart cast of a local delegated val — you definitely have found a bug here and rather major one, so it’s better to report it right to the tracker: http://kotl.in/issue. The compiler should prevent such smart casts, as it does for delegated properties or any other properties with custom getters, such as interface member properties, etc.

Could you point what in the official language documentation makes you think that vals are stated to be always immutable? We may need to clarify these topics then.


#3

While starting to create the issue, I found this similar issue that already exists. Does it cover what is needed to specify the problem, or is it too narrow?

https://youtrack.jetbrains.com/issue/KT-18129

When I say “we see everywhere” I am not being precise, and neither am I intending to fault the official documentation. Browsing stackoverflow a lot, and digesting the documentation at a surface level can just lead to this misapprehension, because it is true that assigning a simple val will give you an unchanging value (e.g. in situations where people wish they could smart cast vars, solution is to create a local val). Then people (like formerly me) just conjure the idea that immutability is intended to implied by the keyword, and not just one possible result of it. I did a google search for “kotlin val immutable” and the first result is a blog post that adequately describes the misapprehension. https://artemzin.com/blog/kotlin-val-does-not-mean-immutable-it-just-means-readonly-yeah/

While not faulting the official documentation, there may be room to pro-actively fight the misapprehension there by adding a note or extra warning to not fall for the mistake.


#4

Yes, it seems to cover this case too.


#5

I feel like this is just an extension of Java’s “what does it mean for a collection to be final” discussion. I’ve been asking that as an interview question for 10 years now! :slight_smile:

But yes, it might be good for the Kotlin team to say, “hey, val just applies to pointers, not to contents of pointers!”


#6

That’s not the issue discussed here. The discussed confusion stems from that the “pointer” can also change, if val property/variable has a custom getter.


#7

Hopefully you do not see such advertising by IntelliJ because it isn’t true as your example proves once again.