Fix methods in mixin being remapped when they should not be

9 files changed, 101 insertions, 0 deletions

diff --git a/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt b/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt
index 7e8162f..a8d9f2a 100644
--- a/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt
+++ b/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt
@@ -169,7 +169,15 @@ internal class PsiMapper(
 
         var name = declaringClass!!.qualifiedName
         if (name != null) {
+            // If this method is declared in a mixin class, we want to consider the hierarchy of the target as well
             mapping = mixinMappings[name]
+            // but only if the method conceptually belongs to the target class
+            val isShadow = method.getAnnotation(CLASS_SHADOW) != null
+            val isOverwrite = method.getAnnotation(CLASS_OVERWRITE) != null
+            val isOverride = method.getAnnotation(CLASS_OVERRIDE) != null
+            if (mapping != null && !isShadow && !isOverwrite && !isOverride) {
+                return null // otherwise, it belongs to the mixin and never gets remapped
+            }
         }
         while (true) {
             if (mapping != null) {
@@ -592,6 +600,8 @@ internal class PsiMapper(
 
     companion object {
         private const val CLASS_MIXIN = "org.spongepowered.asm.mixin.Mixin"
+        private const val CLASS_SHADOW = "org.spongepowered.asm.mixin.Shadow"
+        private const val CLASS_OVERWRITE = "org.spongepowered.asm.mixin.Overwrite"
         private const val CLASS_ACCESSOR = "org.spongepowered.asm.mixin.gen.Accessor"
         private const val CLASS_INVOKER = "org.spongepowered.asm.mixin.gen.Invoker"
         private const val CLASS_AT = "org.spongepowered.asm.mixin.injection.At"
@@ -601,6 +611,7 @@ internal class PsiMapper(
         private const val CLASS_MODIFY_CONSTANT = "org.spongepowered.asm.mixin.injection.ModifyConstant"
         private const val CLASS_MODIFY_VARIABLE = "org.spongepowered.asm.mixin.injection.ModifyVariable"
         private const val CLASS_REDIRECT = "org.spongepowered.asm.mixin.injection.Redirect"
+        private const val CLASS_OVERRIDE = "java.lang.Override"
 
         private fun isSwitchCase(e: PsiElement): Boolean {
             if (e is PsiSwitchLabelStatement) {
diff --git a/src/test/java/a/pkg/A.java b/src/test/java/a/pkg/A.java
index 94e8a77..b5480d4 100644
--- a/src/test/java/a/pkg/A.java
+++ b/src/test/java/a/pkg/A.java
@@ -1,6 +1,7 @@
 package a.pkg;
 
 public class A extends AParent implements AInterface {
+    private A a;
     private int aField;
 
     public A() {
@@ -13,6 +14,10 @@ public class A extends AParent implements AInterface {
         aInterfaceMethod();
     }
 
+    public A getA() {
+        return this;
+    }
+
     public void aOverloaded() {
     }
 
diff --git a/src/test/java/a/pkg/AParent.java b/src/test/java/a/pkg/AParent.java
index aaf35fb..ac0ea7c 100644
--- a/src/test/java/a/pkg/AParent.java
+++ b/src/test/java/a/pkg/AParent.java
@@ -1,4 +1,7 @@
 package a.pkg;
 
 public class AParent {
+    public AParent aParentMethod() {
+        return this;
+    }
 }
diff --git a/src/test/java/b/pkg/B.java b/src/test/java/b/pkg/B.java
index 9524513..88f46cf 100644
--- a/src/test/java/b/pkg/B.java
+++ b/src/test/java/b/pkg/B.java
@@ -1,6 +1,7 @@
 package b.pkg;
 
 public class B extends BParent implements BInterface {
+    private B b;
     private int bField;
 
     public B() {
@@ -13,6 +14,10 @@ public class B extends BParent implements BInterface {
         bInterfaceMethod();
     }
 
+    public B getB() {
+        return this;
+    }
+
     public void bOverloaded() {
     }
 
diff --git a/src/test/java/b/pkg/BParent.java b/src/test/java/b/pkg/BParent.java
index 29cae54..d4b20fa 100644
--- a/src/test/java/b/pkg/BParent.java
+++ b/src/test/java/b/pkg/BParent.java
@@ -1,4 +1,7 @@
 package b.pkg;
 
 public class BParent {
+    public BParent bParentMethod() {
+        return this;
+    }
 }
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinAccessors.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinAccessors.kt
index 4b9450f..9933606 100644
--- a/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinAccessors.kt
+++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinAccessors.kt
@@ -62,4 +62,21 @@ class TestMixinAccessors {
             }
         """.trimIndent()
     }
+
+    @Test
+    fun `does not change @Accessor method name even when it happens to be the same as a method in the target`() {
+        TestData.remap("""
+            @org.spongepowered.asm.mixin.Mixin(a.pkg.A.class)
+            interface MixinA {
+                @org.spongepowered.asm.mixin.gen.Accessor
+                a.pkg.A getA();
+            }
+        """.trimIndent()) shouldBe """
+            @org.spongepowered.asm.mixin.Mixin(b.pkg.B.class)
+            interface MixinA {
+                @org.spongepowered.asm.mixin.gen.Accessor("b")
+                b.pkg.B getA();
+            }
+        """.trimIndent()
+    }
 }
\ No newline at end of file
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinOverride.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinOverride.kt
new file mode 100644
index 0000000..6caaeba
--- /dev/null
+++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinOverride.kt
@@ -0,0 +1,28 @@
+package com.replaymod.gradle.remap.mapper
+
+import com.replaymod.gradle.remap.util.TestData
+import io.kotest.matchers.shouldBe
+import org.junit.jupiter.api.Test
+
+class TestMixinOverride {
+    @Test
+    fun `remaps overridden method`() {
+        TestData.remap("""
+            @org.spongepowered.asm.mixin.Mixin(a.pkg.A.class)
+            abstract class MixinA extends a.pkg.AParent {
+                @Override
+                public a.pkg.AParent aParentMethod() {
+                    return this;
+                }
+            }
+        """.trimIndent()) shouldBe """
+            @org.spongepowered.asm.mixin.Mixin(b.pkg.B.class)
+            abstract class MixinA extends b.pkg.BParent {
+                @Override
+                public b.pkg.BParent bParentMethod() {
+                    return this;
+                }
+            }
+        """.trimIndent()
+    }
+}
\ No newline at end of file
diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinShadow.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinShadow.kt
new file mode 100644
index 0000000..bc4ebe6
--- /dev/null
+++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinShadow.kt
@@ -0,0 +1,26 @@
+package com.replaymod.gradle.remap.mapper
+
+import com.replaymod.gradle.remap.util.TestData
+import io.kotest.matchers.shouldBe
+import org.junit.jupiter.api.Test
+
+class TestMixinShadow {
+    @Test
+    fun `remaps shadow method and references to it`() {
+        TestData.remap("""
+            @org.spongepowered.asm.mixin.Mixin(a.pkg.A.class)
+            abstract class MixinA {
+                @org.spongepowered.asm.mixin.Shadow
+                protected abstract a.pkg.A getA();
+                private void test() { this.getA(); }
+            }
+        """.trimIndent()) shouldBe """
+            @org.spongepowered.asm.mixin.Mixin(b.pkg.B.class)
+            abstract class MixinA {
+                @org.spongepowered.asm.mixin.Shadow
+                protected abstract b.pkg.B getB();
+                private void test() { this.getB(); }
+            }
+        """.trimIndent()
+    }
+}
\ No newline at end of file
diff --git a/src/test/resources/mappings.srg b/src/test/resources/mappings.srg
index 91f6822..2a14ef5 100644
--- a/src/test/resources/mappings.srg
+++ b/src/test/resources/mappings.srg
@@ -1,6 +1,8 @@
 CL: a/pkg/A b/pkg/B
+FD: a/pkg/A/a b/pkg/B/b
 FD: a/pkg/A/aField b/pkg/B/bField
 MD: a/pkg/A/aMethod ()V b/pkg/B/bMethod ()V
+MD: a/pkg/A/getA ()La/pkg/A; b/pkg/B/getB ()Lb/pkg/B;
 MD: a/pkg/A/aOverloaded ()V b/pkg/B/bOverloaded ()V
 MD: a/pkg/A/aOverloaded (I)V b/pkg/B/bOverloaded (I)V
 MD: a/pkg/A/aOverloaded (Z)V b/pkg/B/bOverloaded (Z)V
@@ -12,5 +14,6 @@ FD: a/pkg/A$Inner/aField b/pkg/B$Inner/bField
 CL: a/pkg/A$InnerA b/pkg/B$InnerB
 CL: a/pkg/A$GenericA b/pkg/B$GenericB
 CL: a/pkg/AParent b/pkg/BParent
+MD: a/pkg/AParent/aParentMethod ()V b/pkg/BParent/bParentMethod ()V
 CL: a/pkg/AInterface b/pkg/BInterface
 MD: a/pkg/AInterface/aInterfaceMethod ()V b/pkg/BInterface/bInterfaceMethod ()V