Make EventBus works without EventSubscriberTransformer

src/main/java/net/minecraftforge/fml/common/eventhandler/EventBus.java

@@ -22,11 +22,10 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
-import java.util.ArrayList;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+import java.util.stream.Collectors;
 
 import javax.annotation.Nonnull;
 
@@ -79,59 +78,47 @@ public void register(Object target)
         }
         listenerOwners.put(target, activeModContainer);
 
-        boolean isStatic;
-        Set<? extends Class<?>> supers;
-        Class<?> scanTarget;
+
+        Collection<Method> methods;
         if (target instanceof Class<?> clazz) {
-            isStatic = true;
-            supers = Set.of(clazz);
-            scanTarget = clazz;
+            // static listener: subscribed methods must be declared by the class
+            methods = Arrays.stream(clazz.getDeclaredMethods())
+                .filter(m -> !m.isSynthetic()
+                             && Modifier.isStatic(m.getModifiers())
+                             // private not allowed to keep legacy behaviour
+                             && !Modifier.isPrivate(m.getModifiers())
+                             && m.isAnnotationPresent(SubscribeEvent.class))
+                .toList();
         } else {
-            isStatic = false;
-            supers = TypeToken.of(target.getClass()).getTypes().rawTypes();
-            scanTarget = target.getClass();
+            // instance listener: methods overriding a subscribed method is also valid.
+            //
+            // In this case, we will register the subscribed parent method instead. JVM will
+            // handle it if a subclass overrides subscribed method
+            methods = TypeToken.of(target.getClass())
+                .getTypes()
+                .rawTypes()
+                // get self & superclass & interface
+                .stream()
+                .map(Class::getDeclaredMethods)
+                .flatMap(Arrays::stream)
+                .filter(m -> !m.isSynthetic()
+                             && !Modifier.isStatic(m.getModifiers())
+                             // private not allowed because it does not participate in inheritance
+                             && !Modifier.isPrivate(m.getModifiers())
+                             && m.isAnnotationPresent(SubscribeEvent.class))
+                // deduplicate by signature
+                .collect(Collectors.toMap(
+                    m -> Map.entry(m.getName(), Arrays.asList(m.getParameterTypes())),
+                    Function.identity(),
+                    (a, b) -> a,
+                    LinkedHashMap::new
+                ))
+                .values();
         }
 
-        for (Method method : scanTarget.getMethods())
+        for (Method method : methods)
         {
-            if (isStatic != Modifier.isStatic(method.getModifiers()))
-                continue;
-
-            try {
-                // do `.getDeclaredMethod(...)` to force JVM to walk through declared methods and load their parameter
-                // types. This is for preventing shortcut below from skipping classloading
-                //
-                // mod developers should be responsible for not loading non-existent class, but :(
-                // related issue: https://github.com/CleanroomMC/Cleanroom/issues/349
-                method.getDeclaringClass().getDeclaredMethod("forceClassLoadingForDeclaredMethods", Event.class);
-            } catch (NoSuchMethodException e) {
-                // swallow this specific exception, other exceptions, like ClassNotFoundException, will fall through
-            }
-
             var parameterTypes = method.getParameterTypes();
-            var matched = supers.stream()
-                .map(cls -> {
-                    if (cls == method.getDeclaringClass()) {
-                        // shortcut for most event handler classes with no explicit superclass
-                        return method;
-                    }
-                    try {
-                        return cls.getDeclaredMethod(method.getName(), parameterTypes);
-                    } catch (NoSuchMethodException e) {
-                        // Eat the error, this is not unexpected
-                        return null;
-                    }
-                })
-                .filter(Objects::nonNull)
-                .filter(m -> m.isAnnotationPresent(SubscribeEvent.class))
-                .findFirst()
-                .orElse(null);
-
-            if (matched == null)
-            {
-                continue;
-            }
-
             if (parameterTypes.length != 1)
             {
                 throw new IllegalArgumentException(
@@ -141,15 +128,12 @@ public void register(Object target)
             }
 
             Class<?> eventType = parameterTypes[0];
-
             if (!Event.class.isAssignableFrom(eventType))
             {
                 throw new IllegalArgumentException("Method " + method + " has @SubscribeEvent annotation, but takes a argument that is not an Event " + eventType);
             }
 
-            // the method to be registered here is "matched", not "method", it should be a bug of
-            // the original event bus, since the exceptions above are all referencing "method"
-            register(eventType, target, matched, activeModContainer);
+            register(eventType, target, method, activeModContainer);
         }
     }
 

src/main/java/net/minecraftforge/fml/common/eventhandler/EventListenerFactory.java

@@ -39,15 +39,16 @@ private static MethodHandle createListenerFactory(
         Object instance
     ) {
         try {
-            var handle = LOOKUP.unreflect(callback);
+            var lookup = MethodHandles.privateLookupIn(callback.getDeclaringClass(), LOOKUP);
+            var handle = lookup.unreflect(callback);
 
             var factoryType = isStatic
                 ? Constants.RETURNS_IT
                 // implicit null check on "instance" via ".getClass()"
                 : Constants.RETURNS_IT.insertParameterTypes(0, instance.getClass());
 
             var factoryHandle = LambdaMetafactory.metafactory(
-                LOOKUP,
+                lookup,
                 Constants.METHOD_NAME,
                 factoryType,
                 Constants.METHOD_TYPE,

src/main/java/net/minecraftforge/fml/relauncher/FMLCorePlugin.java

@@ -31,7 +31,6 @@ public String[] getASMTransformerClass()
         return new String[] {
                              "net.minecraftforge.fml.common.asm.transformers.SideTransformer",
                              "net.minecraftforge.fml.common.asm.transformers.EventSubscriptionTransformer",
-                             "net.minecraftforge.fml.common.asm.transformers.EventSubscriberTransformer",
                              "net.minecraftforge.fml.common.asm.transformers.SoundEngineFixTransformer",
                              "net.minecraftforge.fml.common.asm.transformers.LWJGLTransformer",
                             };

src/test/java/net/minecraftforge/fml/common/eventhandler/EventBusTest.java

@@ -2,15 +2,13 @@
 
 import net.minecraftforge.fml.common.Loader;
 import net.minecraftforge.fml.common.ModContainer;
-import net.minecraftforge.fml.common.eventhandler.impl.AbnormalListeners;
-import net.minecraftforge.fml.common.eventhandler.impl.ExampleEvent;
-import net.minecraftforge.fml.common.eventhandler.impl.InstanceListeners;
-import net.minecraftforge.fml.common.eventhandler.impl.StaticListeners;
+import net.minecraftforge.fml.common.eventhandler.impl.*;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import java.lang.reflect.Method;
 import java.util.List;
+import java.util.Set;
 
 /**
  * @author ZZZank
@@ -86,4 +84,32 @@ public void registerIllegalParamType() throws Exception {
 
         Assertions.assertEquals(event.id, listener.captured);
     }
+
+    @Test
+    public void registerNonPublic() {
+        var bus = new EventBus();
+
+        // static
+        {
+            bus.register(NonPublicListeners.class);
+
+            var event = new ExampleEvent();
+            bus.post(event);
+            Assertions.assertEquals(Set.of("packaged static", "protected static"), event.sink);
+
+            bus.unregister(NonPublicListeners.class);
+        }
+
+        // instance
+        {
+            var listener = new NonPublicListeners.OverrideWithNoSub();
+            bus.register(listener);
+
+            var event = new ExampleEvent();
+            bus.post(event);
+            Assertions.assertEquals(Set.of("packaged", "protected (subclass)"), event.sink);
+
+            bus.unregister(listener);
+        }
+    }
 }

src/test/java/net/minecraftforge/fml/common/eventhandler/impl/ExampleEvent.java

@@ -2,13 +2,17 @@
 
 import net.minecraftforge.fml.common.eventhandler.Event;
 
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * @author ZZZank
  */
 public class ExampleEvent extends Event {
     public static int CURRENT_ID = 0;
 
     public final int id;
+    public final Set<String> sink = new HashSet<>();
 
     public ExampleEvent() {
         this.id = CURRENT_ID++;

src/test/java/net/minecraftforge/fml/common/eventhandler/impl/NonPublicListeners.java

@@ -0,0 +1,47 @@
+package net.minecraftforge.fml.common.eventhandler.impl;
+
+import net.minecraftforge.fml.common.eventhandler.SubscribeEvent;
+
+/**
+ * @author ZZZank
+ */
+public class NonPublicListeners {
+
+    @SubscribeEvent
+    private static void privateStatic(ExampleEvent event) {
+        throw new AssertionError("private (static) method should not be registered");
+    }
+
+    @SubscribeEvent
+    static void packagedStatic(ExampleEvent event) {
+        event.sink.add("packaged static");
+    }
+
+    @SubscribeEvent
+    protected static void protectedStatic(ExampleEvent event) {
+        event.sink.add("protected static");
+    }
+
+    @SubscribeEvent
+    private void privateInstance(ExampleEvent event) {
+        throw new AssertionError("private method should not be registered");
+    }
+
+    @SubscribeEvent
+    void packagedInstance(ExampleEvent event) {
+        event.sink.add("packaged");
+    }
+
+    @SubscribeEvent
+    protected void protectedInstance(ExampleEvent event) {
+        throw new AssertionError("impl by subclass");
+    }
+
+    public static class OverrideWithNoSub extends NonPublicListeners {
+
+        @Override
+        protected void protectedInstance(ExampleEvent event) {
+            event.sink.add("protected (subclass)");
+        }
+    }
+}