Fix the imports after formatting, as the command line does

formatSourceAndFixImports, which the Maven plugin and the Spotless
step run, ordered the imports and removed the unused ones before
formatting. Main formats first and fixes the imports after, because
the ranges of --lines and --offset are offsets into the input. The
order decides where the comment of a removed import ends up:
ImportOrderer moves a comment along with its import, and
RemoveUnusedImports leaves it behind. Fixing them first also dropped
an extraneous semicolon between imports, where the command line fails
the file (#219).

formatSourceAndFixImports now fixes the imports after formatting.
FormatterServiceImplTest holds the in-process service to what Main
prints for whole files.

Author: abashev

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

@@ -296,7 +296,8 @@ public String formatSource(String input) throws FormatterException {
     /**
      * Formats an input string (a Java compilation unit) and fixes imports.
      *
-     * <p>Fixing imports includes ordering, spacing, and removal of unused import statements.
+     * <p>Fixing imports includes ordering, spacing, and removal of unused import statements. Long strings are reflowed
+     * too, and the result is what {@code open-java-format -} prints with no flags.
      *
      * @param input the input string
      * @return the output string
@@ -305,9 +306,10 @@ public String formatSource(String input) throws FormatterException {
      * Style Guide - 3.3.3 Import ordering and spacing</a>
      */
     public String formatSourceAndFixImports(String input) throws FormatterException {
-        input = ImportOrderer.reorderImports(input, options.style());
-        input = RemoveUnusedImports.removeUnusedImports(input);
-        String formatted = formatSource(input);
+        // The imports are fixed after formatting, as the command line fixes them: it has to format first, because the
+        // ranges it formats are offsets into the input. In the other order the comment of a removed import ends up
+        // elsewhere, and an extraneous semicolon between imports is dropped where the command line fails the file.
+        String formatted = fixImports(formatSource(input));
         return StringWrapper.wrap(options.maxLineLength(), formatted, this);
     }
 

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

@@ -0,0 +1,120 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.palantir.javaformat.java;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.io.ByteArrayInputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * The service that formats in the build's own JVM, for {@code formatDiff} without the native binary, the Spotless step
+ * and the Maven plugin, against the command line, which the bootstrapping and native services run for IntelliJ IDEA
+ * and {@code formatDiff}, and the GitHub Action runs to check.
+ */
+final class FormatterServiceImplTest {
+    private static final FormatterService SERVICE = new FormatterServiceImpl();
+
+    // Two long strings, as in a file where only the line of the second one has changed.
+    private static final String LONG_STRINGS = """
+        class S {
+            String outside = "Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat sed.";
+
+            String changed = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna.";
+        }
+        """;
+
+    static Stream<Arguments> sources() {
+        return Stream.of(
+                Arguments.of("long strings", LONG_STRINGS),
+                Arguments.of("joinable literals", """
+                    class J {
+                        String joined = "one " + "two";
+                        String wrapped = "Lorem ipsum dolor sit amet, " + "consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore";
+                    }
+                    """),
+                // The comment stays behind when its import is removed, and the order of fixing the imports and
+                // formatting decides where it ends up.
+                Arguments.of("trailing comment of an unused import", """
+                    package p;
+
+                    // group one
+                    import java.util.Map; // used
+                    import java.io.File; // unused, trailing comment
+                    /* block between */
+                    import java.util.List;
+
+                    class C { List<String> l; Map<String,String> m; }
+                    """),
+                Arguments.of("comment under an unused import", """
+                    package p;
+
+                    import java.io.File;
+                    // comment about the class
+                    class C {}
+                    """));
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @MethodSource("sources")
+    void formatsAFileAsTheCommandLineDoes(String name, String source) throws Exception {
+        assertThat(SERVICE.formatSourceReflowStringsAndFixImports(source)).isEqualTo(commandLine(source, "-"));
+    }
+
+    @Test
+    void failsOnAnExtraneousSemicolonAsTheCommandLineDoes() throws Exception {
+        // Fixing the imports first used to drop the semicolon, which javac 21 rejects between imports, and format
+        // the file that the command line refuses.
+        String source = """
+            package p;
+
+            import java.util.List;;
+            import java.io.File;
+
+            class C { List<String> l; }
+            """;
+        StringWriter err = new StringWriter();
+        Main main = new Main(
+                new PrintWriter(new StringWriter(), true),
+                new PrintWriter(err, true),
+                new ByteArrayInputStream(source.getBytes(UTF_8)));
+
+        assertThat(main.format("-")).isNotZero();
+        assertThat(err.toString()).contains("extraneous semicolon");
+        assertThatThrownBy(() -> SERVICE.formatSourceReflowStringsAndFixImports(source))
+                .isInstanceOf(FormatterException.class)
+                .hasMessageContaining("extraneous semicolon");
+    }
+
+    /** What {@code open-java-format} prints for the source on standard input. */
+    private static String commandLine(String source, String... args) throws UsageException {
+        StringWriter out = new StringWriter();
+        StringWriter err = new StringWriter();
+        Main main = new Main(
+                new PrintWriter(out, true),
+                new PrintWriter(err, true),
+                new ByteArrayInputStream(source.getBytes(UTF_8)));
+        assertThat(main.format(args)).as(err::toString).isZero();
+        return out.toString();
+    }
+}

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

@@ -276,6 +276,27 @@ class T {
         assertThat(ImportOrderer.reorderImports(withoutBlank, Style.OJF)).isEqualTo(withoutBlank);
     }
 
+    @Test
+    void commentUnderARemovedImportStaysOnTheClass() throws FormatterException {
+        String input = """
+            package p;
+
+            import java.io.File;
+            // comment about the class
+            class C {}
+            """;
+        String output = """
+            package p;
+
+            // comment about the class
+            class C {}
+            """;
+
+        // The imports are fixed after formatting, as the command line fixes them. Ordering them first left a blank
+        // line after the import, which stayed between the comment and the class once the import was gone.
+        assertThat(FORMATTER.formatSourceAndFixImports(input)).isEqualTo(output);
+    }
+
     @Test
     void canParse_java9_private_interface_methods() {
         assertThatCode(() -> FORMATTER.formatSourceAndFixImports(""