From b00673f859058ff180dc16c22e12ac5698ffbab2 Mon Sep 17 00:00:00 2001 From: Jonas Herzig Date: Fri, 12 Nov 2021 19:00:04 +0100 Subject: Implement synthetic property to/from setter conversion --- .../kotlin/com/replaymod/gradle/remap/PsiMapper.kt | 128 ++++++++++++++++++++- .../mapper/kotlin/TestKotlinSyntheticProperties.kt | 44 +++++-- 2 files changed, 161 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt b/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt index 58224a5..412b6bf 100644 --- a/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt +++ b/src/main/kotlin/com/replaymod/gradle/remap/PsiMapper.kt @@ -20,7 +20,9 @@ import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.getNonStrictParentOfType +import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.psi.synthetics.findClassDescriptor import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull @@ -28,6 +30,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.getAllSuperclassesWithoutAny import org.jetbrains.kotlin.resolve.descriptorUtil.overriddenTreeAsSequence import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor.Companion.propertyNameByGetMethodName +import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor.Companion.propertyNameBySetMethodName import java.util.* internal class PsiMapper( @@ -143,10 +146,66 @@ internal class PsiMapper( replace(expr.parent, maybeGetter.identifier) return } + + val parent = expr.parent + if (parent is KtCallExpression) { + val argumentList = parent.valueArgumentList + val maybeSetter = getSyntheticPropertyForSetter(expr, method, mapping) + if (maybeSetter != null && argumentList != null) { + val leftParen = argumentList.leftParenthesis!! + val rightParen = argumentList.rightParenthesis!! + replace(TextRange( + expr.startOffset, + leftParen.endOffset, + ), "$maybeSetter =" + if (leftParen.nextSibling is PsiWhiteSpace) "" else " ") + replace(rightParen, "") + return + } + } + replaceIdentifier(expr, mapped) } } + private fun getSyntheticPropertyForSetter(expr: PsiElement, method: PsiMethod, mapping: MethodMapping): String? { + // Check if the setter method qualifies for synthetic property conversion + if (method.returnType != PsiType.VOID) return null + if (method.hasModifier(JvmModifier.STATIC)) return null + val parameter = method.parameterList.parameters.singleOrNull() ?: return null + val type = ClassUtil.getBinaryPresentation(parameter.type) + + // `super.setDebugInfo(value)` is a special case which cannot be replaced with a synthetic property + if (expr.parent.parent.let { it is KtDotQualifiedExpression && it.firstChild is KtSuperExpression }) { + return null + } + + val setterName = mapping.deobfuscatedName + + for (withIsPrefix in listOf(false, true)) { + if (withIsPrefix && type != "Z") continue // only boolean types may use the `is` prefix + val propertyName = propertyNameBySetMethodName(Name.identifier(setterName), withIsPrefix) ?: continue + val property = propertyName.identifier + + val getterName = getterNameByPropertyName(property, withIsPrefix) + mapping.parent.methodMappings.find { + it.deobfuscatedName == getterName + && it.descriptor.paramTypes.isEmpty() + && it.descriptor.returnType.toString() == type + } ?: continue + + if (isSyntheticPropertyShadowedByField(property, mapping, expr)) { + return null + } + + return property + } + + return null + } + + private fun getterNameByPropertyName(propertyName: String, withIsPrefix: Boolean): String = + if (withIsPrefix) propertyName else "get" + propertyName.replaceFirstChar { it.uppercaseChar() } + private fun map(expr: PsiElement, method: KtNamedFunction) { val psiMethod = method.getRepresentativeLightMethod() val mapped = findMapping(psiMethod ?: return)?.deobfuscatedName @@ -156,10 +215,34 @@ internal class PsiMapper( } private fun map(expr: PsiElement, property: SyntheticJavaPropertyDescriptor) { - val getter = property.getMethod - .overriddenTreeAsSequence(false) - .firstNotNullOfOrNull { it.findPsi() } - as? PsiMethod ?: return + val assignment = findAssignment(expr) + if (assignment != null) { + mapSetter(expr, assignment, property) + } else { + mapGetter(expr, property) + } + } + + private fun findAssignment(expr: PsiElement): KtBinaryExpression? { + val parent = expr.parent + // Our parent will either be the assignment (`expr = value`) or a qualified expression (`receiver.expr = value`) + val leftSide = if (parent is KtQualifiedExpression) { + if (parent.selectorExpression != expr) return null // we are on the wrong side: `expr.selector = value` + parent + } else { + expr + } + val assignment = leftSide.parent as? KtBinaryExpression ?: return null // not an assignment after all + if (assignment.left != leftSide) return null // turns out we are on the right side of the assignment + if (assignment.operationToken != KtTokens.EQ) return null // not actually an assignment + return assignment + } + + private fun FunctionDescriptor.findPsiInHierarchy() = + overriddenTreeAsSequence(false).firstNotNullOfOrNull { it.findPsi() } as? PsiMethod + + private fun mapGetter(expr: PsiElement, property: SyntheticJavaPropertyDescriptor) { + val getter = property.getMethod.findPsiInHierarchy() ?: return val mapping = findMapping(getter) val mappedGetter = mapping?.deobfuscatedName ?: return if (mappedGetter != getter.name) { @@ -176,6 +259,43 @@ internal class PsiMapper( } } + private fun mapSetter(expr: PsiElement, assignment: KtBinaryExpression, property: SyntheticJavaPropertyDescriptor) { + val getter = property.getMethod.findPsiInHierarchy() ?: return + val setter = property.setMethod?.findPsiInHierarchy() ?: return + val mappingGetter = findMapping(getter) + val mappingSetter = findMapping(setter) + val mappedGetter = mappingGetter?.deobfuscatedName // if getter is gone, we need to switch to method invocation + val mappedSetter = mappingSetter?.deobfuscatedName ?: return + if (mappedGetter == null || mappedSetter != setter.name) { + val maybeMapped = if (mappedGetter != null) { + val isBooleanField = mappingGetter.deobfuscatedDescriptor.endsWith("Z") + val withIsPrefix = isBooleanField && mappedGetter.startsWith("is") + val propertyByGetter = propertyNameByGetMethodName(Name.identifier(mappedGetter)) + val propertyBySetter = propertyNameBySetMethodName(Name.identifier(mappedSetter), withIsPrefix) + if (propertyByGetter == propertyBySetter) { + propertyBySetter + } else { + null // remapped setter does not match remapped getter, use accessor method instead + } + } else { + null + } + if (maybeMapped == null || isSyntheticPropertyShadowedByField(maybeMapped.identifier, mappingSetter, expr)) { + val op = assignment.operationReference + replace(TextRange( + expr.startOffset, + // If there is a single whitespace after the operation element, eat it, otherwise don't + op.endOffset + if ((op.nextSibling as? PsiWhiteSpace)?.textLength == 1) 1 else 0 + ), "$mappedSetter(") + val right = assignment.right!! + replace(TextRange(right.endOffset, right.endOffset), ")") + } else { + val mapped = maybeMapped.identifier + replaceIdentifier(expr, mapped) + } + } + } + // See caller for why this exists private fun map(expr: PsiElement, method: FunctionDescriptor) { for (overriddenDescriptor in method.overriddenDescriptors) { diff --git a/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinSyntheticProperties.kt b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinSyntheticProperties.kt index 65459f2..7a4ab4e 100644 --- a/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinSyntheticProperties.kt +++ b/src/test/kotlin/com/replaymod/gradle/remap/mapper/kotlin/TestKotlinSyntheticProperties.kt @@ -2,7 +2,6 @@ package com.replaymod.gradle.remap.mapper.kotlin import com.replaymod.gradle.remap.util.TestData import io.kotest.matchers.shouldBe -import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test class TestKotlinSyntheticProperties { @@ -63,37 +62,43 @@ class TestKotlinSyntheticProperties { } @Test - @Disabled("not yet implemented") fun `converts synthetic property to setter if no longer synthetic`() { TestData.remapKt(""" import a.pkg.A fun test() { A().nonSyntheticA = A() - A().isNonSyntheticBooleanA = true + A().isNonSyntheticBooleanA = + // Comment + true // More comment } """.trimIndent()) shouldBe """ import b.pkg.B fun test() { B().setterB(B()) - B().setterBooleanB(true) + B().setterBooleanB( + // Comment + true) // More comment } """.trimIndent() } @Test - @Disabled("not yet implemented") fun `converts setter to synthetic property if now synthetic`() { TestData.remapKt(""" import a.pkg.A fun test() { A().setterA(A()) - A().setterBooleanA(true) + A().setterBooleanA( + // Comment + true) // More comment } """.trimIndent()) shouldBe """ import b.pkg.B fun test() { B().nonSyntheticB = B() - B().isNonSyntheticBooleanB = true + B().isNonSyntheticBooleanB = + // Comment + true // More comment } """.trimIndent() } @@ -170,4 +175,29 @@ class TestKotlinSyntheticProperties { fun test() { Kt().syntheticB = Kt() } """.trimIndent() } + + @Test + fun `does not replace super calls with synthetic properties`() { + TestData.remapKt(""" + import a.pkg.A + class C : A() { + init { + super.getSyntheticA() + super.isSyntheticBooleanA() + super.setSyntheticA(A()) + super.setSyntheticBooleanA(true) + } + } + """.trimIndent()) shouldBe """ + import b.pkg.B + class C : B() { + init { + super.getSyntheticB() + super.isSyntheticBooleanB() + super.setSyntheticB(B()) + super.setSyntheticBooleanB(true) + } + } + """.trimIndent() + } } -- cgit