From bac92c01e4a614350168b03ce70fb4ee20fff31c Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 12:55:20 +0000 Subject: [PATCH 01/10] add test we don't like --- .../java/testdata/palantir-10.input | 10 ++++++++++ .../java/testdata/palantir-10.output | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input create mode 100644 palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input new file mode 100644 index 000000000..e6a8ee808 --- /dev/null +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input @@ -0,0 +1,10 @@ +class Palantir10 { + @SqlUpdate("CREATE TABLE things " + + "(id VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + + "title VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + + "description VARCHAR(" + MAX_DESCRIPTION_LENGTH + ") NOT NULL, " + + "standard BOOLEAN NOT NULL, " + + "documentJson VARCHAR(" + MAX_DOCUMENT_LENGTH + ") NOT NULL, " + + "PRIMARY KEY (id))") + void createTable(); +} diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output new file mode 100644 index 000000000..8e8bda46c --- /dev/null +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output @@ -0,0 +1,19 @@ +class Palantir10 { + @SqlUpdate( + "CREATE TABLE things " + + "(id VARCHAR(" + + MAX_LENGTH + + ") NOT NULL, " + + "title VARCHAR(" + + MAX_LENGTH + + ") NOT NULL, " + + "description VARCHAR(" + + MAX_DESCRIPTION_LENGTH + + ") NOT NULL, " + + "standard BOOLEAN NOT NULL, " + + "documentJson VARCHAR(" + + MAX_DOCUMENT_LENGTH + + ") NOT NULL, " + + "PRIMARY KEY (id))") + void createTable(); +} From 330de8045a075297a976d7894cff8f70d2102091 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 14:17:35 +0000 Subject: [PATCH 02/10] preserve existing string concat newlines --- .../com/palantir/javaformat/OpsBuilder.java | 13 ++++++- .../com/palantir/javaformat/doc/Token.java | 2 +- .../javaformat/java/JavaInputAstVisitor.java | 35 +++++++++++++++++-- .../java/testdata/palantir-10.output | 17 +++------ 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java b/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java index 949193e88..41b4b769a 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java @@ -15,11 +15,13 @@ package com.palantir.javaformat; import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; import com.palantir.javaformat.Indent.Const; +import com.palantir.javaformat.Input.Tok; import com.palantir.javaformat.doc.Break; import com.palantir.javaformat.doc.BreakTag; import com.palantir.javaformat.doc.Comment; @@ -29,6 +31,7 @@ import com.palantir.javaformat.doc.NonBreakingSpace; import com.palantir.javaformat.doc.State; import com.palantir.javaformat.doc.Token; +import com.palantir.javaformat.doc.Token.RealOrImaginary; import com.palantir.javaformat.java.FormatterDiagnostic; import com.palantir.javaformat.java.InputMetadata; import com.palantir.javaformat.java.InputMetadataBuilder; @@ -303,6 +306,14 @@ public Optional peekToken() { return peekToken(0); } + /** Return whether the last token emitted is followed by a newline, but not a {@code //} comment. */ + public boolean lastTokenFollowedByNewline() { + Preconditions.checkState(tokenI > 0, "No token was emitted yet"); + ImmutableList afterLast = + input.getTokens().get(tokenI - 1).getToksAfter(); + return afterLast.stream().anyMatch(Tok::isNewline) && afterLast.stream().noneMatch(Tok::isSlashSlashComment); + } + /** Return the text of an upcoming {@link Input.Token}, or absent if there is none. */ public Optional peekToken(int skip) { ImmutableList tokens = input.getTokens(); @@ -329,7 +340,7 @@ public void token( if (token.equals(peekToken().orElse(null))) { // Found the input token. Output it. add(Token.make( tokens.get(tokenI++), - Token.RealOrImaginary.REAL, + RealOrImaginary.REAL, plusIndentCommentsBefore, breakAndIndentTrailingComment)); } else { diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java b/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java index 6227a3410..d8f68fc3e 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java @@ -73,7 +73,7 @@ public Optional breakAndIndentTrailingComment() { * @param plusIndentCommentsBefore extra {@code plusIndent} for comments just before this token * @return the new {@code Token} */ - public static Op make( + public static Token make( Input.Token token, RealOrImaginary realOrImaginary, Indent plusIndentCommentsBefore, diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java index e5f6a6fc1..452064148 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java @@ -1144,18 +1144,24 @@ public Void visitBinary(BinaryTree node, Void unused) { List operands = new ArrayList<>(); List operators = new ArrayList<>(); walkInfix(precedence(node), node, operands, operators); - FillMode fillMode = hasOnlyShortItems(operands) ? INDEPENDENT : UNIFIED; + boolean isStringConcat = isStringConcat(node); + FillMode fillMode = hasOnlyShortItems(operands) || isStringConcat ? INDEPENDENT : UNIFIED; builder.open( plusFour, BreakBehaviours.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN_IF_SIMPLE_OTHERWISE_CHECK_INNER); scan(operands.get(0), null); + FillMode nextFillMode = builder.lastTokenFollowedByNewline() ? UNIFIED : fillMode; int operatorsN = operators.size(); for (int i = 0; i < operatorsN; i++) { - builder.breakOp(fillMode, " ", ZERO); + builder.breakOp(nextFillMode, " ", ZERO); builder.op(operators.get(i)); + boolean shouldEnforceNewline = builder.lastTokenFollowedByNewline(); builder.space(); scan(operands.get(i + 1), null); + shouldEnforceNewline |= builder.lastTokenFollowedByNewline(); + + nextFillMode = isStringConcat && shouldEnforceNewline ? UNIFIED : fillMode; } builder.close(); return null; @@ -3148,12 +3154,35 @@ private boolean isFormatMethod(List arguments) { if (arguments.size() < 2) { return false; } - return isStringConcat(arguments.get(0)); + return isFormatString(arguments.get(0)); } private static final Pattern FORMAT_SPECIFIER = Pattern.compile("%|\\{[0-9]\\}"); private boolean isStringConcat(ExpressionTree first) { + final boolean[] stringConcat = {false}; + new TreeScanner() { + @Override + public void scan(JCTree tree) { + if (tree == null) { + return; + } + switch (tree.getKind()) { + case STRING_LITERAL: + stringConcat[0] = true; + break; + case PLUS: + super.scan(tree); + break; + default: + break; + } + } + }.scan((JCTree) first); + return stringConcat[0]; + } + + private boolean isFormatString(ExpressionTree first) { final boolean[] stringLiteral = {true}; final boolean[] formatString = {false}; new TreeScanner() { diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output index 8e8bda46c..30f47c458 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output @@ -1,19 +1,10 @@ class Palantir10 { @SqlUpdate( - "CREATE TABLE things " - + "(id VARCHAR(" - + MAX_LENGTH - + ") NOT NULL, " - + "title VARCHAR(" - + MAX_LENGTH - + ") NOT NULL, " - + "description VARCHAR(" - + MAX_DESCRIPTION_LENGTH - + ") NOT NULL, " + "CREATE TABLE things " + "(id VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + + "title VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + + "description VARCHAR(" + MAX_DESCRIPTION_LENGTH + ") NOT NULL, " + "standard BOOLEAN NOT NULL, " - + "documentJson VARCHAR(" - + MAX_DOCUMENT_LENGTH - + ") NOT NULL, " + + "documentJson VARCHAR(" + MAX_DOCUMENT_LENGTH + ") NOT NULL, " + "PRIMARY KEY (id))") void createTable(); } From 986c6fb337dadd59e01f54d1cb06c27998446a81 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 14:26:52 +0000 Subject: [PATCH 03/10] allow straight up inlining single annotation param --- .../javaformat/java/JavaInputAstVisitor.java | 5 ++++- .../javaformat/java/testdata/B21305044.output | 8 +++----- .../com/palantir/javaformat/java/testdata/S.output | 5 ++--- .../javaformat/java/testdata/palantir-10.output | 14 +++++++------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java index 452064148..f0fe3ff79 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java @@ -1785,7 +1785,10 @@ boolean visitSingleMemberAnnotation(AnnotationTree node) { return false; } boolean isArrayInitializer = value.getKind() == NEW_ARRAY; - builder.open(isArrayInitializer ? ZERO : plusFour); + builder.open( + isArrayInitializer ? ZERO : plusFour, + BreakBehaviours.preferBreakingLastInnerLevel(true), + LastLevelBreakability.CHECK_INNER); token("@"); scan(node.getAnnotationType(), null); token("("); diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/B21305044.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/B21305044.output index 63a0009bd..16f11eeec 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/B21305044.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/B21305044.output @@ -17,11 +17,9 @@ class B21305044 { ? super @Crazy( key = value, - other = - @Crazier({ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, - 20 - })) + other = @Crazier({ + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 + })) Object> c; } diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/S.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/S.output index 7faa33254..5814f56f2 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/S.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/S.output @@ -11,9 +11,8 @@ class S { int x = 0; - @SingleMemberAnnotation( - 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 - + 0 + 0 + 0 + 0 + 0) + @SingleMemberAnnotation(0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0 + 0) S() { super(); } diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output index 30f47c458..e6a8ee808 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output @@ -1,10 +1,10 @@ class Palantir10 { - @SqlUpdate( - "CREATE TABLE things " + "(id VARCHAR(" + MAX_LENGTH + ") NOT NULL, " - + "title VARCHAR(" + MAX_LENGTH + ") NOT NULL, " - + "description VARCHAR(" + MAX_DESCRIPTION_LENGTH + ") NOT NULL, " - + "standard BOOLEAN NOT NULL, " - + "documentJson VARCHAR(" + MAX_DOCUMENT_LENGTH + ") NOT NULL, " - + "PRIMARY KEY (id))") + @SqlUpdate("CREATE TABLE things " + + "(id VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + + "title VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + + "description VARCHAR(" + MAX_DESCRIPTION_LENGTH + ") NOT NULL, " + + "standard BOOLEAN NOT NULL, " + + "documentJson VARCHAR(" + MAX_DOCUMENT_LENGTH + ") NOT NULL, " + + "PRIMARY KEY (id))") void createTable(); } From d23f312362467d7bafcbb442baa0d3cd46343b38 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 14:32:28 +0000 Subject: [PATCH 04/10] undo that --- .../src/main/java/com/palantir/javaformat/doc/Token.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java b/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java index d8f68fc3e..6227a3410 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/doc/Token.java @@ -73,7 +73,7 @@ public Optional breakAndIndentTrailingComment() { * @param plusIndentCommentsBefore extra {@code plusIndent} for comments just before this token * @return the new {@code Token} */ - public static Token make( + public static Op make( Input.Token token, RealOrImaginary realOrImaginary, Indent plusIndentCommentsBefore, From 5c965832b906385171ee67520c165a214bf8c493 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 14:33:29 +0000 Subject: [PATCH 05/10] simplify --- .../src/main/java/com/palantir/javaformat/OpsBuilder.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java b/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java index 41b4b769a..552805b10 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java @@ -306,12 +306,10 @@ public Optional peekToken() { return peekToken(0); } - /** Return whether the last token emitted is followed by a newline, but not a {@code //} comment. */ + /** Return whether the last token emitted is followed by a newline. */ public boolean lastTokenFollowedByNewline() { Preconditions.checkState(tokenI > 0, "No token was emitted yet"); - ImmutableList afterLast = - input.getTokens().get(tokenI - 1).getToksAfter(); - return afterLast.stream().anyMatch(Tok::isNewline) && afterLast.stream().noneMatch(Tok::isSlashSlashComment); + return input.getTokens().get(tokenI - 1).getToksAfter().stream().anyMatch(Tok::isNewline); } /** Return the text of an upcoming {@link Input.Token}, or absent if there is none. */ From 08cf07838d5ec107d95776bbb8952e6385535c05 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 14:33:29 +0000 Subject: [PATCH 06/10] Add generated changelog entries --- changelog/@unreleased/pr-192.v2.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/@unreleased/pr-192.v2.yml diff --git a/changelog/@unreleased/pr-192.v2.yml b/changelog/@unreleased/pr-192.v2.yml new file mode 100644 index 000000000..ac8aa99b0 --- /dev/null +++ b/changelog/@unreleased/pr-192.v2.yml @@ -0,0 +1,6 @@ +type: improvement +improvement: + description: Preserve the user's newlines in a long string concatenation (defined + as a concatenation with `+` where at least one expression is a literal string). + links: + - https://github.com/palantir/palantir-java-format/pull/192 From c4c7ad8a794ec4ce9aeacb1bf1e1628122845624 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 15:31:13 +0000 Subject: [PATCH 07/10] short-circuit --- .../java/com/palantir/javaformat/java/JavaInputAstVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java index f0fe3ff79..0357900a2 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java @@ -1159,7 +1159,7 @@ public Void visitBinary(BinaryTree node, Void unused) { boolean shouldEnforceNewline = builder.lastTokenFollowedByNewline(); builder.space(); scan(operands.get(i + 1), null); - shouldEnforceNewline |= builder.lastTokenFollowedByNewline(); + shouldEnforceNewline = shouldEnforceNewline || builder.lastTokenFollowedByNewline(); nextFillMode = isStringConcat && shouldEnforceNewline ? UNIFIED : fillMode; } From 344fdea0ed529873bc5d92fc52d9d14338f3f773 Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 15:31:45 +0000 Subject: [PATCH 08/10] mostRecentToken --- .../src/main/java/com/palantir/javaformat/OpsBuilder.java | 2 +- .../com/palantir/javaformat/java/JavaInputAstVisitor.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java b/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java index 552805b10..c768ac467 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java @@ -307,7 +307,7 @@ public Optional peekToken() { } /** Return whether the last token emitted is followed by a newline. */ - public boolean lastTokenFollowedByNewline() { + public boolean mostRecentTokenFollowedByNewline() { Preconditions.checkState(tokenI > 0, "No token was emitted yet"); return input.getTokens().get(tokenI - 1).getToksAfter().stream().anyMatch(Tok::isNewline); } diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java index 0357900a2..c67dbb971 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java @@ -1151,15 +1151,15 @@ public Void visitBinary(BinaryTree node, Void unused) { BreakBehaviours.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN_IF_SIMPLE_OTHERWISE_CHECK_INNER); scan(operands.get(0), null); - FillMode nextFillMode = builder.lastTokenFollowedByNewline() ? UNIFIED : fillMode; + FillMode nextFillMode = builder.mostRecentTokenFollowedByNewline() ? UNIFIED : fillMode; int operatorsN = operators.size(); for (int i = 0; i < operatorsN; i++) { builder.breakOp(nextFillMode, " ", ZERO); builder.op(operators.get(i)); - boolean shouldEnforceNewline = builder.lastTokenFollowedByNewline(); + boolean shouldEnforceNewline = builder.mostRecentTokenFollowedByNewline(); builder.space(); scan(operands.get(i + 1), null); - shouldEnforceNewline = shouldEnforceNewline || builder.lastTokenFollowedByNewline(); + shouldEnforceNewline = shouldEnforceNewline || builder.mostRecentTokenFollowedByNewline(); nextFillMode = isStringConcat && shouldEnforceNewline ? UNIFIED : fillMode; } From 13b2d47de5908216281f1b48a9507dbe549400bc Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 15:56:26 +0000 Subject: [PATCH 09/10] make logic ignore binary expressions spanning less than 3 lines --- .../javaformat/java/JavaInputAstVisitor.java | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java index c67dbb971..b798d6524 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/JavaInputAstVisitor.java @@ -84,6 +84,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.IntFunction; import java.util.regex.Pattern; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -1145,23 +1146,24 @@ public Void visitBinary(BinaryTree node, Void unused) { List operators = new ArrayList<>(); walkInfix(precedence(node), node, operands, operators); boolean isStringConcat = isStringConcat(node); + boolean shouldPreserveNewlines = isStringConcat && lineSpan(node) > 2; FillMode fillMode = hasOnlyShortItems(operands) || isStringConcat ? INDEPENDENT : UNIFIED; + builder.open( plusFour, BreakBehaviours.breakThisLevel(), LastLevelBreakability.ACCEPT_INLINE_CHAIN_IF_SIMPLE_OTHERWISE_CHECK_INNER); scan(operands.get(0), null); - FillMode nextFillMode = builder.mostRecentTokenFollowedByNewline() ? UNIFIED : fillMode; int operatorsN = operators.size(); + boolean shouldEnforceNewline = builder.mostRecentTokenFollowedByNewline(); for (int i = 0; i < operatorsN; i++) { + FillMode nextFillMode = shouldPreserveNewlines && shouldEnforceNewline ? UNIFIED : fillMode; builder.breakOp(nextFillMode, " ", ZERO); builder.op(operators.get(i)); - boolean shouldEnforceNewline = builder.mostRecentTokenFollowedByNewline(); + shouldEnforceNewline = builder.mostRecentTokenFollowedByNewline(); builder.space(); scan(operands.get(i + 1), null); shouldEnforceNewline = shouldEnforceNewline || builder.mostRecentTokenFollowedByNewline(); - - nextFillMode = isStringConcat && shouldEnforceNewline ? UNIFIED : fillMode; } builder.close(); return null; @@ -3300,6 +3302,17 @@ private Integer actualColumn(ExpressionTree expression) { return positionToColumnMap.get(builder.actualStartColumn(getStartPosition(expression))); } + /** How many lines does this node take up in the input. Returns at least 1. */ + int lineSpan(Tree node) { + IntFunction lineNumberAt = tokenPosition -> { + Input.Token token = builder.getInput().getPositionTokenMap().get(tokenPosition); + return builder.getInput().getLineNumber(token.getTok().getPosition()); + }; + return lineNumberAt.apply(getEndPosition(node, getCurrentPath())) + - lineNumberAt.apply(getStartPosition(node)) + + 1; + } + /** Returns true if {@code atLeastM} of the expressions in the given column are the same kind. */ private static boolean expressionsAreParallel(List> rows, int column, int atLeastM) { Multiset nodeTypes = HashMultiset.create(); From 09aed98e5a02d03c1b06d6c3e6e5ce7021b380ee Mon Sep 17 00:00:00 2001 From: Dan Sanduleac Date: Thu, 13 Feb 2020 15:58:57 +0000 Subject: [PATCH 10/10] test that we still reflow short string concatenations --- .../javaformat/java/testdata/palantir-10.input | 10 ++++++++++ .../javaformat/java/testdata/palantir-10.output | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input index e6a8ee808..2ffc0edcc 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.input @@ -1,4 +1,5 @@ class Palantir10 { + // We preserve input newlines for this long string concatenation (more than 2 lines) @SqlUpdate("CREATE TABLE things " + "(id VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + "title VARCHAR(" + MAX_LENGTH + ") NOT NULL, " @@ -7,4 +8,13 @@ class Palantir10 { + "documentJson VARCHAR(" + MAX_DOCUMENT_LENGTH + ") NOT NULL, " + "PRIMARY KEY (id))") void createTable(); + + void foo() { + // Test that we still reflow short string concatenations (spanning exactly 2 lines) + foo( + "foo" + + "bar" + THING + "baz"); + foo("foo" + + "bar" + THING + "baz"); + } } diff --git a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output index e6a8ee808..c15a8d2f5 100644 --- a/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output +++ b/palantir-java-format/src/test/resources/com/palantir/javaformat/java/testdata/palantir-10.output @@ -1,4 +1,5 @@ class Palantir10 { + // We preserve input newlines for this long string concatenation (more than 2 lines) @SqlUpdate("CREATE TABLE things " + "(id VARCHAR(" + MAX_LENGTH + ") NOT NULL, " + "title VARCHAR(" + MAX_LENGTH + ") NOT NULL, " @@ -7,4 +8,10 @@ class Palantir10 { + "documentJson VARCHAR(" + MAX_DOCUMENT_LENGTH + ") NOT NULL, " + "PRIMARY KEY (id))") void createTable(); + + void foo() { + // Test that we still reflow short string concatenations (spanning exactly 2 lines) + foo("foo" + "bar" + THING + "baz"); + foo("foo" + "bar" + THING + "baz"); + } }