Limit the max width of method chain dots (#70)

Limit how far dots may appear in long method chains to 80 chars.

Note that this doesn't currently apply to prefixes (such as `foo.bar().baz().stream()`) because prefixes are also used to group together fully qualified class names and it's a bit trickier to handle that case.

Fixes #69.

changelog/@unreleased/pr-70.v2.yml

@@ -0,0 +1,8 @@
+type: improvement
+improvement:
+  description: |-
+    Limit how far dots may appear in long method chains to 80 chars.
+
+    Note that this doesn't currently apply to prefixes (such as `foo.bar().baz().stream()`) because prefixes are also used to group together fully qualified class names and it's a bit trickier to handle that case.
+  links:
+  - https://github.com/palantir/palantir-java-format/pull/70

palantir-java-format/src/main/java/com/palantir/javaformat/OpenOp.java

@@ -18,6 +18,7 @@
 import com.palantir.javaformat.doc.DocBuilder;
 import com.palantir.javaformat.doc.Level;
 import java.util.Optional;
+import java.util.OptionalInt;
 import org.immutables.value.Value;
 import org.immutables.value.Value.Default;
 
@@ -28,20 +29,29 @@
  */
 @Value.Immutable
 public abstract class OpenOp implements Op {
+    /** The extra indent inside this level. */
     public abstract Indent plusIndent();
 
+    /**
+     * When this level doesn't fit on one line, controls whether this level is to be broken (its breaks taken) or
+     * partially inlined onto the current line.
+     */
     @Default
     public BreakBehaviour breakBehaviour() {
         return BreakBehaviours.breakThisLevel();
     }
 
+    /** If it's the last level of its parent, when to inline this level rather than break the parent. */
     @Default
     public LastLevelBreakability breakabilityIfLastLevel() {
         return LastLevelBreakability.ABORT;
     }
 
     public abstract Optional<String> debugName();
 
+    /** Custom max column limit that contents of this level <em>before the last break</em> may not exceed. */
+    public abstract OptionalInt columnLimitBeforeLastBreak();
+
     /**
      * Make an ordinary {@code OpenOp}.
      *
@@ -69,7 +79,7 @@ public static Op make(
 
     @Override
     public void add(DocBuilder builder) {
-        builder.open(plusIndent(), breakBehaviour(), breakabilityIfLastLevel(), debugName());
+        builder.open(this);
     }
 
     public static Builder builder() {

palantir-java-format/src/main/java/com/palantir/javaformat/doc/Break.java

@@ -16,7 +16,6 @@
 
 package com.palantir.javaformat.doc;
 
-import com.google.common.base.MoreObjects;
 import com.google.common.collect.Range;
 import com.google.errorprone.annotations.Immutable;
 import com.palantir.javaformat.CommentsHelper;
@@ -25,25 +24,19 @@
 import com.palantir.javaformat.Output;
 import com.palantir.javaformat.doc.State.BreakState;
 import java.util.Optional;
+import org.immutables.value.Value;
 
 /** A leaf node in a {@link Doc} for an optional break. */
 @Immutable
-public final class Break extends Doc implements Op {
-    private final FillMode fillMode;
-    private final String flat;
-    private final Indent plusIndent;
-    private final Optional<BreakTag> optTag;
-
-    private Break(FillMode fillMode, String flat, Indent plusIndent, Optional<BreakTag> optTag) {
-        this.fillMode = fillMode;
-        this.flat = flat;
-        this.plusIndent = plusIndent;
-        this.optTag = optTag;
-    }
+@Value.Immutable
+public abstract class Break extends Doc implements Op {
+    public abstract FillMode fillMode();
 
-    public FillMode getFillMode() {
-        return fillMode;
-    }
+    public abstract String flat();
+
+    public abstract Indent plusIndent();
+
+    public abstract Optional<BreakTag> optTag();
 
     /**
      * Make a {@code Break}.
@@ -54,7 +47,7 @@ public FillMode getFillMode() {
      * @return the new {@code Break}
      */
     public static Break make(FillMode fillMode, String flat, Indent plusIndent) {
-        return new Break(fillMode, flat, plusIndent, /* optTag= */ Optional.empty());
+        return builder().fillMode(fillMode).flat(flat).plusIndent(plusIndent).build();
     }
 
     /**
@@ -67,7 +60,7 @@ public static Break make(FillMode fillMode, String flat, Indent plusIndent) {
      * @return the new {@code Break}
      */
     public static Break make(FillMode fillMode, String flat, Indent plusIndent, Optional<BreakTag> optTag) {
-        return new Break(fillMode, flat, plusIndent, optTag);
+        return builder().fillMode(fillMode).flat(flat).plusIndent(plusIndent).optTag(optTag).build();
     }
 
     /**
@@ -76,21 +69,16 @@ public static Break make(FillMode fillMode, String flat, Indent plusIndent, Opti
      * @return the new forced {@code Break}
      */
     public static Break makeForced() {
-        return make(FillMode.FORCED, "", Indent.Const.ZERO);
+        return builder().fillMode(FillMode.FORCED).flat("").plusIndent(Indent.Const.ZERO).build();
     }
 
     /**
      * Return the {@code Break}'s extra indent.
      *
      * @return the extra indent
-     * @param state
      */
     public int evalPlusIndent(State state) {
-        return plusIndent.eval(state);
-    }
-
-    Indent getPlusIndent() {
-        return plusIndent;
+        return plusIndent().eval(state);
     }
 
     /**
@@ -99,7 +87,7 @@ Indent getPlusIndent() {
      * @return whether the {@code Break} is forced
      */
     public boolean isForced() {
-        return fillMode == FillMode.FORCED;
+        return fillMode() == FillMode.FORCED;
     }
 
     @Override
@@ -108,22 +96,22 @@ public void add(DocBuilder builder) {
     }
 
     @Override
-    float computeWidth() {
-        return isForced() ? Float.POSITIVE_INFINITY : (float) flat.length();
+    protected float computeWidth() {
+        return isForced() ? Float.POSITIVE_INFINITY : (float) flat().length();
     }
 
     @Override
-    String computeFlat() {
-        return flat;
+    protected String computeFlat() {
+        return flat();
     }
 
     @Override
-    Range<Integer> computeRange() {
+    protected Range<Integer> computeRange() {
         return EMPTY_RANGE;
     }
 
     public State computeBreaks(State stateIn, boolean broken) {
-        State state = optTag.map(breakTag -> stateIn.breakTaken(breakTag, broken)).orElse(stateIn);
+        State state = optTag().map(breakTag -> stateIn.breakTaken(breakTag, broken)).orElse(stateIn);
         return state.withBreak(this, broken);
     }
 
@@ -143,17 +131,13 @@ public void write(State state, Output output) {
             output.append(state, "\n", EMPTY_RANGE);
             output.indent(breakState.newIndent());
         } else {
-            output.append(state, flat, range());
+            output.append(state, flat(), range());
         }
     }
 
-    @Override
-    public String toString() {
-        return MoreObjects.toStringHelper(this)
-                .add("fillMode", fillMode)
-                .add("flat", flat)
-                .add("plusIndent", plusIndent)
-                .add("optTag", optTag)
-                .toString();
+    public static class Builder extends ImmutableBreak.Builder {}
+
+    public static Builder builder() {
+        return new Builder();
     }
 }

palantir-java-format/src/main/java/com/palantir/javaformat/doc/Doc.java

@@ -69,21 +69,21 @@ final Range<Integer> range() {
      *
      * @return the width, or {@code Float.POSITIVE_INFINITY} if it must be broken
      */
-    abstract float computeWidth();
+    protected abstract float computeWidth();
 
     /**
      * Compute the {@code Doc}'s flat value. Not defined (and never called) if contains forced breaks.
      *
      * @return the flat value
      */
-    abstract String computeFlat();
+    protected abstract String computeFlat();
 
     /**
      * Compute the {@code Doc}'s {@link Range} of {@link Input.Token}s.
      *
      * @return the {@link Range}
      */
-    abstract Range<Integer> computeRange();
+    protected abstract Range<Integer> computeRange();
 
     /**
      * Make breaking decisions for a {@code Doc}.

palantir-java-format/src/main/java/com/palantir/javaformat/doc/DocBuilder.java

@@ -17,20 +17,16 @@
 package com.palantir.javaformat.doc;
 
 import com.google.common.base.MoreObjects;
-import com.palantir.javaformat.BreakBehaviour;
-import com.palantir.javaformat.BreakBehaviours;
 import com.palantir.javaformat.Indent;
-import com.palantir.javaformat.LastLevelBreakability;
 import com.palantir.javaformat.Op;
+import com.palantir.javaformat.OpenOp;
 import com.palantir.javaformat.OpsBuilder;
 import java.util.ArrayDeque;
 import java.util.List;
-import java.util.Optional;
 
 /** A {@code DocBuilder} converts a sequence of {@link Op}s into a {@link Doc}. */
 public final class DocBuilder {
-    private final Level base = Level.make(
-            Indent.Const.ZERO, BreakBehaviours.breakThisLevel(), LastLevelBreakability.ABORT, Optional.of("root"));
+    private final Level base = Level.make(OpenOp.builder().plusIndent(Indent.Const.ZERO).debugName("root").build());
     private final ArrayDeque<Level> stack = new ArrayDeque<>();
 
     /**
@@ -68,20 +64,9 @@ public DocBuilder withOps(List<Op> ops) {
         return this;
     }
 
-    /**
-     * Open a new {@link Level}.
-     *
-     * @param plusIndent the extra indent for the {@link Level}
-     * @param breakBehaviour how to decide whether to break this level or not
-     * @param breakabilityIfLastLevel if last level, when to break this rather than parent
-     * @param debugName
-     */
-    public void open(
-            Indent plusIndent,
-            BreakBehaviour breakBehaviour,
-            LastLevelBreakability breakabilityIfLastLevel,
-            Optional<String> debugName) {
-        Level level = Level.make(plusIndent, breakBehaviour, breakabilityIfLastLevel, debugName);
+    /** Open a new {@link Level}. */
+    public void open(OpenOp openOp) {
+        Level level = Level.make(openOp);
         stack.addLast(level);
     }