Fail the build on four Error Prone checks

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

@@ -18,6 +18,12 @@ tasks.withType(JavaCompile).configureEach {
     options.errorprone {
         // Annotation processor output (Immutables) is not ours to fix.
         excludedPaths = '.*/build/generated/.*'
+        error('FormatStringConcatenation')
+        error('JUnitClassModifiers')
+        error('JUnitMethodDeclaration')
+        error('RefactorSwitch')
+        error('SameNameButDifferent')
+        error('UnusedVariable')
     }
 }
 
@@ -27,27 +33,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/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 @@
         return LOADED.computeIfAbsent(key, _key -> loadUncached(jars));
     }
 
-    @SuppressWarnings("for-rollout:NullAway")
     private static FormatterService loadUncached(Collection<File> jars) {
         URL[] jarUris = jars.stream()
                 .map(file -> {
@@ -83,7 +82,7 @@
 
         ClassLoader classLoader = new URLClassLoader(jarUris, FormatterService.class.getClassLoader());
         checkJavacIsExportedTo(classLoader);
         return Iterables.getOnlyElement(ServiceLoader.load(FormatterService.class, classLoader));
     }
 
     /**

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/FormatDiffTest.java

@@ -104,7 +104,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 {

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

@@ -159,15 +159,15 @@ private SnippetWrapper snippetWrapper(SnippetKind kind, String source, int initi
          * dummy class is correctly formatted -- the blocks use correct indentation,
          * etc.
          */
-        switch (kind) {
+        return switch (kind) {
             case COMPILATION_UNIT -> {
                 SnippetWrapper wrapper = new SnippetWrapper();
                 for (int i = 1; i <= initialIndent; i++) {
                     wrapper.append("class Dummy {\n").append(createIndentationString(i));
                 }
                 wrapper.appendSource(source);
                 wrapper.closeBraces(initialIndent);
-                return wrapper;
+                yield wrapper;
             }
             case CLASS_BODY_DECLARATIONS -> {
                 SnippetWrapper wrapper = new SnippetWrapper();
@@ -176,7 +176,7 @@ private SnippetWrapper snippetWrapper(SnippetKind kind, String source, int initi
                 }
                 wrapper.appendSource(source);
                 wrapper.closeBraces(initialIndent);
-                return wrapper;
+                yield wrapper;
             }
             case STATEMENTS -> {
                 SnippetWrapper wrapper = new SnippetWrapper();
@@ -186,7 +186,7 @@ private SnippetWrapper snippetWrapper(SnippetKind kind, String source, int initi
                 }
                 wrapper.appendSource(source);
                 wrapper.closeBraces(initialIndent);
-                return wrapper;
+                yield wrapper;
             }
             case EXPRESSION -> {
                 SnippetWrapper wrapper = new SnippetWrapper();
@@ -198,9 +198,9 @@ private SnippetWrapper snippetWrapper(SnippetKind kind, String source, int initi
                 wrapper.appendSource(source);
                 wrapper.append(";");
                 wrapper.closeBraces(initialIndent);
-                return wrapper;
+                yield wrapper;
             }
             default -> throw new IllegalArgumentException("Unknown snippet kind: " + kind);
-        }
+        };
     }
 }

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

@@ -37,28 +37,25 @@
 
 import static java.util.Optional.ofNullable;
 
-@SuppressWarnings("for-rollout:SameNameButDifferent")
 @State(
         name = "PalantirJavaFormatSettings",
         storages = {@Storage("open-java-format.xml")})
-public class PalantirJavaFormatSettings implements PersistentStateComponent<PalantirJavaFormatSettings.State> {
+public class PalantirJavaFormatSettings implements PersistentStateComponent<PalantirJavaFormatSettings.StoredSettings> {
 
-    @SuppressWarnings("for-rollout:SameNameButDifferent")
-    private State state = new State();
+    private StoredSettings state = new StoredSettings();
 
     static PalantirJavaFormatSettings getInstance(Project project) {
         return project.getService(PalantirJavaFormatSettings.class);
     }
 
-    @SuppressWarnings("for-rollout:SameNameButDifferent")
     @Nullable
     @Override
-    public State getState() {
+    public StoredSettings getState() {
         return state;
     }
 
     @Override
-    public void loadState(@SuppressWarnings("for-rollout:SameNameButDifferent") State state) {
+    public void loadState(StoredSettings state) {
         this.state = state;
     }
 
@@ -152,7 +149,7 @@ enum EnabledState {
         DISABLED
     }
 
-    static class State {
+    static class StoredSettings {
 
         private EnabledState enabled = EnabledState.UNKNOWN;
         private Optional<List<URI>> implementationClassPath = Optional.empty();
@@ -167,7 +164,6 @@ public void setImplementationClassPath(@Nullable List<String> value) {
                     .map(strings -> strings.stream().map(URI::create).collect(Collectors.toList()));
         }
 
-        @SuppressWarnings("for-rollout:NullAway")
         public List<String> getImplementationClassPath() {
             return implementationClassPath
                     .map(paths -> paths.stream().map(URI::toString).collect(Collectors.toList()))
@@ -185,7 +181,6 @@ public void setEnabled(@Nullable String enabledStr) {
             }
         }
 
-        @SuppressWarnings("for-rollout:NullAway")
         public String getEnabled() {
             return switch (enabled) {
                 case ENABLED -> "true";

open-java-format-idea-plugin/src/test/java/com/palantir/javaformat/intellij/FormatterProviderTest.java

@@ -22,7 +22,7 @@
 import com.intellij.testFramework.fixtures.JavaTestFixtureFactory;
 import com.palantir.javaformat.bootstrap.BootstrappingFormatterService;
 import com.palantir.javaformat.bootstrap.NativeImageFormatterService;
-import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.State;
+import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.StoredSettings;
 import java.nio.file.Path;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
@@ -45,7 +45,7 @@ void setUp() throws Exception {
         fixture = JavaTestFixtureFactory.getFixtureFactory().createCodeInsightFixture(projectBuilder.getFixture());
         fixture.setUp();
         settings = PalantirJavaFormatSettings.getInstance(fixture.getProject());
-        settings.loadState(new State());
+        settings.loadState(new StoredSettings());
     }
 
     @AfterEach

open-java-format-idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatConfigurableTest.java

@@ -34,7 +34,7 @@
 import com.palantir.javaformat.intellij.EmbeddedNativeImage.Installed;
 import com.palantir.javaformat.intellij.EmbeddedNativeImage.Status;
 import com.palantir.javaformat.intellij.PalantirJavaFormatConfigurable.StatusLine;
-import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.State;
+import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.StoredSettings;
 import java.awt.Component;
 import java.awt.Container;
 import java.io.IOException;
@@ -74,7 +74,7 @@ void setUp() throws Exception {
         fixture = JavaTestFixtureFactory.getFixtureFactory().createCodeInsightFixture(projectBuilder.getFixture());
         fixture.setUp();
         settings = PalantirJavaFormatSettings.getInstance(fixture.getProject());
-        settings.loadState(new State());
+        settings.loadState(new StoredSettings());
         settings.setEnabled(true);
         // Nothing downloaded, and nowhere to download from.
         var embedded = new EmbeddedNativeImage(
@@ -104,7 +104,7 @@ void hasNoChoiceOfCodeStyle() {
 
     @Test
     void readsAndWritesTheEnabledSetting() throws Throwable {
-        State disabled = new State();
+        var disabled = new StoredSettings();
         disabled.setEnabled("false");
         settings.loadState(disabled);
 
@@ -176,7 +176,7 @@ void showsTheActiveFormatterAsThePageStands() throws Throwable {
 
             useNativeImage.setSelected(true);
             assertThat(activeFormatter().getText())
-                    .isEqualTo("Need to download version " + pluginVersion + " from Maven Central");
+                    .isEqualTo("Need to download version %s from Maven Central", pluginVersion);
 
             path.setText("/opt/open-java-format");
             assertThat(activeFormatter().getText()).isEqualTo("/opt/open-java-format");
@@ -268,7 +268,7 @@ void refusesAPathThatIsNotAnExecutableFile() throws Throwable {
 
             assertThatThrownBy(configurable::apply)
                     .isInstanceOf(ConfigurationException.class)
-                    .hasMessage(missing + " is not an executable file");
+                    .hasMessage("%s is not an executable file", missing);
         });
     }
 

open-java-format-idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingServiceTest.java

@@ -35,7 +35,7 @@
 import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture;
 import com.intellij.testFramework.fixtures.JavaTestFixtureFactory;
 import com.intellij.testFramework.fixtures.TestFixtureBuilder;
-import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.State;
+import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.StoredSettings;
 import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -70,7 +70,7 @@ void setUp() throws Exception {
 
         settings = PalantirJavaFormatSettings.getInstance(fixture.getProject());
 
-        State resetState = new State();
+        var resetState = new StoredSettings();
         resetState.setEnabled("true");
         settings.loadState(resetState);
     }
@@ -127,7 +127,6 @@ private PsiFile createPsiFile(String path, String... contents) throws IOExceptio
     }
 
     private static final class DelegatingFormatter extends PalantirJavaFormatFormattingService {
-        @SuppressWarnings("DataFlowIssue")
         public DelegatingFormatter() {
             this.setPluginDescriptor(FormattingService.EP_NAME
                     .findExtension(PalantirJavaFormatFormattingService.class)

open-java-format-idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatSettingsTest.java

@@ -18,7 +18,7 @@
 
 import com.intellij.openapi.util.JDOMUtil;
 import com.intellij.util.xmlb.XmlSerializer;
-import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.State;
+import com.palantir.javaformat.intellij.PalantirJavaFormatSettings.StoredSettings;
 import java.nio.file.Path;
 import java.util.Optional;
 import org.intellij.lang.annotations.Language;
@@ -42,7 +42,7 @@ void loadsAFileThatStillHasTheCodeStyle() throws Exception {
 
     @Test
     void doesNotWriteACodeStyle() {
-        var xml = JDOMUtil.write(XmlSerializer.serialize(new State()));
+        var xml = JDOMUtil.write(XmlSerializer.serialize(new StoredSettings()));
 
         assertThat(xml).doesNotContain("style");
     }
@@ -78,15 +78,15 @@ void ignoresTheNativeImageTheGradlePluginUsedToWrite() throws Exception {
 
     @Test
     void takesABlankPathForNone() {
-        var state = new State();
+        var state = new StoredSettings();
         state.nativeImagePath = " ";
         var settings = new PalantirJavaFormatSettings();
         settings.loadState(state);
 
         assertThat(settings.getNativeImagePath()).isEmpty();
     }
 
-    private static State deserialize(@Language("xml") String xml) throws Exception {
-        return XmlSerializer.deserialize(JDOMUtil.load(xml), State.class);
+    private static StoredSettings deserialize(@Language("xml") String xml) throws Exception {
+        return XmlSerializer.deserialize(JDOMUtil.load(xml), StoredSettings.class);
     }
 }

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

@@ -84,11 +84,10 @@
                 jvmArgsForVersion(jdkMajorVersion),
                 implementationClassPath,
                 /* outputReplacements= */ true,
                 ranges.stream().map(RangeUtils::toStringRange).collect(Collectors.toList()));
 
-        @SuppressWarnings("for-rollout:NullAway")
         Optional<String> output =
                 FormatterCommandRunner.runWithStdin(command.toArgs(), input, Optional.of(jdkPath.getParent()));
         if (output.isEmpty() || output.get().isEmpty()) {
             return ImmutableList.of();
         }

open-java-format/src/main/java/com/palantir/javaformat/Newlines.java

@@ -53,7 +53,6 @@ public static int hasNewlineAt(String input, int idx) {
     }
 
     /** Returns the terminating line break in the input, or {@code null} if the input does not end in a break. */
-    @SuppressWarnings("for-rollout:NullAway")
     public static String getLineEnding(String input) {
         for (String b : BREAKS) {
             if (input.endsWith(b)) {
@@ -160,7 +159,6 @@ private static final class LineIterator implements Iterator<String> {
         private final String input;
         private final Iterator<Integer> indices;
 
-        @SuppressWarnings("for-rollout:NullAway")
         private LineIterator(String input) {
             this.input = input;
             this.indices = lineOffsetIterator(input);

open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java

@@ -50,15 +50,13 @@ public final class OpsBuilder {
     /** Return the actual size of the AST node at position, including comments. */
     public int actualSize(int position, int length) {
         Input.Token startToken = input.getPositionTokenMap().get(position);
-        @SuppressWarnings("for-rollout:NullAway")
         int start = startToken.getTok().getPosition();
         for (Input.Tok tok : startToken.getToksBefore()) {
             if (tok.isComment()) {
                 start = Math.min(start, tok.getPosition());
             }
         }
         Input.Token endToken = input.getPositionTokenMap().get(position + length - 1);
-        @SuppressWarnings("for-rollout:NullAway")
         int end = endToken.getTok().getPosition() + endToken.getTok().length();
         for (Input.Tok tok : endToken.getToksAfter()) {
             if (tok.isComment()) {
@@ -71,7 +69,6 @@ public int actualSize(int position, int length) {
     /** Return the start column of the token at {@code position}, including leading comments. */
     public Integer actualStartColumn(int position) {
         Input.Token startToken = input.getPositionTokenMap().get(position);
-        @SuppressWarnings("for-rollout:NullAway")
         int start = startToken.getTok().getPosition();
         int line0 = input.getLineNumber(start);
         for (Input.Tok tok : startToken.getToksBefore()) {

open-java-format/src/main/java/com/palantir/javaformat/doc/Comment.java

@@ -157,7 +157,6 @@ protected Range<Integer> computeRange() {
     public State computeBreaks(
             CommentsHelper commentsHelper, int maxWidth, State state, Obs.ExplorationNode observationNode) {
         String text = commentsHelper.rewrite(tok, maxWidth, state.column());
-        @SuppressWarnings("for-rollout:NullAway")
         int lastLineStart = Iterators.getLast(Newlines.lineOffsetIterator(text));
         int lastLineLength = text.length() - lastLineStart;
         // rewrite() indents every line after the first to state.column(), so after a comment that spans lines the