Rename: Implement Rename conflict analysis for classes. Qualify class references to resove rename conflicts when possible

#KT-8611 Fixed #KT-8562 Fixed (cherry picked from commit 8989ccc)
2016-06-14 15:16:59 +03:00
parent 77b0bb9849
commit c9f659e89b
32 changed files with 301 additions and 7 deletions
@@ -226,6 +226,8 @@
 - [`KT-6363`](https://youtrack.jetbrains.com/issue/KT-6363) Do not rename ambiguous references in import directives
 - [`KT-8541`](https://youtrack.jetbrains.com/issue/KT-8541), [`KT-8786`](https://youtrack.jetbrains.com/issue/KT-8786) Do now show 'Rename overloads' options if target function has no overloads
 - [`KT-8544`](https://youtrack.jetbrains.com/issue/KT-8544) Show more detailed description in Rename dialog
+- [`KT-8562`](https://youtrack.jetbrains.com/issue/KT-8562) Show conflicts dialog on attempt of redeclaration
+- [`KT-8611`](https://youtrack.jetbrains.com/issue/KT-8732) Qualify class references to resove rename conflicts when possible
 - [`KT-8732`](https://youtrack.jetbrains.com/issue/KT-8732) Implement Rename conflict analysis and fixes for properties/parameters
 - [`KT-8860`](https://youtrack.jetbrains.com/issue/KT-8860) Allow renaming class by constructor delegation call referencing primary constructor
 - [`KT-8892`](https://youtrack.jetbrains.com/issue/KT-8892) Suggest renaming base declarations on overriding members in object literals
@@ -100,7 +100,7 @@ class CreateKotlinSubClassIntention : SelfTargetingRangeIntention<KtClass>(KtCla
        val classFromText = KtPsiFactory(project).createClass(builder.asString())
        val body = sealedClass.getOrCreateBody()
        val klass = body.addBefore(classFromText, body.rBrace) as KtClass
-        PsiElementRenameHandler.rename(klass, project, sealedClass, editor)
+        runInteractiveRename(klass, project, sealedClass, editor)
        chooseAndImplementMethods(project, klass, editor)
    }

@@ -142,11 +142,16 @@ class CreateKotlinSubClassIntention : SelfTargetingRangeIntention<KtClass>(KtCla
                                           baseClass, name, visibility)
            val classFromText = factory.createClass(builder.asString())
            val klass = container.parent.addAfter(classFromText, container) as KtClass
-            PsiElementRenameHandler.rename(klass, project, container, editor)
+            runInteractiveRename(klass, project, container, editor)
            chooseAndImplementMethods(project, klass, editor)
        }
    }

+    private fun runInteractiveRename(klass: KtClass, project: Project, container: KtClassOrObject, editor: Editor) {
+        if (ApplicationManager.getApplication().isUnitTestMode) return
+        PsiElementRenameHandler.rename(klass, project, container, editor)
+    }
+
    private fun chooseSubclassToCreate(baseClass: KtClass, baseName: String): CreateClassDialog? {
        val sourceDir = baseClass.containingFile.containingDirectory

@@ -19,16 +19,23 @@ package org.jetbrains.kotlin.idea.refactoring.rename
 import com.intellij.openapi.editor.Editor
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiReference
+import com.intellij.refactoring.listeners.RefactoringElementListener
+import com.intellij.usageView.UsageInfo
 import org.jetbrains.kotlin.asJava.KtLightClass
 import org.jetbrains.kotlin.asJava.KtLightClassForExplicitDeclaration
 import org.jetbrains.kotlin.asJava.KtLightClassForFacade
+import org.jetbrains.kotlin.asJava.namedUnwrappedElement
+import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
 import org.jetbrains.kotlin.psi.KtClassOrObject
 import org.jetbrains.kotlin.psi.KtConstructor
+import org.jetbrains.kotlin.psi.KtNamedDeclaration
 import org.jetbrains.kotlin.psi.KtObjectDeclaration
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
+import org.jetbrains.kotlin.utils.SmartList

 class RenameKotlinClassProcessor : RenameKotlinPsiProcessor() {
    override fun canProcessElement(element: PsiElement): Boolean {
@@ -68,6 +75,23 @@ class RenameKotlinClassProcessor : RenameKotlinPsiProcessor() {
        return bindingContext[BindingContext.SHORT_REFERENCE_TO_COMPANION_OBJECT, element] != null
    }

+    override fun findCollisions(
+            element: PsiElement,
+            newName: String?,
+            allRenames: MutableMap<out PsiElement, String>,
+            result: MutableList<UsageInfo>
+    ) {
+        if (newName == null) return
+        val declaration = element.namedUnwrappedElement as? KtNamedDeclaration ?: return
+        val descriptor = declaration.resolveToDescriptor() as ClassDescriptor
+
+        val collisions = SmartList<UsageInfo>()
+        checkRedeclarations(descriptor, newName, collisions)
+        checkOriginalUsagesRetargeting(declaration, newName, result, collisions)
+        checkNewNameUsagesRetargeting(declaration, newName, collisions)
+        result += collisions
+    }
+
    private fun getClassOrObject(element: PsiElement?): PsiElement? = when (element) {
        is KtLightClass ->
            when (element) {
@@ -82,4 +106,10 @@ class RenameKotlinClassProcessor : RenameKotlinPsiProcessor() {
        else ->
            element as? KtClassOrObject
    }
+
+    override fun renameElement(element: PsiElement, newName: String?, usages: Array<out UsageInfo>, listener: RefactoringElementListener?) {
+        super.renameElement(element, newName, usages, listener)
+
+        usages.forEach { (it as? KtResolvableCollisionUsageInfo)?.apply() }
+    }
 }
@@ -311,7 +311,7 @@ class RenameKotlinPropertyProcessor : RenameKotlinPsiProcessor() {
                            refKindUsages[UsageKind.SIMPLE_PROPERTY_USAGE]?.toTypedArray() ?: arrayOf<UsageInfo>(),
                            null)

-        usages.forEach { (it as? UsageInfoWithReplacement)?.apply() }
+        usages.forEach { (it as? KtResolvableCollisionUsageInfo)?.apply() }

        dropOverrideKeywordIfNecessary(element)

@@ -37,6 +37,7 @@ import org.jetbrains.kotlin.idea.search.restrictToKotlinSources
 import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
 import org.jetbrains.kotlin.idea.util.getAllAccessibleVariables
 import org.jetbrains.kotlin.idea.util.getResolutionScope
+import org.jetbrains.kotlin.incremental.components.NoLookupLocation
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
@@ -47,13 +48,17 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
 import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
 import org.jetbrains.kotlin.resolve.calls.resolvedCallUtil.getExplicitReceiverValue
 import org.jetbrains.kotlin.resolve.calls.resolvedCallUtil.getImplicitReceiverValue
+import org.jetbrains.kotlin.resolve.descriptorUtil.getImportableDescriptor
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter
 import org.jetbrains.kotlin.resolve.scopes.LexicalScope
 import org.jetbrains.kotlin.resolve.scopes.getDescriptorsFiltered
+import org.jetbrains.kotlin.resolve.scopes.utils.findClassifier
 import org.jetbrains.kotlin.resolve.scopes.utils.getImplicitReceiversHierarchy
 import org.jetbrains.kotlin.resolve.source.getPsi
+import org.jetbrains.kotlin.types.ErrorUtils
 import org.jetbrains.kotlin.utils.SmartList
+import org.jetbrains.kotlin.utils.addToStdlib.singletonOrEmptyList

 internal fun ResolvedCall<*>.noReceivers() = dispatchReceiver == null && extensionReceiver == null

@@ -81,7 +86,12 @@ internal fun checkRedeclarations(
        is PackageFragmentDescriptor -> containingDescriptor.getMemberScope()
        else -> return
    }
-    containingScope.getDescriptorsFiltered(DescriptorKindFilter.VARIABLES) { it.asString() == newName }.firstOrNull()?.let { candidateDescriptor ->
+    val descriptorKindFilter = when (descriptor) {
+        is ClassDescriptor -> DescriptorKindFilter.CLASSIFIERS
+        is PropertyDescriptor -> DescriptorKindFilter.VARIABLES
+        else -> return
+    }
+    containingScope.getDescriptorsFiltered(descriptorKindFilter) { it.asString() == newName }.firstOrNull()?.let { candidateDescriptor ->
        val candidate = (candidateDescriptor as? DeclarationDescriptorWithSource)?.source?.getPsi() as? KtNamedDeclaration ?: return
        val what = candidate.renderDescription().capitalize()
        val where = candidate.representativeContainer()?.renderDescription() ?: return
@@ -97,6 +107,7 @@ private fun LexicalScope.getRelevantDescriptors(
    val nameAsName = Name.identifier(name)
    return when (declaration) {
        is KtProperty, is KtParameter, is PsiField -> getAllAccessibleVariables(nameAsName)
+        is KtClassOrObject, is PsiClass -> findClassifier(nameAsName, NoLookupLocation.FROM_IDE).singletonOrEmptyList()
        else -> emptyList()
    }
 }
@@ -141,7 +152,28 @@ private fun checkUsagesRetargeting(

        val psiFactory = KtPsiFactory(declaration)

-        val resolvedCall = refElement.getResolvedCall(context) ?: continue
+        val resolvedCall = refElement.getResolvedCall(context)
+        if (resolvedCall == null) {
+            val typeReference = refElement.getStrictParentOfType<KtTypeReference>() ?: continue
+            val referencedClass = context[BindingContext.TYPE, typeReference]?.constructor?.declarationDescriptor ?: continue
+            val referencedClassFqName = FqName(IdeDescriptorRenderers.SOURCE_CODE.renderClassifierName(referencedClass))
+            val newFqName = if (isNewName) referencedClassFqName.parent().child(Name.identifier(name)) else referencedClassFqName
+            val fakeVar = psiFactory.createDeclaration<KtProperty>("val __foo__: ${newFqName.asString()}")
+            val newContext = fakeVar.analyzeInContext(scope, refElement)
+            val referencedClassInNewContext = newContext[BindingContext.TYPE, fakeVar.typeReference!!]?.constructor?.declarationDescriptor
+            val candidateText = referencedClassInNewContext?.canonicalRender()
+            if (referencedClassInNewContext == null
+                || ErrorUtils.isError(referencedClassInNewContext)
+                || referencedClass.canonicalRender() == candidateText
+                || accessibleDescriptors.any { it.canonicalRender() == candidateText }) {
+                usageIterator.set(UsageInfoWithFqNameReplacement(refElement, declaration, newFqName))
+            }
+            else {
+                reportShadowing(declaration, elementToBindUsageInfosTo, referencedClassInNewContext, refElement, newUsages)
+            }
+            continue
+        }
+
        val callExpression = resolvedCall.call.callElement as? KtExpression ?: continue
        val fullCallExpression = callExpression.getQualifiedExpressionForSelectorOrThis()

@@ -171,7 +203,8 @@ private fun checkUsagesRetargeting(
                    it.value.type.constructor.declarationDescriptor?.getThisLabelName() == expectedLabelName
                }

-                val canQualifyThis = receiversWithExpectedName.size <= 1
+                val canQualifyThis = receiversWithExpectedName.isEmpty()
+                                     || receiversWithExpectedName.size == 1 && (declaration !is KtClassOrObject || expectedLabelName != name)
                if (canQualifyThis) {
                    psiFactory.createExpressionByPattern("${implicitReceiver.explicateAsText()}.$0", callExpression)
                }
@@ -198,7 +231,7 @@ private fun checkUsagesRetargeting(
        val newContext = qualifiedExpression.analyzeInContext(scope, refElement)

        val newResolvedCall = newCallee.getResolvedCall(newContext)
-        val candidateText = newResolvedCall?.candidateDescriptor?.canonicalRender()
+        val candidateText = newResolvedCall?.candidateDescriptor?.getImportableDescriptor()?.canonicalRender()

        if (newResolvedCall != null
            && !accessibleDescriptors.any { it.canonicalRender() == candidateText }
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    class A {
+        class C {
+            val a: A = A()
+        }
+    }
+}
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    class /*rename*/B {
+        class C {
+            val b: B = B()
+        }
+    }
+}
@@ -0,0 +1,6 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/test.kt",
+  "newName": "A",
+  "withRuntime": "true"
+}
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    inner class C {
+        inner class C {
+            val c: A.C = this@A.C()
+        }
+    }
+}
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    inner class /*rename*/B {
+        inner class C {
+            val b: B = B()
+        }
+    }
+}
@@ -0,0 +1,6 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/test.kt",
+  "newName": "C",
+  "withRuntime": "true"
+}
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    class C {
+        class C {
+            val c: A.C = A.C()
+        }
+    }
+}
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    class /*rename*/B {
+        class C {
+            val b: B = B()
+        }
+    }
+}
@@ -0,0 +1,6 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/test.kt",
+  "newName": "C",
+  "withRuntime": "true"
+}
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    inner class A {
+        inner class C {
+            val a: A = A()
+        }
+    }
+}
@@ -0,0 +1,9 @@
+package test
+
+class A {
+    inner class /*rename*/B {
+        inner class C {
+            val b: B = B()
+        }
+    }
+}
@@ -0,0 +1,6 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/test.kt",
+  "newName": "A",
+  "withRuntime": "true"
+}
@@ -0,0 +1,6 @@
+package test
+
+class A {
+    class Foo
+    class /*rename*/Bar
+}
@@ -0,0 +1,7 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/test.kt",
+  "newName": "Foo",
+  "withRuntime": "true",
+  "hint": "Class 'Foo' is already declared in class 'A'"
+}
@@ -0,0 +1,3 @@
+package lib
+
+class LibType
@@ -0,0 +1,8 @@
+package ref
+
+import lib.LibType
+
+class String {}
+
+class Referrer1 { fun method(p1a: String, p1b: kotlin.String) {} }
+class Referrer2 { fun method(p2a: String, p2b: LibType) {} }
@@ -0,0 +1,3 @@
+package lib
+
+class LibType
@@ -0,0 +1,8 @@
+package ref
+
+import lib.LibType
+
+class /*rename*/CustomType {}
+
+class Referrer1 { fun method(p1a: CustomType, p1b: String) {} }
+class Referrer2 { fun method(p2a: CustomType, p2b: LibType) {} }
@@ -0,0 +1,6 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/ref.kt",
+  "newName": "String",
+  "withRuntime": "true"
+}
@@ -0,0 +1,3 @@
+package lib
+
+class LibType
@@ -0,0 +1,8 @@
+package ref
+
+import lib.LibType
+
+class LibType {}
+
+class Referrer1 { fun method(p1a: ref.LibType, p1b: String) {} }
+class Referrer2 { fun method(p2a: ref.LibType, p2b: LibType) {} }
@@ -0,0 +1,3 @@
+package lib
+
+class LibType
@@ -0,0 +1,8 @@
+package ref
+
+import lib.LibType
+
+class /*rename*/CustomType {}
+
+class Referrer1 { fun method(p1a: CustomType, p1b: String) {} }
+class Referrer2 { fun method(p2a: CustomType, p2b: LibType) {} }
@@ -0,0 +1,6 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/ref.kt",
+  "newName": "LibType",
+  "withRuntime": "true"
+}
@@ -0,0 +1,4 @@
+package test
+
+class Foo
+class /*rename*/Bar
@@ -0,0 +1,7 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/test.kt",
+  "newName": "Foo",
+  "withRuntime": "true",
+  "hint": "Class 'Foo' is already declared in package 'test'"
+}
@@ -113,6 +113,30 @@ public class RenameTestGenerated extends AbstractRenameTest {
        doTest(fileName);
    }

+    @TestMetadata("clashOfNestedWithOuterClass/clashOfNestedWithOuterClass.test")
+    public void testClashOfNestedWithOuterClass_ClashOfNestedWithOuterClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/clashOfNestedWithOuterClass/clashOfNestedWithOuterClass.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("clashWithInnerClass/clashWithInnerClass.test")
+    public void testClashWithInnerClass_ClashWithInnerClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/clashWithInnerClass/clashWithInnerClass.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("clashWithNestedClass/clashWithNestedClass.test")
+    public void testClashWithNestedClass_ClashWithNestedClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/clashWithNestedClass/clashWithNestedClass.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("clashWithOuterClass/clashWithOuterClass.test")
+    public void testClashWithOuterClass_ClashWithOuterClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/clashWithOuterClass/clashWithOuterClass.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("companionObject/companionObject.test")
    public void testCompanionObject_CompanionObject() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/companionObject/companionObject.test");
@@ -155,6 +179,12 @@ public class RenameTestGenerated extends AbstractRenameTest {
        doTest(fileName);
    }

+    @TestMetadata("nestedClassRedeclaration/nestedClassRedeclaration.test")
+    public void testNestedClassRedeclaration_NestedClassRedeclaration() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/nestedClassRedeclaration/nestedClassRedeclaration.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("propertyAccidentalOverrideSubclass/propertyAccidentalOverrideSubclass.test")
    public void testPropertyAccidentalOverrideSubclass_PropertyAccidentalOverrideSubclass() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/propertyAccidentalOverrideSubclass/propertyAccidentalOverrideSubclass.test");
@@ -239,6 +269,18 @@ public class RenameTestGenerated extends AbstractRenameTest {
        doTest(fileName);
    }

+    @TestMetadata("renamedClassShadowingImplicitlyImportedClassUsage/renamedClassShadowingImplicitlyImportedClassUsage.test")
+    public void testRenamedClassShadowingImplicitlyImportedClassUsage_RenamedClassShadowingImplicitlyImportedClassUsage() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/renamedClassShadowingImplicitlyImportedClassUsage/renamedClassShadowingImplicitlyImportedClassUsage.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("renamedClassShadowingImportedClassUsage/renamedClassShadowingImportedClassUsage.test")
+    public void testRenamedClassShadowingImportedClassUsage_RenamedClassShadowingImportedClassUsage() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/renamedClassShadowingImportedClassUsage/renamedClassShadowingImportedClassUsage.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("renameEmptyKotlinFile/renameFile.test")
    public void testRenameEmptyKotlinFile_RenameFile() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/renameEmptyKotlinFile/renameFile.test");
@@ -845,6 +887,12 @@ public class RenameTestGenerated extends AbstractRenameTest {
        doTest(fileName);
    }

+    @TestMetadata("topLevelClassRedeclaration/topLevelClassRedeclaration.test")
+    public void testTopLevelClassRedeclaration_TopLevelClassRedeclaration() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/topLevelClassRedeclaration/topLevelClassRedeclaration.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("topLevelPropertyRedeclaration/topLevelPropertyRedeclaration.test")
    public void testTopLevelPropertyRedeclaration_TopLevelPropertyRedeclaration() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/topLevelPropertyRedeclaration/topLevelPropertyRedeclaration.test");