Contracts not working as expected when dividing into functions

Support
1

Consider the following piece of code in Kotlin 1.3:

fun main() {
    val foo: String? = "bar"
    requireNotNull(foo)
    println(foo.length)
}

It compiles and runs as expected, allowing to write foo.length instead of e.g. foo!!.length or foo?.length ?: 0. It’s because requireNotNull “emits” a contract that foo is not null.

But let’s take a look at this snippet:

fun main() {
    val foo: String? = "bar"
    validate(foo)
    println(foo.length)
}

fun validate(foo: String?) {
    requireNotNull(foo)
}

Just a simple refactoring, right? Extracting requireNotNull to a separate function. Yet I’m getting Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type String?. The contract doesn’t work anymore. It looks like contracts work only in scope of a single function.

What I did to understand it:

  • inlined the validate function - doesn’t change the behavior

  • took a look at contracts’ KEEP and this part brought my attention:

    “Contracts are allowed only on functions, which are (…) Top-level (because inheritance not implemented yet)”

    Assuming that “inheritance” here means conveying contracts between functions, it would mean that it’s an area for improvement for contracts.

  • skimmed through the bug tracker and this brought my attention, however I’m not sure if it relates to the issue I’m describing here: https://youtrack.jetbrains.net/issue/KT-26856

My question is: is it by design, to be done in the future, or I’m doing something wrong?

Thanks!

2

Correct. AFAIK this is by design and will not change. If you want a function to have a contract you need to specify it explicitly.

No. “Inheritance” means the same as it does in OOP, inheriting functions from a parent class. The part you quoted means that you can only use contracts for top level functions. The rest will probably come later but it’s design is not yet done.

Yes. The behavior you see is by design. The reason is quite simple. Removing a contract from a function is a breaking change of it’s API. Therefor the compiler will and should never infer a contract for a function. Imagine you have this code.

/**
    @returns true a result was calculated
*/
fun calculateIfPossible(foo: Foo?): Boolean {
    if(foo == null) return false   // TODO temp restriction, will implement later
    // some other restrictions
    // do some calculation
    return true
}

You now release a version of this library. If the compiler would now create a contract for this that foo != null if true is returned, you could not change this function without breaking the code of everyone using this library.
That’s why you have to explicitly add the contract to each function.

3 Likes
3

Thanks for your reply!

I get the argument about specifying contracts explicitly for each function. However, I’m half-satisfied with such approach from Kotlin’s side. Let me explain why.

You mention an example of a library, and an inferred contract. IMO there are also local use cases of contracts, which should be maybe referred to as smart casts regarding nullability. Let me iterate on my example. When I write such piece of code

fun main() {
    val foo: String? = "bar"
    validate(foo)
    println(foo!!.length) // Saying 'not null' 2st time (chronologically).
}

private fun validate(foo: String?) {
    requireNotNull(foo) // Saying 'not null' 1st time (chronologically).
}

I’m telling the compiler twice that foo is not null. I feel like it should be enough to tell it once. Hence my little disappointment about how contracts work right now.

If I were to propose some approach that doesn’t break the current approach but only adds to it, how about something like this?

fun main() {
    val foo: String? = "bar"
    validate(foo)
    println(foo.length)
}

private fun validate(foo: String?) {
    // The below block says "whatever contracts say about the functions
    // inside the block, apply also to this function `validate`".
    bubbleUpContract {
        requireNotNull(foo) // Ensuring not null only once.
    }
}

Don’t pay attention to the name bubbleUpContract. My point is to make the contracts more refactoring-friendly, so that such extracting of validation to separate methods works intuitively. Functions are meant to divide the code in a logical manner, and it’s generally encouraged to have more well-named functions than less. The moment the compiler makes it harder to divide the code into functions, it discourages me from writing code that I perceive as clean. Plus, null-safety is one of flagship features of Kotlin, so it should be intuitive and pleasant to use.

Remark: So far, I haven’t considered other types of contracts besides asserting non-null. Let’s put it aside for a moment, despite the general title of this topic.

What do you think about all this?

4

Your example has almost as much ceremony as adding the contract explicitly to your validate function.

5

Regarding the amount of code - true. But the point is to not state the same fact twice, which is against the DRY principle and doesn’t scale well (consider more than 1 requireNotNull in the validate method). With explicit contracts we would have these both cons.
I’m not saying that such DSL for “bubbling” this contract is the best way to go, I just see a need to achieve such thing.

6

I don’t agree. DRY has nothing to do with the contract of a function. DRY is about not repeating the implementation. You still use interfaces even though it requires you to write out the function definitions twice. Contracts are part of the API and not part of the implementation.
Also when talking about “bubbling up” the contract from one function into another, this is only possible in a very limited fashion, since many contracts are defined based on the return value and not only the successful execution of the function.
Adding a special syntax just to pull up the returns implies someValue != null case just makes contracts harder to read and in no way more efficient to write.

1 Like