Add a couple of errorprone plugins

- Check that common code does not reference client-only classes.
 - Check that @ForgeOverride really overrides a method in Forge
   projects.

buildSrc/src/main/kotlin/cc-tweaked.forge.gradle.kts

@@ -24,6 +24,10 @@ minecraft {
 
 MinecraftConfigurations.setup(project)
 
+extensions.configure(CCTweakedExtension::class.java) {
+    linters(minecraft = true, loader = "forge")
+}
+
 dependencies {
     val libs = project.extensions.getByType<VersionCatalogsExtension>().named("libs")
     "minecraft"("net.minecraftforge:forge:$mcVersion-${libs.findVersion("forge").get()}")

buildSrc/src/main/kotlin/cc-tweaked.vanilla.gradle.kts

@@ -25,3 +25,7 @@ dependencies {
 }
 
 MinecraftConfigurations.setup(project)
+
+extensions.configure(CCTweakedExtension::class.java) {
+    linters(minecraft = true, loader = null)
+}

buildSrc/src/main/kotlin/cc/tweaked/gradle/CCTweakedExtension.kt

@@ -155,6 +155,31 @@ abstract class CCTweakedExtension(
         if (isIdeSync) project.dependencies.add(sourceSet.apiConfigurationName, sourceSet.output)
     }
 
+    fun linters(@Suppress("UNUSED_PARAMETER") vararg unused: UseNamedArgs, minecraft: Boolean, loader: String?) {
+        val java = project.extensions.getByType(JavaPluginExtension::class.java)
+        val sourceSets = java.sourceSets
+
+        project.dependencies.run { add("errorprone", project(mapOf("path" to ":lints"))) }
+        sourceSets.all {
+            val name = name
+            project.tasks.named(compileJavaTaskName, JavaCompile::class.java) {
+                options.errorprone {
+                    // Only the main source set should run the side checker
+                    check("SideChecker", if (minecraft && name == "main") CheckSeverity.DEFAULT else CheckSeverity.OFF)
+
+                    // The MissingLoaderOverride check superseeds the MissingOverride one, so disable that.
+                    if (loader != null) {
+                        check("MissingOverride", CheckSeverity.OFF)
+                        option("ModLoader", loader)
+                    } else {
+                        check("LoaderOverride", CheckSeverity.OFF)
+                        check("MissingLoaderOverride", CheckSeverity.OFF)
+                    }
+                }
+            }
+        }
+    }
+
     fun jacoco(task: NamedDomainObjectProvider<JavaExec>) {
         val classDump = project.buildDir.resolve("jacocoClassDump/${task.name}")
         val reportTaskName = "jacoco${task.name.capitalized()}Report"

config/checkstyle/checkstyle.xml

@@ -111,7 +111,7 @@
         </module>
         <module name="MethodTypeParameterName" />
         <module name="PackageName">
-            <property name="format" value="^dan200\.computercraft(\.[a-z][a-z0-9]*)*" />
+            <property name="format" value="^(dan200\.computercraft|cc\.tweaked)(\.[a-z][a-z0-9]*)*" />
         </module>
         <module name="ParameterName" />
         <module name="StaticVariableName">

gradle/libs.versions.toml

@@ -3,6 +3,7 @@
 # Minecraft
 # MC version is specified in gradle.properties, as we need that in settings.gradle.
 forge = "43.1.1"
+forgeSpi = "6.0.0"
 parchment = "2022.10.16"
 parchmentMc = "1.19.2"
 mixin = "0.8.5"
@@ -61,6 +62,7 @@ autoService = { module = "com.google.auto.service:auto-service", version.ref = "
 checkerFramework = { module = "org.checkerframework:checker-qual", version.ref = "checkerFramework" }
 cobalt = { module = "org.squiddev:Cobalt", version.ref = "cobalt" }
 fastutil = { module = "it.unimi.dsi:fastutil", version.ref = "fastutil" }
+forgeSpi = { module = "net.minecraftforge:forgespi", version.ref = "forgeSpi" }
 guava = { module = "com.google.guava:guava", version.ref = "guava" }
 jetbrainsAnnotations = { module = "org.jetbrains:annotations", version.ref = "jetbrainsAnnotations" }
 jsr305 = { module = "com.google.code.findbugs:jsr305", version.ref = "jsr305" }

projects/common/build.gradle.kts

@@ -31,8 +31,6 @@ dependencies {
     testImplementation(project(":forge-stubs"))
     testRuntimeOnly(libs.bundles.testRuntime)
 
-    errorprone(project(":lints"))
-
     testModImplementation(testFixtures(project(":core")))
     testModImplementation(libs.bundles.kotlin)
 }

projects/forge/build.gradle.kts

@@ -138,8 +138,6 @@ configurations {
 dependencies {
     annotationProcessor("org.spongepowered:mixin:0.8.5-SQUID:processor")
 
-    errorprone(project(":lints"))
-
     compileOnly(libs.jetbrainsAnnotations)
     annotationProcessorEverywhere(libs.autoService)
 

projects/lints/build.gradle.kts

@@ -16,6 +16,12 @@ dependencies {
     implementation(libs.kotlin.stdlib)
     implementation(libs.errorProne.api)
     implementation(libs.nullAway)
+
+    testImplementation(libs.bundles.test)
+    testImplementation(libs.errorProne.testHelpers)
+    testImplementation(libs.forgeSpi)
+    testCompileOnly(project(":core-api"))
+    testRuntimeOnly(libs.bundles.testRuntime)
 }
 
 tasks.test {

projects/lints/src/main/kotlin/cc/tweaked/linter/Extensions.kt

@@ -0,0 +1,69 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+@file:Suppress("JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE")
+
+package cc.tweaked.linter
+
+import com.google.errorprone.VisitorState
+import com.google.errorprone.util.ASTHelpers
+import com.sun.tools.javac.code.Symbol
+import com.sun.tools.javac.code.Types
+import javax.lang.model.element.AnnotationMirror
+import javax.lang.model.element.AnnotationValue
+import javax.lang.model.element.Element
+import javax.lang.model.element.TypeElement
+import javax.lang.model.util.SimpleAnnotationValueVisitor8
+
+inline fun <reified T> VisitorState.getContext(create: () -> T): T {
+    val provider: T? = context.get(T::class.java)
+    if (provider != null) return provider
+
+    val newProvider = create()
+    context.put(T::class.java, newProvider)
+    return newProvider
+}
+
+fun Symbol.MethodSymbol.getAnySuperMethod(types: Types): Symbol.MethodSymbol? =
+    ASTHelpers.findSuperMethods(this, types).firstOrNull()
+
+fun Symbol.MethodSymbol.getSuperMethods(types: Types): Collection<Symbol.MethodSymbol> =
+    ASTHelpers.findSuperMethods(this, types)
+
+// Annotation helpers
+
+typealias AnnotationGetter<T> = (AnnotationValue) -> T
+
+object AnnotationGetters {
+    fun <E : Enum<E>> enum(type: Class<E>): AnnotationGetter<E> =
+        { value -> java.lang.Enum.valueOf(type, value.toString()) }
+
+    inline fun <reified E : Enum<E>> enum(): AnnotationGetter<E> = enum(E::class.java)
+
+    inline fun <reified T> exact(): AnnotationGetter<T> = { value -> value.value as T }
+
+    fun <T> list(element: AnnotationGetter<T>): AnnotationGetter<List<T>> = { value ->
+        value.accept(
+            object : SimpleAnnotationValueVisitor8<List<T>, Unit>() {
+                override fun visitArray(vals: MutableList<out AnnotationValue>, state: Unit): List<T> =
+                    vals.map(element)
+            },
+            Unit,
+        )!!
+    }
+}
+
+fun AnnotationMirror.getValue(name: String): AnnotationValue? {
+    val value = elementValues.entries.find { it.key.simpleName.contentEquals(name) } ?: return null
+    return value.value
+}
+
+fun <T> AnnotationMirror.getValue(name: String, getter: AnnotationGetter<T>): T? = getValue(name)?.let(getter)
+
+fun Element.getAnnotation(name: String): AnnotationMirror? =
+    annotationMirrors.find {
+        val type = it.annotationType.asElement() as TypeElement
+        type.qualifiedName.contentEquals(name)
+    }

projects/lints/src/main/kotlin/cc/tweaked/linter/LoaderOverride.kt

@@ -0,0 +1,98 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+@file:Suppress("JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE")
+
+package cc.tweaked.linter
+
+import com.google.errorprone.BugPattern
+import com.google.errorprone.ErrorProneFlags
+import com.google.errorprone.VisitorState
+import com.google.errorprone.bugpatterns.BugChecker
+import com.google.errorprone.fixes.SuggestedFix
+import com.google.errorprone.matchers.Description
+import com.google.errorprone.util.ASTHelpers
+import com.sun.source.tree.MethodTree
+import com.sun.tools.javac.code.Symbol
+import java.util.*
+import javax.lang.model.element.Modifier
+
+internal object LoaderOverrides {
+    private const val FORGE_ANNOTATION: String = "dan200.computercraft.annotations.ForgeOverride"
+    private const val FABRIC_ANNOTATION: String = "dan200.computercraft.annotations.FabricOverride"
+
+    fun hasOverrideAnnotation(symbol: Symbol.MethodSymbol, state: VisitorState) =
+        ASTHelpers.hasAnnotation(symbol, Override::class.java, state)
+
+    fun getAnnotation(flags: ErrorProneFlags) = when (flags.get("ModLoader").orElse(null)) {
+        "forge" -> FORGE_ANNOTATION
+        "fabric" -> FABRIC_ANNOTATION
+        null -> null
+        else -> throw IllegalArgumentException("Unknown mod loader")
+    }
+}
+
+@BugPattern(
+    summary = "Require an @Override or @ForgeOverride/@FabricOverride annotation on a class.",
+    explanation = """
+        All methods which override a method or implement an interface should use @Override. When overriding
+        loader-specific methods, you should use @ForgeOverride or @FabricOverride instead.
+    """,
+    severity = BugPattern.SeverityLevel.WARNING,
+    tags = [BugPattern.StandardTags.STYLE],
+)
+class MissingLoaderOverride(flags: ErrorProneFlags? = null) : BugChecker(), BugChecker.MethodTreeMatcher {
+    private val annotation = if (flags == null) null else LoaderOverrides.getAnnotation(flags)
+
+    override fun matchMethod(tree: MethodTree, state: VisitorState): Description {
+        if (annotation == null) throw IllegalStateException("MissingLoaderOverride requires the ModLoader flag")
+
+        val symbol = ASTHelpers.getSymbol(tree)
+        if (symbol.isStatic) return Description.NO_MATCH
+
+        // If the method is annotated with @Override or our annotation, then skip.
+        if (LoaderOverrides.hasOverrideAnnotation(symbol, state) || ASTHelpers.hasAnnotation(symbol, annotation, state)) {
+            return Description.NO_MATCH
+        }
+
+        return Optional.ofNullable(symbol.getAnySuperMethod(state.types)).map { override ->
+            buildDescription(tree)
+                .addFix(SuggestedFix.prefixWith(tree, "@Override "))
+                .setMessage(
+                    String.format(
+                        "%s %s method in %s; expected @Override",
+                        symbol.simpleName,
+                        if (override.enclClass().isInterface || override.modifiers.contains(Modifier.ABSTRACT)) "implements" else "overrides",
+                        override.enclClass().simpleName,
+                    ),
+                )
+                .build()
+        }.orElse(Description.NO_MATCH)
+    }
+}
+
+@BugPattern(
+    summary = "@ForgeOverride does not match a super method.",
+    severity = BugPattern.SeverityLevel.ERROR,
+    tags = [BugPattern.StandardTags.LIKELY_ERROR],
+)
+class LoaderOverride(flags: ErrorProneFlags? = null) : BugChecker(), BugChecker.MethodTreeMatcher {
+    private val annotation = if (flags == null) null else LoaderOverrides.getAnnotation(flags)
+
+    override fun matchMethod(tree: MethodTree, state: VisitorState): Description {
+        if (annotation == null) throw IllegalStateException("LoaderOverride requires the ModLoader flag")
+
+        val symbol = ASTHelpers.getSymbol(tree)
+        if (symbol.isStatic) return Description.NO_MATCH
+
+        // Check we're annotated and missing a parent method.
+        if (!ASTHelpers.hasAnnotation(symbol, annotation, state)) return Description.NO_MATCH
+        if (symbol.getAnySuperMethod(state.types) != null) return Description.NO_MATCH
+
+        return buildDescription(tree)
+            .setMessage("Method ${symbol.simpleName} does not override method from its superclass")
+            .build()
+    }
+}

projects/lints/src/main/kotlin/cc/tweaked/linter/SideChecker.kt

@@ -0,0 +1,120 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+@file:Suppress("JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE")
+
+package cc.tweaked.linter
+
+import com.google.errorprone.BugPattern
+import com.google.errorprone.VisitorState
+import com.google.errorprone.bugpatterns.BugChecker
+import com.google.errorprone.matchers.Description
+import com.google.errorprone.util.ASTHelpers
+import com.sun.source.tree.IdentifierTree
+import com.sun.source.tree.MemberSelectTree
+import com.sun.source.tree.Tree
+import com.sun.tools.javac.code.Symbol
+import java.util.*
+import java.util.stream.Stream
+import javax.lang.model.element.AnnotationMirror
+import javax.lang.model.element.ElementKind
+
+enum class Side { CLIENT, SERVER, BOTH }
+
+@BugPattern(
+    summary = "Checks client-only code is not used.",
+    explanation = """
+        Errors on any reference to client-only classes, fields or identifiers.
+    """,
+    severity = BugPattern.SeverityLevel.ERROR,
+    tags = [BugPattern.StandardTags.LIKELY_ERROR],
+)
+class SideChecker : BugChecker(), BugChecker.IdentifierTreeMatcher, BugChecker.MemberSelectTreeMatcher {
+    override fun matchIdentifier(tree: IdentifierTree, state: VisitorState): Description {
+        val sym = ASTHelpers.getSymbol(tree)
+        return when (sym.getKind()) {
+            ElementKind.LOCAL_VARIABLE, ElementKind.TYPE_PARAMETER -> Description.NO_MATCH
+            else -> report(tree, state)
+        }
+    }
+
+    override fun matchMemberSelect(tree: MemberSelectTree, state: VisitorState): Description {
+        // Skip imports: We'll catch these later on.
+        if (state.path.any { it.kind == Tree.Kind.IMPORT }) return Description.NO_MATCH
+
+        val reify = ASTHelpers.getSymbol(tree)
+        return if (reify is Symbol.TypeSymbol) report(tree, state) else Description.NO_MATCH
+    }
+
+    private fun report(tree: Tree, state: VisitorState): Description {
+        return when (state.sideProvider.getSide(tree)) {
+            Side.CLIENT -> buildDescription(tree).setMessage("Using client-only symbol in common source set").build()
+            Side.SERVER -> buildDescription(tree).setMessage("Using server-only symbol in common source set").build()
+            Side.BOTH -> Description.NO_MATCH
+        }
+    }
+}
+
+internal class SideProvider {
+    private val cache = mutableMapOf<Symbol, Optional<Side>>()
+
+    fun getSide(tree: Tree?): Side {
+        val sym = ASTHelpers.getSymbol(tree)
+        return if (sym == null) Side.BOTH else getSide(sym).orElse(Side.BOTH)
+    }
+
+    private fun getSide(sym: Symbol): Optional<Side> {
+        val existing = cache[sym]
+        if (existing != null) return existing
+
+        val side = getSideImpl(sym)
+        cache[sym] = side
+        return side
+    }
+
+    private fun getSideImpl(sym: Symbol): Optional<Side> = when (sym.getKind()) {
+        ElementKind.MODULE -> Optional.empty()
+        ElementKind.PACKAGE -> {
+            val pkg = sym.toString()
+            when {
+                (clientPackages.any { pkg.startsWith(it) } || pkg.splitToSequence(".").contains("client")) &&
+                    !notClientPackages.contains(pkg) -> Optional.of(Side.CLIENT)
+
+                else -> Optional.empty()
+            }
+        }
+
+        else ->
+            fromAnnotationStream(sym.annotationMirrors.stream())
+                .or { Optional.ofNullable(sym.enclosingElement).flatMap { getSide(it) } }
+    }
+
+    private fun fromAnnotationStream(annotations: Stream<out AnnotationMirror>) =
+        annotations.flatMap {
+            when (it.annotationType.toString()) {
+                "net.minecraftforge.api.distmarker.OnlyIn" -> {
+                    val value = it.getValue("value", AnnotationGetters.enum<Side>())!!
+                    Stream.of(value)
+                }
+
+                else -> Stream.empty()
+            }
+        }.findFirst()
+
+    companion object {
+        private val notClientPackages = listOf(
+            // Ugly! But we do what we must.
+            "net.fabricmc.fabric.api.client.itemgroup",
+            "dan200.computercraft.shared.network.client",
+        )
+
+        private val clientPackages = listOf(
+            "org.lwjgl.",
+        )
+    }
+}
+
+internal val VisitorState.sideProvider: SideProvider
+    get() = getContext { SideProvider() }

projects/lints/src/main/resources/META-INF/services/com.google.errorprone.bugpatterns.BugChecker

@@ -0,0 +1,3 @@
+cc.tweaked.linter.LoaderOverride
+cc.tweaked.linter.MissingLoaderOverride
+cc.tweaked.linter.SideChecker

projects/lints/src/test/java/cc/tweaked/linter/AnnotatedClientClass.java

@@ -0,0 +1,17 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+package cc.tweaked.linter;
+
+import net.minecraftforge.api.distmarker.Dist;
+import net.minecraftforge.api.distmarker.OnlyIn;
+
+@OnlyIn(Dist.CLIENT)
+public class AnnotatedClientClass {
+    public static void doSomething() {
+    }
+
+    public static int field = 0;
+}

projects/lints/src/test/java/cc/tweaked/linter/TestSideChecker.java

@@ -0,0 +1,73 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+package cc.tweaked.linter;
+
+import com.google.common.base.Predicates;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.Test;
+
+public class TestSideChecker {
+    private final CompilationTestHelper compilationHelper = CompilationTestHelper.newInstance(SideChecker.class, getClass());
+
+    @Test
+    public void textExtendsAnnotated() {
+        compilationHelper
+            .addSourceLines("UsesClientOnly.java", """
+                // BUG: Diagnostic matches: X
+                class UsesClientOnly extends cc.tweaked.linter.AnnotatedClientClass {
+                }
+                """)
+            .expectErrorMessage("X", Predicates.containsPattern("Using client-only symbol in common source set"))
+            .doTest();
+    }
+
+    @Test
+    public void testImportsAnnotated() {
+        compilationHelper
+            .addSourceLines("UsesClientOnly.java", """
+                import cc.tweaked.linter.AnnotatedClientClass;
+                // BUG: Diagnostic matches: X
+                class UsesClientOnly extends AnnotatedClientClass {
+                }
+                """)
+            .expectErrorMessage("X", Predicates.containsPattern("Using client-only symbol in common source set"))
+            .doTest();
+    }
+
+    @Test
+    public void textUsesAnnotated() {
+        compilationHelper
+            .addSourceLines("UsesClientOnly.java", """
+                import cc.tweaked.linter.AnnotatedClientClass;
+                class UsesClientOnly {
+                    public void f() {
+                        // BUG: Diagnostic matches: X
+                        AnnotatedClientClass.doSomething();
+                        // BUG: Diagnostic matches: Y
+                        System.out.println(AnnotatedClientClass.field);
+                        // BUG: Diagnostic matches: Z
+                        AnnotatedClientClass.field = 0;
+                    }
+                }
+                """)
+            .expectErrorMessage("X", Predicates.containsPattern("Using client-only symbol in common source set"))
+            .expectErrorMessage("Y", Predicates.containsPattern("Using client-only symbol in common source set"))
+            .expectErrorMessage("Z", Predicates.containsPattern("Using client-only symbol in common source set"))
+            .doTest();
+    }
+
+    @Test
+    public void testExtendsPackage() {
+        compilationHelper
+            .addSourceLines("UsesClientOnly.java", """
+                // BUG: Diagnostic matches: X
+                class UsesClientOnly extends cc.tweaked.linter.client.PackageClientClass {
+                }
+                """)
+            .expectErrorMessage("X", Predicates.containsPattern("Using client-only symbol in common source set"))
+            .doTest();
+    }
+}

projects/lints/src/test/java/cc/tweaked/linter/client/PackageClientClass.java

@@ -0,0 +1,9 @@
+/*
+ * This file is part of ComputerCraft - http://www.computercraft.info
+ * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission.
+ * Send enquiries to dratcliffe@gmail.com
+ */
+package cc.tweaked.linter.client;
+
+public class PackageClientClass {
+}