Change "qualified Invocations" wrapping option to improve fluent API formatting

[redhead]

I was surprised that custom wrapped code in a long method call chains is reformatted back to a single line. Affects stream() or any builder methods. For example:

    return GraphQLArgument.newArgument()
            .name(argName)
            .type(argType)
            .description(description)
            .defaultValue(argDefaultValue)
            .build();

Becomes:

	return GraphQLArgument.newArgument().name(argName).type(argType).description(description)
			.defaultValue(argDefaultValue).build();

This does not improve readability IMHO.

[wilkinsona]

This is how Eclipse's formatter, upon which Java Format is based, works. If you want to keep the separate lines in certain cases you can disable the formatter.

[gberche-orange]

@wilkinsona with large adoption of builder, fluent apis, including reactor codebase, the proportion of affected code by this limitation increases, and this becomes impracticable to disable formatting for such large proportion of code. This prevents uses of the spring-javaformat in affected projects (e.g. spring-cloud/spring-cloud-app-broker#291 (comment))

Any chance to reopen this issue to discuss how to find solutions ?

Googling "eclipse formatter for fluent apis" leads to the discussions/issues below, could they be applicable and useful to spring-javaformat ?

https://bugs.eclipse.org/bugs/show_bug.cgi?id=547558 Formatter for lamda expression
https://bugs.eclipse.org/bugs/show_bug.cgi?id=489444 [Formatter] Chained method calls when using Stream API or builders should have different behavior from other method calls.
https://stackoverflow.com/questions/4172937/how-to-indent-the-fluent-interface-pattern-correctly-with-eclipse?rq=1

/CC @royclarkson

[wilkinsona]

Thanks for the references. I think they show that there's nothing that we can do here at the moment, particularly https://bugs.eclipse.org/bugs/show_bug.cgi?id=489444 which remains open.

[gberche-orange]

I wonder whether the existing flag line wrapping -> function calls -> qualified invocations was considered as an improvement over the single liner reported in this issue ?

https://bugs.eclipse.org/bugs/show_bug.cgi?id=547558#c2

[redhead]

In the end, we went the way that it's up to the programmer to decide the line breaks in every particular case. Thus, we don't restrict everything to one line nor splitting it per line every time (sometimes some short builder method chain or simple stream pipeline don't need to be wrapped). This, however, makes it relaxed enough to the point programmers are able to do line breaks anywhere they want, for example, in the middle of parameter list. It's not ideal, as we need to keep an eye out in the reviews, but still better than forcing everything on one line.

[JanecekPetr]

Hello, I'm sorry to resurrect this, but I feel even though Mr. Wilkinson is probably aware of what we're talking about, he didn't actually address it explicitly. Eclipse already supports nice formatting of chained method calls, like the stream calls or any fluent calls, via the Qualified Invocations option. How does this not solve the problem?

In Eclipse, this:

results in this code:

return SomeClass.builder()
	.aha(AhaConverter.convert(message))
	.uuid(UUID.randomUUID())
	.timeReceived(ZonedDateTime.now())
	.build();

That's exactly what we want and should also be available programatically. If there's consensus on the change I'll gladly try and help with the code changes and the required communication to downstream projects (might require a major/minor version bump?).

Let's please make the plugin actually useful by providing readable code :(

[philwebb]

I seem to remember the last time I looked at this I wasn't too happy with the results when we applied them to the Spring Boot codebase. I'll reopen the issue to check again, perhaps I was using a different setting.

[Hccake]

May I ask if there is a way to implement the line break of chained calls in the current version? except @Formatter:off

[philwebb]

@Hccake There's no way to do it with the current version.

[Hccake]

@Hccake There's no way to do it with the current version.

It's really sad 😢, when will this feature be implemented?

[philwebb]

@Hccake we don't have a target date, we're currently quite stacked up with other work.

[maximerichrd]

May I again, it's really sad 😢

[SavvyShark]

@wilkinsona ask sincerely, when will implement this feature?

Hello, I'm sorry to resurrect this, but I feel even though Mr. Wilkinson is probably aware of what we're talking about, he didn't actually address it explicitly. Eclipse already supports nice formatting of chained method calls, like the stream calls or any fluent calls, via the Qualified Invocations option. How does this not solve the problem?

In Eclipse, this:

results in this code:

return SomeClass.builder()
	.aha(AhaConverter.convert(message))
	.uuid(UUID.randomUUID())
	.timeReceived(ZonedDateTime.now())
	.build();

That's exactly what we want and should also be available programatically. If there's consensus on the change I'll gladly try and help with the code changes and the required communication to downstream projects (might require a major/minor version bump?).

Let's please make the plugin actually useful by providing readable code :(

[wilkinsona]

There's no target date at the moment. We have lots of other work on our plates at the moment that is higher priority.

[andreasevers]

@philwebb since you've looked into this before and it didn't work out well for the Spring Boot project, would you consider allowing the qualified invocations setting to be configurable in the .springjavaformatconfig file?

If so, I'd be willing to put in a PR based on your preferred approach.

[philwebb]

@andreasevers I just tried again using a branch with this change. The resulted in the following reformatting.

I'd like to chat with the Boot team to see what we prefer.

[philwebb]

Here's some more tweaks and the result.

[andreasevers]

Wow, this is exactly what we were looking for. As I understand the impact will be huge on the Spring Boot projects, and a release of this change is likely to take some consideration, I'll fork the repo and release my own version internally from your branch to unblock us. If there is anything I can do to help out anywhere let me know.

[wilkinsona]

I like the result of the further tweaks quite a lot. While it increases the line count, I think it makes the code easier to read. My preference is to go with philwebb/spring-javaformat@15397b3.

[simonbasle]

I was really looking forward to such a change in the spring-javaformat, definite +1.

On a side note, applying this change to a code base can lead to a large modification, which can represent some noise in e.g. a git blame. Fortunately it is now possible to ignore specific commits in git blame, including on GitHub : just add the formatting commit to .git-blame-ignore-revs file and make sure to use git blame --ignore-revs-file .git-blame-ignore-revs 👍

[lfvjimisola]

So glad to see the result of @philwebb. Thank you! Is this likely to change in spring-javaformat and if so when?
Meanwhile, we'll go on the same route as @andreasevers.

[philwebb]

@lfvjimisola I'll merge this fix today but I want to try and iron out the IntelliJ plugin bugs before cutting a release.

[lfvjimisola]

@philwebb When is it likely that 0.0.36 will be released?

[philwebb]

@lfvjimisola We don't have a scheduled date, but I suspect sometime next week. You can try the SNAPSHOTs today if you want to test things.