Stop an unused import from leaving two blank lines behind

The command line formats first and fixes imports afterwards. When
RemoveUnusedImports deleted an import that sat between two blank lines,
such as the only import between the package line and the class, it
left both blank lines, and nothing ran after it to collapse them. A
second run did, so --replace wrote files that --set-exit-if-changed
then reported (#37, from google/google-java-format#598 and
google/google-java-format#1436). The Gradle and Spotless entry point
fixes imports before it formats and was not affected, so the two
disagreed about these files.

This ports google/google-java-format#1437 by arimu1: adjacent unused
imports are deleted as one range, and a range that sits between blank
lines, or at the start of the file, takes one of them along. The code
is adapted to this codebase. The four RemoveUnusedImportsTest cases
come from the upstream PR, and a MainTest checks that the command line
and formatSourceAndFixImports now give the same file.

Of the 15,747 files of the JDK 21 sources, 304 format differently, by
305 blank lines fewer in all and no other change. All 304 are files the
old code changed again on a second run, and the new output is stable on
every one of them.

Author: arimu1

open-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedImports.java

@@ -226,6 +226,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 (JCImport importTree : unit.getImports()) {
             String simpleName = getSimpleName(importTree);
             if (!isUnused(unit, usedNames, usedInJavadoc, importTree, simpleName)) {
@@ -234,16 +235,62 @@ private static RangeMap<Integer, String> buildReplacements(
             // delete the import
             int endPosition = importTree.getEndPosition(unit.endPositions);
             endPosition = Math.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 range, so the blank-line cleanup below sees 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 (package, blank, imports, blank, type). Collapse one of them, so a single formatting pass leaves
+        // one blank line, as the second one did.
+        return collapseBlankLinesAroundDeletedImports(contents, replacements, sep);
+    }
+
+    /**
+     * Extends contiguous deleted-import ranges so that 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 line 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) {
+            // The 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(ImportTree importTree) {

open-java-format/src/test/java/com/palantir/javaformat/java/MainTest.java

@@ -266,6 +266,46 @@ public void importRemovalLines() throws Exception {
         assertThat(out.toString()).isEqualTo(joiner.join(expected));
     }
 
+    // An unused import between two blank lines must not leave both of them behind: one run of the command line gives
+    // what a second run would, and what the entry point of the Gradle and Spotless step gives (#37, from
+    // google/google-java-format#1436).
+    @Test
+    public void unusedImportRemovalLeavesOneBlankLine() 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 {}",
+            "",
+        };
+        StringWriter out = new StringWriter();
+        Main main = new Main(
+                new PrintWriter(out, true),
+                new PrintWriter(new BufferedWriter(new OutputStreamWriter(System.err, UTF_8)), true),
+                new ByteArrayInputStream(joiner.join(input).getBytes(UTF_8)));
+        assertThat(main.format("-")).isEqualTo(0);
+        assertThat(out.toString()).isEqualTo(joiner.join(expected));
+
+        Formatter formatter = Formatter.createFormatter(JavaFormatterOptions.builder()
+                .style(JavaFormatterOptions.Style.OJF)
+                .build());
+        assertThat(formatter.formatSourceAndFixImports(joiner.join(input))).isEqualTo(joiner.join(expected));
+    }
+
     // test that errors are reported on the right line when imports are removed
     @Test
     public void importRemoveErrorParseError() throws Exception {

open-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedImportsTest.java

@@ -254,6 +254,74 @@ public static List<Object[]> parameters() {
                     "interface Test { private static void foo() {} }",
                 },
             },
+            // An unused import between blank lines takes one of them with it (#37, from
+            // google/google-java-format#1436 and google/google-java-format#1437).
+            {
+                {
+                    "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) {