Skip to content

Commit f64dd35

Browse files
dansanduleacbulldozer-bot[bot]
authored andcommitted
Fix: don't inline level prefix if breaks occurred inside (#85)
When inlining a level's leading docs, check that no breaks were introduced more robustly. We already did some validation that the leading docs (1) don't contain forced breaks, and (2) can fit onto the current line However with the new logic added in #71, inner levels might decide to break even when the above two conditions are satisfied. We guard against this by checking whether the state after the inlining of leading docs has recorded new lines, which would be caused by an inner break being taken.
1 parent 6fd07f2 commit f64dd35

File tree

3 files changed

+15
-4
lines changed

3 files changed

+15
-4
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
type: fix
2+
fix:
3+
description: "When inlining a level's leading docs, check that no breaks were introduced
4+
more robustly."
5+
links:
6+
- https://github.com/palantir/palantir-java-format/pull/85

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,11 @@ private Optional<State> tryBreakLastLevel(
295295
SplitsBreaks prefixSplitsBreaks = splitByBreaks(leadingDocs);
296296

297297
State state1 = tryToLayOutLevelOnOneLine(commentsHelper, maxWidth, state, prefixSplitsBreaks, explorationNode);
298-
Preconditions.checkState(
299-
!state1.mustBreak(), "We messed up, it wants to break a bunch of splits that shouldn't be broken");
298+
// If a break was still forced somehow even though we could fit the leadingWidth, then abort.
299+
// This could happen if inner levels have set a `columnLimitBeforeLastBreak` or something like that.
300+
if (state1.numLines() != state.numLines()) {
301+
return Optional.empty();
302+
}
300303

301304
// Ok now how to handle the last level?
302305
// There are two options:

palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-conjure-java-runtime-1285-1.output

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ class PalantirConjureJavaRuntime1285_SslSocketFactories {
88
KeyManager[] keyManagers = null;
99
if (config.keyStorePath().isPresent()) {
1010
keyManagers = createKeyManagerFactory(
11-
config.keyStorePath().get(), config.keyStorePassword()
12-
.get(), config.keyStoreType(), config.keyStoreKeyAlias())
11+
config.keyStorePath().get(),
12+
config.keyStorePassword().get(),
13+
config.keyStoreType(),
14+
config.keyStoreKeyAlias())
1315
.getKeyManagers();
1416
}
1517

0 commit comments

Comments
 (0)