Keep a Mutiny group call on one line with the call that completes it

[abashev]

Motivation

SmallRye Mutiny is designed so that a chain reads as a sentence. A "group" call selects what you are talking about, and the next call says what to do: onItem().transform(…), onFailure(IOException.class).recoverWithItem(…), subscribe().with(…). The formatter puts every call on its own line, which cuts each phrase in two.

Today, 2.98.0.1:

request.ifNoItem()
        .after(ofMillis(100))
        .failWith(() -> new TooSlowException("boom"))
        .onFailure(IOException.class)
        .recoverWithItem(fail -> "fallback")
        .subscribe()
        .with(item -> log("ok " + item), err -> log(err.getMessage()));

Wanted:

request.ifNoItem().after(ofMillis(100)).failWith(() -> new TooSlowException("boom"))
        .onFailure(IOException.class).recoverWithItem(fail -> "fallback")
        .subscribe().with(item -> log("ok " + item), err -> log(err.getMessage()));

It fits the current design

The chain layout already knows a few method names. LOG_METHODS keeps Flogger's logger.atInfo().log(…) together, and handleStream keeps stream(), parallelStream() and toBuilder() with the expression before them.

Breaks in a chain are emitted in two loops, in visitRegularDot and visitDotWithPrefix: one breakOp before every dot. Keeping two calls together means not emitting that break before the second one. Nothing in doc/ has to change.

The formatter has no type information, so it cannot know that a receiver is a Uni. The rule therefore has to go by method names, and to keep it from firing on unrelated code it can be switched on only in a compilation unit that imports io.smallrye.mutiny.*. visitCompilationUnit sees the imports before anything else.

Prototype

A spike on top of 7f06ca16 does exactly that: a set of group method names, a gluedToPrevious(items, i) check in both loops, and the import gate. It is one file, +41 −9 lines. A group call counts only when it has no arguments, so that collect(Collectors.toList()) on a stream is left alone, with onFailure(…) and after(…) as the exceptions.

Results:

• the example above comes out as the "wanted" block;
• uni.onItem().transform(String::toUpperCase) and multi.collect().asList() style phrases stay on one line, and a plain stream() chain in the same file is unchanged;
• formatting the result again changes nothing;
• the same code without the Mutiny import is byte-identical to 2.98.0.1;
• 116 files of this repository, about 25,000 lines of output, format identically with the release and with the spike.

To design properly

• The list. Which calls are groups, including the second-level ones. The spike leaves .onFailure().retry() on one line and .atMost(3) on the next, because retry() is a group too. ifNull(), ifNotNull() and delayIt() are in the same family.
• A phrase that does not fit. The spike never breaks inside a phrase, so an over-long one breaks inside its arguments instead, which looks worse than today. The break should stay available but be taken only when the phrase does not fit on the line, that is, a level of its own with an independent break instead of no break at all.
• The gate. A file that uses Mutiny without importing it, for example through var, keeps today's layout. That seems an acceptable fallback.
• Built-in knowledge or configuration. The formatter has no options, and this should not become the first one. A small built-in table of import prefix and group method names would also cover other libraries with the same grammar, if any turn up.

Compatibility

Files that do not import Mutiny are unaffected. Files that do will format differently from palantir-java-format 2.98.0, so under the 2.x promise this is a 3.0 change unless decided otherwise.

The spike, as a diff

diff --git a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java
index 3917911e..6e8b1876 100644
--- a/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java
+++ b/open-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java
@@ -345,6 +345,8 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> {
 
     @Override
     public Void visitCompilationUnit(CompilationUnitTree node, Void unused) {
+        fluentGroupsEnabled = node.getImports().stream()
+                .anyMatch(i -> i.getQualifiedIdentifier().toString().startsWith("io.smallrye.mutiny."));
         boolean first = true;
         if (node.getPackageName() != null) {
             markForPartialFormat();
@@ -1610,6 +1612,33 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> {
             "withCause",
             "withStackTrace");
 
+    /** SPIKE: Mutiny "group" calls read as one phrase with the call that follows them. */
+    private boolean fluentGroupsEnabled = false;
+
+    private static final ImmutableSet<String> FLUENT_GROUP_METHODS = ImmutableSet.of(
+            "onItem", "onFailure", "onItemOrFailure", "onSubscription", "onTermination", "onCancellation",
+            "onCompletion", "onOverflow", "onRequest", "ifNoItem", "subscribe", "await", "collect", "select",
+            "skip", "group", "broadcast", "memoize", "convert", "after");
+
+    /** Whether items[i] must stay on the line of items[i - 1]. */
+    private boolean gluedToPrevious(List<ExpressionTree> items, int i) {
+        if (!fluentGroupsEnabled || i == 0 || !(items.get(i) instanceof MethodInvocationTree)) {
+            return false;
+        }
+        ExpressionTree previous = items.get(i - 1);
+        if (!(previous instanceof MethodInvocationTree)) {
+            return false;
+        }
+        MethodInvocationTree call = (MethodInvocationTree) previous;
+        String name = getMethodName(call).toString();
+        if (!FLUENT_GROUP_METHODS.contains(name)) {
+            return false;
+        }
+        // `collect(toList())` on a stream is an action; Mutiny's groups take no arguments,
+        // except onFailure(Class) and ifNoItem().after(Duration).
+        return call.getArguments().isEmpty() || name.equals("onFailure") || name.equals("after");
+    }
+
     private static List<Long> handleStream(List<ExpressionTree> parts) {
         return indexes(parts.stream(), p -> {
                     if (!(p instanceof MethodInvocationTree)) {
@@ -2857,9 +2886,10 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> {
         // chain starts with another expression
         int minLength = indentMultiplier * 4;
         int length = needDot0 ? minLength : 0;
-        for (ExpressionTree e : items) {
+        for (int itemIndex = 0; itemIndex < items.size(); itemIndex++) {
+            ExpressionTree e = items.get(itemIndex);
             if (needDot) {
-                if (length > minLength) {
+                if (length > minLength && !gluedToPrevious(items, itemIndex)) {
                     builder.breakOp(Break.builder()
                             .fillMode(FillMode.UNIFIED)
                             .flat("")
@@ -2966,13 +2996,15 @@ public class JavaInputAstVisitor extends TreePathScanner<Void, Void> {
                     fillMode = FillMode.UNIFIED;
                 }
 
-                builder.breakOp(Break.builder()
-                        .fillMode(fillMode)
-                        .flat("")
-                        .plusIndent(ZERO)
-                        .optTag(Optional.of(nameTag))
-                        .hasColumnLimit(shouldHaveColumnLimit(e))
-                        .build());
+                if (!gluedToPrevious(items, i)) {
+                    builder.breakOp(Break.builder()
+                            .fillMode(fillMode)
+                            .flat("")
+                            .plusIndent(ZERO)
+                            .optTag(Optional.of(nameTag))
+                            .hasColumnLimit(shouldHaveColumnLimit(e))
+                            .build());
+                }
                 token(".");
             }
             BreakTag tyargTag = new BreakTag();