diff --git a/changelog/@unreleased/pr-257.v2.yml b/changelog/@unreleased/pr-257.v2.yml new file mode 100644 index 000000000..dd52a3dd4 --- /dev/null +++ b/changelog/@unreleased/pr-257.v2.yml @@ -0,0 +1,6 @@ +type: fix +fix: + description: Fix edge case with empty levels causing formattings very close to the + column limit to throw. + links: + - https://github.com/palantir/palantir-java-format/pull/257 diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Level.java b/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Level.java index abb28e883..be040e01a 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Level.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Level.java @@ -42,6 +42,7 @@ import java.util.OptionalInt; import java.util.stream.Collector; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.immutables.value.Value; /** A {@code Level} inside a {@link Doc}. */ @@ -262,10 +263,7 @@ private Optional handle_breakOnlyIfInnerLevelsThenFitOnOneLine( // Note: we are not checking if the brokenState produced one extra line compared to state, as this can be // misleading if there is no level but a single comment that got reflowed onto multiple lines (see palantir-11). - boolean anyLevelWasBroken = docs.stream() - .filter(doc -> doc instanceof Level) - .map(doc -> ((Level) doc)) - .anyMatch(level -> !brokenState.isOneLine(level)); + boolean anyLevelWasBroken = getNonEmptyInnerLevels().anyMatch(level -> !brokenState.isOneLine(level)); if (!anyLevelWasBroken) { return Optional.of(brokenState); @@ -278,10 +276,7 @@ private Optional handle_breakOnlyIfInnerLevelsThenFitOnOneLine( } State partiallyInlinedState = partiallyInlinedStateOpt.get(); - boolean bodyIsComplex = this.docs.stream() - .filter(doc -> doc instanceof Level) - .map(doc -> ((Level) doc)) - .anyMatch(il -> il.openOp.complexity() == Complexity.COMPLEX); + boolean bodyIsComplex = getNonEmptyInnerLevels().anyMatch(il -> il.openOp.complexity() == Complexity.COMPLEX); if (bodyIsComplex || partiallyInlinedState.numLines() < brokenState.numLines()) { return Optional.of(partiallyInlinedState); @@ -289,6 +284,13 @@ private Optional handle_breakOnlyIfInnerLevelsThenFitOnOneLine( return Optional.empty(); } + private Stream getNonEmptyInnerLevels() { + return docs.stream() + .filter(doc -> doc instanceof Level) + .map(doc -> ((Level) doc)) + .filter(doc -> StartsWithBreakVisitor.INSTANCE.visit(doc) != Result.EMPTY); + } + private Optional tryInlinePrefixOntoCurrentLine( CommentsHelper commentsHelper, int maxWidth, @@ -297,15 +299,9 @@ private Optional tryInlinePrefixOntoCurrentLine( Obs.ExplorationNode explorationNode) { // Find the last level, skipping empty levels (that contain nothing, or are made up // entirely of other empty levels). - List innerLevels = this.docs.stream() - .filter(doc -> doc instanceof Level) - .map(doc -> ((Level) doc)) - .collect(Collectors.toList()); - // Last level because there might be other in-between levels after the initial break like `new - // int[] - // {`, and we want to skip those. - Level lastLevel = innerLevels.stream() + // int[] {`, and we want to skip those. + Level lastLevel = getNonEmptyInnerLevels() .filter(doc -> StartsWithBreakVisitor.INSTANCE.visit(doc) != Result.EMPTY) .collect(GET_LAST_COLLECTOR) .orElseThrow(() -> diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-12.input b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-12.input new file mode 100644 index 000000000..fd39ea725 --- /dev/null +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-12.input @@ -0,0 +1,25 @@ +public class Palantir12 { + + @Override + public + void + foo() { + new DogWalker() + .walk( + DogBreed.FOX_TERRIER, + new RoadWalker() { + @Override + public Void call(Dog dog) { + routes.forEach((route) -> route.getCoordinates() + .forEach((coordinate) -> { + coordinate.getOtherDogs().forEach(otherDog -> { + if (!dog.likes(otherDog)) { + badEncounters.computeIfAbsent(coordinate, _x -> new HashSet<>()) + .add(dog).add(otherDog); + } + }); + })); + } + }); + } +} diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-12.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-12.output new file mode 100644 index 000000000..faf9e251a --- /dev/null +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-12.output @@ -0,0 +1,21 @@ +public class Palantir12 { + + @Override + public void foo() { + new DogWalker().walk(DogBreed.FOX_TERRIER, new RoadWalker() { + @Override + public Void call(Dog dog) { + routes.forEach((route) -> route.getCoordinates().forEach((coordinate) -> { + coordinate.getOtherDogs().forEach(otherDog -> { + if (!dog.likes(otherDog)) { + badEncounters + .computeIfAbsent(coordinate, _x -> new HashSet<>()) + .add(dog) + .add(otherDog); + } + }); + })); + } + }); + } +}