diff --git a/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java b/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java index 063843622..c0f16e9b5 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java +++ b/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java @@ -239,9 +239,10 @@ static int hasEscapedNewlineAt(String input, int idx) { * @param separator the line separator * @param columnLimit the number of columns to wrap at * @param startColumn the column position of the beginning of the original text - * @param trailing extra space to leave after the last line - * @param components the text to reflow - * @param first0 true if the text includes the beginning of its enclosing concat chain, i.e. a + * @param trailing extra space to leave after the last line, to accommodate a ; or ) + * @param components the text to reflow. This is a list of “words” of a single literal. Its first + * and last quotes have been stripped + * @param first0 true if the text includes the beginning of its enclosing concat chain */ private static String reflow( String separator, @@ -251,7 +252,7 @@ private static String reflow( ImmutableList components, boolean first0) { // We have space between the start column and the limit to output the first line. - // Reserve two spaces for the quotes. + // Reserve two spaces for the start and end quotes. int width = columnLimit - startColumn - 2; Deque input = new ArrayDeque<>(components); List lines = new ArrayList<>(); @@ -259,10 +260,13 @@ private static String reflow( while (!input.isEmpty()) { int length = 0; List line = new ArrayList<>(); + // If we know this is going to be the last line, then remove a bit of width to account for the + // trailing characters. if (input.stream().mapToInt(String::length).sum() <= width) { + // This isn’t quite optimal, but arguably good enough. See b/179561701 width -= trailing; } - while (!input.isEmpty() && (length <= 4 || (length + input.peekFirst().length()) < width)) { + while (!input.isEmpty() && (length <= 4 || (length + input.peekFirst().length()) <= width)) { String text = input.removeFirst(); line.add(text); length += text.length(); diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java index 44ba63925..fdcf6f8a8 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java @@ -125,7 +125,9 @@ public FormatterIntegrationTest(String name, String input, String expected) { @Test public void format() { try { - String output = new Formatter().formatSource(input); + Formatter formatter = new Formatter(); + String output = formatter.formatSource(input); + output = StringWrapper.wrap(output, formatter); assertEquals("bad output for " + name, expected, output); } catch (FormatterException e) { fail(String.format("Formatter crashed on %s: %s", name, e.getMessage())); diff --git a/core/src/test/java/com/google/googlejavaformat/java/MainTest.java b/core/src/test/java/com/google/googlejavaformat/java/MainTest.java index fa23486fd..a7e10fee7 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/MainTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/MainTest.java @@ -525,8 +525,8 @@ public void reflowLongStrings() throws Exception { "class T {", " String s =", " \"one long incredibly unbroken sentence moving from topic to topic so that no one had" - + " a\"", - " + \" chance to interrupt\";", + + " a chance\"", + " + \" to interrupt\";", "}", "", }; diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B173808510.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B173808510.input new file mode 100644 index 000000000..e3e849399 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B173808510.input @@ -0,0 +1,8 @@ +class B173808510 { + // b/173808510 + @FlagSpec( + name = "myFlag", + help = + "areallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyloongword word1 word2") + Flag dummy = null; +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B173808510.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B173808510.output new file mode 100644 index 000000000..45a939efb --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B173808510.output @@ -0,0 +1,9 @@ +class B173808510 { + // b/173808510 + @FlagSpec( + name = "myFlag", + help = + "areallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyloongword word1" + + " word2") + Flag dummy = null; +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/LiteralReflow.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/LiteralReflow.input new file mode 100644 index 000000000..2aa4de330 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/LiteralReflow.input @@ -0,0 +1,44 @@ +class LiteralReflow { + static class TestLineBreak { + String doesNotBreakAt100 = "A very long long long long long long long long long loong sentence"; + String breaksAt101 = "A very long long long long long long long long long long loooong sentence"; + } + + static class TestReflowLimit { + String doesNotReflowAt100 = + "A very long long long long long long long long long long long long long looooong sentence"; + String reflowsWhenLongerThan100 = + "A very long long long long long long long long long long long long long long long sentence"; + } + + static class TestReflowLocation { + String accommodatesWordsUpTo100 = + "A very long long long long long long long long long long long long long long long looooong sentence"; + String breaksBeforeWordsReach101 = + "A very long long long long long long long long long long long long long long long loooooong sentence"; + } + + static class Test2LineReflowLimit { + String doesNotReflowEitherLinesAt100 = + "A very long long long long long long long long long long long long long looooong sentence. And a second very long long long long long long long long long long loong sentence"; + String reflowsLastLineAt101 = + "A very long long long long long long long long long long long long long looooong sentence. And a second very long long long long long long long long long long looong sentence"; + } + + static class TestWithTrailingCharacters { + String fitsLastLineUpTo100WithTrailingCharacters = + f( + f( + "A very long long long long long long long long long long long long loong sentence. And a second very long long long long long long long long loong sentence")); + String reflowsLastLineToAccommodateTrailingCharacters = + f( + f( + "A very long long long long long long long long long long long long loong sentence. And a second very long long long long long long long long looong sentence")); + // Tests an off-by-one issue, but see b/179561701 for a similar issue that is not yet fixed + String doesNotOverTriggerLastLineReflow = + f( + f( + "A very long long long long long long long long long long long long loong sentence." + + " And a second very loong sentence with trailing a a a a a a a a a a a a a a a")); + } +} diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/LiteralReflow.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/LiteralReflow.output new file mode 100644 index 000000000..50ed7bda1 --- /dev/null +++ b/core/src/test/resources/com/google/googlejavaformat/java/testdata/LiteralReflow.output @@ -0,0 +1,55 @@ +class LiteralReflow { + static class TestLineBreak { + String doesNotBreakAt100 = "A very long long long long long long long long long loong sentence"; + String breaksAt101 = + "A very long long long long long long long long long long loooong sentence"; + } + + static class TestReflowLimit { + String doesNotReflowAt100 = + "A very long long long long long long long long long long long long long looooong sentence"; + String reflowsWhenLongerThan100 = + "A very long long long long long long long long long long long long long long long" + + " sentence"; + } + + static class TestReflowLocation { + String accommodatesWordsUpTo100 = + "A very long long long long long long long long long long long long long long long looooong" + + " sentence"; + String breaksBeforeWordsReach101 = + "A very long long long long long long long long long long long long long long long" + + " loooooong sentence"; + } + + static class Test2LineReflowLimit { + String doesNotReflowEitherLinesAt100 = + "A very long long long long long long long long long long long long long looooong sentence." + + " And a second very long long long long long long long long long long loong sentence"; + String reflowsLastLineAt101 = + "A very long long long long long long long long long long long long long looooong sentence." + + " And a second very long long long long long long long long long long looong" + + " sentence"; + } + + static class TestWithTrailingCharacters { + String fitsLastLineUpTo100WithTrailingCharacters = + f( + f( + "A very long long long long long long long long long long long long loong sentence." + + " And a second very long long long long long long long long loong sentence")); + String reflowsLastLineToAccommodateTrailingCharacters = + f( + f( + "A very long long long long long long long long long long long long loong sentence." + + " And a second very long long long long long long long long looong" + + " sentence")); + // Tests an off-by-one issue, but see b/179561701 for a similar issue that is not yet fixed + String doesNotOverTriggerLastLineReflow = + f( + f( + "A very long long long long long long long long long long long long loong sentence." + + " And a second very loong sentence with trailing a a a a a a a a a a a a a a" + + " a")); + } +}