Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ private static RangeMap<Integer, String> buildReplacements(
Set<String> usedNames,
Multimap<String, Range<Integer>> usedInJavadoc) {
RangeMap<Integer, String> replacements = TreeRangeMap.create();
String sep = Newlines.guessLineSeparator(contents);
for (JCTree importTree : unit.getImports()) {
if (isModuleImport(importTree)) {
continue;
Expand All @@ -245,14 +246,62 @@ private static RangeMap<Integer, String> buildReplacements(
// delete the import
int endPosition = getEndPosition(importTree, unit);
endPosition = max(CharMatcher.isNot(' ').indexIn(contents, endPosition), endPosition);
String sep = Newlines.guessLineSeparator(contents);
if (endPosition + sep.length() < contents.length()
&& contents.subSequence(endPosition, endPosition + sep.length()).toString().equals(sep)) {
endPosition += sep.length();
}
replacements.put(Range.closedOpen(importTree.getStartPosition(), endPosition), "");
// putCoalescing merges adjacent unused imports into one span so blank-line cleanup can see
// the whole deleted import block (TreeRangeMap.put does not coalesce).
replacements.putCoalescing(Range.closedOpen(importTree.getStartPosition(), endPosition), "");
}
return replacements;
// Removing a whole import block can leave the blank line that preceded it stacked on the
// blank line that followed it (e.g. package → blank → imports → blank → type). Collapse one
// of those blanks so a single formatting pass stays style-compliant (#1436).
return collapseBlankLinesAroundDeletedImports(contents, replacements, sep);
}

/**
* Extends contiguous deleted-import ranges so a blank line that both preceded and followed the
* imports is not left doubled after the deletion.
*/
private static RangeMap<Integer, String> collapseBlankLinesAroundDeletedImports(
String contents, RangeMap<Integer, String> replacements, String sep) {
if (replacements.asMapOfRanges().isEmpty()) {
return replacements;
}
RangeMap<Integer, String> adjusted = TreeRangeMap.create();
for (Range<Integer> range : replacements.asMapOfRanges().keySet()) {
int start = range.lowerEndpoint();
int end = range.upperEndpoint();
// Eat one trailing blank line when the deletion sits between blank lines, or at the start of
// the file (where a leading blank would otherwise remain after the last import is removed).
if (isBlankLineAfter(contents, end, sep)
&& (start == 0 || isBlankLineBefore(contents, start, sep))) {
end += sep.length();
}
adjusted.putCoalescing(Range.closedOpen(start, end), "");
}
return adjusted;
}

/** True if {@code pos} is immediately preceded by an empty line. */
private static boolean isBlankLineBefore(String contents, int pos, String sep) {
if (pos < sep.length() || !contents.regionMatches(pos - sep.length(), sep, 0, sep.length())) {
return false;
}
int endOfPreviousLine = pos - sep.length();
if (endOfPreviousLine == 0) {
// File begins with a blank line before the deleted import.
return true;
}
return endOfPreviousLine >= sep.length()
&& contents.regionMatches(endOfPreviousLine - sep.length(), sep, 0, sep.length());
}

/** True if {@code pos} is immediately followed by an empty line (a line break). */
private static boolean isBlankLineAfter(String contents, int pos, String sep) {
return pos + sep.length() <= contents.length()
&& contents.regionMatches(pos, sep, 0, sep.length());
}

private static String getSimpleName(JCTree importTree) {
Expand Down
36 changes: 36 additions & 0 deletions core/src/test/java/com/google/googlejavaformat/java/MainTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,42 @@ class Test extends ArrayList {}
assertThat(out.toString()).isEqualTo(expected);
}

// https://github.com/google/google-java-format/issues/1436
@Test
public void unusedImportRemovalDoesNotLeaveDoubleBlankBeforeJavadoc() throws Exception {
String input =
"""
package com.example;

import static io.grpc.MethodDescriptor.generateFullMethodName;

/**
* Javadoc for class.
*/
public class TestBug {
}
""";
String expected =
"""
package com.example;

/** Javadoc for class. */
public class TestBug {}
""";

assertThat(new Formatter().formatSourceAndFixImports(input)).isEqualTo(expected);

InputStream in = new ByteArrayInputStream(input.getBytes(UTF_8));
StringWriter out = new StringWriter();
Main main =
new Main(
new PrintWriter(out, true),
new PrintWriter(new BufferedWriter(new OutputStreamWriter(System.err, UTF_8)), true),
in);
assertThat(main.format("-")).isEqualTo(0);
assertThat(out.toString()).isEqualTo(expected);
}

// test that -lines handling works with import removal
@Test
public void importRemovalLines() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,73 @@ interface Test { private static void foo() {} }
interface Test { private static void foo() {} }
""",
},
// #1436: unused import between package and class Javadoc must not leave a double blank line
{
"""
package com.example;

import static io.grpc.MethodDescriptor.generateFullMethodName;

/**
* Javadoc for class.
*/
public class TestBug {}
""",
"""
package com.example;

/**
* Javadoc for class.
*/
public class TestBug {}
""",
},
{
"""
package com.example;

import com.foo.Unused1;
import com.foo.Unused2;

public class TestBug {}
""",
"""
package com.example;

public class TestBug {}
""",
},
{
"""
import com.foo.Unused;

public class TestBug {}
""",
"""
public class TestBug {}
""",
},
{
"""
package com.example;

import java.util.List;
import com.foo.Unused;

public class TestBug {
List<String> xs;
}
""",
"""
package com.example;

import java.util.List;

public class TestBug {
List<String> xs;
}
""",
},
};
ImmutableList.Builder<Object[]> builder = ImmutableList.builder();
for (String[] inputAndOutput : inputsOutputs) {
Expand Down