Unnecessary line return inserted in expression

[romainguy]

The following code:

private inline fun createLinks(node: Long, previous: Int, next: Int, mapping: IntArray): Long {
    return (node and NodeMetaMask) or
        (if (previous == NodeInvalidLink) NodeInvalidLink else mapping[previous]).toLong() shl 31 or
        (if (next == NodeInvalidLink) NodeInvalidLink else mapping[next]).toLong()
}

gets formatted as:

private inline fun createLinks(node: Long, previous: Int, next: Int, mapping: IntArray): Long {
    return (node and NodeMetaMask) or
        (if (previous == NodeInvalidLink) NodeInvalidLink else mapping[previous]).toLong() shl
        31 or
        (if (next == NodeInvalidLink) NodeInvalidLink else mapping[next]).toLong()
}

Note the extra line break before 31 or even though the original version fits within 100 columns. If ktfmt must break, it should at least break before or to not split the left shift expression in two.

[davidtorosyan]

Thanks for creating this issue, and sorry for the delay in getting to it.

even though the original version fits within 100 columns.

Note that this doesn't really matter, since it's a multi-line statement. That is, the overall statement is really 205 characters long, we're just reflowing it according to how we think it should be broken down.

If ktfmt must break, it should at least break before or to not split the left shift expression in two.

I need to experiment to figure out exactly what is being broken here in an unexpected way. In general though there's a bunch of competing priorities (operations, parenthesis) so I can't tell if it's being inconsistent in this particular case.

[davidtorosyan]

I took your example and put it in the online formatter with a line length of 50.

Here's what I got:

fun example1(): Long {
    return someApple or
        someBanana shl
        31 or
        someCitrus
}

This has the same shape as your code. We could do what you suggest and move the break so that we have shl 31 on the same line, but notice what happens when I replace 31 with a variable:

fun example2(): Long {
    return someApple or
        someBanana shl
        someNumber or
        someCitrus
}

This better demonstrates the logic. We're simply breaking at each binary operator, and shl doesn't have higher priority than or.

Now if you did want to raise the priority you could wrap it in parenthesis:

fun example3(): Long {
    return someApple or
        (someBanana shl 31) or
        someCitrus
}

Unfortunately this doesn't work for your example as it would push it over being 100 characters. You could do something like this though (at this point, I'm just having some fun):

private inline fun createLinks(node: Long, previous: Int, next: Int, mapping: IntArray): Long {
    return (node and NodeMetaMask) or
        ((previous.letValid { mapping[it] }).toLong() shl 31) or
        (next.letValid { mapping[next] }).toLong()
}

fun Int.letValid(action: (Int) -> Int) {
    if (this == NodeInvalidLink) {
        return this
    }
    return action(this)
}

Closing as by design. Let me know if you still have concerns!