[7.0] Introduce compile-time validation, enhance strict runtime checks (#86)

Author: PaintNinja

README.md

@@ -21,6 +21,7 @@ repositories {
 
 dependencies {
     implementation "net.minecraftforge:eventbus:<version>"
+    annotationProcessor "net.minecraftforge:eventbus-validator:<version>"
 }
 ```
 
@@ -32,7 +33,7 @@ import net.minecraftforge.eventbus.api.event.RecordEvent;
 import net.minecraftforge.eventbus.api.bus.EventBus;
 
 // Define an event and a bus for it
-record PlayerLoggedInEvent(String username) implements RecordEvent {
+public record PlayerLoggedInEvent(String username) implements RecordEvent {
     public static final EventBus<PlayerLoggedInEvent> BUS = EventBus.create(PlayerLoggedInEvent.class);
 }
 
@@ -54,6 +55,30 @@ Attempting to pass a `null` value to a method param that isn't explicitly marked
 operation and won't be considered a breaking change if a future version throws an exception in such cases when it didn't
 before.
 
+## Validation
+To improve startup performance, EventBus 7 relies heavily on dev-time static analysis and compile-time validation to
+ensure correct API usage, reducing the need for runtime checks. This also helps catch potential issues early on and
+provides more informative error messages that suggest solutions.
+
+It is highly recommended to use the `eventbus-validator` annotation processor during development to enable enhanced
+validation.
+
+### Runtime checks
+EventBus performs limited validation at runtime and assumes API usage is mostly correct. Incorrect usage of the API may
+lead to unexpected behaviour and breaking changes when updating EventBus due to reliance on internal implementation
+details.
+
+For use-cases where you need to debug issues in production or are unable to use the annotation processor, you can enable
+strict runtime checks by setting the `eventbus.api.strictRuntimeChecks` system property to `true` when launching your
+application. This will enable more exhaustive runtime checks for most API usage, including bulk registration of
+listeners and EventBus creation, but not field declarations.
+
+Alternatively, you can selectively enable strict runtime checks for specific subsets of the API by using the
+`eventbus.api.strictRegistrationChecks` and `eventbus.api.strictBusCreationChecks` system properties for bulk
+registration and EventBus creation, respectively. This may be useful if you only intend on adding listeners to an
+existing event made by another library, where you don't need strict checks for the creation of its associated EventBus
+due to it being outside your control.
+
 ## Contributing
 One of the main goals of EventBus is performance. As such, any changes should be benchmarked with the `jmh` Gradle task
 to help ensure that there are no unintended performance regressions. If you are unsure how to do this or would like

eventbus-test/build.gradle

@@ -19,17 +19,22 @@ license {
 dependencies {
     testImplementation rootProject
     testImplementation projects.eventbusTestJar
+    testImplementation projects.eventbusValidator
     testImplementation libs.junit.api
     testImplementation libs.jspecify.annotations
+    testImplementation libs.compile.testing
+    testImplementation libs.jetbrains.annotations
     testRuntimeOnly libs.bundles.junit.runtime
 }
 
 extraJavaModuleInfo {
     failOnMissingModuleInfo = false
+    automaticModule(libs.compile.testing, "compile.testing")
 }
 
 tasks.named('test', Test) {
     useJUnitPlatform()
+    systemProperties = ['eventbus.api.strictBusCreationChecks': true]
 }
 
 tasks.register('testAll', AggregateTest) {

eventbus-test/src/test/java/module-info.java

@@ -4,8 +4,14 @@
  */
 open module net.minecraftforge.eventbus.test {
     requires net.minecraftforge.eventbus;
+    requires net.minecraftforge.eventbus.validator;
     requires org.junit.jupiter.api;
     requires org.jspecify;
+    requires java.compiler;
+    requires compile.testing;
+    requires org.jetbrains.annotations;
+    requires net.minecraftforge.eventbus.testjars;
 
     exports net.minecraftforge.eventbus.test;
+    exports net.minecraftforge.eventbus.test.compiletime;
 }

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/BulkEventListenerTests.java

@@ -288,7 +288,7 @@ public static void monitoringListener(TestEvent event) {
             monitoringCalled = true;
         }
 
-        @SubscribeEvent
+        @SubscribeEvent(priority = Priority.MONITOR)
         public static void cancellationAwareMonitoringListener(CancellableTestEvent event, boolean wasCancelled) {
             cancellationAwareMonitoringCalled = true;
         }
@@ -653,7 +653,7 @@ static void wrongParamCountStatic(TestEvent event, boolean wrong) {}
     }
 
     /**
-     * Tests that strict bulk registeration on a class with a non-monitoring priority on a cancellation-aware monitoring event listener method throws an exception.
+     * Tests that strict bulk registration on a class with a non-monitoring priority on a cancellation-aware monitoring event listener method throws an exception.
      */
     @Test
     public void testStrictBulkRegistrationValidationWrongPriorityMonitoring() {

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/compiletime/CompileTestHelper.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) Forge Development LLC
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+package net.minecraftforge.eventbus.test.compiletime;
+
+import com.google.testing.compile.Compilation;
+import com.google.testing.compile.Compiler;
+import com.google.testing.compile.JavaFileObjects;
+import net.minecraftforge.eventbus.api.bus.EventBus;
+import net.minecraftforge.eventbus.testjar.events.EventWithData;
+import net.minecraftforge.eventbus.validator.EventBusValidator;
+import net.minecraftforge.eventbus.validator.EventTypeValidator;
+import net.minecraftforge.eventbus.validator.SubscribeEventValidator;
+import org.intellij.lang.annotations.Language;
+import org.jspecify.annotations.NullMarked;
+
+import java.io.File;
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.util.List;
+
+final class CompileTestHelper {
+    private CompileTestHelper() {}
+
+    private static final List<File> CLASSPATH;
+
+    @Language("Java")
+    static final String SOURCE_PREFIX = """
+            import net.minecraftforge.eventbus.api.bus.*;
+            import net.minecraftforge.eventbus.api.event.*;
+            import net.minecraftforge.eventbus.api.event.characteristic.*;
+            import net.minecraftforge.eventbus.api.listener.*;
+            
+            import net.minecraftforge.eventbus.testjar.events.*;
+            
+            import org.jspecify.annotations.NullMarked;
+            import org.jspecify.annotations.Nullable;
+            
+            @NullMarked
+            """;
+
+    static {
+        try {
+            CLASSPATH = List.of(
+                    getClassPath(EventBus.class).toFile(),
+                    getClassPath(EventWithData.class).toFile(),
+                    getClassPath(NullMarked.class).toFile()
+            );
+        } catch (URISyntaxException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private static Path getClassPath(Class<?> clazz) throws URISyntaxException {
+        return Path.of(clazz
+                .getProtectionDomain()
+                .getCodeSource()
+                .getLocation()
+                .toURI()
+        );
+    }
+
+    static Compilation compile(@Language(value = "Java", prefix = SOURCE_PREFIX) String sourceCode) {
+        return compileWithoutDefaultPrefix(SOURCE_PREFIX + sourceCode);
+    }
+
+    static Compilation compileWithoutDefaultPrefix(@Language(value = "Java") String sourceCode) {
+        return Compiler.javac()
+                .withProcessors(new EventBusValidator(), new EventTypeValidator(), new SubscribeEventValidator())
+                .withClasspath(CLASSPATH)
+                .compile(JavaFileObjects.forSourceString("", sourceCode));
+    }
+}

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/compiletime/EventBusValidatorTests.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) Forge Development LLC
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+package net.minecraftforge.eventbus.test.compiletime;
+
+import org.junit.jupiter.api.Test;
+
+import static com.google.testing.compile.CompilationSubject.assertThat;
+import static net.minecraftforge.eventbus.test.compiletime.CompileTestHelper.compile;
+
+public class EventBusValidatorTests {
+    /**
+     * Tests that compile-time validation emits a warning for EventBus fields that are not final.
+     */
+    @Test
+    public void testBusFieldModifiers() {
+        var compilation = compile("""
+            record RecordTestEvent() implements RecordEvent {
+                static EventBus<RecordTestEvent> BUS = EventBus.create(RecordTestEvent.class);
+            }
+        """);
+        assertThat(compilation).hadWarningContaining("should be final");
+    }
+
+    /**
+     * Tests that compile-time validation emits a warning for EventBus fields that are not of the correct type.
+     */
+    @Test
+    public void testBusFieldType() {
+        var compilation = compile("""
+            record CancellableEvent() implements Cancellable, RecordEvent {
+                static final EventBus<CancellableEvent> BUS = EventBus.create(CancellableEvent.class);
+            }
+        """);
+        assertThat(compilation).hadWarningContaining("should be CancellableEventBus");
+    }
+}

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/compiletime/EventTypeValidatorTests.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) Forge Development LLC
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+package net.minecraftforge.eventbus.test.compiletime;
+
+import org.junit.jupiter.api.Test;
+
+import static com.google.testing.compile.CompilationSubject.assertThat;
+import static net.minecraftforge.eventbus.test.compiletime.CompileTestHelper.compile;
+
+public class EventTypeValidatorTests {
+    /**
+     * Tests that compile-time validation throws an error for non-record classes that implement RecordEvent.
+     */
+    @Test
+    public void testRecordEventValidation() {
+        var compilation = compile("final class ClassTestEvent implements RecordEvent {}");
+        assertThat(compilation).hadErrorContaining("implements RecordEvent but is not a record class");
+
+        compilation = compile("record RecordTestEvent() implements RecordEvent {}");
+        assertThat(compilation).succeededWithoutWarnings();
+    }
+
+    /**
+     * Tests that compile-time validation throws an error for MonitorAware on classes that do not extend MutableEvent.
+     */
+    @Test
+    public void testMonitorAwareValidation() {
+        var compilation = compile("record RecordTestEvent() implements RecordEvent, MonitorAware {}");
+        assertThat(compilation).hadErrorContaining("implements MonitorAware but is not a mutable event");
+
+        compilation = compile("final class ClassTestEvent extends MutableEvent implements MonitorAware {}");
+        assertThat(compilation).succeededWithoutWarnings();
+    }
+
+    /**
+     * Tests that compile-time validation throws an error for InheritableEvent on classes that are not inheritable.
+     */
+    @Test
+    public void testInheritableEventValidation() {
+        var compilation = compile("final class ClassTestEvent implements InheritableEvent {}");
+        assertThat(compilation).hadErrorContaining("directly implements InheritableEvent but is not inheritable - extend MutableEvent instead");
+
+        compilation = compile("record RecordTestEvent() implements InheritableEvent {}");
+        assertThat(compilation).hadErrorContaining("directly implements InheritableEvent but is not inheritable - implement RecordEvent instead");
+
+        compilation = compile("final class ClassTestEvent extends MutableEvent {}");
+        assertThat(compilation).succeededWithoutWarnings();
+    }
+}