Setup errorprone and resolve issues

Author: me4502

build-logic/build.gradle.kts

@@ -15,6 +15,7 @@ dependencies {
     implementation(libs.levelHeadered)
     implementation(libs.shadow)
     implementation(libs.jfrog.buildinfo)
+    implementation(libs.errorprone.gradle.plugin)
     implementation(libs.gson)
 
     constraints {

build-logic/src/main/kotlin/buildlogic.common-java.gradle.kts

@@ -1,10 +1,12 @@
 import buildlogic.stringyLibs
 import buildlogic.getLibrary
+import net.ltgt.gradle.errorprone.errorprone
 
 plugins {
     id("eclipse")
     id("idea")
     id("checkstyle")
+    id("net.ltgt.errorprone")
     id("buildlogic.common")
 }
 
@@ -13,7 +15,7 @@ tasks
     .matching { it.name == "compileJava" || it.name == "compileTestJava" }
     .configureEach {
         val disabledLint = listOf(
-            "processing", "path", "fallthrough", "serial", "overloads", "this-escape"
+            "processing", "path", "fallthrough", "serial", "overloads", "this-escape",
         )
         options.release.set(21)
         options.compilerArgs.addAll(listOf("-Xlint:all") + disabledLint.map { "-Xlint:-$it" })
@@ -23,11 +25,24 @@ tasks
         if (project.name.contains("-core")) {
             options.compilerArgs.add("-Werror")
         }
+        options.errorprone {
+            // We use -Werror, so we don't need errorprone to fail the build separately
+            allErrorsAsWarnings = true
+            // Obviously we don't need to fix generated code
+            disableWarningsInGeneratedCode = true
+            // We use reference equality intentionally in several places
+            // Perhaps we should consider testing the performance impact of using .equals() instead?
+            // Especially for the types that are only compared by reference equality, we could consider
+            // removing their .equals() implementations to avoid confusion.
+            disable("ReferenceEquality")
+            // We're on JDK 21, so System.console() can still be null
+            disable("SystemConsoleNull")
+        }
     }
 
 configure<CheckstyleExtension> {
     configFile = rootProject.file("config/checkstyle/checkstyle.xml")
-    toolVersion = "10.16.0"
+    toolVersion = "12.3.1"
 }
 
 tasks.withType<Test>().configureEach {
@@ -38,6 +53,8 @@ tasks.withType<Test>().configureEach {
 
 dependencies {
     "compileOnly"(stringyLibs.getLibrary("jspecify"))
+    "compileOnly"(stringyLibs.getLibrary("errorprone-annotations"))
+    "errorprone"(stringyLibs.getLibrary("errorprone-core"))
     "testImplementation"(platform(stringyLibs.getLibrary("junit-bom")))
     "testImplementation"(stringyLibs.getLibrary("junit-jupiter-api"))
     "testImplementation"(stringyLibs.getLibrary("junit-jupiter-params"))

config/checkstyle/checkstyle.xml

@@ -201,6 +201,9 @@ Checks based on Google Checks, modified for EngineHub.
             <property name="format" value="(?!Character)\.to(Lower|Upper)Case\(\)"/>
             <property name="illegalPattern" value="true"/>
         </module>
+        <module name="PatternVariableAssignment"/>
+        <module name="UnnecessaryNullCheckWithInstanceOf"/>
+        <module name="WhenShouldBeUsed"/>
     </module>
     <!-- Validate that command annotations are formatted correctly -->
     <module name="RegexpMultiline">
@@ -249,4 +252,4 @@ Checks based on Google Checks, modified for EngineHub.
     <!-- Enforce package-info.java -->
     <module name="JavadocPackage"/>
 
-</module>
+</module>

craftbook-bukkit/src/main/java/org/enginehub/craftbook/bukkit/mechanics/BukkitReadableBookshelf.java

@@ -35,7 +35,7 @@
 import org.enginehub.craftbook.util.ProtectionUtil;
 
 /**
- * This mechanism allow players to read bookshelves and get a random line from a file as as "book."
+ * This mechanism allow players to read bookshelves and get a random line from a file as "book".
  */
 public class BukkitReadableBookshelf extends ReadableBookshelf implements Listener {
     public BukkitReadableBookshelf(MechanicType<? extends CraftBookMechanic> mechanicType) {

craftbook-bukkit/src/main/java/org/enginehub/craftbook/mechanics/minecart/blocks/CartMechanismBlocks.java

@@ -34,11 +34,10 @@
 import org.jspecify.annotations.Nullable;
 
 /**
- * <p>
  * Stores the tuple of three blocks over which any typical CartMechanism is implemented,
  * and also performs detection of two of the blocks if only one
  * is given.
- * </p>
+ *
  * <p>
  * Sign text and base block type are not validated by any constructors; this must be performed
  * explicitly by calling
@@ -47,7 +46,7 @@
  * fine,
  * since if you have two signs in an area where
  * they could concievably contend for control of the mechanism, you would be doing something that
- * ought be physically
+ * ought to be physically
  * impossible anyway (though yes,
  * it is possible if editing the world directly without physics).
  * </p>
@@ -155,13 +154,12 @@ public ChangedSign getChangedSign(Side side) {
     }
 
     /**
-     * <p>
      * Detecting factory, based on the position of the rails. The base must be one block below and
      * the sign if it
      * exists must be two or three blocks
      * below. Signs are guaranteed to be signs (unless they're null) and rails are guaranteed to be
      * rails.
-     * </p>
+     *
      * <p>
      * This is the most important constructor, since it is the one invoked when processing cart move
      * events.

craftbook-bukkit/src/main/java/org/enginehub/craftbook/util/SignUtil.java

@@ -27,9 +27,8 @@
 import org.jspecify.annotations.Nullable;
 
 /**
- * <p>
  * Convenience methods for dealing with some sign block data.
- * </p>
+ *
  * <p>
  * If you intend to care about the eight further directions (as opposed to the four cardinal
  * directions and the four

craftbook-core/src/main/java/org/enginehub/craftbook/AbstractCraftBookMechanic.java

@@ -32,6 +32,7 @@ public AbstractCraftBookMechanic(MechanicType<? extends CraftBookMechanic> mecha
         this.mechanicType = mechanicType;
     }
 
+    @Override
     public MechanicType<? extends CraftBookMechanic> getMechanicType() {
         return this.mechanicType;
     }

craftbook-core/src/main/java/org/enginehub/craftbook/CraftBook.java

@@ -78,7 +78,7 @@ public void setup() {
             try {
                 Files.createDirectories(cacheDir);
             } catch (IOException e) {
-                e.printStackTrace();
+                LOGGER.warn("Failed to create cache directory, profile cache may not work correctly.", e);
             }
         }
 

craftbook-core/src/main/java/org/enginehub/craftbook/PlatformCommandManager.java

@@ -52,7 +52,6 @@
 
 import java.util.Collections;
 import java.util.List;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 public class PlatformCommandManager {
@@ -117,8 +116,8 @@ private MemoizingValueAccess initializeInjectedValues(Arguments arguments, Actor
         InjectedValueStore store = MapBackedValueStore.create();
 
         store.injectValue(Key.of(Actor.class), ValueProvider.constant(actor));
-        if (actor instanceof CraftBookPlayer) {
-            store.injectValue(Key.of(CraftBookPlayer.class), ValueProvider.constant((CraftBookPlayer) actor));
+        if (actor instanceof CraftBookPlayer player) {
+            store.injectValue(Key.of(CraftBookPlayer.class), ValueProvider.constant(player));
         } else {
             store.injectValue(Key.of(CraftBookPlayer.class), context -> {
                 throw new CommandException(
@@ -202,7 +201,7 @@ public List<Substring> handleCommandSuggestion(Actor actor, String arguments) {
             List<Substring> split = parseArgs(arguments).toList();
             List<String> argStrings = split.stream()
                 .map(Substring::getSubstring)
-                .collect(Collectors.toList());
+                .toList();
             MemoizingValueAccess access = initializeInjectedValues(() -> arguments, actor);
             ImmutableSet<Suggestion> suggestions;
             try {
@@ -227,8 +226,9 @@ public List<Substring> handleCommandSuggestion(Actor actor, String arguments) {
                         original.getStart() + 1,
                         original.getEnd() + 1
                     );
-                }).collect(Collectors.toList());
+                }).toList();
         } catch (ConditionFailedException ignored) {
+            // Ignore condition failures.
         }
 
         return Collections.emptyList();

craftbook-core/src/main/java/org/enginehub/craftbook/command/CraftBookExceptionConverter.java

@@ -20,7 +20,6 @@
 import com.sk89q.worldedit.internal.command.exception.ExceptionConverterHelper;
 import com.sk89q.worldedit.internal.command.exception.ExceptionMatch;
 import com.sk89q.worldedit.util.formatting.text.Component;
-import com.sk89q.worldedit.util.formatting.text.TextComponent;
 import org.enginehub.craftbook.CraftBook;
 import org.enginehub.craftbook.exception.CraftBookException;
 import org.enginehub.piston.exception.CommandException;
@@ -29,15 +28,8 @@
 
 public class CraftBookExceptionConverter extends ExceptionConverterHelper {
 
-    private final CraftBook craftBook;
-
     public CraftBookExceptionConverter(CraftBook craftBook) {
         checkNotNull(craftBook);
-        this.craftBook = craftBook;
-    }
-
-    private CommandException newCommandException(String message, Throwable cause) {
-        return newCommandException(TextComponent.of(String.valueOf(message)), cause);
     }
 
     private CommandException newCommandException(Component message, Throwable cause) {