Skip to content

Commit

Permalink
Fix an off-by-one issue on the reflowing of string literals.
Browse files Browse the repository at this point in the history
During the initial width calculation, it seems we are off by one.

The existing tests where actually also wrong (wrapped at 99).

PiperOrigin-RevId: 356697501
  • Loading branch information
java-team-github-bot authored and google-java-format Team committed Feb 10, 2021
1 parent 69ba30f commit b9fd8d2
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -251,18 +252,21 @@ private static String reflow(
ImmutableList<String> 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<String> input = new ArrayDeque<>(components);
List<String> lines = new ArrayList<>();
boolean first = first0;
while (!input.isEmpty()) {
int length = 0;
List<String> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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\";",
"}",
"",
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class B173808510 {
// b/173808510
@FlagSpec(
name = "myFlag",
help =
"areallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyloongword word1 word2")
Flag<Integer> dummy = null;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class B173808510 {
// b/173808510
@FlagSpec(
name = "myFlag",
help =
"areallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyloongword word1"
+ " word2")
Flag<Integer> dummy = null;
}
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Original file line number Diff line number Diff line change
@@ -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"));
}
}

0 comments on commit b9fd8d2

Please sign in to comment.