Repository navigation
Replace Guava's immutable collections with java.util and ban them outside the SPI - #218
Merged
abashev merged 6 commits intoNov 12, 2026
Conversation
ImmutableList, ImmutableMap, ImmutableSet and ImmutableSortedSet give way to List.of, List.copyOf, Map.of, Stream.toList() and, where a builder filled them, an ArrayList or a TreeSet handed out with List.copyOf or an unmodifiable view. FileBasedTests lists its test resources with Files.walk rather than Guava's ClassPath, whose getResources() returns an ImmutableSet. What differs between the two kinds of collection decided three places: - Newlines.BREAKS is a List: it is tried in order, "\r\n" before "\n", and Set.of keeps no order. StringWrapper's ranges are a LinkedHashSet for the same reason. - JavaInput.buildToks asks its stop tokens whether they contain the kind of a comment token, which is null, and Set.of throws on contains(null). They are a HashSet. - Error Prone's @immutable cannot prove a java.util List or Map immutable, so it is gone from Input.Token, JavaInput.Token, OpsBuilder.BlankLineWanted, InputMetadata and doc.Token. The SPI does not change. FormatterService.getFormatReplacements still returns ImmutableList, so that code compiled against an older SPI keeps linking, and its three implementations still declare it: FormatterServiceImpl copies Formatter's list into one. Formatter's own getFormatReplacements, Input.getPositionTokenMap (a RangeMap now) and SnippetFormatter.format return java.util types.
forbidden-apis 3.11 checks the bytecode of every module but the SPI against config/forbidden-apis/guava-immutables.txt, which names ImmutableList, ImmutableMap and ImmutableSet, and fails the build on a use of one. Error Prone has no check that bans a class, and Picnic's error-prone-support pushes towards Guava's immutables rather than away. The check sees what a source-level ban would miss: a var whose type is one of them, a subclass such as ImmutableSortedSet, and a call on what a Guava method like ClassPath.getResources() returns. The SPI is left out because FormatterService.getFormatReplacements returns ImmutableList for compatibility. The methods that implement it in open-java-format and open-java-format-jdk-bootstrap carry @SuppressForbidden, an annotation of each module's own: the default one would put forbidden-apis on their compile classpath. Code that calls the method and keeps the result as a List passes the check. ignoreSignaturesOfMissingClasses, because the benchmarks have no Guava on their classpath and would otherwise fail to parse the signatures.
The build job ran ./gradlew test, and forbidden-apis hangs off check, so the previous commit's ban would never have failed a pull request. check also runs Checkstyle and the Gradle plugin's validatePlugins, which pass. The jdk jobs, which run the same code on other JDKs, keep running only the tests. The README and CLAUDE.md say what CI runs.
Collectors.toUnmodifiableList() was there only because the list was declared List<Tree>, and Stream.toList() on getMembers(), a List<? extends Tree>, gives a list of the wildcard's capture. addBodyDeclarations takes a List<? extends Tree>, so the variable is a var and the stream ends in toList(), as CLAUDE.md asks.
FormatDiff decoded what a failed git command printed to stdout and stderr in the platform's charset, while the output of one that succeeds, three lines further down, is read as UTF-8. Both are UTF-8 now.
The block comment above JavaInput's fields described "the following
lists" of tok indices on each line, which live in InputOutput.ranges,
and Error Prone's AlmostJavadoc flagged its {@code} tags. What it said
that the field's Javadoc did not, that a /* comment spans lines and
that the lists give the correspondence between input and output, is
in that Javadoc now.
abashev
deleted the
abashev/replace-guava-immutablelist-immutablemap-immutab
branch
November 12, 2026 17:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before this PR
The code used Guava's
ImmutableList,ImmutableMapandImmutableSetthroughout, althoughCLAUDE.mdasks new code for plainjava.utilcollections, and nothing stopped them from coming back. #202 has the plan and the pitfalls found on the way.After this PR
==COMMIT_MSG==
Guava's ImmutableList, ImmutableMap and ImmutableSet give way to java.util collections, and forbidden-apis fails the build on them in every module but the SPI, whose FormatterService keeps returning ImmutableList for compatibility.
==COMMIT_MSG==
Closes #202.
List.of,List.copyOf,Map.of,Stream.toList(), and anArrayListorTreeSetwhere a builder was filled. Three places depended on how Guava's collections behave:Newlines.BREAKSis aList, since it is tried in order andSet.ofkeeps none;JavaInput.buildTokskeeps its stop tokens in aHashSet, since it asks them about a null token kind;@Immutableis gone from five classes, since Error Prone cannot prove ajava.utilListorMapimmutable.FileBasedTestslists its resources withFiles.walkinstead of Guava'sClassPath.open-java-format.forbidden-apis-conventions, the signatures inconfig/forbidden-apis/guava-immutables.txt. It works on bytecode, so it also catches avar, a subclass such asImmutableSortedSet, and a call on what a Guava method returns.FormatterService.getFormatReplacementsreturnsImmutableList, so that code compiled against an older SPI keeps linking. Its three implementations,FormatterServiceImpl,BootstrappingFormatterServiceandNativeImageFormatterService, have to declare it too; those methods carry a package-private@SuppressForbidden(reason = …)of their module, and the rest of each class is checked. Code that calls the method and keeps the result as aListpasses the check../gradlew checkrather than./gradlew test, since forbidden-apis hangs offcheck. That adds Checkstyle andvalidatePlugins, which pass. Thejdkjobs still run only the tests.DefaultCharsetis an error (last commit). Its two findings were inFormatDiff, which decoded what a failed git command printed in the platform's charset, while a successful one's output is read as UTF-8; both are UTF-8 now.How it was checked
./gradlew check: 1422 tests inopen-java-format(14 skipped, as on21.X.X), 141 in the Gradle plugin, 65 in the IntelliJ plugin, then Checkstyle, forbidden-apis andvalidatePlugins.nativejob does, on GraalVM CE 25.0.2:-PnativeImage=true :open-java-format-native:nativeCompile, then:open-java-format-jdk-bootstrap:test :gradle-open-java-format:testagainst the image, 8 and 152 tests.ImmutableList.copyOfput intoNativeImageFormatterService'stoArgs()failsforbiddenApisMain(tried, then reverted).Possible downsides?
Formatter.getFormatReplacements,Input.getPositionTokenMapandSnippetFormatter.formatare public and now returnListandRangeMap, so code compiled against the oldImmutableListandImmutableRangeMapsignatures would no longer link. Spotless's glue calls none of them (Replace Guava ImmutableList/ImmutableMap/ImmutableSet with java.util and ban them with forbidden-apis #202 lists what it links against).List.ofand its kin throw on null elements and oncontains(null), andSet.ofandMap.ofkeep no order. The places the tests showed are fixed; a path no test reaches could still hit one of these.CheckForbiddenApisis not cacheable. It is up to date when nothing changed and takes some 0.03 s a module when it runs.