[Enhancement] Only Send Updates When Formatting Changed

idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingService.java

@@ -36,7 +36,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.jetbrains.annotations.NotNull;
 
 class PalantirJavaFormatFormattingService extends AsyncDocumentFormattingService {
@@ -91,28 +90,32 @@ public void run() {
                 return;
             }
 
+            String preFormatText = request.getDocumentText();
+
             try {
                 if (logger.isDebugEnabled()) {
                     logger.debug(String.format(
                             "Received request to format file=%s, length=%s with ranges=%s",
                             Optional.ofNullable(request.getIOFile())
                                     .map(file -> file.toPath().toString())
                                     .orElse("null"),
-                            request.getDocumentText().length(),
+                            preFormatText.length(),
                             request.getFormattingRanges()));
                 }
-                List<Replacement> replacements =
-                        formatterService.get().getFormatReplacements(request.getDocumentText(), toRanges(request));
-                if (logger.isDebugEnabled()) {
-                    logger.debug(String.format(
-                            "Applying %s replacements with ranges=%s",
-                            replacements.size(),
-                            replacements.stream()
-                                    .map(Replacement::getReplaceRange)
-                                    .collect(Collectors.toSet())));
+
+                // Note: I attempted to implement this using getFormatReplacements and elide an update when
+                // there were no replacements. There is a bug in the underlying formatter where it _always_
+                // returns a change. Thus we must perform a content aware diff / branching.
+                String formattedText = applyReplacements(
+                        preFormatText, formatterService.get().getFormatReplacements(preFormatText, toRanges(request)));
+
+                // The Javadoc of this API says that you should set it to null when the document is unchanged
+                // We should not be trying to format a document that is already formatted
+                if (preFormatText.equals(formattedText)) {
+                    request.onTextReady(null);
+                } else {
+                    request.onTextReady(formattedText);
                 }
-                String formattedText = applyReplacements(request.getDocumentText(), replacements);
-                request.onTextReady(formattedText);
             } catch (FormatterException e) {
                 logger.error(
                         String.format(

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

@@ -21,6 +21,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import com.google.common.base.Joiner;
+import com.google.common.collect.Range;
 import com.google.common.io.CharStreams;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
@@ -31,6 +32,7 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.api.parallel.Execution;
@@ -452,4 +454,36 @@ void canParse_java9_private_interface_methods() {
                                 + "}"))
                 .doesNotThrowAnyException();
     }
+
+    @Test
+    void producesFormattingChangesOnAlreadyFormattedFiles() throws FormatterException {
+        Formatter formatter = Formatter.create();
+        String simpleClass = "package com.palantir;\n"
+                + "\n"
+                + "public final class Formatted {\n"
+                + "  public static int count(int number) {\n"
+                + "    return number;\n"
+                + "  }\n"
+                + "}";
+        // This is kinda overkill but the point of the test is to show that it
+        // is for sure formatted and still producing a replacement.
+        String formattedClass = formatter.formatSourceAndFixImports(simpleClass);
+        String reformattedClass = formatter.formatSourceAndFixImports(formattedClass);
+
+        // The formatSourceAndFixImports code path flows through getFormatReplacements and
+        // this shows that despite the replacements happening they are superfluous
+        assertThat(formattedClass).isEqualTo(reformattedClass);
+
+        assertWithMessage("""
+            If this test is failing and you are reading this message it means that an underlying bug
+            in the formatting service was fixed. Previously, this formatter always produced a "replacement"
+            even when the document was fully formatted. You can see evidence of this in
+            https://github.com/palantir/palantir-java-format/pull/1188. To fix this failing test, consider
+            relying on the new behavior and instead checking that the 'replacements' is empty instead of
+            full string comparison.
+            """)
+                .that(formatter.getFormatReplacements(
+                        formattedClass, List.of(Range.closedOpen(0, formattedClass.length()))))
+                .isNotEmpty();
+    }
 }