From 5c5b7b9e818a007f2cfe73b3d889ff678895c9ea Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Wed, 27 Nov 2019 16:06:50 +0000 Subject: [PATCH 1/3] fix bug --- .../src/main/java/com/palantir/javaformat/doc/Level.java | 7 +++++-- .../testdata/palantir-conjure-java-runtime-1285-1.output | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) 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 1a21c0b33..a513f2821 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 @@ -295,8 +295,11 @@ private Optional tryBreakLastLevel( SplitsBreaks prefixSplitsBreaks = splitByBreaks(leadingDocs); State state1 = tryToLayOutLevelOnOneLine(commentsHelper, maxWidth, state, prefixSplitsBreaks, explorationNode); - Preconditions.checkState( - !state1.mustBreak(), "We messed up, it wants to break a bunch of splits that shouldn't be broken"); + // If a break was still forced somehow even though we could fit the leadingWidth, then abort. + // This could happen if inner levels have set a `columnLimitBeforeLastBreak` or something like that. + if (state1.numLines() != state.numLines()) { + return Optional.empty(); + } // Ok now how to handle the last level? // There are two options: diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-conjure-java-runtime-1285-1.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-conjure-java-runtime-1285-1.output index a011da382..0c6194789 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-conjure-java-runtime-1285-1.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-conjure-java-runtime-1285-1.output @@ -8,8 +8,10 @@ class PalantirConjureJavaRuntime1285_SslSocketFactories { KeyManager[] keyManagers = null; if (config.keyStorePath().isPresent()) { keyManagers = createKeyManagerFactory( - config.keyStorePath().get(), config.keyStorePassword() - .get(), config.keyStoreType(), config.keyStoreKeyAlias()) + config.keyStorePath().get(), + config.keyStorePassword().get(), + config.keyStoreType(), + config.keyStoreKeyAlias()) .getKeyManagers(); } From f417f99ad9c723f052d8189eb2875728a2835fd0 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Wed, 27 Nov 2019 16:06:50 +0000 Subject: [PATCH 2/3] Add generated changelog entries --- changelog/@unreleased/pr-85.v2.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changelog/@unreleased/pr-85.v2.yml diff --git a/changelog/@unreleased/pr-85.v2.yml b/changelog/@unreleased/pr-85.v2.yml new file mode 100644 index 000000000..51a8e5c00 --- /dev/null +++ b/changelog/@unreleased/pr-85.v2.yml @@ -0,0 +1,11 @@ +type: fix +fix: + description: "When inlining a level's leading docs, check that no breaks were introduced + more robustly.\n\nWe already did some validation that the leading docs \n (1) + don't contain forced breaks, and\n (2) can fit onto the current line\nHowever + with the new logic added in #71, inner levels might decide to break even when + the above two conditions are satisfied.\n\nWe 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." + links: + - https://github.com/palantir/palantir-java-format/pull/85 From b5f160257bc1181cb482df466836dd3539e24ad0 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Wed, 27 Nov 2019 16:30:12 +0000 Subject: [PATCH 3/3] fix changelog --- changelog/@unreleased/pr-85.v2.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/changelog/@unreleased/pr-85.v2.yml b/changelog/@unreleased/pr-85.v2.yml index 51a8e5c00..751833647 100644 --- a/changelog/@unreleased/pr-85.v2.yml +++ b/changelog/@unreleased/pr-85.v2.yml @@ -1,11 +1,6 @@ type: fix fix: description: "When inlining a level's leading docs, check that no breaks were introduced - more robustly.\n\nWe already did some validation that the leading docs \n (1) - don't contain forced breaks, and\n (2) can fit onto the current line\nHowever - with the new logic added in #71, inner levels might decide to break even when - the above two conditions are satisfied.\n\nWe 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." + more robustly." links: - https://github.com/palantir/palantir-java-format/pull/85