Skip to content

Commit

Permalink
Fix possible exception with initializer body close to column limit (#257
Browse files Browse the repository at this point in the history
)

Fix edge case with empty levels causing formattings very close to the column limit to throw.
  • Loading branch information
dansanduleac authored Mar 30, 2020
1 parent 1848986 commit 438feb7
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 16 deletions.
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-257.v2.yml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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}. */
Expand Down Expand Up @@ -262,10 +263,7 @@ private Optional<State> 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);
Expand All @@ -278,17 +276,21 @@ private Optional<State> 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);
}
return Optional.empty();
}

private Stream<Level> getNonEmptyInnerLevels() {
return docs.stream()
.filter(doc -> doc instanceof Level)
.map(doc -> ((Level) doc))
.filter(doc -> StartsWithBreakVisitor.INSTANCE.visit(doc) != Result.EMPTY);
}

private Optional<State> tryInlinePrefixOntoCurrentLine(
CommentsHelper commentsHelper,
int maxWidth,
Expand All @@ -297,15 +299,9 @@ private Optional<State> tryInlinePrefixOntoCurrentLine(
Obs.ExplorationNode explorationNode) {
// Find the last level, skipping empty levels (that contain nothing, or are made up
// entirely of other empty levels).
List<Level> 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(() ->
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
public class Palantir12 {

@Override
public
void
foo() {
new DogWalker()
.walk(
DogBreed.FOX_TERRIER,
new RoadWalker<Void>() {
@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);
}
});
}));
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
public class Palantir12 {

@Override
public void foo() {
new DogWalker().walk(DogBreed.FOX_TERRIER, new RoadWalker<Void>() {
@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);
}
});
}));
}
});
}
}

0 comments on commit 438feb7

Please sign in to comment.