Format the same on every route: formatDiff on the JVM, ranges, and the order of fixing imports

gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/FormatDiffTest.java

@@ -69,11 +69,7 @@ void parsing_git_diff_output_works() throws IOException {
     @MethodSource("getFormatters")
     void reformat_a_subpath_of_a_git_directory_for_only_changed_lines(FormatterService formatterService)
             throws IOException, InterruptedException {
-        runCommandInRepo("git", "init");
-        runCommandInRepo("git", "config", "user.name", "Test User");
-        runCommandInRepo("git", "config", "user.email", "test-user@palantir.com");
-        runCommandInRepo("git", "config", "commit.gpgsign", "false");
-        runCommandInRepo("git", "commit", "--allow-empty", "-m", "Init");
+        initRepo();
 
         Path subdir = repo.resolve("subdir");
         Files.createDirectories(subdir);
@@ -92,6 +88,52 @@ void reformat_a_subpath_of_a_git_directory_for_only_changed_lines(FormatterServi
         assertThat(dontTouchMe).hasContent("                                 class DontTouchMe {}");
     }
 
+    @ParameterizedTest
+    @MethodSource("getFormatters")
+    void reflows_the_long_strings_of_changed_lines_only(FormatterService formatterService)
+            throws IOException, InterruptedException {
+        initRepo();
+        String outside = "    String outside = \"Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris"
+                + " nisi ut aliquip ex ea commodo consequat sed.\";";
+        Path longStrings = repo.resolve("LongStrings.java");
+        Files.write(longStrings, List.of("class LongStrings {", outside, "}"), UTF_8);
+        runCommandInRepo("git", "add", ".");
+        runCommandInRepo("git", "commit", "-m", "A long string");
+
+        Files.write(
+                longStrings,
+                List.of(
+                        "class LongStrings {",
+                        outside,
+                        "",
+                        "    String changed = \"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod"
+                                + " tempor incididunt ut labore et dolore magna.\";",
+                        "}"),
+                UTF_8);
+
+        FormatDiff.formatDiff(repo, formatterService);
+
+        assertThat(longStrings)
+                .hasContent(String.join(
+                        "\n",
+                        "class LongStrings {",
+                        outside,
+                        "",
+                        "    String changed =",
+                        "            \"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor"
+                                + " incididunt ut labore et\"",
+                        "                    + \" dolore magna.\";",
+                        "}"));
+    }
+
+    private void initRepo() throws IOException, InterruptedException {
+        runCommandInRepo("git", "init");
+        runCommandInRepo("git", "config", "user.name", "Test User");
+        runCommandInRepo("git", "config", "user.email", "test-user@palantir.com");
+        runCommandInRepo("git", "config", "commit.gpgsign", "false");
+        runCommandInRepo("git", "commit", "--allow-empty", "-m", "Init");
+    }
+
     private void runCommandInRepo(String... args) throws IOException, InterruptedException {
         Process process =
                 new ProcessBuilder().command(args).directory(repo.toFile()).start();
@@ -107,6 +149,9 @@ private void runCommandInRepo(String... args) throws IOException, InterruptedExc
 
     private static Stream<FormatterService> getFormatters() throws IOException {
         return Stream.of(
+                // In this JVM, as formatDiff runs it without the native binary.
+                FormatterServiceLoader.load(
+                        getClasspath().stream().map(Path::toFile).toList()),
                 new BootstrappingFormatterService(
                         javaBinPath(), Runtime.version().feature(), getClasspath()),
                 new NativeImageFormatterService(

open-java-format-jdk-bootstrap/src/test/java/com/palantir/javaformat/bootstrap/FormatterServicesTest.java

@@ -24,6 +24,7 @@
 import com.palantir.javaformat.Utils;
 import com.palantir.javaformat.java.FormatterException;
 import com.palantir.javaformat.java.FormatterService;
+import com.palantir.javaformat.java.FormatterServiceImpl;
 import com.palantir.javaformat.java.Replacement;
 import java.io.File;
 import java.net.URI;
@@ -96,13 +97,47 @@ public void main(     String    aaaa ) {
                 .isEmpty();
     }
 
+    @ParameterizedTest
+    @MethodSource("getFormattersAndTheInProcessOne")
+    void reflows_the_long_strings_of_the_ranges_only(FormatterService formatterService) throws FormatterException {
+        String input = """
+            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.";
+            }
+            """;
+        String expectedOutput = """
+            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.";
+            }
+            """;
+        int start = input.indexOf("    String changed");
+        int end = input.indexOf('\n', start);
+
+        List<Replacement> replacements =
+                formatterService.getFormatReplacements(input, List.of(Range.closedOpen(start, end)));
+
+        assertThat(Utils.applyReplacements(input, replacements)).isEqualTo(expectedOutput);
+    }
+
     private static Stream<FormatterService> getFormatters() {
         return Stream.of(
                 new BootstrappingFormatterService(
                         javaBinPath(), Runtime.version().feature(), getClasspath()),
                 new NativeImageFormatterService(Path.of(System.getenv("NATIVE_IMAGE_CLASSPATH"))));
     }
 
+    // With the one that formats in this JVM, as formatDiff does without the native binary. On input that does not parse
+    // it throws, where the other two return no replacements, so it is not among getFormatters.
+    private static Stream<FormatterService> getFormattersAndTheInProcessOne() {
+        return Stream.concat(Stream.of(new FormatterServiceImpl()), getFormatters());
+    }
+
     private String getTestResourceContent(String resourceName) {
         try {
             URI resource = getClass().getClassLoader().getResource(resourceName).toURI();

open-java-format-spi/src/main/java/com/palantir/javaformat/java/FormatterService.java

@@ -28,6 +28,9 @@ public interface FormatterService {
     /**
      * Emit a list of {@link Replacement}s to convert from input to formatted output.
      *
+     * <p>The ranges are formatted, and the long strings in the code that formatting them rewrites are reflowed; the
+     * imports, and the strings elsewhere, stay as they are.
+     *
      * @param input the input compilation unit
      * @param ranges the character ranges to reformat
      * @return a list of {@link Replacement}s, sorted from low index to high index, without overlaps

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

@@ -24,7 +24,6 @@
 import com.palantir.javaformat.Utils;
 import java.io.UncheckedIOException;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.Callable;
 
 /** Encapsulates information about a file to be formatted, including which parts of the file to format. */
@@ -51,23 +50,10 @@ public String call() throws FormatterException {
         Formatter formatter = Formatter.createFormatter(options);
         if (parameters.outputReplacements()) {
             RangeSet<Integer> characterRanges = characterRanges(input);
-            Set<Range<Integer>> rangesToChange = characterRanges.asRanges();
-            List<Replacement> replacements = formatter.getFormatReplacements(input, rangesToChange);
-            if (parameters.reflowLongStrings()) {
-                String formattedText = Utils.applyReplacements(input, replacements);
-                // avoid trying to wrap the input unless the lines needing wrapping are part of the "characterRanges"
-                if (!StringWrapper.linesNeedWrapping(options.maxLineLength(), formattedText, characterRanges)) {
-                    return writeFormatReplacements(replacements);
-                }
-                String wrappedFormattedText = StringWrapper.wrap(options.maxLineLength(), formattedText, formatter);
-                // if no wrapping change happened, return the initial replacements
-                if (wrappedFormattedText.equals(formattedText)) {
-                    return writeFormatReplacements(replacements);
-                }
-                return writeFormatReplacements(List.of(Replacement.create(0, input.length(), wrappedFormattedText)));
-            } else {
-                return writeFormatReplacements(replacements);
-            }
+            return writeFormatReplacements(
+                    parameters.reflowLongStrings()
+                            ? formatter.getFormatAndReflowReplacements(input, characterRanges)
+                            : formatter.getFormatReplacements(input, characterRanges.asRanges()));
         }
         return formatFile(formatter);
     }

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);
     }
 
@@ -364,4 +366,27 @@ public List<Replacement> getFormatReplacements(String input, Collection<Range<In
         RangeSet<Integer> tokenRangeSet = javaInput.characterRangesToTokenRanges(characterRanges);
         return javaOutput.getFormatReplacements(tokenRangeSet);
     }
+
+    /**
+     * Emit the {@link Replacement}s that format the given character ranges and reflow the long strings in the code
+     * that formatting them rewrites; the imports, and the strings elsewhere, stay as they are. The command line prints
+     * these for {@code --output-replacements}, and {@link FormatterServiceImpl#getFormatReplacements} returns them,
+     * so that {@code formatDiff} and IntelliJ IDEA format the same on the JVM and with the native binary.
+     */
+    List<Replacement> getFormatAndReflowReplacements(String input, RangeSet<Integer> characterRanges)
+            throws FormatterException {
+        List<Replacement> replacements = getFormatReplacements(input, characterRanges.asRanges());
+        String formatted = Utils.applyReplacements(input, replacements);
+        RangeSet<Integer> formattedRanges = StringWrapper.moveRanges(characterRanges, replacements);
+        // avoid trying to wrap the input unless the lines needing wrapping are part of the ranges
+        if (!StringWrapper.linesNeedWrapping(options.maxLineLength(), formatted, formattedRanges)) {
+            return replacements;
+        }
+        String wrapped = StringWrapper.wrap(options.maxLineLength(), formatted, formattedRanges, this);
+        // if no wrapping change happened, return the initial replacements
+        if (wrapped.equals(formatted)) {
+            return replacements;
+        }
+        return List.of(Replacement.create(0, input.length(), wrapped));
+    }
 }

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

@@ -18,6 +18,7 @@
 import com.google.auto.service.AutoService;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Range;
+import com.google.common.collect.TreeRangeSet;
 import com.palantir.javaformat.java.JavaFormatterOptions.Style;
 import java.util.Collection;
 
@@ -36,7 +37,8 @@ public FormatterServiceImpl() {
     @SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
     public ImmutableList<Replacement> getFormatReplacements(String text, Collection<Range<Integer>> toRanges)
             throws FormatterException {
-        return ImmutableList.copyOf(formatter.getFormatReplacements(text, toRanges));
+        // What the command line prints for --output-replacements, which the bootstrapping and native services run.
+        return ImmutableList.copyOf(formatter.getFormatAndReflowReplacements(text, TreeRangeSet.create(toRanges)));
     }
 
     @Override