From 212d5fe582087f537fb69924d3ba4ae3c4ad180f Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Fri, 16 Oct 2026 04:33:19 -0700 Subject: [PATCH] Hug lambda arguments in `google-java-format`. When a call doesn't fit on one line and its only argument is a lambda, keep the lambda's head on the call's line and indent its body relative to that line: ```java executor.submit(() -> { run(); }); ``` instead of ```java executor.submit( () -> { run(); }); ``` The lambda is hugged only if everything up to `->` fits on the line; otherwise the existing layout is used. However, if breaking before the lambda allows the entire lambda to fit flat on the next line, prefer breaking before the lambda rather than hugging `params ->` onto the call line and breaking inside the lambda body. Lambdas preceded by other arguments are not hugged, since the preceding arguments wouldn't line up with the lambda's body. This addresses https://github.com/google/google-java-format/issues/19. This adds a `Doc.Choice` node to the layout IR, which chooses between a preferred and a fallback layout with the same flat text. `LayoutBuilder` derives the preferred layout by flattening the argument list up to the hug point. PiperOrigin-RevId: 1000770039 --- .../java/JavaInputAstVisitor.java | 66 ++++++- .../google/googlejavaformat/layout/Doc.java | 42 ++++- .../layout/LayoutBuilder.java | 161 ++++++++++++++++-- .../googlejavaformat/layout/LayoutSolver.java | 161 +++++++++++++++++- .../googlejavaformat/layout/DocTest.java | 12 ++ .../layout/LayoutSolverTest.java | 122 +++++++++++++ .../java/testdata/B20128760.output | 18 +- .../java/testdata/B22873322.output | 32 ++-- .../java/testdata/LambdaHug.output | 97 +++++------ 9 files changed, 610 insertions(+), 101 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java index af29352ed..83a2c9579 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java @@ -167,6 +167,7 @@ import java.util.Comparator; import java.util.Deque; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -1356,6 +1357,7 @@ public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) { builder.close(); builder.space(); builder.op("->"); + markHugPoint(node); builder.open(statementBody ? ZERO : plusFour); if (statementBody) { builder.space(); @@ -3368,10 +3370,16 @@ private void addTypeArguments(List typeArguments, Indent plusInd /// @param arguments the arguments /// @param plusIndent the extra indent for the arguments private void addArguments(List arguments, Indent plusIndent) { - builder.open(plusIndent); + boolean tabular = arguments.size() % 2 == 0 && argumentsAreTabular(arguments) == 2; + boolean hug = canHugOnlyArgument(arguments); + if (hug) { + builder.openHuggable(plusIndent, hugPoint(getOnlyElement(arguments))); + } else { + builder.open(plusIndent); + } token("("); if (!arguments.isEmpty()) { - if (arguments.size() % 2 == 0 && argumentsAreTabular(arguments) == 2) { + if (tabular) { builder.forcedBreak(); builder.open(ZERO); boolean afterFirstToken = false; @@ -3403,15 +3411,65 @@ private void addArguments(List arguments, Indent plusI builder.close(); } else { builder.breakOp(); - argList(arguments); + argList(arguments, hug); } } token(")"); builder.close(); } + /** + * The {@link LayoutBuilder.HugPoint}s of constructs that may be hugged, keyed by the construct. + * They are marked when the construct is visited. + */ + private final IdentityHashMap hugPoints = new IdentityHashMap<>(); + + /** Returns the hug point for {@code tree}, which is marked when {@code tree} is visited. */ + private LayoutBuilder.HugPoint hugPoint(Tree tree) { + return hugPoints.computeIfAbsent(tree, t -> LayoutBuilder.newHugPoint()); + } + + /** Marks the hug point of {@code tree}, if it may be hugged. */ + private void markHugPoint(Tree tree) { + LayoutBuilder.HugPoint hugPoint = hugPoints.remove(tree); + if (hugPoint != null) { + builder.hugPoint(hugPoint); + } + } + + /** + * Returns true for constructs that may be hugged, i.e. laid out starting on the line of the + * preceding content: lambdas. + */ + private static boolean isHuggable(ExpressionTree tree) { + return tree instanceof LambdaExpressionTree; + } + + /** + * Returns true if the argument list is a single argument that may be hugged, e.g. {@code f(x -> { + * ... })}. Lambdas preceded by other arguments aren't hugged: the preceding arguments would share + * a line with the lambda's head, and wouldn't line up with the lambda's body. + */ + private static boolean canHugOnlyArgument(List arguments) { + return arguments.size() == 1 && isHuggable(getOnlyElement(arguments)); + } + private void argList(List arguments) { - builder.open(ZERO); + argList(arguments, /* hug= */ false); + } + + /** + * Adds an argument list. + * + * @param hug whether the only argument may be hugged, i.e. laid out starting on the line of the + * call; see {@link LayoutBuilder#openHuggable} + */ + private void argList(List arguments, boolean hug) { + if (hug) { + builder.openHuggedArguments(); + } else { + builder.open(ZERO); + } boolean afterFirstToken = false; FillMode fillMode = hasOnlyShortItems(arguments) ? FillMode.INDEPENDENT : FillMode.UNIFIED; for (ExpressionTree argument : arguments) { diff --git a/core/src/main/java/com/google/googlejavaformat/layout/Doc.java b/core/src/main/java/com/google/googlejavaformat/layout/Doc.java index 687651959..7c8308fb1 100644 --- a/core/src/main/java/com/google/googlejavaformat/layout/Doc.java +++ b/core/src/main/java/com/google/googlejavaformat/layout/Doc.java @@ -53,7 +53,10 @@ /// group. Trailing text that must stay on the same line as a group (e.g. a `;`) is instead placed /// inside the group by [LayoutBuilder]. These are the rules of the existing /// [com.google.googlejavaformat.Doc] engine, and are kept for output compatibility. -public sealed interface Doc permits Doc.Text, Doc.Tok, Doc.Space, Doc.Break, Doc.Group { +/// +/// A [Choice] offers a preferred alternative layout for a group, used when the group doesn't fit +/// flat but the preferred layout's first line does. +public sealed interface Doc permits Doc.Text, Doc.Tok, Doc.Space, Doc.Break, Doc.Group, Doc.Choice { /// Width used for documents that cannot be laid out on a single line. int MAX_LINE_WIDTH = 1000; @@ -260,4 +263,41 @@ public void appendFlat(StringBuilder sb) { } } } + + /// A choice between two layouts of the same content. + /// + /// If the content fits flat it is laid out flat, as for a [Group]. Otherwise, `fallback` is used + /// if taking a single direct break in `fallback` allows the rest of it to fit flat on the next + /// line (e.g. `f(\n x -> g(x))` rather than breaking inside the lambda). Otherwise, `preferred` + /// is used if, when laid out, its first line fits and contains at least its first `prefixWidth` + /// columns; `fallback` is used if not. + /// + /// Both alternatives must have the same flat text, so the choice never affects layouts in which + /// it is flat, or the fit decisions of enclosing groups. They usually share subtrees. + /// + /// @param preferred the preferred layout + /// @param prefixWidth the width of the part of `preferred` that must be on the current line, e.g. + /// `f(x ->` for a hugged lambda argument + /// @param fallback the layout to use if `preferred` isn't chosen + record Choice(Group preferred, int prefixWidth, Group fallback) implements Doc { + public Choice { + requireNonNull(preferred, "preferred"); + requireNonNull(fallback, "fallback"); + } + + @Override + public int width() { + return fallback.width(); + } + + @Override + public void appendFlat(StringBuilder sb) { + fallback.appendFlat(sb); + } + + @Override + public Range range() { + return fallback.range(); + } + } } diff --git a/core/src/main/java/com/google/googlejavaformat/layout/LayoutBuilder.java b/core/src/main/java/com/google/googlejavaformat/layout/LayoutBuilder.java index 85d281076..bcc2a4f93 100644 --- a/core/src/main/java/com/google/googlejavaformat/layout/LayoutBuilder.java +++ b/core/src/main/java/com/google/googlejavaformat/layout/LayoutBuilder.java @@ -127,8 +127,47 @@ public void drain() { } public void open(Indent plusIndent) { + open(plusIndent, HugRole.NONE, null); + } + + private void open(Indent plusIndent, HugRole hugRole, HugPoint hugPoint) { depth++; - events.add(new Open(plusIndent)); + events.add(new Open(plusIndent, hugRole, hugPoint)); + } + + /// The start of the body of a construct that can be hugged, i.e. laid out starting on the same + /// line as preceding content, e.g. the body of a lambda. See [#openHuggable]. + public static final class HugPoint { + /// A zero-width marker for this point in the [Doc]. + private final Doc marker = new Doc.Text(""); + + private HugPoint() {} + } + + /// Returns a new [HugPoint], to be passed to [#hugPoint] and an enclosing level. + public static HugPoint newHugPoint() { + return new HugPoint(); + } + + /// Marks the position of `hugPoint`. + public void hugPoint(HugPoint hugPoint) { + events.add(new HugPointOp(hugPoint)); + } + + /// Opens a level for an argument list whose argument may be hugged, e.g. `f(x -> { ... })`. The + /// level's direct breaks, and the direct breaks of a nested level opened with + /// [#openHuggedArguments], are the ones that are not taken when hugging, and everything up to + /// `hugPoint` must then fit on the current line. + /// + /// When hugged, the hugged content is indented relative to the line containing the call, instead + /// of by the level's extra indent. + public void openHuggable(Indent plusIndent, HugPoint hugPoint) { + open(plusIndent, HugRole.HUGGABLE, hugPoint); + } + + /// Opens the level holding the arguments of an enclosing [#openHuggable] level. + public void openHuggedArguments() { + open(ZERO, HugRole.ARGUMENTS, null); } public void close() { @@ -493,8 +532,9 @@ private List spliceEvents(ListMultimap tokEvents) { continue; } result.add(ev); - // An `Open` doesn't output anything, so it doesn't end the run that follows a forced break. - if (!(ev instanceof Open)) { + // An `Open` or hug point doesn't output anything, so it doesn't end the run that follows a + // forced break. + if (!(ev instanceof Open || ev instanceof HugPointOp)) { afterForcedBreak = isForcedBreak(ev); } } @@ -534,13 +574,14 @@ private static boolean isSpaceLike(Event ev) { /// closed group) into the preceding group, so it's accounted for when deciding whether that group /// fits on the line. private static Doc buildTree(List events) { - LevelBuilder base = new LevelBuilder(ZERO); + LevelBuilder base = new LevelBuilder(ZERO, HugRole.NONE, null); ArrayDeque stack = new ArrayDeque<>(); stack.addLast(base); LevelBuilder appendLevel = base; for (Event ev : events) { switch (ev) { - case Open open -> stack.addLast(new LevelBuilder(open.plusIndent())); + case Open open -> + stack.addLast(new LevelBuilder(open.plusIndent(), open.hugRole(), open.hugPoint())); case Close close -> { LevelBuilder top = stack.removeLast(); stack.getLast().add(top); @@ -556,6 +597,7 @@ private static Doc buildTree(List events) { tok.getOriginalText(), Range.singleton(tok.getIndex()).canonical(INTEGERS))); } case TokOp tokOp -> appendLevel.add(new Doc.Tok(tokOp.tok())); + case HugPointOp hugPointOp -> appendLevel.add(hugPointOp.hugPoint().marker); case SpaceOp spaceOp -> appendLevel.add(Doc.Space.INSTANCE); } } @@ -577,28 +619,127 @@ private static LineBreak forcedLineBreak(Indent plusIndent) { /// A mutable group under construction; see [#buildTree]. private static final class LevelBuilder { final Indent plusIndent; + final HugRole hugRole; + final HugPoint hugPoint; final List items = new ArrayList<>(); - LevelBuilder(Indent plusIndent) { + LevelBuilder(Indent plusIndent, HugRole hugRole, HugPoint hugPoint) { this.plusIndent = plusIndent; + this.hugRole = hugRole; + this.hugPoint = hugPoint; } void add(Object item) { items.add(item); } - Doc.Group build() { + Doc build() { + return hugRole == HugRole.HUGGABLE + ? buildHuggable() + : new Doc.Group(plusIndent, buildItems()); + } + + ImmutableList buildItems() { ImmutableList.Builder docs = ImmutableList.builder(); for (Object item : items) { docs.add(item instanceof LevelBuilder lb ? lb.build() : (Doc) item); } - return new Doc.Group(plusIndent, docs.build()); + return docs.build(); + } + + /// Builds a choice between hugging and the regular layout. + /// + /// The hugged layout has the same content as the regular one, but the breaks of this level and + /// of its [HugRole#ARGUMENTS] level (if any) are replaced by their flat text, and this level's + /// extra indent is dropped. Everything up to the hug point must fit on the current line. + private Doc buildHuggable() { + ImmutableList.Builder fallback = ImmutableList.builder(); + ImmutableList.Builder preferred = ImmutableList.builder(); + boolean canHug = true; + for (Object item : items) { + ImmutableList flattened; + Doc fallbackDoc; + switch (item) { + case LevelBuilder lb when lb.hugRole == HugRole.ARGUMENTS -> { + flattened = lb.buildItems(); + fallbackDoc = new Doc.Group(lb.plusIndent, flattened); + } + case LevelBuilder lb -> { + fallbackDoc = lb.build(); + flattened = ImmutableList.of(fallbackDoc); + } + case Doc doc -> { + fallbackDoc = doc; + flattened = ImmutableList.of(doc); + } + default -> throw new AssertionError(item); + } + fallback.add(fallbackDoc); + for (Doc doc : flattened) { + if (doc instanceof Doc.Break b) { + canHug &= canFlatten(b); + preferred.add(new Doc.Text(b.flatText())); + } else { + preferred.add(doc); + } + } + } + Doc.Group fallbackGroup = new Doc.Group(plusIndent, fallback.build()); + Doc.Group preferredGroup = new Doc.Group(ZERO, preferred.build()); + int prefixWidth = canHug ? widthBefore(preferredGroup, hugPoint.marker) : -1; + if (prefixWidth < 0) { + return fallbackGroup; + } + return new Doc.Choice(preferredGroup, prefixWidth, fallbackGroup); + } + + /// Forced breaks (e.g. after line comments) and tagged breaks must not be flattened. + private static boolean canFlatten(Doc.Break b) { + return b.mode() != Doc.BreakMode.FORCED && b.tag().isEmpty(); + } + + /// Returns the flat width of the content of `doc` before `marker`, or -1 if `doc` doesn't + /// contain `marker` within fewer than [Doc#MAX_LINE_WIDTH] columns. + // Hug point markers are equal to any other empty Text, so they must be found by identity. + @SuppressWarnings("ReferenceEquality") + private static int widthBefore(Doc doc, Doc marker) { + if (doc == marker) { + return 0; + } + return switch (doc) { + case Doc.Group g -> { + int width = 0; + for (Doc child : g.docs()) { + int before = widthBefore(child, marker); + if (before >= 0) { + yield min(width + before, Doc.MAX_LINE_WIDTH); + } + width += child.width(); + if (width >= Doc.MAX_LINE_WIDTH) { + yield -1; + } + } + yield -1; + } + case Doc.Choice c -> widthBefore(c.fallback(), marker); + default -> -1; + }; } } - private sealed interface Event permits Open, Close, LineBreak, TokenOp, TokOp, SpaceOp {} + private sealed interface Event + permits Open, Close, LineBreak, TokenOp, TokOp, SpaceOp, HugPointOp {} + + private record Open(Indent plusIndent, HugRole hugRole, HugPoint hugPoint) implements Event {} - private record Open(Indent plusIndent) implements Event {} + private record HugPointOp(HugPoint hugPoint) implements Event {} + + /// The role of a level in a huggable argument list; see [#openHuggable]. + private enum HugRole { + NONE, + HUGGABLE, + ARGUMENTS + } private record Close() implements Event {} diff --git a/core/src/main/java/com/google/googlejavaformat/layout/LayoutSolver.java b/core/src/main/java/com/google/googlejavaformat/layout/LayoutSolver.java index 0ee8a8d9a..c381e84cd 100644 --- a/core/src/main/java/com/google/googlejavaformat/layout/LayoutSolver.java +++ b/core/src/main/java/com/google/googlejavaformat/layout/LayoutSolver.java @@ -23,7 +23,9 @@ import com.google.googlejavaformat.Indent; import com.google.googlejavaformat.Newlines; import com.google.googlejavaformat.Output; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Deque; import java.util.List; /// Lays out a [Doc]. @@ -105,6 +107,33 @@ private static final class Solver { private final CommentsHelper commentsHelper; private final List actions = new ArrayList<>(); + /// Whether the innermost trial layout has reached the end of its first line. + private boolean firstLineEnded = false; + + /// The column at which the innermost trial's first line ended, if [#firstLineEnded]. + private int firstLineEndColumn; + + /// The indent of the current line. + private int lineIndent = 0; + + /// The state of a break tag before it was modified during a trial layout. + /// + /// @param tag the break tag + /// @param wasBroken the tag's [Output.BreakTag#wasBreakTaken] value before it was modified + private record SavedBreakTag(Output.BreakTag tag, boolean wasBroken) { + void restore() { + tag.recordBroken(wasBroken); + } + } + + /// A stack of active trial layouts (see [#firstLineEnd]). While non-empty, ending the current + /// line stops the innermost trial: [#endLine] sets [#firstLineEnded], and layout methods return + /// as soon as they see it set. + /// + /// Each entry records the previous state of every [Output.BreakTag] modified during that trial, + /// so that the tags can be restored when the trial ends. + private final Deque> trialTagsStack = new ArrayDeque<>(); + Solver(int maxWidth, CommentsHelper commentsHelper) { this.maxWidth = maxWidth; this.commentsHelper = commentsHelper; @@ -132,6 +161,9 @@ private State broken(Doc.Group group, State state) { if (doc instanceof Doc.Break b) { SegmentResult result = segment(before, segment, previousFits, state); state = result.state(); + if (firstLineEnded) { + return state; + } previousFits = result.fits(); before = b; segment = new ArrayList<>(); @@ -163,18 +195,30 @@ private SegmentResult segment( !previousFits || state.column() + before.width() + segmentWidth > maxWidth; }; state = breakOp(before, taken, state); + if (firstLineEnded) { + // The trial is over, so whether the segment fits doesn't matter. + return new SegmentResult(state, true); + } } boolean fits = state.column() + segmentWidth <= maxWidth; for (Doc doc : segment) { state = leaf(doc, state); + if (firstLineEnded) { + return new SegmentResult(state, fits); + } } return new SegmentResult(state, fits); } private State breakOp(Doc.Break b, boolean taken, State state) { - b.tag().ifPresent(t -> t.recordBroken(taken)); + b.tag().ifPresent(t -> recordTag(t, taken)); if (taken) { + endLine(state.column()); + if (firstLineEnded) { + return state; + } int indent = max(state.indent() + b.plusIndent().eval(), 0); + lineIndent = indent; actions.add(new Newline()); actions.add(new IndentTo(indent)); return state.withColumn(indent); @@ -186,7 +230,12 @@ private State breakOp(Doc.Break b, boolean taken, State state) { private State leaf(Doc doc, State state) { return switch (doc) { case Doc.Group g -> group(g, state); + case Doc.Choice c -> choice(c, state); case Doc.Text t -> { + int newline = Newlines.firstBreak(t.text()); + if (newline >= 0) { + endLine(state.column() + newline); + } actions.add(new Append(t.text(), t.range())); yield state.withColumn(state.column() + t.width()); } @@ -199,6 +248,10 @@ private State leaf(Doc doc, State state) { commentsHelper != null ? commentsHelper.rewrite(c.tok(), maxWidth, state.column()) : c.flat(); + int newline = Newlines.firstBreak(text); + if (newline >= 0) { + endLine(state.column() + newline); + } actions.add(new Append(text, c.range())); int lastLineLength = text.length() - Newlines.lastLineStart(text); yield state.withColumn(state.column() + lastLineLength); @@ -206,5 +259,111 @@ private State leaf(Doc doc, State state) { case Doc.Break b -> throw new AssertionError(b); }; } + + /// Lays out a choice. If it doesn't fit flat, the fallback is used if breaking at its direct + /// break allows the rest of it to fit flat on the next line (e.g. `f(\n x -> g(x))` rather than + /// breaking inside the lambda). Otherwise, the preferred layout is used when its prefix fits + /// flat on the current line, its first line fits, and that line contains the whole prefix (e.g. + /// a hugged lambda isn't preceded by a line comment); otherwise the fallback is used. + /// + /// The preferred layout is indented relative to the indent of the current line, rather than the + /// indent of the enclosing groups. It starts partway along a line, so enclosing groups whose + /// breaks weren't taken shouldn't contribute to its indentation. For example, a hugged lambda + /// body is indented relative to the line containing the call. + private State choice(Doc.Choice choice, State state) { + int width = choice.width(); + if (state.column() + width <= maxWidth) { + actions.add(new Append(choice.flat(), choice.range())); + return state.withColumn(state.column() + width); + } + if (fallbackFitsOnNextLine(choice.fallback(), state)) { + return group(choice.fallback(), state); + } + State preferredState = new State(lineIndent, state.column()); + int prefixEnd = state.column() + choice.prefixWidth(); + if (prefixEnd <= maxWidth) { + Trial trial = firstLineEnd(choice.preferred(), preferredState); + int firstLineEnd = trial.column(); + if (prefixEnd <= firstLineEnd && firstLineEnd <= maxWidth) { + if (!trialTagsStack.isEmpty() && trial.lineEnded()) { + // The enclosing trial ends on the line the preferred layout's trial just ended, so + // laying out the preferred layout again would only rediscover that column. Skipping it + // keeps the cost of nested choices linear, instead of doubling it at each level. + endLine(firstLineEnd); + return state; + } + return state.withColumn(group(choice.preferred(), preferredState).column()); + } + } + return group(choice.fallback(), state); + } + + /// Returns whether `fallback` can be laid out by taking a single direct break and fitting + /// everything after that break flat on the next line. + private boolean fallbackFitsOnNextLine(Doc.Group fallback, State state) { + ImmutableList docs = fallback.docs(); + int breakIndex = -1; + for (int i = 0; i < docs.size(); i++) { + if (docs.get(i) instanceof Doc.Break) { + if (breakIndex != -1) { + return false; + } + breakIndex = i; + } + } + if (breakIndex == -1 + || state.column() + Doc.Group.totalWidth(docs.subList(0, breakIndex)) > maxWidth) { + return false; + } + Doc.Break theBreak = (Doc.Break) docs.get(breakIndex); + int indent = + max(state.indent() + fallback.plusIndent().eval() + theBreak.plusIndent().eval(), 0); + return indent + Doc.Group.totalWidth(docs.subList(breakIndex + 1, docs.size())) <= maxWidth; + } + + /// The result of a trial layout. + /// + /// @param column the column at which the first line ended, or at which the layout ended if it + /// fit on one line + /// @param lineEnded whether the first line ended before the end of the layout + private record Trial(int column, boolean lineEnded) {} + + /// Lays out `group` provisionally, up to the end of its first line, and returns the column at + /// which that line ends. The layout's output and break tags are discarded. + /// + /// Only the first line is laid out, so the cost of a trial is bounded by the line width rather + /// than by the size of `group`. A nested trial consumes [#firstLineEnded] before returning, so + /// the enclosing trial continues. + private Trial firstLineEnd(Doc.Group group, State state) { + int mark = actions.size(); + int savedLineIndent = lineIndent; + trialTagsStack.push(new ArrayList<>()); + State end = group(group, state); + Trial trial = + firstLineEnded ? new Trial(firstLineEndColumn, true) : new Trial(end.column(), false); + firstLineEnded = false; + lineIndent = savedLineIndent; + actions.subList(mark, actions.size()).clear(); + for (SavedBreakTag saved : trialTagsStack.pop().reversed()) { + saved.restore(); + } + return trial; + } + + /// Records that the current line ends at `column`. In a trial layout, this ends the trial; + /// callers must check [#firstLineEnded] and return. + private void endLine(int column) { + if (!trialTagsStack.isEmpty()) { + firstLineEnded = true; + firstLineEndColumn = column; + } + } + + private void recordTag(Output.BreakTag tag, boolean taken) { + if (!trialTagsStack.isEmpty()) { + trialTagsStack.peek().add(new SavedBreakTag(tag, tag.wasBreakTaken())); + } + tag.recordBroken(taken); + } } } diff --git a/core/src/test/java/com/google/googlejavaformat/layout/DocTest.java b/core/src/test/java/com/google/googlejavaformat/layout/DocTest.java index 3b31e69c7..daf042b54 100644 --- a/core/src/test/java/com/google/googlejavaformat/layout/DocTest.java +++ b/core/src/test/java/com/google/googlejavaformat/layout/DocTest.java @@ -175,4 +175,16 @@ public void groupRangeSpansChildren() { assertThat(doc.range()).isEqualTo(Range.closedOpen(3, 7)); assertThat(group(text("a"), line(), text("b")).range()).isEqualTo(Doc.EMPTY_RANGE); } + + @Test + public void choiceWidthFlatAndRange() { + Doc.Group preferred = + (Doc.Group) group(text("a"), text(" "), new Doc.Text("b", Range.closedOpen(2, 3))); + Doc.Group fallback = + (Doc.Group) group(text("a"), line(), new Doc.Text("b", Range.closedOpen(2, 3))); + Doc choice = new Doc.Choice(preferred, 1, fallback); + assertThat(choice.width()).isEqualTo(3); + assertThat(choice.flat()).isEqualTo("a b"); + assertThat(choice.range()).isEqualTo(Range.closedOpen(2, 3)); + } } diff --git a/core/src/test/java/com/google/googlejavaformat/layout/LayoutSolverTest.java b/core/src/test/java/com/google/googlejavaformat/layout/LayoutSolverTest.java index 9b082e5ac..96115a9cc 100644 --- a/core/src/test/java/com/google/googlejavaformat/layout/LayoutSolverTest.java +++ b/core/src/test/java/com/google/googlejavaformat/layout/LayoutSolverTest.java @@ -132,4 +132,126 @@ public void spacesDoNotBreak() { Doc doc = group(text("a"), space(), text("b"), space(), text("c")); assertThat(LayoutSolver.format(doc, 3)).isEqualTo("a b c"); } + + /// `f(a, x -> body)`, with a preferred layout that hugs the lambda. + private static Doc hugChoice(String body) { + Doc.Group lambdaBody = (Doc.Group) group(4, line(), text(body)); + Doc.Group fallback = + (Doc.Group) + group( + 4, + text("f("), + softline(), + group(text("a,"), line(), text("x ->"), lambdaBody), + text(")")); + Doc.Group preferred = + (Doc.Group) group(text("f("), text("a,"), text(" "), text("x ->"), lambdaBody, text(")")); + return new Doc.Choice(preferred, "f(a, ".length(), fallback); + } + + @Test + public void choiceFlat() { + assertThat(LayoutSolver.format(hugChoice("y"), 20)).isEqualTo("f(a, x -> y)"); + // Also exercise a Choice that fits flat inside an enclosing broken Group, where Solver.choice + // is reached directly instead of the root Group being flattened up front. + Doc inBrokenGroup = group(text("prefix"), line(), hugChoice("y")); + assertThat(LayoutSolver.format(inBrokenGroup, 15)).isEqualTo("prefix\nf(a, x -> y)"); + } + + @Test + public void choicePreferred() { + // The body is indented relative to the line, not the fallback's +4 argument indent. + assertThat(LayoutSolver.format(hugChoice("longBody"), 13)) + .isEqualTo("f(a, x ->\n longBody)"); + } + + @Test + public void choiceFallbackWhenFirstLineDoesNotFit() { + assertThat(LayoutSolver.format(hugChoice("longBody"), 8)) + .isEqualTo("f(\n a,\n x ->\n longBody)"); + } + + @Test + public void choiceFallbackWhenRestFitsOnNextLine() { + Doc.Group lambdaBody = (Doc.Group) group(4, line(), text("body")); + Doc.Group fallback = + (Doc.Group) + group( + 4, text("someLongMethod("), softline(), group(text("x ->"), lambdaBody), text(")")); + Doc.Group preferred = + (Doc.Group) group(text("someLongMethod("), text("x ->"), lambdaBody, text(")")); + Doc choice = new Doc.Choice(preferred, "someLongMethod(x ->".length(), fallback); + assertThat(LayoutSolver.format(choice, 20)).isEqualTo("someLongMethod(\n x -> body)"); + } + + @Test + public void choicePreferredWhenFallbackCannotFitOnSingleNextLine() { + Doc.Group lambdaBody = (Doc.Group) group(4, line(), text("body")); + Doc.Group preferred = (Doc.Group) group(text("f("), text("x ->"), lambdaBody, text(")")); + + // Multiple direct breaks in fallback: fallbackFitsOnNextLine returns false, so preferred hugs. + Doc.Group multiBreakFallback = + (Doc.Group) + group( + 4, text("f("), softline(), text("x ->"), softline(), group(lambdaBody), text(")")); + assertThat( + LayoutSolver.format( + new Doc.Choice(preferred, "f(x ->".length(), multiBreakFallback), 8)) + .isEqualTo("f(x ->\n body)"); + + // No direct breaks in fallback: fallbackFitsOnNextLine returns false, so preferred hugs. + Doc.Group noBreakFallback = + (Doc.Group) group(4, text("f("), group(text("x ->"), lambdaBody), text(")")); + assertThat( + LayoutSolver.format(new Doc.Choice(preferred, "f(x ->".length(), noBreakFallback), 8)) + .isEqualTo("f(x ->\n body)"); + + // Prefix before the single break in fallback is wider than maxWidth: fallbackFitsOnNextLine + // returns false, and the choice falls back to breaking the group. + Doc.Group widePrefixFallback = + (Doc.Group) + group( + 4, text("someLongMethod("), softline(), group(text("x ->"), lambdaBody), text(")")); + Doc.Group widePrefixPreferred = + (Doc.Group) group(text("someLongMethod("), text("x ->"), lambdaBody, text(")")); + assertThat( + LayoutSolver.format( + new Doc.Choice( + widePrefixPreferred, "someLongMethod(x ->".length(), widePrefixFallback), + 10)) + .isEqualTo("someLongMethod(\n x ->\n body)"); + } + + @Test + public void choicePreferredIndentIsRelativeToLine() { + // The enclosing group's +8 indent doesn't apply, since its break wasn't taken. + Doc doc = group(8, text("g("), hugChoice("longBody")); + assertThat(LayoutSolver.format(doc, 14)).isEqualTo("g(f(a, x ->\n longBody)"); + } + + @Test + public void choiceTrialDoesNotLeakBreakTags() { + BreakTag tag = new BreakTag(); + Doc.Group preferred = (Doc.Group) group(text("aaaa"), taggedLine("", tag), text("bbbbbbbbbb")); + Doc.Group fallback = (Doc.Group) group(text("aaaa"), line(""), text("bbbbbbbbbb")); + // The preferred layout's first line ends before its prefix, so it's rejected. The tag recorded + // while trying it is reset. + assertThat(LayoutSolver.format(new Doc.Choice(preferred, 5, fallback), 8)) + .isEqualTo("aaaa\nbbbbbbbbbb"); + assertThat(tag.wasBreakTaken()).isFalse(); + } + + @Test + public void nestedChoicesAreLinear() { + // Each choice prefers to keep its contents on the current line, so every trial reaches the + // innermost choice. Re-laying out each accepted preferred layout would take 2^depth steps. + int depth = 100; + Doc doc = group(text("{"), hardline(), text("}")); + for (int i = 0; i < depth; i++) { + Doc.Group preferred = (Doc.Group) group(text("a"), doc); + Doc.Group fallback = (Doc.Group) group(4, text("a"), softline(), doc); + doc = new Doc.Choice(preferred, 1, fallback); + } + assertThat(LayoutSolver.format(doc, 999)).isEqualTo("a".repeat(depth) + "{\n}"); + } } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B20128760.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B20128760.output index b5b27fa19..a5530d3be 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B20128760.output +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B20128760.output @@ -90,16 +90,12 @@ class B20128760 { { Stream 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)))); + .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)))); } } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B22873322.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B22873322.output index 776838319..a2d08e419 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B22873322.output +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B22873322.output @@ -1,23 +1,17 @@ class B22873322 { { - f( - param -> - veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongExpr( - param)); - f( - (param1, param2) -> - veryLooooooooooooooooooooooooooooooooooooooooooooooooongExpr(param1, param2)); - f( - (int param) -> - veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongExpr(param)); - f( - (param1, param2) -> { - return expr(param1, param2); - }); - f( - (param1, param2) -> { - Object foo = expr(param1, param2); - return foo; - }); + f(param -> + veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongExpr(param)); + f((param1, param2) -> + veryLooooooooooooooooooooooooooooooooooooooooooooooooongExpr(param1, param2)); + f((int param) -> + veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongExpr(param)); + f((param1, param2) -> { + return expr(param1, param2); + }); + f((param1, param2) -> { + Object foo = expr(param1, param2); + return foo; + }); } } diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/LambdaHug.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/LambdaHug.output index 201e05899..85c2e7ef7 100644 --- a/core/src/test/resources/com/google/googlejavaformat/java/testdata/LambdaHug.output +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/LambdaHug.output @@ -1,15 +1,13 @@ class LambdaHug { void statements() { - executor.submit( - () -> { - run(); - }); - executor.submit( - () -> { - runSomethingThatIsLong(); - runSomethingElseThatIsAlsoLong(); - run(); - }); + executor.submit(() -> { + run(); + }); + executor.submit(() -> { + runSomethingThatIsLong(); + runSomethingElseThatIsAlsoLong(); + run(); + }); listenableFuture.addListener( () -> { System.err.println("done"); @@ -50,9 +48,8 @@ class LambdaHug { run(); run(); }); - foo.bar( - (param1, param2) -> - param1.someLongMethodName(param2).anotherLongMethodName().andAnotherOne()); + foo.bar((param1, param2) -> + param1.someLongMethodName(param2).anotherLongMethodName().andAnotherOne()); } int returns() { @@ -65,11 +62,10 @@ class LambdaHug { void assignments() { Runnable r = - executor.submit( - () -> { - runSomethingThatIsLong(); - runSomethingElseThatIsAlsoLong(); - }); + executor.submit(() -> { + runSomethingThatIsLong(); + runSomethingElseThatIsAlsoLong(); + }); x = list.stream() .map(element -> element.transformWithSomethingLong().andSomethingElse()) @@ -91,25 +87,22 @@ class LambdaHug { void f() {} }; callbacks += - register( - event -> { - handle(event); - handleAgain(event); - handleOnceMore(event); - }); + register(event -> { + handle(event); + handleAgain(event); + handleOnceMore(event); + }); x = someObject .withSomeMethod(argument) .andAnotherMethod(argument) - .thenAccept( - y -> { - consume(y); - }); - ImmutableMap> someLongVariableName = - compute( - k -> { - return k; + .thenAccept(y -> { + consume(y); }); + ImmutableMap> someLongVariableName = + compute(k -> { + return k; + }); } private final Runnable field = @@ -121,32 +114,27 @@ class LambdaHug { void chains() { list.stream() .filter(x -> x.isSomethingThatIsLong() && x.isSomethingElseThatIsLong() && x.isThirdThing()) - .map( - x -> { - Object y = x.transform(); - return y.transformAgainWithSomethingLonger(); - }) + .map(x -> { + Object y = x.transform(); + return y.transformAgainWithSomethingLonger(); + }) .forEach(System.out::println); CompletableFuture.supplyAsync(() -> computeSomethingExpensive(input, parameters)) - .thenAccept( - result -> { - consume(result); - consumeAgain(result); - }); + .thenAccept(result -> { + consume(result); + consumeAgain(result); + }); someObject.someModerateLengthFunctionName( foo -> doOtherModerateLengthName(foo, bar, baz, qux, quux)); } void nested() { - outer( - a -> - inner( - b -> - innermost( - c -> { - doSomethingWith(a, b, c); - doSomethingElseWith(a, b, c); - }))); + outer(a -> + inner(b -> + innermost(c -> { + doSomethingWith(a, b, c); + doSomethingElseWith(a, b, c); + }))); } void comments() { @@ -154,9 +142,8 @@ class LambdaHug { // comment x -> System.err.println("hello, world, this is a long string that doesn't fit on the line")); - f( - x -> { // trailing comment - run(); - }); + f(x -> { // trailing comment + run(); + }); } }