From 982982c63027e416bd78ca40708c7238e7eed26b Mon Sep 17 00:00:00 2001 From: Jonas Herzig Date: Thu, 11 Nov 2021 16:52:27 +0100 Subject: Fix remapping of qualified inner class reference in Kotlin code When we used to remap `a.pkg.A.Inner` we would apply both mappings (the one for the inner class and the one for the outer class) at the same time, resulting in `b.pkg.B.Inner.Inner`. The direct cause being that we only checked conflicts for the range of the respective expression (which is just `A` and `Inner` for Kotlin, and therefore not conflicting) instead of the parent as we should have. With that fixed, it now also becomes apparent that we need to apply mappings for dot qualified expressions back to front (otherwise the outer class takes priority), so that is the second thing this commit change. --- .../kotlin/com/replaymod/gradle/remap/PsiMapper.kt | 22 +++++++++- src/test/java/a/pkg/A.java | 5 +++ src/test/java/b/pkg/B.java | 5 +++ .../remap/mapper/kotlin/TestKotlinGenerics.kt | 49 ++++++++++++++++++++++ .../remap/mapper/kotlin/TestKotlinImports.kt | 42 +++++++++++++++++++ .../remap/mapper/kotlin/TestKotlinTypeAliases.kt | 42 +++++++++++++++++++ .../com/replaymod/gradle/remap/util/TestData.kt | 3 ++ src/test/resources/mappings.srg | 2 + 8 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinGenerics.kt create mode 100644 src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinImports.kt create mode 100644 src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinTypeAliases.kt diff --git a/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt b/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt index 07d7d1d..f7b50ba 100644 --- a/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt +++ b/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt @@ -219,7 +219,9 @@ internal class PsiMapper( } val parent: PsiElement? = expr.parent if ((parent is KtUserType || parent is KtQualifiedExpression) && parent.text == name) { - replace(parent, mapped) + if (valid(parent)) { + replace(parent, mapped) + } return } // FIXME this incorrectly filters things like "Packet" and doesn't filter same-name type aliases @@ -549,6 +551,24 @@ internal class PsiMapper( return super.visitNamedFunction(function, data) } + override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression, data: Void?): Void? { + // Dot qualified expressions such as "a.pkg.A.Inner" we want to remap back to front because the + // latter parts are more specific. + // I.e. we start with the inner class, and only if there is no mapping for that, do we try to remap + // the outer class. + expression.selectorExpression?.accept(this) + expression.receiverExpression.accept(this) + return null + } + + override fun visitUserType(type: KtUserType, data: Void?): Void? { + // Same as visitDotQualifiedExpression but for typealias declarations + type.referenceExpression?.accept(this) + type.qualifier?.accept(this) + type.typeArgumentList?.accept(this) + return null + } + override fun visitReferenceExpression(expression: KtReferenceExpression, data: Void?): Void? { if (valid(expression)) { val target = bindingContext[BindingContext.REFERENCE_TARGET, expression] diff --git a/src/test/java/a/pkg/A.java b/src/test/java/a/pkg/A.java index ff779b2..94e8a77 100644 --- a/src/test/java/a/pkg/A.java +++ b/src/test/java/a/pkg/A.java @@ -42,4 +42,9 @@ public class A extends AParent implements AInterface { public class Inner { private int aField; } + + public class InnerA { + } + + public class GenericA {} } diff --git a/src/test/java/b/pkg/B.java b/src/test/java/b/pkg/B.java index 6ed26c1..9524513 100644 --- a/src/test/java/b/pkg/B.java +++ b/src/test/java/b/pkg/B.java @@ -42,4 +42,9 @@ public class B extends BParent implements BInterface { public class Inner { private int bField; } + + public class InnerB { + } + + public class GenericB {} } diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinGenerics.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinGenerics.kt new file mode 100644 index 0000000..19a3574 --- /dev/null +++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinGenerics.kt @@ -0,0 +1,49 @@ +package com.replaymod.gradle.remap.mapper.kotlin + +import com.replaymod.gradle.remap.util.TestData +import io.kotest.matchers.collections.shouldHaveSize +import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldContain +import org.junit.jupiter.api.Test + +class TestKotlinGenerics { + @Test + fun `remaps generic type argument`() { + TestData.remapKt(""" + val test: List = TODO() + """.trimIndent()) shouldBe """ + val test: List = TODO() + """.trimIndent() + } + + @Test + fun `remaps generic type argument with import`() { + TestData.remapKt(""" + import a.pkg.A + val test: List = TODO() + """.trimIndent()) shouldBe """ + import b.pkg.B + val test: List = TODO() + """.trimIndent() + } + + @Test + fun `remaps generic type`() { + TestData.remapKt(""" + val test: a.pkg.A.GenericA = TODO() + """.trimIndent()) shouldBe """ + val test: b.pkg.B.GenericB = TODO() + """.trimIndent() + } + + @Test + fun `remaps generic type with import`() { + TestData.remapKt(""" + import a.pkg.A.GenericA + val test: GenericA = TODO() + """.trimIndent()) shouldBe """ + import b.pkg.B.GenericB + val test: GenericB = TODO() + """.trimIndent() + } +} \ No newline at end of file diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinImports.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinImports.kt new file mode 100644 index 0000000..3193c2f --- /dev/null +++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinImports.kt @@ -0,0 +1,42 @@ +package com.replaymod.gradle.remap.mapper.kotlin + +import com.replaymod.gradle.remap.util.TestData +import io.kotest.matchers.collections.shouldHaveSize +import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldContain +import org.junit.jupiter.api.Test + +class TestKotlinImports { + @Test + fun `remaps simple import`() { + TestData.remapKt(""" + import a.pkg.A + val test: A = TODO() + """.trimIndent()) shouldBe """ + import b.pkg.B + val test: B = TODO() + """.trimIndent() + } + + @Test + fun `remaps outer class of inner class import`() { + TestData.remapKt(""" + import a.pkg.A.Inner + val test: Inner = TODO() + """.trimIndent()) shouldBe """ + import b.pkg.B.Inner + val test: Inner = TODO() + """.trimIndent() + } + + @Test + fun `remaps inner class import`() { + TestData.remapKt(""" + import a.pkg.A.InnerA + val test: InnerA = TODO() + """.trimIndent()) shouldBe """ + import b.pkg.B.InnerB + val test: InnerB = TODO() + """.trimIndent() + } +} \ No newline at end of file diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinTypeAliases.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinTypeAliases.kt new file mode 100644 index 0000000..0d69341 --- /dev/null +++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinTypeAliases.kt @@ -0,0 +1,42 @@ +package com.replaymod.gradle.remap.mapper.kotlin + +import com.replaymod.gradle.remap.util.TestData +import io.kotest.matchers.collections.shouldHaveSize +import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldContain +import org.junit.jupiter.api.Test + +class TestKotlinTypeAliases { + @Test + fun `remaps simple alias`() { + TestData.remapKt(""" + typealias A = a.pkg.A + val test: A = TODO() + """.trimIndent()) shouldBe """ + typealias A = b.pkg.B + val test: A = TODO() + """.trimIndent() + } + + @Test + fun `remaps outer class of inner class alias`() { + TestData.remapKt(""" + typealias Inner = a.pkg.A.Inner + val test: Inner = TODO() + """.trimIndent()) shouldBe """ + typealias Inner = b.pkg.B.Inner + val test: Inner = TODO() + """.trimIndent() + } + + @Test + fun `remaps inner class alias`() { + TestData.remapKt(""" + typealias InnerA = a.pkg.A.InnerA + val test: InnerA = TODO() + """.trimIndent()) shouldBe """ + typealias InnerA = b.pkg.B.InnerB + val test: InnerA = TODO() + """.trimIndent() + } +} \ No newline at end of file diff --git a/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt b/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt index cb0ad44..0f83373 100644 --- a/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt +++ b/src/test/kotlin/com/replaymod/gradle/remap/util/TestData.kt @@ -45,4 +45,7 @@ object TestData { fun remap(content: String): String = transformer.remap(mapOf("test.java" to content))["test.java"]!!.first fun remapWithErrors(content: String) = transformer.remap(mapOf("test.java" to content))["test.java"]!! + + fun remapKt(content: String): String = transformer.remap(mapOf("test.kt" to content))["test.kt"]!!.first + fun remapKtWithErrors(content: String) = transformer.remap(mapOf("test.kt" to content))["test.kt"]!! } \ No newline at end of file diff --git a/src/test/resources/mappings.srg b/src/test/resources/mappings.srg index 4213107..91f6822 100644 --- a/src/test/resources/mappings.srg +++ b/src/test/resources/mappings.srg @@ -9,6 +9,8 @@ MD: a/pkg/A/commonOverloaded (La/pkg/A;)V b/pkg/B/commonOverloaded (La/pkg/B;)V CL: a/pkg/A$1 b/pkg/B$1 CL: a/pkg/A$Inner b/pkg/B$Inner 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 CL: a/pkg/AInterface b/pkg/BInterface MD: a/pkg/AInterface/aInterfaceMethod ()V b/pkg/BInterface/bInterfaceMethod ()V -- cgit