A pitfall with multiple default function arguments and passing lambdas

Language Design
1

There are 2 language features involving default arguments and lambdas in Kotlin:

  1. When using default arguments, the trailing default parameters can be omitted.

  2. In Kotlin, there is a convention: if the last parameter of a function is a function, then a lambda expression passed as the corresponding argument can be placed outside the parentheses:
    …
    Such syntax is also known as trailing lambda.
    If the lambda is the only argument to that call, the parentheses can be omitted entirely:
    …

This can cause a pitfall when you use multiple default arguments of the same function type and pass one lambda: if you put the lambda inside the parentheses, the following lambdas are omitted; if you put it outside, the preceding ones are omitted. So syntactically, the omission of parentheses is not omission anymore but changes the semantics. See the code:

fun runBlocks(block1: () -> Unit = { println("default 1") }, block2: () -> Unit = { println("default 2") }) {
    block1()
    block2()
}

fun main() {
    runBlocks({ println("param") })
    println()
    runBlocks { println("param") }
}

Output:

param
default 2

default 1
param

I wonder how many people come across using these 2 features together in a function like this and make a mistake of omitting parentheses. For such cases, IntelliJ IDEA doesn’t show the “Move lambda argument out of parentheses” tip and action, but doesn’t warn about such uses either. I think this can sometimes be quite misleading for people reading the code.

1 Like
2

Here’s a runnable example:

// Warn author about API confusion
fun runBlocks(block1: () -> Unit = { println("default 1") }, block2: () -> Unit = { println("default 2") }) {
    block1()
    block2()
}

fun main() {
    runBlocks({ println("param") }) // Warn caller to use named param
    println()
    runBlocks { println("param") }
}

Interesting.

The first thing off the top of my head is to warn the caller when using parentheses and recommend they used a named param.
Another angle is to warn the author about their API–which might be incorrect since I can’t say an API with default and trailing lambdas is something worth warning against.

2 Likes
3

Maybe the solution is to disable the syntax without the parentheses if a function takes multiple lambda arguments. Sure that would be a breaking change, but a small one. This could be introduced in a 1.x release with a simple tool to update existing code.
I also like the idea of maybe adding an intellij intrinsic that warns to use named params if there are multiple lambda parameters. Not sure I want to see it as an actual compiler warning, but that’s debatable. I’d be fine with a compiler warning if KT-8087(disable specific warnings via compiler argument) gets implemented.

3 Likes
4

If such a breaking change were to be implemented, the change would likely also break functional interfaces since SAM conversions could lead to the same confusion.

More use-cases should probably help–a part of me thinks taking away trailing lambdas in any capacity is too much break from the norm. Since both locations are valid, I’d prefer a solution that makes it more obvious which location you’ve chosen in your call instead of a hard break.

This issue reminds me of the confusion using nested this and it since they are also clarity of usage issues. Those issues are solved with inspections so an inspection for lambda location would fit nicely. Those inspections recommend using explicit naming for this and it so a recommendation for using named params for the lambda argument would match the pattern.
Good point on about the difference for a full compiler-warning.

@ShreckYe, what was your use-case that brought up the issue? Was there any specific library?

3 Likes
6

Yeah I was developing a small desktop app with TornadoFX. And in this library there are a lot of functions with both default arguments and trailing lambdas. I don’t exactly remember whether there is one with multiple trailing default lambdas in the library, but I defined one myself and this led me into thinking about this question:

fun showSaveWarningIfEdited(continuation: () -> Unit = {}, cancelContinuation: () -> Unit = {}) {
    val bytes = getContentBytes()
    if (!(bytes contentEquals (savedContentBytesProperty.get() ?: ByteArray(0))))
        currentWindowAlert(
            Alert.AlertType.WARNING, "The file has been edited. Save it?",
            buttons = arrayOf(ButtonType.YES, ButtonType.NO, ButtonType.CANCEL)
        ) {
            when (it) {
                ButtonType.YES -> {
                    save(bytes)
                    continuation()
                }
                ButtonType.NO -> continuation()
                ButtonType.CANCEL -> cancelContinuation()
            }
        }
    else continuation()
}
7

Just a small nitpick, if you are dealing with any kind of callback or continuation interface, it’s probably a good idea to just use coroutines instead.

2 Likes
9

Yeah that’s a good idea. But TornadoFX is based on JavaFX which doesn’t natively support coroutines, this is the only function that needs callbacks so I didn’t bother to write all the boilerplate code for that.

1 Like
10

You can use the kotlinx.coroutines JavaFx dispatcher to add coroutines. It’s actually pretty easy, if you’re interested in doing that you can reference my GitLab time tracking app that I built for my work which uses TornadoFX with coroutines: GitHub - emanguy/GitlabTimeTracker: Allows you to record time on GitLab issues with less effort.

3 Likes