Make method generation order sorted by MethodKey

helps maintain a more deterministic order

Author: Machine-Maker

src/main/java/io/papermc/asm/rules/RewriteRule.java

@@ -2,6 +2,7 @@
 
 import io.papermc.asm.ClassProcessingContext;
 import io.papermc.asm.rules.builder.RuleFactory;
+import io.papermc.asm.util.DescriptorUtils;
 import java.lang.constant.ClassDesc;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -26,7 +27,7 @@ static RewriteRule forOwnerClass(final Class<?> owner, final Consumer<? super Ru
 
     @SafeVarargs
     static RewriteRule forOwnerClasses(final Set<Class<?>> owners, final Consumer<? super RuleFactory> firstFactoryConsumer, final Consumer<? super RuleFactory>... factoryConsumers) {
-        return forOwners(owners.stream().map(c -> c.describeConstable().orElseThrow()).collect(Collectors.toUnmodifiableSet()), firstFactoryConsumer, factoryConsumers);
+        return forOwners(owners.stream().map(DescriptorUtils::desc).collect(Collectors.toUnmodifiableSet()), firstFactoryConsumer, factoryConsumers);
     }
 
     @SafeVarargs
@@ -49,7 +50,13 @@ static RewriteRule chain(final RewriteRule... rules) {
     }
 
     static RewriteRule chain(final Collection<? extends RewriteRule> rules) {
-        return new Chain(List.copyOf(rules));
+        final List<? extends RewriteRule> filteredRules = rules.stream().filter(r -> r != EMPTY).toList();
+        if (filteredRules.isEmpty()) {
+            return EMPTY;
+        } else if (filteredRules.size() == 1) {
+            return filteredRules.iterator().next();
+        }
+        return new Chain(filteredRules);
     }
 
     static ChainBuilder chain() {
@@ -73,16 +80,17 @@ default ClassVisitor createVisitor(final int api, final ClassVisitor parent, fin
         }
     }
 
-    record Chain(List<RewriteRule> rules) implements RewriteRule {
-        public Chain(final List<RewriteRule> rules) {
-            this.rules = List.copyOf(rules);
+    record Chain(List<? extends RewriteRule> rules) implements RewriteRule {
+        public Chain {
+            rules = List.copyOf(rules);
         }
 
         @Override
         public ClassVisitor createVisitor(final int api, final ClassVisitor parent, final ClassProcessingContext context) {
             ClassVisitor visitor = parent;
-            for (final RewriteRule rule : this.rules) {
-                visitor = rule.createVisitor(api, visitor, context);
+            // iterate over this.rules backwards to ensure the first rule is the outermost visitor
+            for (int i = this.rules.size() - 1; i >= 0; i--) {
+                visitor = this.rules.get(i).createVisitor(api, visitor, context);
             }
             return visitor;
         }
@@ -109,7 +117,7 @@ public ChainBuilder then(final RewriteRule... rules) {
         }
 
         public RewriteRule build() {
-            return new Chain(this.rules);
+            return RewriteRule.chain(this.rules);
         }
     }
 }

src/main/java/io/papermc/asm/rules/builder/RuleFactoryImpl.java

@@ -101,9 +101,6 @@ private static Method isStatic(final Method staticHandler) {
 
     @Override
     public RewriteRule build() {
-        if (this.rules.size() == 1) {
-            return this.rules.get(0);
-        }
         return RewriteRule.chain(this.rules);
     }
 }

src/main/java/io/papermc/asm/rules/method/MethodRewriteRule.java

@@ -5,8 +5,9 @@
 import io.papermc.asm.rules.method.rewrite.MethodRewrite;
 import java.lang.constant.ClassDesc;
 import java.lang.constant.MethodTypeDesc;
-import java.util.LinkedHashMap;
+import java.util.Comparator;
 import java.util.Map;
+import java.util.TreeMap;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.Handle;
@@ -56,9 +57,17 @@ default boolean shouldProcess(final ClassProcessingContext context, final int op
 
     @Override
     default ClassVisitor createVisitor(final int api, final ClassVisitor parent, final ClassProcessingContext context) {
-        record MethodKey(String owner, String name, MethodTypeDesc descriptor) {
+        record MethodKey(String owner, String name, MethodTypeDesc descriptor) implements Comparable<MethodKey> {
+            private static final Comparator<MethodKey> COMPARATOR = Comparator.comparing(MethodKey::owner)
+                .thenComparing(MethodKey::name)
+                .thenComparing(key -> key.descriptor().descriptorString());
+
+            @Override
+            public int compareTo(final MethodKey o) {
+                return COMPARATOR.compare(this, o);
+            }
         }
-        final Map<MethodKey, MethodRewrite.MethodGenerator> methodsToGenerate = new LinkedHashMap<>();
+        final Map<MethodKey, MethodRewrite.MethodGenerator> methodsToGenerate = new TreeMap<>();
         return new ClassVisitor(api, parent) {
 
             @Override