odd formatting for lambdas

[scottbessler]

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))));

[kevinb9n]

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))));

[cpovirk]

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))));

[kevinb9n]

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.

[cushon]

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()
            ...

[kevinb9n]

The decision might not have been officially made, but it has been now. Keep -> { together, but otherwise break after ->.

[GuiSim]

Has the code been modified to reflect this decision?

[cushon]

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.

[jbduncan]

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?

[basil]

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.

[jbduncan]

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...

[alexkleiman]

@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.

[ilya40umov]

+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);
                      });
            });

[wangxufire]

+1 too many break line

[stephenh]

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?

[dmvk]

Hello, is there any update on this issue?

[jkaldon]

Does anyone have any updates on this bug?

[theqp]

@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.

[basil]

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.

[linusjf]

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.

[linusjf]

Checkstyle indentation rule.

[linusjf]

For some reason, the xml snippet is not accepted by this comment box. Kindly checkout the Module Indentation in csrules.xml in root directory.

[linusjf]

Would you prefer I list the above as a new issue?

[linusjf]

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?

[sormuras]

@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.

[linusjf]

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.

[tbroyer]

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.

[linusjf]

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.

[linusjf]

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

[tbroyer]

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

[linusjf]

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.

[linusjf]

You can check out this clang-format file and let me know if it matches the Google Java Formatter.

It's modified since the default config.

Regards, Linus.

[linusjf]

You might want to change the property AlignAfterOpenBracket to the original Align.

[jbduncan]

Hi @Fernal73 - which clang-format file are you referring to? :)

[linusjf]

This one:

Language:        Java
# BasedOnStyle:  Google
AccessModifierOffset: -1
AlignAfterOpenBracket: DontAlign
AlignConsecutiveAssignments: false
AlignConsecutiveDeclarations: false
AlignEscapedNewlines: Left
AlignOperands:   true
AlignTrailingComments: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: false
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: false
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: true
AlwaysBreakTemplateDeclarations: Yes
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
  AfterClass:      false
  AfterControlStatement: false
  AfterEnum:       false
  AfterFunction:   false
  AfterNamespace:  false
  AfterObjCDeclaration: false
  AfterStruct:     false
  AfterUnion:      false
  AfterExternBlock: false
  BeforeCatch:     false
  BeforeElse:      false
  IndentBraces:    false
  SplitEmptyFunction: true
  SplitEmptyRecord: true
  SplitEmptyNamespace: true
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeBraces: Attach
BreakBeforeInheritanceComma: false
BreakInheritanceList: BeforeColon
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializers: BeforeColon
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: true
ColumnLimit:     80
CommentPragmas:  '^ IWYU pragma:'
CompactNamespaces: false
ConstructorInitializerAllOnOneLineOrOnePerLine: true
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DerivePointerAlignment: true
DisableFormat:   false
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: true
ForEachMacros:
  - foreach
  - Q_FOREACH
  - BOOST_FOREACH
IncludeBlocks:   Preserve
IncludeCategories:
  - Regex:           '^<ext/.*\.h>'
    Priority:        2
  - Regex:           '^<.*\.h>'
    Priority:        1
  - Regex:           '^<.*'
    Priority:        2
  - Regex:           '.*'
    Priority:        3
IncludeIsMainRegex: '([-_](test|unittest))?$'
IndentCaseLabels: true
IndentPPDirectives: None
IndentWidth:     2
IndentWrappedFunctionNames: false
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLinesAtTheStartOfBlocks: false
MacroBlockBegin: ''
MacroBlockEnd:   ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBinPackProtocolList: Never
ObjCBlockIndentWidth: 2
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PenaltyBreakAssignment: 2
PenaltyBreakBeforeFirstCallParameter: 1
PenaltyBreakComment: 300
PenaltyBreakFirstLessLess: 120
PenaltyBreakString: 1000
PenaltyBreakTemplateDeclaration: 10
PenaltyExcessCharacter: 1000000
PenaltyReturnTypeOnItsOwnLine: 200
PointerAlignment: Left
RawStringFormats:
  - Language:        Cpp
    Delimiters:
      - cc
      - CC
      - cpp
      - Cpp
      - CPP
      - 'c++'
      - 'C++'
    CanonicalDelimiter: ''
    BasedOnStyle:    google
  - Language:        TextProto
    Delimiters:
      - pb
      - PB
      - proto
      - PROTO
    EnclosingFunctions:
      - EqualsProto
      - EquivToProto
      - PARSE_PARTIAL_TEXT_PROTO
      - PARSE_TEST_PROTO
      - PARSE_TEXT_PROTO
      - ParseTextOrDie
      - ParseTextProtoOrDie
    CanonicalDelimiter: ''
    BasedOnStyle:    google
ReflowComments:  true
SortIncludes:    true
SortUsingDeclarations: true
SpaceAfterCStyleCast: false
SpaceAfterTemplateKeyword: true
SpaceBeforeAssignmentOperators: true
SpaceBeforeCpp11BracedList: false
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: true
SpaceBeforeParens: ControlStatements
SpaceBeforeRangeBasedForLoopColon: true
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 2
SpacesInAngles:  false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard:        Auto
StatementMacros:
  - Q_UNUSED
  - QT_REQUIRE_VERSION
TabWidth:        2
UseTab:          Never

[linusjf]

You might want to set AllowShortLambdasOnASingleLine to false if you're able to access version 9 of clang-format.

[MrIvanPlays]

Any progress on this?

[linusjf]

What do you mean? No, I don't have access to clang-9 on termux, as yet.

The only workaround for now is to manually format the code as expected and wrap it in // clang-format off and // clang-format on markers. That works if you're using clang-format. It will not prevent Google-Java-format from reformatting your code if it deems it necessary.

[linusjf]

My usecase is to use Google-Java-format and clang-format in that order. If I don't like what clang-format does, I comment-block that section. Google-Java-format does a slightly better job, overall, but then I haven't access to clang-format 9.

[mpern]

Since almost a year passed after the last comment and there was a release of GJF recently (so at least the project isn't dead):

Is there any progress on improving the formatting of lambdas by GJF?

[allensuner]

I would like to echo the above comment I really don't feel like adding a line break or new line solely for the purpose of using a language feature is a good workaround.

[mpern]

If you want to have a solution now:

Get rid of GJF and use spotless to run the Eclipse JDT formatter.

[kevinb9n]

Wow. I apologize for not having given any update on this issue in so long (and while so much discussion happened). As I suppose is obvious, we haven't been paying attention in an "organized/committed" way but have just watched for low-hanging fruit.

This one seems to be... high-hanging fruit:

• The formatter is built on the principle of the "rectangle rule" and that's the source of most of what is good about it
• The best behavior would be if it can recognize when exceptions to that rule are justified, and back off in those cases
• This is sure as heck one of those cases
• Yet it has proven surprisingly difficult to figure out how to make it do that without undue collateral damage
• It seems that we (Google) don't necessarily have enough motivation to work on figuring this out. It's less nasty for us internally because of our 2-space indents and because we generally prefer to refactor nested lambdas into private methods (used via method reference).