google-java-format
google-java-format copied to clipboard
google
odd formatting for lambdas
are the extra newlines intentional?
before:
Stream<ItemKey> itemIdsStream = stream(members)
.flatMap(m -> m.getFieldValues()
.entrySet()
.stream()
.filter(fv -> itemLinkFieldIds.contains(fv.getKey()))
.flatMap(fv -> FieldDTO.deserializeStringToListOfStrings(fv.getValue())
.stream()
.map(id -> new ItemKey(fieldsById.get(fv.getKey()).getItemTypeId(), id))));
or even
Stream<ItemKey> itemIdsStream =
stream(members)
.flatMap(
m -> m.getFieldValues()
.entrySet()
.stream()
.filter(fv -> itemLinkFieldIds.contains(fv.getKey()))
.flatMap(
fv -> FieldDTO.deserializeStringToListOfStrings(fv.getValue())
.stream()
.map(
id -> new ItemKey(
fieldsById.get(fv.getKey()).getItemTypeId(), id))));
after:
// items
Stream<ItemKey> itemIdsStream =
stream(members)
.flatMap(
m ->
m.getFieldValues().entrySet().stream()
.filter(fv -> itemLinkFieldIds.contains(fv.getKey()))
.flatMap(
fv ->
FieldDTO.deserializeStringToListOfStrings(fv.getValue()).stream()
.map(
id ->
new ItemKey(
fieldsById.get(fv.getKey()).getItemTypeId(), id))));
I don't think the problem is lambda-specific, but this lambda-using code is doing a really good job of tripping up on the general issue. If we can solve that issue, ideally this code ought to be able to show up as
Stream<ItemKey> itemIdsStream =
stream(members)
.flatMap(m ->
m.getFieldValues()
.entrySet()
.stream()
.filter(fv -> itemLinkFieldIds.contains(fv.getKey()))
.flatMap(fv ->
FieldDTO.deserializeStringToListOfStrings(fv.getValue())
.stream()
.map(id -> new ItemKey(fieldsById.get(fv.getKey()).getItemTypeId(), id))));
In addition to the general problem, do we also have something lambda-specific? Your example has the line break after ->, but the formatter apparently puts it before. Should that change?
A result of the current behavior is that any chained calls or arguments to the next expression are indented 1 space from the expression, rather than 4. This is probably to spec, and IIRC it also comes up with the ternary operator, but it looks even weirder here because it's 1 space, rather than the 3 we see with the ternary operator. I'm talking about lines like this one...
-> m.getFieldValues()
.entrySet()
...(which kind of works out because m happens to be one letter, perhaps a common case for lambda variables) and this one (which is hard to love)...
-> new ItemKey(
fieldsById.get(fv.getKey()).getItemTypeId(), id))));
Yes, the issue of whether to break before or after -> is on our list of style guide issues to resolve before we move to Java 8.
The formatter started always breaking after -> in b8e67444bf432d070c2c4da4de1207946052e8a0. I thought that decision had been made already, but I think I misread @kevinb9n's comment in #2.
Anyway, the current behaviour is:
x ->
y()
x -> {
y();
}
Instead of:
x
-> y()
x
-> {
y();
}
I like the consistency between expression and statement lambdas, and it seems like the only way to support the suggested formatting of this example:
Stream<ItemKey> itemIdsStream =
stream(members)
.flatMap(m ->
m.getFieldValues()
...
The decision might not have been officially made, but it has been now. Keep -> { together, but otherwise break after ->.
It has been modified to break after -> and -> {. It hasn't been modified to keep e.g. m -> on the previous line (we haven't decided not to do that, it just hasn't happened).
There should be another release soon, or you can build it at head to see the current behaviour.
Hmm, I have some code which, before I tried applying google-java-format to it, I half-expected to end up looking like this:
FooTraverser.of(fooGraph).preOrderTraversal(root).forEach(e ->
fooGraph.getSuccessors(e).forEach(child ->
treeCopy.addChild(graph.findEdge(e, child), e, child)));
but when I actually did format it (using the version built from https://github.com/google/google-java-format/commit/00530c0b5ac7cb119222af430b29bae1e04e68c7), it looked like this:
FooTraverser.of(fooGraph)
.preOrderTraversal(root)
.forEach(
e ->
fooGraph
.getSuccessors(e)
.forEach(
child ->
treeCopy.addChild(graph.findEdge(e, child), e, child)));
Is the current behaviour intentional?
It hasn't been modified to keep e.g. m -> on the previous line (we haven't decided not to do that, it just hasn't happened).
Has a decision been made about this issue? It's really annoying that the lambda parameter and the arrow token always begin a new line. For example, consider how google-java-format indents this example:
import java.util.Arrays;
import java.util.stream.Stream;
class LambdaExample {
public static void main(String[] args) {
String[] stringArr = {"a", "b", "c", "d"};
Stream<String> stream = Arrays.stream(stringArr);
stream.forEach(
letter -> {
System.out.print(letter);
});
}
}
This would look much nicer as follows:
import java.util.Arrays;
import java.util.stream.Stream;
class LambdaExample {
public static void main(String[] args) {
String[] stringArr = {"a", "b", "c", "d"};
Stream<String> stream = Arrays.stream(stringArr);
stream.forEach(letter -> {
System.out.print(letter);
});
}
}
This problem is exacerbated in AOSP mode. Here's how google-java-format indents the same example when in AOSP mode:
import java.util.Arrays;
import java.util.stream.Stream;
class LambdaExample {
public static void main(String[] args) {
String[] stringArr = {"a", "b", "c", "d"};
Stream<String> stream = Arrays.stream(stringArr);
stream.forEach(
letter -> {
System.out.print(letter);
});
}
}
Lambdas in AOSP mode are indented 12 spaces! This creates a huge amount of unnecessary whitespace and inhibits readability, especially compared to using a traditional for loop. I want to encourage people to use lambdas in our codebase, but it's difficult to do that when using them triples the indentation level and makes the code less readable. In contrast, it looks much nicer if the lambda parameter and arrow token are kept on the preceding line:
import java.util.Arrays;
import java.util.stream.Stream;
class LambdaExample {
public static void main(String[] args) {
String[] stringArr = {"a", "b", "c", "d"};
Stream<String> stream = Arrays.stream(stringArr);
stream.forEach(letter -> {
System.out.print(letter);
});
}
}
Notice how in the above, the body of the lambda is indented only four spaces from the forEach, the same amount as it would have been in a traditional for loop.
Hi @basil, I think you can workaround this nasty lambda indentation behaviour for a good number of cases in the meantime, by using method references[1] whenever possible.
If I were to use the example you gave above as a template, then here's what I'd turn it into:
import java.util.Arrays;
import java.util.stream.Stream;
class MethodReferenceExample {
public static void main(String[] args) {
String[] stringArr = {"a", "b", "c", "d"};
Stream<String> stream = Arrays.stream(stringArr);
stream.forEach(System.out::print);
}
}
[1] Or refactoring the lambda expression into a private method, and calling it from a method reference or a new lambda...
@jbduncan that may work in some individual cases, but unfortunately, it is not feasible to apply this workaround when reformatting a codebase which already makes heavy use of lambdas. That being said, it is certainly a useful workaround to apply on a case-by-case basis until this bug is resolved.
+1 Formatting of lambdas in the current version (1.3) is quite annoying (gets unreadable quite easily). e.g.
assertThat(response.getBody())
.satisfies(
body -> {
assertThat(body.data())
.hasValueSatisfying(
place -> {
assertThat(place.getId()).isEqualTo(253955);
});
});
Not a huge fan of "+1" comments, but I'll commit the faux pax anyway to note that I'm surprised the "new line solely for lambda variable declaration" is not seen as a bigger deal/fixed by now.
Curious why this is still unresolved? E.g. it is technically challenging? Or do the contributors think the current behavior is actually preferred? If so, maybe just decide so and close the issue?
@jkaldon @dmvk @stephenh @wangxufire I have forked https://github.com/google/google-java-format/pull/163 and merged master into it. https://github.com/theqp/google-java-format
Examples how it handles lambdas currently: https://github.com/theqp/google-java-format/blob/master/core/src/test/resources/com/google/googlejavaformat/java/testdata/B20128760.output https://github.com/theqp/google-java-format/blob/master/core/src/test/resources/com/google/googlejavaformat/java/testdata/B21305044.output https://github.com/theqp/google-java-format/blob/459401334186a6a5cf9d298f456f4ddbbdc5be41/core/src/test/resources/com/google/googlejavaformat/java/testdata/B22873322.output https://github.com/theqp/google-java-format/blob/459401334186a6a5cf9d298f456f4ddbbdc5be41/core/src/test/resources/com/google/googlejavaformat/java/testdata/B33358723.output
Note that it is not perfect as mentioned by the original author at https://github.com/google/google-java-format/pull/163#issuecomment-307542440 However the cases he mentioned were really unreal and far less annoying than the current situation.
It hasn't been modified to keep e.g.
m ->on the previous line (we haven't decided not to do that, it just hasn't happened).
@cushon @ronshapiro I am posting to beg for the elimination of mandatory line breaks before the lambda parameter and arrow token. This is needed to avoid unreadable code. Please also see this explanation.
I have set up indentation as the above in my checkstyle configuration. The issue I have with Google Java format is that it auto-indents lambda expressions by 4 spaces on the next line thus breaking the above rule. Every time I run the formatter it reformats the lambda expressions by an additional 2 spaces.
My source code is hosted at https://github.com/Fernal73/LearnJava and the project you'd want to look at is Switch under the Switch directory. The checkstyle rules are listed in csrules.xml under the root directory. Run the build.xml under the root directory to generate the classpath for Google java format and then either run the format and checkstyle scripts with the project name as parameter or run the build.xml under the Project Switch. The ant tasks are checkstyle and format respectively. The default build runs the formatter and then checkstyle.
For some reason, the xml snippet is not accepted by this comment box. Kindly checkout the Module Indentation in csrules.xml in root directory.
I'm wondering if this is a Google java format error. Line wrapping from the formatter is always four spaces. If I set the forceStrictCindition property in the checkstyle to true, then it flags every other line wap indentation as well as formatted by Google Java Formatter. One workaround would be to set that value to 4. I'm not happy with that. The question is why is the strict check enforced for lambda expressions by Checkstyle when it's set to false?
@Fernal73 Using google-java-format means that you don't need a style checking tool at all. Just format the code with google-java-format and that's it. By definition, all .java files touched by google-java-format are formatted "in style".
When running google-java-format on the command line, you may pass --dry-run --set-exit-if-changed to check the format of .java files. A zero exit codes means, all touched files are "in good style", a non-zero value means some format issues were detected. IIRC, a list of bad formatted files is printed on the console as well.
Thanks, Christian.
Your suggestion makes eminent sense especially about the indentation rule. I use Checkstyle and PMD for more than just 'style', though.
Regards, Linus.
Then disable the style-related checks in Checkstyle to prevent any "conflict" between the two tools, and only keep the non-style-related checks that you know won't conflict.
I understand that Google-java-format is not configurable. Deliberately so.
However, I was wondering if it was possible for it to not disturb existing formatting if it found it well within certain guidelines.
That's probably out of the question, though.
Yes, it does appear that the lambda not being complied with is a well-known bug with checkstyle. I've disabled the indentation module in checkstyle and everything's fine once more!
Thanks
However, I was wondering if it was possible for it to not disturb existing formatting if it found it well within certain guidelines.
See https://github.com/google/google-java-format/wiki/FAQ#why-didnt-it-leave-my-perfectly-valid-code-alone
Thanks. I'm aware of that. In other words, if developers wish to tweak the Google Java Formatter, they would be better off using other formatters that allow them to configure the default Google style. That's fair enough.