odd formatting for lambdas #19

scottbessler · 2015-12-17T03:33:17Z

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 · 2015-12-17T08:08:06Z

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 · 2015-12-17T15:22:43Z

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 · 2015-12-17T15:34:37Z

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 · 2015-12-17T17:51:43Z

The formatter started always breaking after -> in b8e6744. 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 · 2015-12-17T19:15:20Z

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

GuiSim · 2016-03-11T16:12:08Z

Has the code been modified to reflect this decision?

cushon · 2016-03-11T17:21:01Z

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 · 2016-08-17T16:03:33Z

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 00530c0), 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 · 2016-12-23T01:57:35Z

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 · 2017-02-10T01:53:33Z

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 · 2017-02-10T16:07:40Z

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

google#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 going in 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.

ilya40umov · 2017-07-10T14:38:44Z

+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 · 2017-10-19T09:08:33Z

+1 too many break line

stephenh · 2017-11-18T20:02:03Z

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 · 2018-03-22T09:55:45Z

Hello, is there any update on this issue?

jkaldon · 2018-05-16T16:30:07Z

Does anyone have any updates on this bug?

theqp · 2019-01-04T03:00:30Z

@jkaldon @dmvk @stephenh @wangxufire
I have forked #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
#163 (comment)
However the cases he mentioned were really unreal and far less annoying than the current situation.

basil · 2019-05-22T17:20:19Z

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 · 2019-06-16T10:38:48Z

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 · 2019-06-16T10:39:54Z

Checkstyle indentation rule.

linusjf · 2019-06-16T10:41:47Z

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 · 2019-06-16T14:39:10Z

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

linusjf · 2019-06-16T15:58:30Z

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 · 2019-06-16T16:05:46Z

@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 · 2019-06-16T16:26:19Z

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 · 2019-06-16T16:42:19Z

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 · 2019-06-16T16:44:29Z

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 · 2019-06-16T16:51:28Z

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 · 2019-06-16T17:07:32Z

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 · 2019-06-16T17:29:35Z

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 · 2019-06-18T07:26:42Z

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 · 2019-06-18T08:17:08Z

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

jbduncan · 2019-06-18T08:51:16Z

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

linusjf · 2019-06-18T10:30:44Z

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 · 2019-06-19T01:09:09Z

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

MrIvanPlays · 2019-09-07T16:13:39Z

Any progress on this?

linusjf · 2019-09-07T18:44:48Z

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 · 2019-09-08T06:05:14Z

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 · 2020-07-07T08:36:56Z

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 · 2021-04-20T00:19:51Z

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 · 2021-04-21T12:20:02Z

If you want to have a solution now:

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

kevinb9n · 2021-08-23T17:11:47Z

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

alexkleiman mentioned this issue Sep 1, 2016

AOSP style should have an option for two space continuations #73

Closed

sormuras mentioned this issue Mar 11, 2017

Formatting stream API methods on multiple lines. diffplug/spotless#87

Closed

novalis mentioned this issue Jun 8, 2017

Improve lambda formatting #163

Open

aionick mentioned this issue May 23, 2018

Add getNrgPrice Java API call aionnetwork/aion#500

Merged

10 tasks

tbroyer mentioned this issue Jun 20, 2019

Inconsistent formatting between eclipse and IntelliJ #381

Closed

tbroyer mentioned this issue Jul 15, 2020

Indenting too much #506

Closed

JavierSegoviaCordoba mentioned this issue Aug 24, 2021

Wrong indentation in a lambda facebook/ktfmt#177

Closed

agrieve mentioned this issue Sep 14, 2023

Avoid excessive newlines for short variable names with --aosp #970

Open

cushon mentioned this issue Dec 11, 2023

Java nested lambdas formatting is incredibly poor #990

Closed

john-h-kastner-aws mentioned this issue Feb 20, 2024

Add code format checks to Java CI cedar-policy/cedar-java#93

Open

2 tasks

odd formatting for lambdas #19

odd formatting for lambdas #19

Comments

scottbessler commented Dec 17, 2015 • edited by cushon

kevinb9n commented Dec 17, 2015

cpovirk commented Dec 17, 2015

kevinb9n commented Dec 17, 2015

cushon commented Dec 17, 2015

kevinb9n commented Dec 17, 2015

GuiSim commented Mar 11, 2016

cushon commented Mar 11, 2016

jbduncan commented Aug 17, 2016

basil commented Dec 23, 2016

jbduncan commented Feb 10, 2017

alexkleiman commented Feb 10, 2017

ilya40umov commented Jul 10, 2017 • edited by cushon

wangxufire commented Oct 19, 2017

stephenh commented Nov 18, 2017

dmvk commented Mar 22, 2018

jkaldon commented May 16, 2018

theqp commented Jan 4, 2019

basil commented May 22, 2019

linusjf commented Jun 16, 2019

linusjf commented Jun 16, 2019

linusjf commented Jun 16, 2019

linusjf commented Jun 16, 2019

linusjf commented Jun 16, 2019

sormuras commented Jun 16, 2019 • edited

linusjf commented Jun 16, 2019 via email • edited

tbroyer commented Jun 16, 2019

linusjf commented Jun 16, 2019 via email • edited

linusjf commented Jun 16, 2019 via email • edited

tbroyer commented Jun 16, 2019

linusjf commented Jun 16, 2019 via email • edited

linusjf commented Jun 18, 2019 via email • edited

linusjf commented Jun 18, 2019 via email • edited

jbduncan commented Jun 18, 2019

linusjf commented Jun 18, 2019 via email • edited

linusjf commented Jun 19, 2019 via email • edited

MrIvanPlays commented Sep 7, 2019

linusjf commented Sep 7, 2019 • edited

linusjf commented Sep 8, 2019

mpern commented Jul 7, 2020

allensuner commented Apr 20, 2021

mpern commented Apr 21, 2021

kevinb9n commented Aug 23, 2021 • edited

scottbessler commented Dec 17, 2015 •

edited by cushon

ilya40umov commented Jul 10, 2017 •

edited by cushon

sormuras commented Jun 16, 2019 •

edited

linusjf commented Jun 16, 2019 via email •

edited

linusjf commented Jun 16, 2019 via email •

edited

linusjf commented Jun 16, 2019 via email •

edited

linusjf commented Jun 16, 2019 via email •

edited

linusjf commented Jun 18, 2019 via email •

edited

linusjf commented Jun 18, 2019 via email •

edited

linusjf commented Jun 18, 2019 via email •

edited

linusjf commented Jun 19, 2019 via email •

edited

linusjf commented Sep 7, 2019 •

edited

kevinb9n commented Aug 23, 2021 •

edited