Unreachable "return" in try/finally block

Support
1

I’m using Kotlin 1.2 targetting the JVM. In the function

fun splitVcsPathFromUrl(vcsUrl: String): Pair<String, String> {
    try {
        val uri = URI(vcsUrl)

        if (uri.host.endsWith("github.com")) {
            val split = vcsUrl.split("/blob/", "/tree/")
            if (split.size == 2) {
                val repo = split.first() + ".git"

                // Remove the blob / tree committish (e.g. a branch name) and any ".git" suffix.
                val path = split.last().substringAfter("/").substringBeforeLast(".git")

                return Pair(repo, path)
            }
        }
    } finally {
        // Fall back to returning just the original URL.
        return Pair(vcsUrl, "")
    }
}

the line return Pair(repo, path) is marked unreachable code. That seems to be because the compiler always runs the code in finally, even if it came across that return Pair(repo, path) statement before. Also changing that line to return@splitVcsPathFromUrl Pair(repo, path) does not help.

As that behavior seems a bit counter-intuitive, is that by design?

2

It looks like you make a wrong assumption about how finally works. You document it with fall back, as if the finally block will only be run if the try block does not return from the function. But this is not the case. finally always (unless the thread is killed hard) runs.

Loose the try statement, and simply move the return from the finally block to below the outer if.

3

But this is not the case. finally always (unless the thread is killed hard) runs.

I indeed wasn’t aware of that, also not in Java times. It’s weird that it even “works” to have two return statements in the code path in this case, but only that last one counts.

1 Like
4

They, that’s why most Java code style guidelines say that you shouldn’t return out of finally blocks. I believe IDEA even has an inspection for that.

5

Indeed, it probably shouldn’t work (from an ideal language design perspective). Bit more detail here.

You may instead want to do something like this:

fun splitStuff(url: String): Pair<String, String> =
  try {
    // blah blah
    Pair(repo, path)
  } catch (e: Exception) {
    e.printStacktrace(); // should do SOME sort of logging,
                         // or even throw and let the caller deal with it
    Pair(vcsUrl, "")
  }

Also, since we’re here, I’d recommend returning something other than a pair, either a data class or a sealed class with two types.

6

Actually, you should not return out of try blocks if you have finally. I believe that’s the rule applicable here.

At least for Kotlin code I saw none.

7

You example is simpler as it contains no if. My goal was to return the same fall-back value when the if is not taken or an exception is throw, without repeating that fall-back value in an else branch.

True, I used a Pair for simplicity here, my real implementation returns a data class.

9

Your understanding of try-finally is incorrect. The finally block is executed no matter how the try block is exited. Write a few simple tests without return, with return and with an exception being thrown, and you will see how it works.

1 Like