From 1aa8b425982a6d30a177bc25a70a325652209ee0 Mon Sep 17 00:00:00 2001 From: Jonas Herzig Date: Thu, 11 Nov 2021 20:37:10 +0100 Subject: Fix methods in mixin being remapped when they should not be --- .../kotlin/com/replaymod/gradle/remap/PsiMapper.kt | 11 +++++++++ src/test/java/a/pkg/A.java | 5 ++++ src/test/java/a/pkg/AParent.java | 3 +++ src/test/java/b/pkg/B.java | 5 ++++ src/test/java/b/pkg/BParent.java | 3 +++ .../gradle/remap/mapper/TestMixinAccessors.kt | 17 +++++++++++++ .../gradle/remap/mapper/TestMixinOverride.kt | 28 ++++++++++++++++++++++ .../gradle/remap/mapper/TestMixinShadow.kt | 26 ++++++++++++++++++++ src/test/resources/mappings.srg | 3 +++ 9 files changed, 101 insertions(+) create mode 100644 src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinOverride.kt create mode 100644 src/test/kotlin/com/replaymod/gradle/remap/mapper/TestMixinShadow.kt 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 -- cgit