transformer: Properly handle abstract methods in ParameterAdder

Previously abstract methods would always crash, even if no parameters were added to them. A test case was also added to check for this.

Author: NotStirred

build.gradle

@@ -10,7 +10,7 @@ plugins {
 
 group 'io.github.notstirred'
 archivesBaseName = "dasm"
-version '2.2.0'
+version '2.2.1'
 
 ext.ossrhUsername = project.findProperty("ossrhUsername") ?: "" // Enter in ~/.gradle/gradle.properties, not here.
 ext.ossrhPassword = project.findProperty("ossrhPassword") ?: "" // Enter in ~/.gradle/gradle.properties, not here.

src/main/java/io/github/notstirred/dasm/annotation/parse/RedirectSetImpl.java

@@ -133,7 +133,7 @@ public static RedirectSetImpl parse(ClassNode redirectSetClassNode, ClassNodePro
                 // Verify that there are no non-static members
                 boolean nonStaticMembersExist = innerClassNode.methods.stream()
                         // filter the default constructor, it's not a valid redirect anyway.
-                        .filter(methodNode -> !(methodNode.name.equals("<init>") && methodNode.desc.equals("()V")))
+                        .filter(methodNode -> !((methodNode.name.equals("<init>") || methodNode.name.equals("<clinit>")) && methodNode.desc.equals("()V")))
                         .anyMatch(method -> (method.access & ACC_STATIC) == 0) |
                         innerClassNode.fields.stream().anyMatch(field -> (field.access & ACC_STATIC) == 0);
                 if (nonStaticMembersExist) {
@@ -174,7 +174,7 @@ private static void parseMethods(ClassNode innerClassNode, Type srcType, Type ds
                                      Set<MethodRedirectImpl> methodRedirects, Set<FieldToMethodRedirectImpl> fieldToMethodRedirects,
                                      Set<ConstructorToFactoryRedirectImpl> constructorToFactoryRedirects, DasmClassExceptions exceptions) {
         for (MethodNode methodNode : innerClassNode.methods) {
-            if (methodNode.name.equals("<init>") && (methodNode.signature == null || methodNode.signature.equals("()V"))) {
+            if ((methodNode.name.equals("<init>") || methodNode.name.equals("<clinit>")) && (methodNode.signature == null || methodNode.signature.equals("()V"))) {
                 continue; // Skip default empty constructor
             }
 

src/main/java/io/github/notstirred/dasm/transformer/ParameterAdder.java

@@ -111,9 +111,12 @@ public void visitMaxs(int maxStack, int maxLocals) {
 
     @Override
     public void visitEnd() {
-        super.visitMaxs(this.maxStack, this.maxLocals + this.addedParameterTypeSize);
+        // endLabel is only null if there is no code (an abstract method), in which case nothing else has been called
+        if (this.endLabel != null) {
+            super.visitMaxs(this.maxStack, this.maxLocals + this.addedParameterTypeSize);
 
-        super.visitLabel(this.endLabel);
+            super.visitLabel(this.endLabel);
+        }
         super.visitEnd();
     }
 }

src/test/java/io/github/notstirred/dasm/test/TestHarness.java

@@ -19,6 +19,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.AccessFlag;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.nio.file.Files;
@@ -230,6 +231,10 @@ private static void callAllMethodsWithDummies(Class<?> actualClass, Class<?> exp
         // Create instances of test classes
         Object actualInstance;
         try {
+            // Can't test abstract classes
+            if (actualClassLoaded.accessFlags().contains(AccessFlag.ABSTRACT)) {
+                return;
+            }
             actualInstance = actualClassLoaded.getConstructor().newInstance();
         } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
             throw new RuntimeException("Failed to create transformed class instance for " + actualClassLoaded.getName(), e);
@@ -251,6 +256,10 @@ private static void callAllMethodsWithDummies(Class<?> actualClass, Class<?> exp
             try {
                 actualMethod = actualClassLoaded.getDeclaredMethod(expectedMethod.getName(), expectedMethod.getParameterTypes());
                 actualMethod.setAccessible(true);
+                // Can't test abstract methods
+                if (actualMethod.accessFlags().contains(AccessFlag.ABSTRACT)) {
+                    continue;
+                }
             } catch (NoSuchMethodException e) {
                 throw new RuntimeException("Transformed class missing expected method `" + expectedMethod + "`", e);
             }

src/test/java/io/github/notstirred/dasm/test/tests/add_parameter_no_code_case/AddParameterAbstractInput.java

@@ -0,0 +1,9 @@
+package io.github.notstirred.dasm.test.tests.add_parameter_no_code_case;
+
+import io.github.notstirred.dasm.test.targets.A;
+
+public abstract class AddParameterAbstractInput {
+    public abstract void method1();
+
+    public abstract void method2(A foo);
+}

src/test/java/io/github/notstirred/dasm/test/tests/add_parameter_no_code_case/AddParameterAbstractOutput.java

@@ -0,0 +1,15 @@
+package io.github.notstirred.dasm.test.tests.add_parameter_no_code_case;
+
+import io.github.notstirred.dasm.test.targets.A;
+import io.github.notstirred.dasm.test.targets.B;
+import io.github.notstirred.dasm.test.targets.C;
+
+public abstract class AddParameterAbstractOutput {
+    public abstract void method1();
+
+    public abstract void method1out();
+
+    public abstract void method2(A foo);
+
+    public abstract void method2out(B foo, C foo2);
+}

src/test/java/io/github/notstirred/dasm/test/tests/add_parameter_no_code_case/TestAddParameterAbstract.java

@@ -0,0 +1,37 @@
+package io.github.notstirred.dasm.test.tests.add_parameter_no_code_case;
+
+import io.github.notstirred.dasm.api.annotations.Dasm;
+import io.github.notstirred.dasm.api.annotations.redirect.redirects.TypeRedirect;
+import io.github.notstirred.dasm.api.annotations.redirect.sets.RedirectSet;
+import io.github.notstirred.dasm.api.annotations.selector.MethodSig;
+import io.github.notstirred.dasm.api.annotations.selector.Ref;
+import io.github.notstirred.dasm.api.annotations.transform.AddUnusedParam;
+import io.github.notstirred.dasm.api.annotations.transform.TransformFromMethod;
+import io.github.notstirred.dasm.test.targets.A;
+import io.github.notstirred.dasm.test.targets.B;
+import io.github.notstirred.dasm.test.targets.C;
+import io.github.notstirred.dasm.test.tests.BaseMethodTest;
+
+import static io.github.notstirred.dasm.test.tests.TestData.single;
+
+@Dasm(TestAddParameterAbstract.Set.class)
+public class TestAddParameterAbstract extends BaseMethodTest {
+    public TestAddParameterAbstract() {
+        super(single(AddParameterAbstractInput.class, AddParameterAbstractOutput.class, TestAddParameterAbstract.class));
+    }
+
+    // Error case for https://discord.com/channels/316679487955927050/317206370359443458/1343090582087532544
+    // An abstract method without an added parameter.
+    @TransformFromMethod(value = @MethodSig("method1()V"))
+    public native void method1out1();
+
+    @TransformFromMethod(value = @MethodSig("method2(Lio/github/notstirred/dasm/test/targets/A;)V"))
+    public native void method2out(B foo, @AddUnusedParam C foo2);
+
+    @RedirectSet
+    public interface Set {
+        @TypeRedirect(from = @Ref(A.class), to = @Ref(B.class))
+        abstract class A_to_B {
+        }
+    }
+}