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

.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);

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

@@ -22,7 +22,6 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Range;
 import com.google.common.collect.RangeSet;
@@ -110,7 +109,7 @@ private static void format(FormatterService formatter, SingleFileDiff diff) {
 
         try {
             System.err.println("Formatting " + diff.path);
-            ImmutableList<Replacement> replacements = formatter.getFormatReplacements(input, charRanges.asRanges());
+            List<Replacement> replacements = formatter.getFormatReplacements(input, charRanges.asRanges());
             String output = applyReplacements(input, replacements);
             Files.write(diff.path, output.getBytes(UTF_8));
         } catch (IOException | FormatterException e) {
@@ -136,8 +135,8 @@ private static String gitCommand(Path dir, String... args) throws IOException, I
         Preconditions.checkState(process.waitFor(30, TimeUnit.SECONDS), "git diff took too long to terminate");
         if (process.exitValue() != 0) {
             throw new RuntimeException("Expected return code of 0 but got " + process.exitValue()
-                    + "\n stdout: " + new String(process.getInputStream().readAllBytes())
-                    + "\n stderr: " + new String(process.getErrorStream().readAllBytes()));
+                    + "\n stdout: " + new String(process.getInputStream().readAllBytes(), UTF_8)
+                    + "\n stderr: " + new String(process.getErrorStream().readAllBytes(), UTF_8));
         }
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         process.getInputStream().transferTo(baos);

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

@@ -16,7 +16,7 @@
 package com.palantir.javaformat.gradle;
 
 import com.diffplug.gradle.spotless.SpotlessExtension;
-import com.google.common.collect.ImmutableList;
+import java.util.List;
 import org.gradle.api.Action;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
@@ -25,8 +25,8 @@
 
 public abstract class PalantirJavaFormatSpotlessPlugin implements Plugin<Project> {
     // The spotless gradle plugin got renamed to 'com.diffplug.spotless' at version 5.0.0
-    private static final ImmutableList<String> SPOTLESS_PLUGINS =
-            ImmutableList.of("com.diffplug.gradle.spotless", "com.diffplug.spotless");
+    private static final List<String> SPOTLESS_PLUGINS =
+            List.of("com.diffplug.gradle.spotless", "com.diffplug.spotless");
 
     @Override
     public void apply(Project project) {

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

@@ -21,7 +21,6 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
-import com.google.common.collect.ImmutableList;
 import com.palantir.javaformat.bootstrap.BootstrappingFormatterService;
 import com.palantir.javaformat.bootstrap.NativeImageFormatterService;
 import com.palantir.javaformat.java.FormatterService;
@@ -80,10 +79,10 @@ void reformat_a_subpath_of_a_git_directory_for_only_changed_lines(FormatterServi
         Files.createDirectories(subdir);
 
         Path reformatMe = subdir.resolve("ReformatMe.java");
-        Files.write(reformatMe, ImmutableList.of("                                 class ReformatMe {}"), UTF_8);
+        Files.write(reformatMe, List.of("                                 class ReformatMe {}"), UTF_8);
 
         Path dontTouchMe = repo.resolve("DontTouchMe.java");
-        Files.write(dontTouchMe, ImmutableList.of("                                 class DontTouchMe {}"), UTF_8);
+        Files.write(dontTouchMe, List.of("                                 class DontTouchMe {}"), UTF_8);
 
         runCommandInRepo("git", "add", "-N", ".");
 

open-java-format-eclipse-plugin/src/main/java/com/palantir/javaformat/java/SnippetFormatter.java

@@ -14,12 +14,9 @@
 
 package com.palantir.javaformat.java;
 
-import static com.google.common.collect.ImmutableList.toImmutableList;
-
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.DiscreteDomain;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Range;
 import com.google.common.collect.RangeSet;
 import com.google.common.collect.TreeRangeSet;
@@ -93,7 +90,7 @@ private static List<Range<Integer>> offsetRanges(List<Range<Integer>> ranges, in
      * Runs the Google Java formatter on the given source, with only the given
      * ranges specified.
      */
-    public ImmutableList<Replacement> format(
+    public List<Replacement> format(
             SnippetKind kind, String source, List<Range<Integer>> ranges, int initialIndent, boolean includeComments)
             throws FormatterException {
         RangeSet<Integer> rangeSet = TreeRangeSet.create();
@@ -115,7 +112,7 @@ public ImmutableList<Replacement> format(
 
         return toReplacements(source, replacement).stream()
                 .filter(r -> rangeSet.encloses(r.getReplaceRange()))
-                .collect(toImmutableList());
+                .toList();
     }
 
     /**

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

@@ -16,7 +16,9 @@
 
 package com.palantir.javaformat.intellij;
 
-import com.google.common.collect.ImmutableList;
+import static java.util.Comparator.comparing;
+import static java.util.Optional.ofNullable;
+
 import com.google.common.collect.Range;
 import com.intellij.formatting.service.AsyncDocumentFormattingService;
 import com.intellij.formatting.service.AsyncFormattingRequest;
@@ -33,19 +35,15 @@
 import com.palantir.javaformat.java.FormatterException;
 import com.palantir.javaformat.java.FormatterService;
 import com.palantir.javaformat.java.Replacement;
-import org.jspecify.annotations.Nullable;
-
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-
-import static java.util.Comparator.comparing;
-import static java.util.Optional.ofNullable;
+import org.jspecify.annotations.Nullable;
 
 class PalantirJavaFormatFormattingService extends AsyncDocumentFormattingService implements PluginAware {
     private static final Logger logger = Logger.getInstance(PalantirJavaFormatFormattingService.class);
     private final FormatterProvider formatterProvider =
             new FormatterProvider(() -> EmbeddedNativeImage.getInstance().installedPath());
 
@@ -191,12 +189,11 @@
             if (isWholeFile(request)) {
                 // The IDE sometimes passes invalid ranges when the file is unsaved before invoking the
                 // formatter. So this is a workaround for that issue.
-                return ImmutableList.of(
-                        Range.closedOpen(0, request.getDocumentText().length()));
+                return List.of(Range.closedOpen(0, request.getDocumentText().length()));
             }
             return request.getFormattingRanges().stream()
                     .map(textRange -> Range.closedOpen(textRange.getStartOffset(), textRange.getEndOffset()))
-                    .collect(ImmutableList.toImmutableList());
+                    .toList();
         }
 
         private static boolean isWholeFile(AsyncFormattingRequest request) {

open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java

@@ -29,6 +29,7 @@
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
@@ -51,6 +52,7 @@ public BootstrappingFormatterService(Path jdkPath, Integer jdkMajorVersion, List
     }
 
     @Override
+    @SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
     public ImmutableList<Replacement> getFormatReplacements(String input, Collection<Range<Integer>> ranges) {
         try {
             return getFormatReplacementsInternal(input, ranges);
@@ -77,6 +79,7 @@ public String fixImports(String input) throws FormatterException {
         }
     }
 
+    @SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
     private ImmutableList<Replacement> getFormatReplacementsInternal(String input, Collection<Range<Integer>> ranges)
             throws IOException {
         FormatterCliArgs command = new FormatterCliArgs(
@@ -125,27 +128,26 @@ record FormatterCliArgs(
             List<String> characterRanges) {
 
         List<String> toArgs() {
-            ImmutableList.Builder<String> args = ImmutableList.<String>builder()
-                    .add(jdkPath.toAbsolutePath().toString())
-                    .addAll(jvmArgs)
-                    .add(
-                            "-cp",
-                            implementationClasspath.stream()
-                                    .map(path -> path.toAbsolutePath().toString())
-                                    .collect(Collectors.joining(System.getProperty("path.separator"))))
-                    .add(FORMATTER_MAIN_CLASS);
+            var args = new ArrayList<String>();
+            args.add(jdkPath.toAbsolutePath().toString());
+            args.addAll(jvmArgs);
+            args.add("-cp");
+            args.add(implementationClasspath.stream()
+                    .map(path -> path.toAbsolutePath().toString())
+                    .collect(Collectors.joining(System.getProperty("path.separator"))));
+            args.add(FORMATTER_MAIN_CLASS);
 
             if (!characterRanges.isEmpty()) {
-                args.add("--character-ranges", Joiner.on(',').join(characterRanges));
+                args.add("--character-ranges");
+                args.add(Joiner.on(',').join(characterRanges));
             }
             if (outputReplacements) {
                 args.add("--output-replacements");
             }
 
-            return args
-                    // Trailing "-" enables formatting stdin -> stdout
-                    .add("-")
-                    .build();
+            // Trailing "-" enables formatting stdin -> stdout
+            args.add("-");
+            return List.copyOf(args);
         }
     }
 }

open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/NativeImageFormatterService.java

@@ -28,6 +28,7 @@
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
@@ -42,6 +43,7 @@ public NativeImageFormatterService(Path nativeImagePath) {
     }
 
     @Override
+    @SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
     public ImmutableList<Replacement> getFormatReplacements(String input, Collection<Range<Integer>> ranges) {
         Optional<String> output = Optional.empty();
         try {
@@ -91,20 +93,20 @@ private String runFormatterCommand(String input) throws IOException {
     record FormatterNativeImageArgs(Path nativeImagePath, boolean outputReplacements, List<String> characterRanges) {
 
         List<String> toArgs() {
-            ImmutableList.Builder<String> args = ImmutableList.<String>builder()
-                    .add(nativeImagePath.toAbsolutePath().toString());
+            var args = new ArrayList<String>();
+            args.add(nativeImagePath.toAbsolutePath().toString());
 
             if (!characterRanges.isEmpty()) {
-                args.add("--character-ranges", Joiner.on(',').join(characterRanges));
+                args.add("--character-ranges");
+                args.add(Joiner.on(',').join(characterRanges));
             }
             if (outputReplacements) {
                 args.add("--output-replacements");
             }
 
-            return args
-                    // Trailing "-" enables formatting stdin -> stdout
-                    .add("-")
-                    .build();
+            // Trailing "-" enables formatting stdin -> stdout
+            args.add("-");
+            return List.copyOf(args);
         }
     }
 }