Fail the build on NullAway and ten more Error Prone checks

CLAUDE.md

@@ -8,8 +8,10 @@ Building and testing: the README's "Building and testing locally" section. Gradl
 - **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.
-- **Nullability.** One annotation, `javax.annotation.Nullable` (jsr305), the one the code uses; do not add another
-  `Nullable`.
+- **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
+  the brackets (`String @Nullable []`).
 - **Local variables.** `var` wherever the initializer gives the type unchanged, in `for` loops too. Not where it
   would box a primitive (`for (var n : List<Integer>)`), and not without an initializer that has a type of its own
   (a lambda, `null`).

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

@@ -11,13 +11,36 @@ dependencies {
 }
 
 nullaway {
+    severity = net.ltgt.gradle.errorprone.CheckSeverity.ERROR
     annotatedPackages.add('com.palantir')
+    jspecifyMode = true
 }
 
 tasks.withType(JavaCompile).configureEach {
+    // JSpecify mode reads @Nullable on types from class files, which javac 21 does only with this flag.
+    options.compilerArgs.add('-XDaddTypeAnnotationsToSymbol=true')
     options.errorprone {
         // Annotation processor output (Immutables) is not ours to fix.
         excludedPaths = '.*/build/generated/.*'
+        nullaway {
+            handleTestAssertionLibraries = true
+        }
+        error('CanonicalAnnotationSyntax')
+        error('CollectorMutability')
+        error('DirectReturn')
+        error('EffectivelyPrivate')
+        error('FormatStringConcatenation')
+        error('JUnitClassModifiers')
+        error('JUnitMethodDeclaration')
+        error('RedundantStringConversion')
+        error('RefactorSwitch')
+        error('SameNameButDifferent')
+        error('StringConcatToTextBlock')
+        error('UnusedVariable')
+        disable('IsInstanceLambdaUsage')
+        disable('LexicographicalAnnotationAttributeListing')
+        disable('LexicographicalAnnotationListing')
+        disable('StringJoining')
     }
 }
 
@@ -27,27 +50,3 @@ pluginManager.withPlugin('java-gradle-plugin') {
         options.errorprone.disable('JavaxInjectOnAbstractMethod')
     }
 }
-
-// The checks vfs-s3 makes build-breaking. The commented ones stay off until the code is fixed, one check at a
-// time: StaticImport 84, LexicographicalAnnotationListing 46, RedundantStringConversion 6, AlmostJavadoc 3
-// findings; the rest have none.
-tasks.withType(JavaCompile).configureEach {
-    options.errorprone {
-        //error('StaticImport')
-        //error('UnnecessarilyFullyQualified')
-        //error('EmptyBlockTag')
-        //error('OperatorPrecedence')
-        //error('RedundantStringConversion')
-        //error('PatternMatchingInstanceof')
-        //error('UnnecessaryParentheses')
-        //error('LexicographicalAnnotationListing')
-        error('JUnitMethodDeclaration')
-        error('JUnitClassModifiers')
-        //error('MissingSummary')
-        //error('AlmostJavadoc')
-        //error('BadImport')
-        //error('JdkObsolete')
-        // NonStaticImport (Picnic) is the inverse of StaticImport and would give contradictory advice.
-        //disable('NonStaticImport')
-    }
-}

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

@@ -14,11 +14,12 @@
 
 package com.palantir.javaformat.gradle;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UncheckedIOException;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.DigestInputStream;
@@ -86,10 +87,7 @@ static List<File> jars(Path gradleUserHome, ClassLoader plugin) {
     }
 
     private static List<Jar> list(ClassLoader plugin) {
-        return new String(resource(plugin, LIST), StandardCharsets.UTF_8)
-                .lines()
-                .map(Jar::parse)
-                .toList();
+        return new String(resource(plugin, LIST), UTF_8).lines().map(Jar::parse).toList();
     }
 
     private static boolean isComplete(Path dir, List<Jar> list) {

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

@@ -129,7 +129,6 @@
         return Paths.get(gitCommand(dir, "git", "rev-parse", "--show-toplevel"));
     }
 
-    @SuppressWarnings("for-rollout:DefaultCharset")
     private static String gitCommand(Path dir, String... args) throws IOException, InterruptedException {
         Process process =
                 new ProcessBuilder().command(args).directory(dir.toFile()).start();
@@ -137,8 +136,8 @@
         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()));
         }
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         process.getInputStream().transferTo(baos);

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

@@ -69,7 +69,6 @@ public static synchronized FormatterService load(Collection<File> jars) {
         return LOADED.computeIfAbsent(key, _key -> loadUncached(jars));
     }
 
-    @SuppressWarnings("for-rollout:NullAway")
     private static FormatterService loadUncached(Collection<File> jars) {
         URL[] jarUris = jars.stream()
                 .map(file -> {

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

@@ -25,16 +25,14 @@
 
     static final String CONFIGURATION_NAME = "palantirJavaFormat";
 
-    @SuppressWarnings({"for-rollout:GradleTypesAsFields", "for-rollout:NonAbstractGradleType"})
     @Override
     public void apply(Project rootProject) {
         Preconditions.checkState(
                 rootProject == rootProject.getRootProject(),
                 "May only apply dev.openjavaformat.java-format-provider to the root project");
 
         rootProject.getPluginManager().apply(NativeImageFormatProviderPlugin.class);
 
-        @SuppressWarnings("for-rollout:ConfigurationAvoidanceRegistration")
         Configuration configuration = rootProject.getConfigurations().create(CONFIGURATION_NAME, conf -> {
             // Empty unless the build declares a formatter: the plugin carries its own, and needs no repository.
             conf.setDescription("Overrides the open-java-format implementation that comes inside the plugin");

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

@@ -84,8 +84,7 @@ Signature sign() throws IOException {
     static final class Signature implements Serializable {
         private static final long serialVersionUID = 1L;
 
-        // Spotless compares the signature in its serialized form, and reads nothing out of it.
-        @SuppressWarnings("UnusedVariable")
+        /** The hash of the plugin's own jars, or empty when the build declared its own. */
         private final String bundled;
 
         private final FileSignature override;
@@ -100,7 +99,7 @@ static final class Signature implements Serializable {
         }
 
         List<File> jars() {
-            return BundledFormatter.jarsUnlessOverridden(override.files(), gradleUserHome);
+            return bundled.isEmpty() ? List.copyOf(override.files()) : BundledFormatter.jars(gradleUserHome);
         }
     }
 

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

@@ -14,6 +14,7 @@
 
 package com.palantir.javaformat.gradle;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -23,7 +24,6 @@
 import java.io.UncheckedIOException;
 import java.net.URL;
 import java.net.URLClassLoader;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.FileTime;
@@ -148,8 +148,7 @@ void a_jar_that_does_not_match_the_list_fails_and_leaves_nothing_behind(@TempDir
         var bundle = Files.createDirectories(plugin.resolve(BUNDLE));
         Files.writeString(bundle.resolve("formatter.jar"), "not what the list says");
         Files.writeString(
-                bundle.resolve("jars.txt"),
-                sha256("what the list says".getBytes(StandardCharsets.UTF_8)) + " formatter.jar\n");
+                bundle.resolve("jars.txt"), sha256("what the list says".getBytes(UTF_8)) + " formatter.jar\n");
 
         try (var loader = new URLClassLoader(new URL[] {plugin.toUri().toURL()}, null)) {
             assertThatThrownBy(() -> BundledFormatter.jars(gradleUserHome, loader))
@@ -161,9 +160,7 @@ void a_jar_that_does_not_match_the_list_fails_and_leaves_nothing_behind(@TempDir
     }
 
     private static List<String> bundledList() {
-        return new String(resource(BUNDLE + "jars.txt"), StandardCharsets.UTF_8)
-                .lines()
-                .toList();
+        return new String(resource(BUNDLE + "jars.txt"), UTF_8).lines().toList();
     }
 
     private static List<String> bundledNames() {

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

@@ -34,7 +34,6 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -58,7 +57,7 @@ void parsing_git_diff_output_works() throws IOException {
 
         List<String> strings = FormatDiff.parseGitDiffOutput(example1)
                 .map(FormatDiff.SingleFileDiff::toString)
-                .collect(Collectors.toList());
+                .toList();
         assertThat(strings)
                 .containsExactly(
                         "SingleFileDiff{path=build.gradle, lineRanges=[[24..25), [29..30)]}",
@@ -104,7 +103,7 @@ private void runCommandInRepo(String... args) throws IOException, InterruptedExc
         process.getErrorStream().transferTo(baos);
         String stderr = baos.toString(UTF_8);
 
-        Preconditions.checkState(process.exitValue() == 0, "Expected return code of 0: " + stderr);
+        Preconditions.checkState(process.exitValue() == 0, "Expected return code of 0: %s", stderr);
     }
 
     private static Stream<FormatterService> getFormatters() throws IOException {
@@ -121,7 +120,7 @@ private static List<Path> getClasspath() throws IOException {
                 .omitEmptyStrings()
                 .splitToStream(Files.readString(CLASSPATH_FILE.toPath()))
                 .map(Path::of)
-                .collect(Collectors.toList());
+                .toList();
     }
 
     private static Path javaBinPath() {

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

@@ -45,8 +45,10 @@ void the_copy_is_executable_and_the_image_is_left_as_it_was() throws IOException
 
         var copy = NativeImageAtomicCopy.copyToCache(image, gradleUserHome.toFile());
 
-        assertThat(copy.getParent().getParent()).isEqualTo(gradleUserHome.resolve("open-java-format-caches"));
-        assertThat(copy.getParent().getFileName().toString()).matches("[0-9a-f]{16}");
+        var cacheDir = copy.getParent();
+        assertThat(cacheDir).isNotNull();
+        assertThat(cacheDir.getParent()).isEqualTo(gradleUserHome.resolve("open-java-format-caches"));
+        assertThat(cacheDir.getFileName().toString()).matches("[0-9a-f]{16}");
         assertThat(copy.getFileName()).isEqualTo(image.getFileName());
         assertThat(copy).hasContent("an image");
         assertThat(Files.isExecutable(copy)).isTrue();

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

@@ -14,6 +14,7 @@
 
 package com.palantir.javaformat.gradle;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.fasterxml.jackson.databind.json.JsonMapper;
@@ -22,7 +23,6 @@
 import com.palantir.javaformat.java.Replacement;
 import java.io.IOException;
 import java.io.UncheckedIOException;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.PosixFilePermissions;
@@ -154,7 +154,7 @@ private static String lddVersion() {
             var process = new ProcessBuilder("ldd", "--version")
                     .redirectErrorStream(true)
                     .start();
-            var output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
+            var output = new String(process.getInputStream().readAllBytes(), UTF_8);
             process.waitFor();
             return output.lines().findFirst().orElse("");
         } catch (IOException e) {

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

@@ -93,11 +93,7 @@ void format_ignores_excluded_directories(String srcDir) {
     }
 
     @ParameterizedTest
-    @ValueSource(
-            strings = {
-                "foo/generated_foo/src/bar",
-                "src/main/java/test/generatedNamespace",
-            })
+    @ValueSource(strings = {"foo/generated_foo/src/bar", "src/main/java/test/generatedNamespace"})
     void format_checks_non_generated_files(String srcDir) {
         project.buildGradle("""
             sourceSets {

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

@@ -16,11 +16,12 @@
 
 package com.palantir.javaformat.gradle.testing;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UncheckedIOException;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -164,7 +165,7 @@ public GradleTestProject writeFile(String relativePath, String content) {
         Path target = file(relativePath);
         try {
             Files.createDirectories(target.getParent());
-            Files.writeString(target, content, StandardCharsets.UTF_8);
+            Files.writeString(target, content, UTF_8);
         } catch (IOException e) {
             throw new UncheckedIOException("Could not write " + relativePath, e);
         }
@@ -189,7 +190,7 @@ public GradleTestProject delete(String relativePath) {
 
     public String readFile(String relativePath) {
         try {
-            return Files.readString(file(relativePath), StandardCharsets.UTF_8);
+            return Files.readString(file(relativePath), UTF_8);
         } catch (IOException e) {
             throw new UncheckedIOException("Could not read " + relativePath, e);
         }

gradle/libs.versions.toml

@@ -39,7 +39,7 @@ jackson-datatype-jdk8 = { module = "com.fasterxml.jackson.datatype:jackson-datat
 jackson-module-parameterNames = { module = "com.fasterxml.jackson.module:jackson-module-parameter-names", version = "2.22.4" }
 jmh-core = { module = "org.openjdk.jmh:jmh-core", version = "1.37" }
 jmh-generator-annprocess = { module = "org.openjdk.jmh:jmh-generator-annprocess", version = "1.37" }
-jsr305 = { module = "com.google.code.findbugs:jsr305", version = "3.0.2" }
+jspecify = { module = "org.jspecify:jspecify", version = "1.0.1" }
 junit-jupiter = { module = "org.junit.jupiter:junit-jupiter", version = "6.1.3" }
 junit-jupiter-migrationsupport = { module = "org.junit.jupiter:junit-jupiter-migrationsupport", version = "6.1.3" }
 junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher", version = "6.1.3" }

open-java-format-eclipse-plugin/build.gradle

@@ -28,7 +28,7 @@ dependencies {
     compileOnly libs.eclipse.text
 
     implementation project(':open-java-format')
-    implementation libs.jsr305
+    implementation libs.jspecify
 }
 
 // The dependencies embedded in the plugin jar. Named once, so the manifest can list exactly what

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

@@ -19,30 +19,30 @@
 import com.palantir.javaformat.java.SnippetFormatter.SnippetKind;
 import java.util.ArrayList;
 import java.util.List;
-import javax.annotation.Nullable;
 import org.eclipse.jdt.core.dom.ASTParser;
 import org.eclipse.jdt.core.formatter.CodeFormatter;
 import org.eclipse.jface.text.IRegion;
 import org.eclipse.jface.text.Region;
 import org.eclipse.text.edits.MultiTextEdit;
 import org.eclipse.text.edits.ReplaceEdit;
 import org.eclipse.text.edits.TextEdit;
+import org.jspecify.annotations.Nullable;
 
 /** Runs the Palantir Java formatter on the given code. */
 public class PalantirJavaFormatter extends CodeFormatter {
 
     private static final int INDENTATION_SIZE = 4;
 
-    @Nullable
     @Override
+    @Nullable
     public TextEdit format(
             int kind, String source, int offset, int length, int indentationLevel, String _lineSeparator) {
         IRegion[] regions = new IRegion[] {new Region(offset, length)};
         return formatInternal(kind, source, regions, indentationLevel);
     }
 
-    @Nullable
     @Override
+    @Nullable
     public TextEdit format(int kind, String source, IRegion[] regions, int indentationLevel, String _lineSeparator) {
         return formatInternal(kind, source, regions, indentationLevel);
     }