Guarding against unintentional return types in if statements

Language Design
1

Kotlin allows following:

var a = 0
if (a == 0) a = 1 else a == 2

In this case, if statement will return a boolean value, since code in else evaluates a condition instead of assigning it as it was the original intention. Is there a way to guard against these unintentional returns that are a result of programming mistakes?

2

Where possible, avoid var in favor of val, and assign the if/else directly. Doesn’t help in all cases, but likely covers most of them.

4 Likes
3

To spell that out, something like:

val a = if (a == 0) 1 else 2

would be more clear, more concise, safer (no opportunity for that sort of error), and making the variable immutable could avoid errors further down. Also, the if could be replaced by a when for more complex processing.

5 Likes
4

Even though that strictly does not answer the question, I tend to agree as I don’t remember ever assigning values in an if/else branch.

1 Like
5

Usually I don’t assign variables to a dummy value, val a: Int or var a: Int do the trick.

2 Likes
6

Thank you all for the replies! As @brunojcm mentioned, this does not strictly answer the question. Firstly, avoiding var is not always easy. You could be working with a data structure that internally holds a var reference, which you want to modify. This is very common in Android, for example. Secondly, never assigning values in if statements is a very strict rule that is difficult, if not impossible to enforce.

Here’s a more realistic scenario where the issue is less obvious and much more likely to be missed:

import kotlin.random.Random

class TextView(var text: String)

fun main() {
    val textView = TextView("Text")
    textView.updateText(Random.nextDouble() > 0.5)
    
    println(textView.text)
}

fun TextView.updateText(shouldUpdate: Boolean) {
     if (shouldUpdate) {
        this.text = "Hi"
    } else {
        this.text == "Bye"
    }
}
7

And again, that could be written more clearly, concisely, and safely, e.g.:

fun TextView.updateText(shouldUpdate: Boolean) {
    this.text = if (shouldUpdate) "Hi" else "Bye"
}
3 Likes
8

My point is not that what I wrote is better. As I cannot enforce this style across the whole organization, this error is bound to happen eventually. I’m a layman in language design, but it’s strange to me that this compiles in the first place as these seem to be two different return types - one is a statement (returns a Unit) and the other retruns a Boolean.

In any case, I think I got the answer. Thank you!

9

I think the whole point of an if statement is that you can do wildly different things. Adding language constructs to limit what you can do in the branches is bound to have limited functionality (not powerful enough because otherwise lots of complex syntax is needed) and/or be unwieldy (it is possible to enforce a lot of restrictions but it requires quite a bit of code).

If you use an if expression, then you have a “return” value. But then you also have an easy way to enforce what the type of that value must be: explicitly specify the type of the variable, the function returning the value, etc.

I think your use case calls more for testing than for enforcement by the language.

10

Well, as an analogy, you can’t force people to make the text property a String. They could have made it Any, and then assigning 42 to it would not be forbidden by the language either.

You cannot force people to be good programmers. The language can protect programmers to some extent, but ultimately there are styles that are inherently safer and styles that are less safe. It’s our job to pick better styles.

That being said, in this specific case, I guess Kotlin could at least warn about “dangling” expressions that are not assigned to anything or used in any way.

5 Likes
11

I agree, I think the only reasonable thing we can do here is to look for expressions where we expect their results to be used somewhere, but they are not. Things like: x == y, list[0], map["a"], async {}, etc. Technically, some of them could have side effects and be used for them, but this is a very “evil” case.

Anyway, I’m not sure if we should do such checks by the compiler itself, or by the IDE. And speaking of IDEs… it’s funny nobody checked/mentioned this, but the discussed code clearly shows as a warning in IntelliJ, because a == 2 expression is unused. Any decent linter tool would probably do the same.

9 Likes
12

it’s funny nobody checked/mentioned this, but the discussed code clearly shows as a warning in IntelliJ, because a == 2 expression is unused

Oh… given the OP’s post I was wrongly assuming there was no such warnings (I was on the phone so I couldn’t check).

Then I really don’t get the origin of the post at all. If developers ignore warnings, what can you do?

6 Likes
13

And speaking of IDEs… it’s funny nobody checked/mentioned this, but the discussed code clearly shows as a warning in IntelliJ, because a == 2 expression is unused. Any decent linter tool would probably do the same.

Thanks @broot, you’re right. I wasn’t aware of the inspection. That surely solves it. As for linters, I’ve tried a couple so far and none of them report UnusedEqualsExpression as an issue. That’s off-topic, but I just want to highlight that it’s not as obvious as it may seem.

Then I really don’t get the origin of the post at all. If developers ignore warnings, what can you do?

I wasn’t aware that the inspection exists, hence the post. The inspection solves the issue, for sure. As for what you could do, you could fail the lint check, which is what I’m going to do. The solution has got nothing to do with the language design at this point, of course :slight_smile:

1 Like
14

Just for a reference and documentation, here’s the original issue that introduced the inspection:
https://youtrack.jetbrains.com/issue/KTIJ-2930/Add-warning-for-unused-equals-expression

2 Likes