Fix `java.lang.RuntimeException: Document is locked by write PSI oper…

…ations` errors Also update to `google-java-format` 1.17.0 Fixes #960 COPYBARA_INTEGRATE_REVIEW=#960 from facboy:master e0925a2 PiperOrigin-RevId: 563163224
google · Sep 6, 2023 · 28b199c · 28b199c
1 parent 4ebb6f5
commit 28b199c
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 13 deletions.
diff --git a/idea_plugin/build.gradle.kts b/idea_plugin/build.gradle.kts
@@ -14,15 +14,15 @@
  * limitations under the License.
  */
 
-plugins { id("org.jetbrains.intellij") version "1.13.3" }
+plugins { id("org.jetbrains.intellij") version "1.15.0" }
 
 apply(plugin = "org.jetbrains.intellij")
 
 apply(plugin = "java")
 
 repositories { mavenCentral() }
 
-val googleJavaFormatVersion = "1.16.0"
+val googleJavaFormatVersion = "1.17.0"
 
 java {
   sourceCompatibility = JavaVersion.VERSION_11
@@ -37,7 +37,7 @@ intellij {
 
 tasks {
   patchPluginXml {
-    version.set("${googleJavaFormatVersion}.2")
+    version.set("${googleJavaFormatVersion}.0")
     sinceBuild.set("213")
     untilBuild.set("")
   }
@@ -49,12 +49,12 @@ tasks {
 
   withType<Test>().configureEach {
     jvmArgs(
-      "--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
-      "--add-exports jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
-      "--add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
-      "--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
-      "--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
-      "--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+      "--add-exports", "jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+      "--add-exports", "jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
+      "--add-exports", "jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+      "--add-exports", "jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+      "--add-exports", "jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+      "--add-exports", "jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
     )
   }
 }

diff --git a/...n/src/main/java/com/google/googlejavaformat/intellij/GoogleJavaFormatImportOptimizer.java b/...n/src/main/java/com/google/googlejavaformat/intellij/GoogleJavaFormatImportOptimizer.java
@@ -55,16 +55,36 @@ public boolean supports(@NotNull PsiFile file) {
 
     JavaFormatterOptions.Style style = GoogleJavaFormatSettings.getInstance(project).getStyle();
 
+    final String origText = document.getText();
     String text;
     try {
-      text =
-          ImportOrderer.reorderImports(
-              RemoveUnusedImports.removeUnusedImports(document.getText()), style);
+      text = ImportOrderer.reorderImports(RemoveUnusedImports.removeUnusedImports(origText), style);
     } catch (FormatterException e) {
       Notifications.displayParsingErrorNotification(project, file.getName());
       return Runnables.doNothing();
     }
 
-    return () -> document.setText(text);
+    /* pointless to change document text if it hasn't changed, plus this can interfere with
+    e.g. GoogleJavaFormattingService's output, i.e. it can overwrite the results from the main
+    formatter. */
+    if (text.equals(origText)) {
+      return Runnables.doNothing();
+    }
+
+    return () -> {
+      if (documentManager.isDocumentBlockedByPsi(document)) {
+        documentManager.doPostponedOperationsAndUnblockDocument(document);
+      }
+
+      /* similarly to above, don't overwrite new document text if it has changed - we use
+      getCharsSequence() as we should have `writeAction()` (which I think means effectively a
+      write-lock) and it saves calling getText(), which apparently is expensive. */
+      CharSequence newText = document.getCharsSequence();
+      if (CharSequence.compare(origText, newText) != 0) {
+        return;
+      }
+
+      document.setText(text);
+    };
   }
 }