Skip to content

Commit 9204cf5

Browse files
cushonronshapiro
authored andcommitted
Improve whitespace handling around partial format ranges
During partial formatting the formatter reformats entire statements, and then splices formatted statements from the reformatted regions together with unformatted statements from outside the reformatted regions. Previously the whitespace between formatted and unformatted statements was preserved from the input. This change causes the formatter to use whitespace from the formatted file at those boundaries. In practice, this means that the formatter does a better job of removing or inserting blank lines around reformatted regions, and of removing trailing whitespace from lines adjacent to reformatted regions. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=155411845
1 parent 6492e36 commit 9204cf5

File tree

3 files changed

+217
-56
lines changed

3 files changed

+217
-56
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ public static boolean isNewline(String input) {
4242
return BREAKS.contains(input);
4343
}
4444

45+
/** Returns the length of the newline sequence at the current offset, or {@code -1}. */
46+
public static int hasNewlineAt(String input, int idx) {
47+
for (String b : BREAKS) {
48+
if (input.startsWith(b, idx)) {
49+
return b.length();
50+
}
51+
}
52+
return -1;
53+
}
54+
4555
/**
4656
* Returns the terminating line break in the input, or {@code null} if the input does not end in a
4757
* break.

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

Lines changed: 49 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,10 @@ public void append(String text, Range<Integer> range) {
145145
break;
146146
default:
147147
while (newlinesPending > 0) {
148-
mutableLines.add(lineBuilder.toString());
148+
// drop leading blank lines
149+
if (!mutableLines.isEmpty() || lineBuilder.length() > 0) {
150+
mutableLines.add(lineBuilder.toString());
151+
}
149152
lineBuilder = new StringBuilder();
150153
rangesSet = false;
151154
--newlinesPending;
@@ -253,44 +256,33 @@ public ImmutableList<Replacement> getFormatReplacements(RangeSet<Integer> iRange
253256
// Add all output lines in the given token range to the replacement.
254257
StringBuilder replacement = new StringBuilder();
255258

256-
boolean needsBreakBefore = false;
257259
int replaceFrom = startTok.getPosition();
260+
// Replace leading whitespace in the input with the whitespace from the formatted file
258261
while (replaceFrom > 0) {
259262
char previous = javaInput.getText().charAt(replaceFrom - 1);
260-
if (previous == '\n' || previous == '\r') {
263+
if (!CharMatcher.whitespace().matches(previous)) {
261264
break;
262265
}
263-
if (CharMatcher.whitespace().matches(previous)) {
264-
replaceFrom--;
265-
continue;
266-
}
267-
needsBreakBefore = true;
268-
break;
266+
replaceFrom--;
269267
}
270268

271-
if (needsBreakBefore) {
272-
replacement.append(lineSeparator);
269+
int i = kToJ.get(startTok.getIndex()).lowerEndpoint();
270+
// Include leading blank lines from the formatted output, unless the formatted range
271+
// starts at the beginning of the file.
272+
while (i > 0 && getLine(i - 1).isEmpty()) {
273+
i--;
273274
}
274-
275-
boolean first = true;
276-
int i;
277-
for (i = kToJ.get(startTok.getIndex()).lowerEndpoint();
278-
i < kToJ.get(endTok.getIndex()).upperEndpoint();
279-
i++) {
275+
// Write out the formatted range.
276+
for (; i < kToJ.get(endTok.getIndex()).upperEndpoint(); i++) {
280277
// It's possible to run out of output lines (e.g. if the input ended with
281278
// multiple trailing newlines).
282279
if (i < getLineCount()) {
283-
if (first) {
284-
first = false;
285-
} else {
280+
if (i > 0) {
286281
replacement.append(lineSeparator);
287282
}
288283
replacement.append(getLine(i));
289284
}
290285
}
291-
replacement.append(lineSeparator);
292-
293-
String trailingLine = i < getLineCount() ? getLine(i) : null;
294286

295287
int replaceTo =
296288
Math.min(endTok.getPosition() + endTok.length(), javaInput.getText().length());
@@ -299,46 +291,50 @@ public ImmutableList<Replacement> getFormatReplacements(RangeSet<Integer> iRange
299291
if (endTok.getIndex() == javaInput.getkN() - 1) {
300292
replaceTo = javaInput.getText().length();
301293
}
302-
303-
// Expand the partial formatting range to include non-breaking trailing
304-
// whitespace. If the range ultimately ends in a newline, then preserve
305-
// whatever original text was on the next line (i.e. don't re-indent
306-
// the next line after the reformatted range). However, if the partial
307-
// formatting range doesn't end in a newline, then break and re-indent.
308-
boolean reIndent = true;
309-
OUTER:
294+
// Replace trailing whitespace in the input with the whitespace from the formatted file.
295+
// If the trailing whitespace in the input includes one or more line breaks, preserve the
296+
// whitespace after the last newline to avoid re-indenting the line following the formatted
297+
// line.
298+
int newline = -1;
310299
while (replaceTo < javaInput.getText().length()) {
311-
char endChar = javaInput.getText().charAt(replaceTo);
312-
switch (endChar) {
313-
case '\r':
314-
if (replaceTo + 1 < javaInput.getText().length()
315-
&& javaInput.getText().charAt(replaceTo + 1) == '\n') {
316-
replaceTo++;
317-
}
318-
// falls through
319-
case '\n':
320-
replaceTo++;
321-
reIndent = false;
322-
break OUTER;
323-
default:
324-
break;
300+
char next = javaInput.getText().charAt(replaceTo);
301+
if (!CharMatcher.whitespace().matches(next)) {
302+
break;
325303
}
326-
if (CharMatcher.whitespace().matches(endChar)) {
304+
int newlineLength = Newlines.hasNewlineAt(javaInput.getText(), replaceTo);
305+
if (newlineLength != -1) {
306+
newline = replaceTo;
307+
// Skip over the entire newline; don't count the second character of \r\n as a newline.
308+
replaceTo += newlineLength;
309+
} else {
327310
replaceTo++;
328-
continue;
329311
}
330-
break;
331312
}
332-
if (reIndent && trailingLine != null) {
333-
int idx = CharMatcher.whitespace().negate().indexIn(trailingLine);
334-
if (idx > 0) {
335-
replacement.append(trailingLine, 0, idx);
313+
if (newline != -1) {
314+
replaceTo = newline;
315+
}
316+
317+
if (newline == -1) {
318+
// There wasn't an existing trailing newline; add one.
319+
replacement.append(lineSeparator);
320+
}
321+
for (; i < getLineCount(); i++) {
322+
String after = getLine(i);
323+
if (after.isEmpty()) {
324+
// Write out trailing blank lines from the formatted output.
325+
replacement.append(lineSeparator);
326+
} else {
327+
if (newline == -1) {
328+
// If there wasn't a trailing newline in the input, indent the next line.
329+
int idx = CharMatcher.whitespace().negate().indexIn(after);
330+
replacement.append(after.substring(0, idx));
331+
}
332+
break;
336333
}
337334
}
338335

339336
result.add(Replacement.create(replaceFrom, replaceTo, replacement.toString()));
340337
}
341-
342338
return result.build();
343339
}
344340

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

Lines changed: 158 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.common.collect.ImmutableList;
2323
import com.google.common.collect.ObjectArrays;
2424
import com.google.common.collect.Range;
25+
import java.io.IOException;
2526
import java.io.PrintWriter;
2627
import java.io.StringWriter;
2728
import java.nio.charset.StandardCharsets;
@@ -361,6 +362,7 @@ public void onlyPackage() throws Exception {
361362
String expectedOutput =
362363
lines(
363364
"package test;", //
365+
"",
364366
"class Test {}",
365367
"");
366368
int idx = input.indexOf("test");
@@ -664,9 +666,9 @@ public void testGetFormatReplacementRanges() throws Exception {
664666
assertThat(replacement.getReplacementString())
665667
.isEqualTo(
666668
lines(
667-
" void f() {}", //
668-
""));
669-
int replaceFrom = input.indexOf("void f");
669+
"", //
670+
" void f() {}"));
671+
int replaceFrom = input.indexOf("void f") - newline.length();
670672
assertThat(replacement.getReplaceRange().lowerEndpoint()).isEqualTo(replaceFrom);
671673
}
672674

@@ -1546,4 +1548,157 @@ public void endOfLineStatementNewline() throws Exception {
15461548
new Formatter().formatSource(in, ImmutableList.of(Range.closedOpen(idx + 1, idx + 1))))
15471549
.isEqualTo(in);
15481550
}
1551+
1552+
@Test
1553+
public void importNewlines() throws Exception {
1554+
String input =
1555+
lines(
1556+
"package p;",
1557+
"import java.util.ArrayList;",
1558+
"class Foo {",
1559+
" ArrayList<String> xs = new ArrayList<>();",
1560+
"}",
1561+
"");
1562+
String expectedOutput =
1563+
lines(
1564+
"package p;",
1565+
"",
1566+
"import java.util.ArrayList;",
1567+
"",
1568+
"class Foo {",
1569+
" ArrayList<String> xs = new ArrayList<>();",
1570+
"}",
1571+
"");
1572+
1573+
String output = runFormatter(input, new String[] {"-lines", "2"});
1574+
assertThat(output).isEqualTo(expectedOutput);
1575+
}
1576+
1577+
@Test
1578+
public void b36458607() throws Exception {
1579+
String input =
1580+
lines(
1581+
"// copyright",
1582+
"",
1583+
"package p;",
1584+
"import static c.g.I.c;",
1585+
"",
1586+
"/** */",
1587+
"class Foo {{ c(); }}",
1588+
"");
1589+
String expectedOutput =
1590+
lines(
1591+
"// copyright",
1592+
"",
1593+
"package p;",
1594+
"",
1595+
"import static c.g.I.c;",
1596+
"",
1597+
"/** */",
1598+
"class Foo {{ c(); }}",
1599+
"");
1600+
1601+
String output = runFormatter(input, new String[] {"-lines", "4"});
1602+
assertThat(output).isEqualTo(expectedOutput);
1603+
}
1604+
1605+
@Test
1606+
public void b32159971() throws Exception {
1607+
String input =
1608+
lines(
1609+
"", //
1610+
"",
1611+
"package p;",
1612+
"class X {}",
1613+
"");
1614+
String expectedOutput =
1615+
lines(
1616+
"package p;", //
1617+
"",
1618+
"class X {}",
1619+
"");
1620+
1621+
String output = runFormatter(input, new String[] {"-lines", "3"});
1622+
assertThat(output).isEqualTo(expectedOutput);
1623+
}
1624+
1625+
@Test
1626+
public void b21668189() throws Exception {
1627+
String input =
1628+
lines(
1629+
"class Foo {", //
1630+
" {",
1631+
" int x = 1;",
1632+
" ",
1633+
" int y = 2;",
1634+
" }",
1635+
"}",
1636+
"");
1637+
String expectedOutput =
1638+
lines(
1639+
"class Foo {", //
1640+
" {",
1641+
" int x = 1;",
1642+
"",
1643+
" int y = 2;",
1644+
" }",
1645+
"}",
1646+
"");
1647+
1648+
String output = runFormatter(input, new String[] {"-lines", "4:5"});
1649+
assertThat(output).isEqualTo(expectedOutput);
1650+
}
1651+
1652+
private String runFormatter(String input, String[] args) throws IOException, UsageException {
1653+
Path tmpdir = testFolder.newFolder().toPath();
1654+
Path path = tmpdir.resolve("Foo.java");
1655+
Files.write(path, input.getBytes(StandardCharsets.UTF_8));
1656+
1657+
StringWriter out = new StringWriter();
1658+
StringWriter err = new StringWriter();
1659+
1660+
Main main = new Main(new PrintWriter(out, true), new PrintWriter(err, true), System.in);
1661+
assertThat(main.format(ObjectArrays.concat(args, path.toString()))).isEqualTo(0);
1662+
return out.toString();
1663+
}
1664+
1665+
@Test
1666+
public void trailing() throws Exception {
1667+
String input =
1668+
lines(
1669+
"package foo.bar.baz;",
1670+
"",
1671+
"public class B {",
1672+
" public void f() {",
1673+
" int a = 7 +4;",
1674+
" int b = 7 +4;",
1675+
" int c = 7 +4;",
1676+
" int d = 7 +4;",
1677+
"",
1678+
" int e = 7 +4;",
1679+
" }",
1680+
"}");
1681+
String expected =
1682+
lines(
1683+
"package foo.bar.baz;",
1684+
"",
1685+
"public class B {",
1686+
" public void f() {",
1687+
" int a = 7 +4;",
1688+
" int b = 7 +4;",
1689+
" int c = 7 + 4;",
1690+
" int d = 7 +4;",
1691+
"",
1692+
" int e = 7 +4;",
1693+
" }",
1694+
"}");
1695+
String actual =
1696+
new Formatter().formatSource(input, ImmutableList.of(rangeOf(input, "int c = 7 +4")));
1697+
assertThat(actual).isEqualTo(expected);
1698+
}
1699+
1700+
private Range<Integer> rangeOf(String input, String needle) {
1701+
int idx = input.indexOf(needle);
1702+
return Range.closedOpen(idx, idx + needle.length());
1703+
}
15491704
}

0 commit comments

Comments
 (0)