Refactoring: change signature should affected expect/actual

#KT-33972 Fixed
2019-09-04 15:13:55 +07:00
parent 484dda478e
commit e3ce799993
53 changed files with 473 additions and 63 deletions
@@ -188,7 +188,7 @@ class ConvertFunctionToPropertyIntention :
            }
        }

-        private fun findMainElement(callables: List<PsiElement>): PsiElement? {
+        private fun findMainElement(callables: Collection<PsiElement>): PsiElement? {
            if (editor == null) return null
            val offset = editor.caretModel.offset
            return callables.find { file == it.containingFile && offset in it.textRange }
@@ -35,31 +35,24 @@ import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
 import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
 import org.jetbrains.kotlin.idea.core.getDeepestSuperDeclarations
 import org.jetbrains.kotlin.idea.core.getDirectlyOverriddenDeclarations
+import org.jetbrains.kotlin.idea.search.declarationsSearch.forEachOverridingElement
 import org.jetbrains.kotlin.idea.util.actualsForExpected
 import org.jetbrains.kotlin.idea.util.getResolutionScope
-import org.jetbrains.kotlin.idea.util.isExpectDeclaration
 import org.jetbrains.kotlin.idea.util.liftToExpected
-import org.jetbrains.kotlin.psi.KtBlockExpression
-import org.jetbrains.kotlin.psi.KtDeclaration
-import org.jetbrains.kotlin.psi.KtDeclarationWithBody
-import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.renderer.DescriptorRenderer
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
 import org.jetbrains.kotlin.resolve.scopes.LexicalScope
 import org.jetbrains.kotlin.resolve.scopes.utils.memberScopeAsImportingScope
-import java.util.*

 abstract class CallableRefactoring<out T : CallableDescriptor>(
    val project: Project,
-    callableDescriptor: T,
+    val callableDescriptor: T,
    val commandName: String
 ) {
    private val LOG = Logger.getInstance(CallableRefactoring::class.java)

-    @Suppress("UNCHECKED_CAST")
-    val callableDescriptor = callableDescriptor.liftToExpected() as? T ?: callableDescriptor
-
    private val kind = (callableDescriptor as? CallableMemberDescriptor)?.kind ?: DECLARATION

    protected open fun forcePerformForSelectedFunctionOnly(): Boolean {
@@ -184,15 +177,34 @@ abstract class CallableRefactoring<out T : CallableDescriptor>(
    }
 }

-fun getAffectedCallables(project: Project, descriptorsForChange: Collection<CallableDescriptor>): List<PsiElement> {
-    val baseCallables = descriptorsForChange.mapNotNull { DescriptorToSourceUtilsIde.getAnyDeclaration(project, it) }
-    return baseCallables + baseCallables.flatMapTo(HashSet<PsiElement>()) { callable ->
-        if (callable is KtDeclaration && callable.isExpectDeclaration()) {
-            callable.actualsForExpected()
-        } else {
-            callable.toLightMethods().flatMap { psiMethod ->
-                val overrides = OverridingMethodsSearch.search(psiMethod).findAll()
-                overrides.map { method -> method.namedUnwrappedElement ?: method }
+fun getAffectedCallables(project: Project, descriptorsForChange: Collection<CallableDescriptor>): Collection<PsiElement> {
+    val results = hashSetOf<PsiElement>()
+    for (descriptor in descriptorsForChange) {
+        val declaration = DescriptorToSourceUtilsIde.getAnyDeclaration(project, descriptor) ?: continue
+        collectAffectedCallables(declaration, results)
+    }
+
+    return results
+}
+
+private fun collectAffectedCallables(declaration: PsiElement, results: MutableCollection<PsiElement>) {
+    if (!results.add(declaration)) return
+    if (declaration is KtDeclaration) {
+        for (it in declaration.actualsForExpected()) {
+            collectAffectedCallables(it, results)
+        }
+
+        declaration.liftToExpected()?.let { collectAffectedCallables(it, results) }
+
+        if (declaration !is KtCallableDeclaration) return
+        declaration.forEachOverridingElement { _, overridingElement ->
+            results += overridingElement.namedUnwrappedElement ?: overridingElement
+            true
+        }
+    } else {
+        for (psiMethod in declaration.toLightMethods()) {
+            OverridingMethodsSearch.search(psiMethod).forEach {
+                results += it.namedUnwrappedElement ?: it
            }
        }
    }
@@ -28,17 +28,13 @@ import org.jetbrains.kotlin.idea.caches.resolve.unsafeResolveToDescriptor
 import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.usages.KotlinCallableDefinitionUsage
 import org.jetbrains.kotlin.idea.search.declarationsSearch.forEachOverridingElement
-import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
-import org.jetbrains.kotlin.idea.util.actualsForExpected
-import org.jetbrains.kotlin.idea.util.isExpectDeclaration
+import org.jetbrains.kotlin.idea.util.*
 import org.jetbrains.kotlin.psi.KtCallableDeclaration
 import org.jetbrains.kotlin.psi.KtClass
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtFunction
 import org.jetbrains.kotlin.psi.psiUtil.quoteIfNeeded
 import org.jetbrains.kotlin.resolve.DescriptorUtils
-import org.jetbrains.kotlin.utils.SmartList
-import java.util.*

 class KotlinChangeSignatureData(
    override val baseDescriptor: CallableDescriptor,
@@ -101,45 +97,76 @@ class KotlinChangeSignatureData(
    }

    override val affectedCallables: Collection<UsageInfo> by lazy {
-        primaryCallables + primaryCallables.flatMapTo(HashSet<UsageInfo>()) { primaryFunction ->
-            val primaryDeclaration = primaryFunction.declaration as? KtDeclaration ?: return@flatMapTo emptyList()
+        val results = hashSetOf<UsageInfo>()

-            if (primaryDeclaration.isExpectDeclaration()) {
-                return@flatMapTo primaryDeclaration.actualsForExpected().mapNotNull {
-                    val callableDescriptor = when (val descriptor = it.unsafeResolveToDescriptor()) {
-                        is CallableDescriptor -> descriptor
-                        is ClassDescriptor -> descriptor.unsubstitutedPrimaryConstructor ?: return@mapNotNull null
-                        else -> return@mapNotNull null
-                    }
-                    KotlinCallableDefinitionUsage<PsiElement>(it, callableDescriptor, primaryFunction, null)
-                }
-            }
-
-            if (primaryDeclaration !is KtCallableDeclaration) return@flatMapTo emptyList()
-
-            val results = SmartList<UsageInfo>()
-
-            primaryDeclaration.forEachOverridingElement { baseElement, overridingElement ->
-                val currentDeclaration = overridingElement.namedUnwrappedElement
-                results += when (currentDeclaration) {
-                    is KtDeclaration -> {
-                        val overridingDescriptor = currentDeclaration.unsafeResolveToDescriptor() as CallableDescriptor
-                        KotlinCallableDefinitionUsage(currentDeclaration, overridingDescriptor, primaryFunction, null)
-                    }
-
-                    is PsiMethod -> {
-                        val baseMethod = baseElement as? PsiMethod ?: return@forEachOverridingElement true
-                        OverriderUsageInfo(currentDeclaration, baseMethod, true, true, true)
-                    }
-
-                    else -> return@forEachOverridingElement true
-                }
-
-                true
-            }
-
-            results
+        results += primaryCallables
+        for (primaryCallable in primaryCallables) {
+            val primaryDeclaration = primaryCallable.declaration as? KtDeclaration ?: continue
+            collectMembers(primaryDeclaration, primaryCallable, results)
        }
+
+        results
+    }
+
+    private fun collectMembers(
+        declaration: KtDeclaration,
+        primaryFunction: KotlinCallableDefinitionUsage<PsiElement>,
+        results: MutableCollection<UsageInfo>
+    ) {
+        if (declaration.isEffectivelyActual()) {
+            declaration.liftToExpected()?.let { collectExpectActualMembers(it, primaryFunction, results) }
+        }
+
+        if (declaration.isExpectDeclaration()) for (it in declaration.actualsForExpected()) collectExpectActualMembers(it, primaryFunction, results)
+
+        if (declaration !is KtCallableDeclaration) return
+
+        declaration.forEachOverridingElement { baseElement, overridingElement ->
+            val currentDeclaration = overridingElement.namedUnwrappedElement
+            results += when (currentDeclaration) {
+                is KtDeclaration -> {
+                    val overridingDescriptor = currentDeclaration.unsafeResolveToDescriptor() as CallableDescriptor
+                    KotlinCallableDefinitionUsage(
+                        currentDeclaration,
+                        overridingDescriptor,
+                        primaryFunction,
+                        null,
+                        canDropOverride = false
+                    )
+                }
+
+                is PsiMethod -> {
+                    val baseMethod = baseElement as? PsiMethod ?: return@forEachOverridingElement true
+                    OverriderUsageInfo(currentDeclaration, baseMethod, true, true, true)
+                }
+
+                else -> return@forEachOverridingElement true
+            }
+
+            true
+        }
+    }
+
+    private fun collectExpectActualMembers(
+        it: KtDeclaration,
+        primaryFunction: KotlinCallableDefinitionUsage<PsiElement>,
+        results: MutableCollection<UsageInfo>
+    ) {
+        val callableDescriptor = when (val descriptor = it.unsafeResolveToDescriptor()) {
+            is CallableDescriptor -> descriptor
+            is ClassDescriptor -> descriptor.unsubstitutedPrimaryConstructor ?: return
+            else -> return
+        }
+
+        val usage = KotlinCallableDefinitionUsage<PsiElement>(
+            it,
+            callableDescriptor,
+            primaryFunction,
+            null,
+            canDropOverride = false
+        )
+
+        if (results.add(usage)) collectMembers(it, primaryFunction, results)
    }

    override fun getParameters(): List<KotlinParameterInfo> = parameters
@@ -994,7 +994,7 @@ fun DialogWrapper.showWithTransaction() {
    TransactionGuard.submitTransaction(disposable, Runnable { show() })
 }

-fun PsiMethod.checkDeclarationConflict(name: String, conflicts: MultiMap<PsiElement, String>, callables: List<PsiElement>) {
+fun PsiMethod.checkDeclarationConflict(name: String, conflicts: MultiMap<PsiElement, String>, callables: Collection<PsiElement>) {
    containingClass
        ?.findMethodsByName(name, true)
        // as is necessary here: see KT-10386
@@ -0,0 +1 @@
+expect fun f1(i: Int, s: String)
@@ -0,0 +1 @@
+expect fun String.f1(i: Int)
@@ -0,0 +1,2 @@
+//
+actual fun f1(i: Int, s: String) {}
@@ -0,0 +1,2 @@
+//
+actual fun String.f1(i: Int) {}
@@ -0,0 +1,2 @@
+// "Convert parameter to receiver" "true"
+actual fun f1(i: Int, <caret>s: String) {}
@@ -0,0 +1,2 @@
+// "Convert parameter to receiver" "true"
+actual fun String.f1(i: Int<caret>) {}
@@ -0,0 +1,2 @@
+// "Convert parameter to receiver" "true"
+expect fun f1(i: Int, <caret>s: String)
@@ -0,0 +1,2 @@
+// "Convert parameter to receiver" "true"
+expect fun String.f1(i: Int<caret>)
@@ -0,0 +1,2 @@
+//
+actual fun f1(i: Int, s: String) {}
@@ -0,0 +1,2 @@
+//
+actual fun String.f1(i: Int) {}
@@ -0,0 +1 @@
+actual fun f1(i: Int, s: String) {}
@@ -0,0 +1 @@
+actual fun String.f1(i: Int) {}
@@ -0,0 +1,3 @@
+expect class A {
+    fun c(a: Int, b: String)
+}
@@ -0,0 +1,3 @@
+expect class A {
+    fun String.c(a: Int)
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, <caret>b: String) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int<caret>) {}
+}
@@ -0,0 +1,3 @@
+expect class A {
+    fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,3 @@
+expect class A {
+    fun String.c(a: Int) {}
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+actual open class A {
+    actual open fun c(a: Int, <caret>b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+actual open class A {
+    actual open fun String.c(a: Int<caret>) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,4 @@
+// "Convert parameter to receiver" "true"
+expect class A {
+    fun c(a: Int, <caret>b: String)
+}
@@ -0,0 +1,4 @@
+// "Convert parameter to receiver" "true"
+expect class A {
+    fun String.c(a: Int<caret>)
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+expect open class A() {
+    open fun c(a: Int, <caret>b: String)
+}
+
+class C : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+expect open class A() {
+    open fun String.c(a: Int<caret>)
+}
+
+class C : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+expect open class A() {
+    open fun c(a: Int, b: String)
+}
+
+class C : A() {
+    override fun c(a: Int, <caret>b: String) {}
+}
@@ -0,0 +1,8 @@
+// "Convert parameter to receiver" "true"
+expect open class A() {
+    open fun String.c(a: Int)
+}
+
+class C : A() {
+    override fun String.c(a: Int<caret>) {}
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,8 @@
+//
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+class B : A() {
+    override fun c(a: Int, b: String) {}
+}
@@ -0,0 +1,7 @@
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+class B : A() {
+    override fun String.c(a: Int) {}
+}
@@ -0,0 +1,11 @@
+expect open class A() {
+    open fun c(a: Int, b: String)
+}
+
+class C : A() {
+    override fun c(a: Int, b: String) {}
+}
+
+public inline fun <T, R> with(receiver: T, block: T.() -> R): R {
+    return receiver.block()
+}
@@ -0,0 +1,11 @@
+expect open class A() {
+    open fun String.c(a: Int)
+}
+
+class C : A() {
+    override fun String.c(a: Int) {}
+}
+
+public inline fun <T, R> with(receiver: T, block: T.() -> R): R {
+    return receiver.block()
+}
@@ -0,0 +1,18 @@
+//
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+open class B : A() {
+    override fun c(a: Int, b: String) {}
+}
+
+open class D : B() {
+    override fun c(a: Int, b: String) {}
+}
+
+fun test(a: Int, b: String) {
+    with(A()) {
+        c(a, b)
+    }
+}
@@ -0,0 +1,18 @@
+//
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+open class B : A() {
+    override fun String.c(a: Int) {}
+}
+
+open class D : B() {
+    override fun String.c(a: Int) {}
+}
+
+fun test(a: Int, b: String) {
+    with(A()) {
+        b.c(a)
+    }
+}
@@ -0,0 +1,18 @@
+// "Convert parameter to receiver" "true"
+actual open class A {
+    actual open fun c(a: Int, b: String) {}
+}
+
+open class B : A() {
+    override fun c(a: Int, <caret>b: String) {}
+}
+
+open class D : B() {
+    override fun c(a: Int, b: String) {}
+}
+
+fun test(a: Int, b: String) {
+    with(A()) {
+        c(a, b)
+    }
+}
@@ -0,0 +1,18 @@
+// "Convert parameter to receiver" "true"
+actual open class A {
+    actual open fun String.c(a: Int) {}
+}
+
+open class B : A() {
+    override fun String.c(a: Int<caret>) {}
+}
+
+open class D : B() {
+    override fun String.c(a: Int) {}
+}
+
+fun test(a: Int, b: String) {
+    with(A()) {
+        b.c(a)
+    }
+}
@@ -122,6 +122,59 @@ public class QuickFixMultiModuleTestGenerated extends AbstractQuickFixMultiModul
        }
    }

+    @TestMetadata("idea/testData/multiModuleQuickFix/changeSignature")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class ChangeSignature extends AbstractQuickFixMultiModuleTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        @TestMetadata("actual")
+        public void testActual() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/actual/");
+        }
+
+        public void testAllFilesPresentInChangeSignature() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/multiModuleQuickFix/changeSignature"), Pattern.compile("^([^\\.]+)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("expect")
+        public void testExpect() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/expect/");
+        }
+
+        @TestMetadata("override")
+        public void testOverride() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/override/");
+        }
+
+        @TestMetadata("override2")
+        public void testOverride2() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/override2/");
+        }
+
+        @TestMetadata("override3")
+        public void testOverride3() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/override3/");
+        }
+
+        @TestMetadata("override4")
+        public void testOverride4() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/override4/");
+        }
+
+        @TestMetadata("override5")
+        public void testOverride5() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/override5/");
+        }
+
+        @TestMetadata("override6")
+        public void testOverride6() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/changeSignature/override6/");
+        }
+    }
+
    @TestMetadata("idea/testData/multiModuleQuickFix/createActual")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)