Merge pull request #218 from openjavaformat/abashev/replace-guava-immutablelist-immutablemap-immutab

Replace Guava's immutable collections with java.util and ban them outside the SPI

Author: abashev

.github/workflows/ci.yml

@@ -51,7 +51,7 @@ jobs:
             caches/*/transforms
 
       - name: Build
-        run: ./gradlew test
+        run: ./gradlew check
 
       - name: Publish Test Report
         uses: mikepenz/action-junit-report@4a26df3e2ae132254ba12a53f65a388d0961e786 # v6.7.0

CLAUDE.md

@@ -1,13 +1,15 @@
 # Working in this repository
 
 Building and testing: the README's "Building and testing locally" section. Gradle runs on the Temurin 21 that
-`mise install` provides, `./gradlew test` is what CI runs, and `./gradlew formatDiff` formats the files you changed.
+`mise install` provides, `./gradlew check` is what CI runs, and `./gradlew formatDiff` formats the files you changed.
 
 ## Conventions for new code
 
 - **Collections.** Plain `java.util` types. `List.of`, `List.copyOf`, `Map.of` and `Stream.toList()` give
-  unmodifiable collections, so new code does not need Guava's `ImmutableList` or `ImmutableMap`; use them only
-  where an existing API already hands one over.
+  unmodifiable collections; unlike Guava's, `Set.of` and `Map.of` do not keep insertion order, and `contains(null)`
+  on them throws. Guava's `ImmutableList`, `ImmutableMap` and `ImmutableSet` are banned by forbidden-apis
+  (`config/forbidden-apis`), which fails the build, in every module but the SPI: `FormatterService` keeps returning
+  `ImmutableList` for compatibility, and the methods that implement it carry their module's `@SuppressForbidden`.
 - **Nullability.** One annotation, JSpecify's `org.jspecify.annotations.Nullable`; do not add another `Nullable`,
   nor a `NotNull` or `NonNull`: what is not `@Nullable` is non-null, and NullAway checks it. It is a type-use
   annotation, so on a nested type it goes before the simple name (`Input.@Nullable Token`), and on an array before

README.md

@@ -134,7 +134,7 @@ fixes made the output differ from upstream, that numbering lost its meaning.
 
 ```bash
 mise trust && mise install
-./gradlew test      # what the CI build job runs
+./gradlew check     # what the CI build job runs: the tests, Checkstyle and forbidden-apis
 ```
 
 `-PjavaRuntime=25` runs the same tests on JDK 25, as CI's `jdk` jobs do for 25, 26 and 27, and for

build.gradle

@@ -83,6 +83,11 @@ subprojects {
     // Java toolchains, javac and test settings; Error Prone. Both in buildSrc/src/main/groovy.
     apply plugin: 'open-java-format.java-conventions'
     apply plugin: 'open-java-format.error-prone-conventions'
+    // Bans Guava's immutable collections, but not in the SPI: FormatterService.getFormatReplacements keeps
+    // returning ImmutableList, so that code compiled against an older SPI still links.
+    if (path != ':open-java-format-spi') {
+        apply plugin: 'open-java-format.forbidden-apis-conventions'
+    }
 
     dependencies {
         // Gives a version to the JUnit artifacts that useJUnitJupiter() adds without one.

buildSrc/build.gradle

@@ -13,5 +13,6 @@ repositories {
 dependencies {
     implementation 'net.ltgt.gradle:gradle-errorprone-plugin:5.2.0'
     implementation 'net.ltgt.gradle:gradle-nullaway-plugin:3.3.0'
+    implementation 'de.thetaphi:forbiddenapis:3.11'
     implementation 'org.jreleaser:org.jreleaser.gradle.plugin:1.26.0'
 }

buildSrc/src/main/groovy/open-java-format.error-prone-conventions.gradle

@@ -27,6 +27,7 @@ tasks.withType(JavaCompile).configureEach {
         }
         error('CanonicalAnnotationSyntax')
         error('CollectorMutability')
+        error('DefaultCharset')
         error('DirectReturn')
         error('EffectivelyPrivate')
         error('FormatStringConcatenation')

buildSrc/src/main/groovy/open-java-format.forbidden-apis-conventions.gradle

@@ -0,0 +1,13 @@
+// Bans Guava's immutable collections in favour of java.util ones; the list is config/forbidden-apis.
+plugins {
+    id 'java'
+    id 'de.thetaphi.forbiddenapis'
+}
+
+forbiddenApis {
+    signaturesFiles = files(rootProject.file('config/forbidden-apis/guava-immutables.txt'))
+    // A module without Guava on its classpath cannot use the classes anyway.
+    ignoreSignaturesOfMissingClasses = true
+    // A module's own package-private SuppressForbidden, so the code does not depend on forbidden-apis.
+    suppressAnnotations = ['**.SuppressForbidden']
+}

config/forbidden-apis/guava-immutables.txt

@@ -0,0 +1,4 @@
+@defaultMessage Use java.util collections: List.of, List.copyOf, Map.of, Stream.toList()
+com.google.common.collect.ImmutableList
+com.google.common.collect.ImmutableMap
+com.google.common.collect.ImmutableSet

gradle-open-java-format/src/main/groovy/com/palantir/javaformat/gradle/PalantirJavaFormatIdeaPlugin.java

@@ -17,7 +17,6 @@
 package com.palantir.javaformat.gradle;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import java.util.stream.Stream;
 import org.gradle.StartParameter;
 import org.gradle.api.Plugin;
@@ -80,9 +79,8 @@ public void apply(Project rootProject) {
     // Adds the tasks to the Gradle start parameters, so they run with whatever the build was asked to do.
     private static void runWithEveryBuild(Project rootProject, TaskProvider<?>... tasks) {
         StartParameter startParameter = rootProject.getGradle().getStartParameter();
-        startParameter.setTaskNames(ImmutableList.<String>builder()
-                .addAll(startParameter.getTaskNames())
-                .addAll(Stream.of(tasks).map(task -> ":" + task.getName()).toList())
-                .build());
+        startParameter.setTaskNames(Stream.concat(
+                        startParameter.getTaskNames().stream(), Stream.of(tasks).map(task -> ":" + task.getName()))
+                .toList());
     }
 }

gradle-open-java-format/src/main/groovy/com/palantir/javaformat/gradle/XmlUtils.java

@@ -16,7 +16,6 @@
 
 package com.palantir.javaformat.gradle;
 
-import com.google.common.collect.ImmutableMap;
 import com.google.common.io.Files;
 import groovy.util.Node;
 import groovy.xml.XmlNodePrinter;
@@ -26,6 +25,7 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.nio.charset.Charset;
+import java.util.Map;
 import java.util.function.Consumer;
 import javax.xml.parsers.ParserConfigurationException;
 import org.xml.sax.SAXException;
@@ -41,7 +41,7 @@ public static void updateIdeaXmlFile(File configurationFile, Consumer<Node> conf
                 throw new RuntimeException("Couldn't parse existing configuration file: " + configurationFile, e);
             }
         } else {
-            rootNode = new Node(null, "project", ImmutableMap.of("version", "4"));
+            rootNode = new Node(null, "project", Map.of("version", "4"));
         }
 
         configure.accept(rootNode);