Skip to content

Commit 40df976

Browse files
committed
Ban Guava's immutable collections outside the SPI
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.
1 parent 14d3ed0 commit 40df976

10 files changed

Lines changed: 83 additions & 2 deletions

File tree

‎CLAUDE.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ Building and testing: the README's "Building and testing locally" section. Gradl
66
## Conventions for new code
77

88
- **Collections.** Plain `java.util` types. `List.of`, `List.copyOf`, `Map.of` and `Stream.toList()` give
9-
unmodifiable collections, so new code does not need Guava's `ImmutableList` or `ImmutableMap`; use them only
10-
where an existing API already hands one over.
9+
unmodifiable collections; unlike Guava's, `Set.of` and `Map.of` do not keep insertion order, and `contains(null)`
10+
on them throws. Guava's `ImmutableList`, `ImmutableMap` and `ImmutableSet` are banned by forbidden-apis
11+
(`config/forbidden-apis`), which fails the build, in every module but the SPI: `FormatterService` keeps returning
12+
`ImmutableList` for compatibility, and the methods that implement it carry their module's `@SuppressForbidden`.
1113
- **Nullability.** One annotation, JSpecify's `org.jspecify.annotations.Nullable`; do not add another `Nullable`,
1214
nor a `NotNull` or `NonNull`: what is not `@Nullable` is non-null, and NullAway checks it. It is a type-use
1315
annotation, so on a nested type it goes before the simple name (`Input.@Nullable Token`), and on an array before

‎build.gradle‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ subprojects {
8383
// Java toolchains, javac and test settings; Error Prone. Both in buildSrc/src/main/groovy.
8484
apply plugin: 'open-java-format.java-conventions'
8585
apply plugin: 'open-java-format.error-prone-conventions'
86+
// Bans Guava's immutable collections, but not in the SPI: FormatterService.getFormatReplacements keeps
87+
// returning ImmutableList, so that code compiled against an older SPI still links.
88+
if (path != ':open-java-format-spi') {
89+
apply plugin: 'open-java-format.forbidden-apis-conventions'
90+
}
8691

8792
dependencies {
8893
// Gives a version to the JUnit artifacts that useJUnitJupiter() adds without one.

‎buildSrc/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,6 @@ repositories {
1313
dependencies {
1414
implementation 'net.ltgt.gradle:gradle-errorprone-plugin:5.2.0'
1515
implementation 'net.ltgt.gradle:gradle-nullaway-plugin:3.3.0'
16+
implementation 'de.thetaphi:forbiddenapis:3.11'
1617
implementation 'org.jreleaser:org.jreleaser.gradle.plugin:1.26.0'
1718
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Bans Guava's immutable collections in favour of java.util ones; the list is config/forbidden-apis.
2+
plugins {
3+
id 'java'
4+
id 'de.thetaphi.forbiddenapis'
5+
}
6+
7+
forbiddenApis {
8+
signaturesFiles = files(rootProject.file('config/forbidden-apis/guava-immutables.txt'))
9+
// A module without Guava on its classpath cannot use the classes anyway.
10+
ignoreSignaturesOfMissingClasses = true
11+
// A module's own package-private SuppressForbidden, so the code does not depend on forbidden-apis.
12+
suppressAnnotations = ['**.SuppressForbidden']
13+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
@defaultMessage Use java.util collections: List.of, List.copyOf, Map.of, Stream.toList()
2+
com.google.common.collect.ImmutableList
3+
com.google.common.collect.ImmutableMap
4+
com.google.common.collect.ImmutableSet

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public BootstrappingFormatterService(Path jdkPath, Integer jdkMajorVersion, List
5252
}
5353

5454
@Override
55+
@SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
5556
public ImmutableList<Replacement> getFormatReplacements(String input, Collection<Range<Integer>> ranges) {
5657
try {
5758
return getFormatReplacementsInternal(input, ranges);
@@ -78,6 +79,7 @@ public String fixImports(String input) throws FormatterException {
7879
}
7980
}
8081

82+
@SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
8183
private ImmutableList<Replacement> getFormatReplacementsInternal(String input, Collection<Range<Integer>> ranges)
8284
throws IOException {
8385
FormatterCliArgs command = new FormatterCliArgs(

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public NativeImageFormatterService(Path nativeImagePath) {
4343
}
4444

4545
@Override
46+
@SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
4647
public ImmutableList<Replacement> getFormatReplacements(String input, Collection<Range<Integer>> ranges) {
4748
Optional<String> output = Optional.empty();
4849
try {
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.palantir.javaformat.bootstrap;
15+
16+
import java.lang.annotation.ElementType;
17+
import java.lang.annotation.Retention;
18+
import java.lang.annotation.RetentionPolicy;
19+
import java.lang.annotation.Target;
20+
21+
/** Lets forbidden-apis accept a use of an API that config/forbidden-apis bans, for the reason given. */
22+
@Retention(RetentionPolicy.CLASS)
23+
@Target({ElementType.TYPE, ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.FIELD})
24+
@interface SuppressForbidden {
25+
String reason();
26+
}

‎open-java-format/src/main/java/com/palantir/javaformat/java/FormatterServiceImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public FormatterServiceImpl() {
3333
}
3434

3535
@Override
36+
@SuppressForbidden(reason = "FormatterService, kept in the SPI for compatibility, returns ImmutableList")
3637
public ImmutableList<Replacement> getFormatReplacements(String text, Collection<Range<Integer>> toRanges)
3738
throws FormatterException {
3839
return ImmutableList.copyOf(formatter.getFormatReplacements(text, toRanges));
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.palantir.javaformat.java;
15+
16+
import java.lang.annotation.ElementType;
17+
import java.lang.annotation.Retention;
18+
import java.lang.annotation.RetentionPolicy;
19+
import java.lang.annotation.Target;
20+
21+
/** Lets forbidden-apis accept a use of an API that config/forbidden-apis bans, for the reason given. */
22+
@Retention(RetentionPolicy.CLASS)
23+
@Target({ElementType.TYPE, ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.FIELD})
24+
@interface SuppressForbidden {
25+
String reason();
26+
}

0 commit comments

Comments
 (0)