Skip to content

Commit b9fd8d2

Browse files
java-team-github-botgoogle-java-format Team
authored and
google-java-format Team
committed
Fix an off-by-one issue on the reflowing of string literals.
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
1 parent 69ba30f commit b9fd8d2

File tree

7 files changed

+130
-8
lines changed

7 files changed

+130
-8
lines changed

core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,10 @@ static int hasEscapedNewlineAt(String input, int idx) {
239239
* @param separator the line separator
240240
* @param columnLimit the number of columns to wrap at
241241
* @param startColumn the column position of the beginning of the original text
242-
* @param trailing extra space to leave after the last line
243-
* @param components the text to reflow
244-
* @param first0 true if the text includes the beginning of its enclosing concat chain, i.e. a
242+
* @param trailing extra space to leave after the last line, to accommodate a ; or )
243+
* @param components the text to reflow. This is a list of “words” of a single literal. Its first
244+
* and last quotes have been stripped
245+
* @param first0 true if the text includes the beginning of its enclosing concat chain
245246
*/
246247
private static String reflow(
247248
String separator,
@@ -251,18 +252,21 @@ private static String reflow(
251252
ImmutableList<String> components,
252253
boolean first0) {
253254
// We have space between the start column and the limit to output the first line.
254-
// Reserve two spaces for the quotes.
255+
// Reserve two spaces for the start and end quotes.
255256
int width = columnLimit - startColumn - 2;
256257
Deque<String> input = new ArrayDeque<>(components);
257258
List<String> lines = new ArrayList<>();
258259
boolean first = first0;
259260
while (!input.isEmpty()) {
260261
int length = 0;
261262
List<String> line = new ArrayList<>();
263+
// If we know this is going to be the last line, then remove a bit of width to account for the
264+
// trailing characters.
262265
if (input.stream().mapToInt(String::length).sum() <= width) {
266+
// This isn’t quite optimal, but arguably good enough. See b/179561701
263267
width -= trailing;
264268
}
265-
while (!input.isEmpty() && (length <= 4 || (length + input.peekFirst().length()) < width)) {
269+
while (!input.isEmpty() && (length <= 4 || (length + input.peekFirst().length()) <= width)) {
266270
String text = input.removeFirst();
267271
line.add(text);
268272
length += text.length();

core/src/test/java/com/google/googlejavaformat/java/FormatterIntegrationTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ public FormatterIntegrationTest(String name, String input, String expected) {
125125
@Test
126126
public void format() {
127127
try {
128-
String output = new Formatter().formatSource(input);
128+
Formatter formatter = new Formatter();
129+
String output = formatter.formatSource(input);
130+
output = StringWrapper.wrap(output, formatter);
129131
assertEquals("bad output for " + name, expected, output);
130132
} catch (FormatterException e) {
131133
fail(String.format("Formatter crashed on %s: %s", name, e.getMessage()));

core/src/test/java/com/google/googlejavaformat/java/MainTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,8 @@ public void reflowLongStrings() throws Exception {
525525
"class T {",
526526
" String s =",
527527
" \"one long incredibly unbroken sentence moving from topic to topic so that no one had"
528-
+ " a\"",
529-
" + \" chance to interrupt\";",
528+
+ " a chance\"",
529+
" + \" to interrupt\";",
530530
"}",
531531
"",
532532
};
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
class B173808510 {
2+
// b/173808510
3+
@FlagSpec(
4+
name = "myFlag",
5+
help =
6+
"areallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyloongword word1 word2")
7+
Flag<Integer> dummy = null;
8+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
class B173808510 {
2+
// b/173808510
3+
@FlagSpec(
4+
name = "myFlag",
5+
help =
6+
"areallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreallyloongword word1"
7+
+ " word2")
8+
Flag<Integer> dummy = null;
9+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
class LiteralReflow {
2+
static class TestLineBreak {
3+
String doesNotBreakAt100 = "A very long long long long long long long long long loong sentence";
4+
String breaksAt101 = "A very long long long long long long long long long long loooong sentence";
5+
}
6+
7+
static class TestReflowLimit {
8+
String doesNotReflowAt100 =
9+
"A very long long long long long long long long long long long long long looooong sentence";
10+
String reflowsWhenLongerThan100 =
11+
"A very long long long long long long long long long long long long long long long sentence";
12+
}
13+
14+
static class TestReflowLocation {
15+
String accommodatesWordsUpTo100 =
16+
"A very long long long long long long long long long long long long long long long looooong sentence";
17+
String breaksBeforeWordsReach101 =
18+
"A very long long long long long long long long long long long long long long long loooooong sentence";
19+
}
20+
21+
static class Test2LineReflowLimit {
22+
String doesNotReflowEitherLinesAt100 =
23+
"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";
24+
String reflowsLastLineAt101 =
25+
"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";
26+
}
27+
28+
static class TestWithTrailingCharacters {
29+
String fitsLastLineUpTo100WithTrailingCharacters =
30+
f(
31+
f(
32+
"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"));
33+
String reflowsLastLineToAccommodateTrailingCharacters =
34+
f(
35+
f(
36+
"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"));
37+
// Tests an off-by-one issue, but see b/179561701 for a similar issue that is not yet fixed
38+
String doesNotOverTriggerLastLineReflow =
39+
f(
40+
f(
41+
"A very long long long long long long long long long long long long loong sentence."
42+
+ " And a second very loong sentence with trailing a a a a a a a a a a a a a a a"));
43+
}
44+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
class LiteralReflow {
2+
static class TestLineBreak {
3+
String doesNotBreakAt100 = "A very long long long long long long long long long loong sentence";
4+
String breaksAt101 =
5+
"A very long long long long long long long long long long loooong sentence";
6+
}
7+
8+
static class TestReflowLimit {
9+
String doesNotReflowAt100 =
10+
"A very long long long long long long long long long long long long long looooong sentence";
11+
String reflowsWhenLongerThan100 =
12+
"A very long long long long long long long long long long long long long long long"
13+
+ " sentence";
14+
}
15+
16+
static class TestReflowLocation {
17+
String accommodatesWordsUpTo100 =
18+
"A very long long long long long long long long long long long long long long long looooong"
19+
+ " sentence";
20+
String breaksBeforeWordsReach101 =
21+
"A very long long long long long long long long long long long long long long long"
22+
+ " loooooong sentence";
23+
}
24+
25+
static class Test2LineReflowLimit {
26+
String doesNotReflowEitherLinesAt100 =
27+
"A very long long long long long long long long long long long long long looooong sentence."
28+
+ " And a second very long long long long long long long long long long loong sentence";
29+
String reflowsLastLineAt101 =
30+
"A very long long long long long long long long long long long long long looooong sentence."
31+
+ " And a second very long long long long long long long long long long looong"
32+
+ " sentence";
33+
}
34+
35+
static class TestWithTrailingCharacters {
36+
String fitsLastLineUpTo100WithTrailingCharacters =
37+
f(
38+
f(
39+
"A very long long long long long long long long long long long long loong sentence."
40+
+ " And a second very long long long long long long long long loong sentence"));
41+
String reflowsLastLineToAccommodateTrailingCharacters =
42+
f(
43+
f(
44+
"A very long long long long long long long long long long long long loong sentence."
45+
+ " And a second very long long long long long long long long looong"
46+
+ " sentence"));
47+
// Tests an off-by-one issue, but see b/179561701 for a similar issue that is not yet fixed
48+
String doesNotOverTriggerLastLineReflow =
49+
f(
50+
f(
51+
"A very long long long long long long long long long long long long loong sentence."
52+
+ " And a second very loong sentence with trailing a a a a a a a a a a a a a a"
53+
+ " a"));
54+
}
55+
}

0 commit comments

Comments
 (0)