facebook / ktfmt

A program that reformats Kotlin source code to comply with the common community standard for Kotlin code conventions.
https://facebook.github.io/ktfmt/
Apache License 2.0
907 stars 75 forks source link

Weird lambda formatting when wrapping long line #454

Open avi-cenna opened 5 months ago

avi-cenna commented 5 months ago

If I have the following code,

val input = "abc"
val example =
    try {
        input.fakeFunction(a = "aaaaaaaaaaaaaaa", b = "bbbbbbbbbbb", c = "cccccccccccc").map { strng -> strng.replace("a", "b") }
    } catch (error: Exception) {
        println()
    }

ktfmt will wrap it as follows:

val input = "abc"
val example =
    try {
        input.fakeFunction(a = "aaaaaaaaaaaaaaa", b = "bbbbbbbbbbb", c = "cccccccccccc").map { strng
            ->
            strng.replace("a", "b")
        }
    } catch (error: Exception) {
        println()
    }

The fact that it doesn’t simply put the whole .map { ... } on a new line is odd to me. And the -> on its own line is also unusual. This issue can be reproduced in the online ktfmt playground by setting style to kotlinlang and line length to 100.

nreid260 commented 5 months ago

This is the logic for block-like formatting of lambdas. It looks weird in this case because of how long the first function call is. Compare with:

val input = "abc"
val example =
    try {
        input.fakeFunction(short).map { strng ->
            strng.replace("a", "b")
        }
    } catch (error: Exception) {
        println()
    }

We can't really tell how long the previous function call was (i.e. if it needed to break), but we could improve the lambda param possitioning:

val input = "abc"
val example =
    try {
        input.fakeFunction(a = "aaaaaaaaaaaaaaa", b = "bbbbbbbbbbb", c = "cccccccccccc").map {
            strng
            ->
            strng.replace("a", "b")
        }
    } catch (error: Exception) {
        println()
    }
avi-cenna commented 5 months ago

@nreid260 would it be possible to implement something similar to what rustfmt does? E.g. with the following Rust,

fn main() {
    let vec = vec![1, 2, 3, 4, 5];
    let result = vec.iter().map(|x| x + 2 + 1 + 2).map(|x| x + 1 + 3 + 123 + 654654 + 1);
    let result: Vec<i32> = result.collect();
    println!("{:?}", result);
}

It formats it to

fn main() {
    let vec = vec![1, 2, 3, 4, 5];
    let result = vec
        .iter()
        .map(|x| x + 2 + 1 + 2)
        .map(|x| x + 1 + 3 + 123 + 654654 + 1);
    let result: Vec<i32> = result.collect();
    println!("{:?}", result);
}

What if ktfmt is updated to do the following: if there is a chain of function calls which goes over the max line length, then each function call should be put onto a new line. Only after each function call is put onto a new line does it attempt to do further wrapping if necessary.

nreid260 commented 5 months ago

Your description is what ktfmt does most of the time. The behaviour you're seeing was specially added to improve the "feel" of code like:

return foo.bar("something")?.block {
  // Some long lambda
}

I recall there being some common example where preceding method calls were important, but perhaps the block-like behaviour could be limited to free calls and calls on simple names.