Skip to content

Commit 1b2e657

Browse files
committed
fix: avoid extra blank line after unused import removal (#1436)
When every import between the package blank and the type blank is removed, collapse one of the stacked blank lines so a single CLI pass stays style-compliant (format-then-fix-imports path).
1 parent cf859e3 commit 1b2e657

3 files changed

Lines changed: 155 additions & 3 deletions

File tree

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

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ private static RangeMap<Integer, String> buildReplacements(
234234
Set<String> usedNames,
235235
Multimap<String, Range<Integer>> usedInJavadoc) {
236236
RangeMap<Integer, String> replacements = TreeRangeMap.create();
237+
String sep = Newlines.guessLineSeparator(contents);
237238
for (JCTree importTree : unit.getImports()) {
238239
if (isModuleImport(importTree)) {
239240
continue;
@@ -245,14 +246,62 @@ private static RangeMap<Integer, String> buildReplacements(
245246
// delete the import
246247
int endPosition = getEndPosition(importTree, unit);
247248
endPosition = max(CharMatcher.isNot(' ').indexIn(contents, endPosition), endPosition);
248-
String sep = Newlines.guessLineSeparator(contents);
249249
if (endPosition + sep.length() < contents.length()
250250
&& contents.subSequence(endPosition, endPosition + sep.length()).toString().equals(sep)) {
251251
endPosition += sep.length();
252252
}
253-
replacements.put(Range.closedOpen(importTree.getStartPosition(), endPosition), "");
253+
// putCoalescing merges adjacent unused imports into one span so blank-line cleanup can see
254+
// the whole deleted import block (TreeRangeMap.put does not coalesce).
255+
replacements.putCoalescing(Range.closedOpen(importTree.getStartPosition(), endPosition), "");
254256
}
255-
return replacements;
257+
// Removing a whole import block can leave the blank line that preceded it stacked on the
258+
// blank line that followed it (e.g. package → blank → imports → blank → type). Collapse one
259+
// of those blanks so a single formatting pass stays style-compliant (#1436).
260+
return collapseBlankLinesAroundDeletedImports(contents, replacements, sep);
261+
}
262+
263+
/**
264+
* Extends contiguous deleted-import ranges so a blank line that both preceded and followed the
265+
* imports is not left doubled after the deletion.
266+
*/
267+
private static RangeMap<Integer, String> collapseBlankLinesAroundDeletedImports(
268+
String contents, RangeMap<Integer, String> replacements, String sep) {
269+
if (replacements.asMapOfRanges().isEmpty()) {
270+
return replacements;
271+
}
272+
RangeMap<Integer, String> adjusted = TreeRangeMap.create();
273+
for (Range<Integer> range : replacements.asMapOfRanges().keySet()) {
274+
int start = range.lowerEndpoint();
275+
int end = range.upperEndpoint();
276+
// Eat one trailing blank line when the deletion sits between blank lines, or at the start of
277+
// the file (where a leading blank would otherwise remain after the last import is removed).
278+
if (isBlankLineAfter(contents, end, sep)
279+
&& (start == 0 || isBlankLineBefore(contents, start, sep))) {
280+
end += sep.length();
281+
}
282+
adjusted.putCoalescing(Range.closedOpen(start, end), "");
283+
}
284+
return adjusted;
285+
}
286+
287+
/** True if {@code pos} is immediately preceded by an empty line. */
288+
private static boolean isBlankLineBefore(String contents, int pos, String sep) {
289+
if (pos < sep.length() || !contents.regionMatches(pos - sep.length(), sep, 0, sep.length())) {
290+
return false;
291+
}
292+
int endOfPreviousLine = pos - sep.length();
293+
if (endOfPreviousLine == 0) {
294+
// File begins with a blank line before the deleted import.
295+
return true;
296+
}
297+
return endOfPreviousLine >= sep.length()
298+
&& contents.regionMatches(endOfPreviousLine - sep.length(), sep, 0, sep.length());
299+
}
300+
301+
/** True if {@code pos} is immediately followed by an empty line (a line break). */
302+
private static boolean isBlankLineAfter(String contents, int pos, String sep) {
303+
return pos + sep.length() <= contents.length()
304+
&& contents.regionMatches(pos, sep, 0, sep.length());
256305
}
257306

258307
private static String getSimpleName(JCTree importTree) {

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,42 @@ class Test extends ArrayList {}
256256
assertThat(out.toString()).isEqualTo(expected);
257257
}
258258

259+
// https://github.com/google/google-java-format/issues/1436
260+
@Test
261+
public void unusedImportRemovalDoesNotLeaveDoubleBlankBeforeJavadoc() throws Exception {
262+
String input =
263+
"""
264+
package com.example;
265+
266+
import static io.grpc.MethodDescriptor.generateFullMethodName;
267+
268+
/**
269+
* Javadoc for class.
270+
*/
271+
public class TestBug {
272+
}
273+
""";
274+
String expected =
275+
"""
276+
package com.example;
277+
278+
/** Javadoc for class. */
279+
public class TestBug {}
280+
""";
281+
282+
assertThat(new Formatter().formatSourceAndFixImports(input)).isEqualTo(expected);
283+
284+
InputStream in = new ByteArrayInputStream(input.getBytes(UTF_8));
285+
StringWriter out = new StringWriter();
286+
Main main =
287+
new Main(
288+
new PrintWriter(out, true),
289+
new PrintWriter(new BufferedWriter(new OutputStreamWriter(System.err, UTF_8)), true),
290+
in);
291+
assertThat(main.format("-")).isEqualTo(0);
292+
assertThat(out.toString()).isEqualTo(expected);
293+
}
294+
259295
// test that -lines handling works with import removal
260296
@Test
261297
public void importRemovalLines() throws Exception {

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,73 @@ interface Test { private static void foo() {} }
265265
interface Test { private static void foo() {} }
266266
""",
267267
},
268+
// #1436: unused import between package and class Javadoc must not leave a double blank line
269+
{
270+
"""
271+
package com.example;
272+
273+
import static io.grpc.MethodDescriptor.generateFullMethodName;
274+
275+
/**
276+
* Javadoc for class.
277+
*/
278+
public class TestBug {}
279+
""",
280+
"""
281+
package com.example;
282+
283+
/**
284+
* Javadoc for class.
285+
*/
286+
public class TestBug {}
287+
""",
288+
},
289+
{
290+
"""
291+
package com.example;
292+
293+
import com.foo.Unused1;
294+
import com.foo.Unused2;
295+
296+
public class TestBug {}
297+
""",
298+
"""
299+
package com.example;
300+
301+
public class TestBug {}
302+
""",
303+
},
304+
{
305+
"""
306+
import com.foo.Unused;
307+
308+
public class TestBug {}
309+
""",
310+
"""
311+
public class TestBug {}
312+
""",
313+
},
314+
{
315+
"""
316+
package com.example;
317+
318+
import java.util.List;
319+
import com.foo.Unused;
320+
321+
public class TestBug {
322+
List<String> xs;
323+
}
324+
""",
325+
"""
326+
package com.example;
327+
328+
import java.util.List;
329+
330+
public class TestBug {
331+
List<String> xs;
332+
}
333+
""",
334+
},
268335
};
269336
ImmutableList.Builder<Object[]> builder = ImmutableList.builder();
270337
for (String[] inputAndOutput : inputsOutputs) {

0 commit comments

Comments
 (0)