Improve lambda formatting

[novalis]

https://github.com/google/google-java-format/issues/19

The style guide says: "A line is never broken adjacent to the arrow in a lambda, except that a break may come immediately after the arrow if the body of the lambda consists of a single unbraced expression."

There are two changes here:

1. Don't put a newline after the arrow.
2. When the only argument to a function is a lambda, don't put a newline after the open-paren of the function. I think this newline was getting added because a lambda is a single expression that is longer than (the remainder of) a line. But generally, it's prettier to break inside the lambda.

[cushon]

This change causes the formatter to never break in these places, even if breaks are necessary to prevent >100 character lines, e.g.:

class T {
  {
    ffffffffffffffffffffffffffffffffffffffffffffffffff(
        xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx -> {});
  }
}

[novalis]

Well, I can definitely see why this issue has been open for a while :). Will try again.

[novalis]

OK, so I hacked together something that works for @cushon's f...(x -> ...) case. But I'm deeply unhappy with it, because (a) it doesn't yet work if you put another pair of parens around the lambda, although I think I can fix that with yet more code, and (b) more importantly, because it doesn't look like any of the other code in the formatter.

My idea was: if you've got a lambda that's an argument to a function, figure out how much space you have on the right to decide whether to break. To do this, I look back in the positionTokenMap until I find the open-paren for the function, and check its column in the positionToColumn map. Then I look at the formatted length of the argument to the lambda, and compare all that to 100; if it's greater, then we add a breakToFill(). Else, we don't.

This does preserve my result for all of the test cases that I modified, as well as a few that I added for @cushon's case and a few related cases (except, as noted above, the case where the lambda is for some reason wrapped in parentheses).

One interesting example is

    ffffffffffffffffffffffffffffffffffffffffffffffffff(
        (morxxxxxxxxxxxxxxxx, fleeeeeeeeeeeeeeem) -> {});

There is enough horizontal space to put the first arg to the lamba on the line with the enclosing function call, but that would be much uglier wrapping before the args. (Notes that this problem comes up regardless of whether we take @jbduncan's suggestion about wrapping after arrows).

Anyway, my hack is probably not how this code is intended to work. What I imagine is something higher-level, like attaching a "priority" to each break, where the higher-priority breaks are taken before the lower-priority ones. So in f((a, b) -> c.d) the break after f( would be higher-priority than the break after the comma, and the break after the dot would be even higher-priority. If the break after the dot doesn't fix the problem, we add one after f(, and then finally we break the arg list. But this seems like a lot of general-purpose machinery for a rare edge case (and I don't know what it would do to the rest of the formatting code).

So I wonder if it is better or just do the hack?

[MrIvanPlays]

Any progress on this?

[novalis]

Not on my end -- it seems really hard to do right, and I haven't had time to look at it lately. Sorry.

[cushon]

What I imagine is something higher-level, like attaching a "priority" to each break, where the higher-priority breaks are taken before the lower-priority ones

This is roughly what clang-format does, e.g.: https://youtu.be/s7JmdCfI__c?t=649

That approach has merit, and it was considered when we settled on the break-at-the-highest-syntactic-level approach used by google-java-format. I don't see a good way to adopt pieces of the clang-format approach to make a targeted change to lambda formatting. Switching the entire formatting algorithm to something closer to clang-format would be an invasive change, and come with trade-offs.