Move: Fix processing of conflicting usages

Exclude conflict-associated usages from refactoring. Move search of file internal usages to MoveKotlinDeclarationsProcessor
2017-03-04 21:28:10 +03:00
parent a21f19d613
commit 13de9aea81
13 changed files with 88 additions and 58 deletions
@@ -67,6 +67,8 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassNotAny
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.resolve.scopes.utils.findClassifier
 import java.util.*
+import kotlin.collections.LinkedHashMap
+import kotlin.collections.LinkedHashSet

 data class ExtractSuperInfo(
        val originalClass: KtClassOrObject,
@@ -155,7 +157,7 @@ class ExtractSuperRefactoring(

            project.runSynchronouslyWithProgress(RefactoringBundle.message("detecting.possible.conflicts"), true) {
                runReadAction {
-                    val usages = ArrayList<UsageInfo>()
+                    val usages = LinkedHashSet<UsageInfo>()
                    for (element in elementsToMove) {
                        ReferencesSearch.search(element).mapTo(usages) { MoveRenameUsageInfo(it, element) }
                        if (element is KtCallableDeclaration) {
@@ -164,7 +166,7 @@ class ExtractSuperRefactoring(
                            }
                        }
                    }
-                    conflictChecker.checkAllConflicts(usages, conflicts)
+                    conflictChecker.checkAllConflicts(usages, LinkedHashSet<UsageInfo>(), conflicts)
                    if (targetParent is PsiDirectory) {
                        ExtractSuperClassUtil.checkSuperAccessible(targetParent, conflicts, originalClass.toLightClass())
                    }
@@ -57,7 +57,7 @@ class KotlinChangePackageRefactoring(val file: KtFile) {
                            override fun verify(file: PsiFile) = null
                        },
                        delegate = MoveDeclarationsDelegate.TopLevel,
-                        updateInternalReferences = false
+                        scanEntireFile = false
                ),
                Mover.Idle // we don't need to move any declarations physically
        )
@@ -99,7 +99,6 @@ class MoveDeclarationToSeparateFileIntention :
                delegate = MoveDeclarationsDelegate.TopLevel,
                searchInCommentsAndStrings = false,
                searchInNonCode = false,
-                updateInternalReferences = true,
                moveCallback = MoveCallback {
                    val newFile = directory.findFile(targetFileName) as KtFile
                    val newDeclaration = newFile.declarations.first()
@@ -34,10 +34,10 @@ import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject

 sealed class MoveDeclarationsDelegate {
    abstract fun getContainerChangeInfo(originalDeclaration: KtNamedDeclaration, moveTarget: KotlinMoveTarget): ContainerChangeInfo
-    abstract fun findUsages(descriptor: MoveDeclarationsDescriptor): List<UsageInfo>
+    abstract fun findInternalUsages(descriptor: MoveDeclarationsDescriptor): List<UsageInfo>
    abstract fun collectConflicts(
            descriptor: MoveDeclarationsDescriptor,
-            usages: MutableList<UsageInfo>,
+            internalUsages: MutableSet<UsageInfo>,
            conflicts: MultiMap<PsiElement, String>
    )
    abstract fun preprocessDeclaration(descriptor: MoveDeclarationsDescriptor, originalDeclaration: KtNamedDeclaration)
@@ -49,11 +49,11 @@ sealed class MoveDeclarationsDelegate {
                                       ContainerInfo.Package(moveTarget.targetContainerFqName!!))
        }

-        override fun findUsages(descriptor: MoveDeclarationsDescriptor): List<UsageInfo> = emptyList()
+        override fun findInternalUsages(descriptor: MoveDeclarationsDescriptor): List<UsageInfo> = emptyList()

        override fun collectConflicts(
                descriptor: MoveDeclarationsDescriptor,
-                usages: MutableList<UsageInfo>,
+                internalUsages: MutableSet<UsageInfo>,
                conflicts: MultiMap<PsiElement, String>
        ) {

@@ -83,17 +83,17 @@ sealed class MoveDeclarationsDelegate {
            return ContainerChangeInfo(originalInfo, newInfo)
        }

-        override fun findUsages(descriptor: MoveDeclarationsDescriptor): List<UsageInfo> {
+        override fun findInternalUsages(descriptor: MoveDeclarationsDescriptor): List<UsageInfo> {
            val classToMove = descriptor.elementsToMove.singleOrNull() as? KtClass ?: return emptyList()
            return collectOuterInstanceReferences(classToMove)
        }

        override fun collectConflicts(
                descriptor: MoveDeclarationsDescriptor,
-                usages: MutableList<UsageInfo>,
+                internalUsages: MutableSet<UsageInfo>,
                conflicts: MultiMap<PsiElement, String>
        ) {
-            val usageIterator = usages.iterator()
+            val usageIterator = internalUsages.iterator()
            while (usageIterator.hasNext()) {
                val usage = usageIterator.next()
                val element = usage.element ?: continue
@@ -37,21 +37,16 @@ import com.intellij.usageView.UsageViewBundle
 import com.intellij.usageView.UsageViewDescriptor
 import com.intellij.usageView.UsageViewUtil
 import com.intellij.util.IncorrectOperationException
-import com.intellij.util.SmartList
 import com.intellij.util.containers.MultiMap
 import gnu.trove.THashMap
 import gnu.trove.TObjectHashingStrategy
 import org.jetbrains.kotlin.asJava.elements.KtLightElement
 import org.jetbrains.kotlin.asJava.toLightElements
-import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithSource
 import org.jetbrains.kotlin.idea.codeInsight.shorten.addToShorteningWaitSet
 import org.jetbrains.kotlin.idea.core.deleteSingle
 import org.jetbrains.kotlin.idea.refactoring.fqName.getKotlinFqName
-import org.jetbrains.kotlin.idea.refactoring.move.UnqualifiableMoveRenameUsageInfo
-import org.jetbrains.kotlin.idea.refactoring.move.createMoveUsageInfoIfPossible
-import org.jetbrains.kotlin.idea.refactoring.move.getInternalReferencesToUpdateOnPackageNameChange
+import org.jetbrains.kotlin.idea.refactoring.move.*
 import org.jetbrains.kotlin.idea.refactoring.move.moveFilesOrDirectories.MoveKotlinClassHandler
-import org.jetbrains.kotlin.idea.refactoring.move.postProcessMoveUsages
 import org.jetbrains.kotlin.idea.references.KtSimpleNameReference.ShorteningMode
 import org.jetbrains.kotlin.idea.search.projectScope
 import org.jetbrains.kotlin.psi.*
@@ -92,7 +87,7 @@ class MoveDeclarationsDescriptor(
        val delegate: MoveDeclarationsDelegate,
        val searchInCommentsAndStrings: Boolean = true,
        val searchInNonCode: Boolean = true,
-        val updateInternalReferences: Boolean = true,
+        val scanEntireFile: Boolean = false,
        val deleteSourceFiles: Boolean = false,
        val moveCallback: MoveCallback? = null,
        val openInEditor: Boolean = false
@@ -125,16 +120,19 @@ class MoveKotlinDeclarationsProcessor(
        return MoveMultipleElementsViewDescriptor(elementsToMove.toTypedArray(), targetContainerFqName)
    }

-    private val usagesToProcessBeforeMove = SmartList<UsageInfo>()
-
    fun getConflictsAsUsages(): List<UsageInfo> = conflicts.entrySet().map { ConflictUsageInfo(it.key, it.value) }

+    class UsagesToProcessBeforeMoveWrapper(
+            sourceFile: KtFile,
+            val usages: Collection<UsageInfo>
+    ): UsageInfo(sourceFile)
+
    public override fun findUsages(): Array<UsageInfo> {
        if (elementsToMove.isEmpty()) return UsageInfo.EMPTY_ARRAY

        val newContainerName = descriptor.moveTarget.targetContainerFqName?.asString() ?: ""

-        fun collectUsages(kotlinToLightElements: Map<KtNamedDeclaration, List<PsiNamedElement>>, result: MutableList<UsageInfo>) {
+        fun collectUsages(kotlinToLightElements: Map<KtNamedDeclaration, List<PsiNamedElement>>, result: MutableCollection<UsageInfo>) {
            kotlinToLightElements.values.flatMap { it }.flatMapTo(result) { lightElement ->
                val newFqName = StringUtil.getQualifiedName(newContainerName, lightElement.name)

@@ -169,24 +167,38 @@ class MoveKotlinDeclarationsProcessor(
            }
        }

+        val usagesToProcessBeforeMove = LinkedHashSet<UsageInfo>()
        val usages = ArrayList<UsageInfo>()
        val conflictChecker = MoveConflictChecker(project, elementsToMove, descriptor.moveTarget, elementsToMove.first())
-        for (kotlinToLightElements in kotlinToLightElementsBySourceFile.values) {
-            kotlinToLightElements.keys.forEach {
-                if (descriptor.updateInternalReferences) {
+        for ((sourceFile, kotlinToLightElements) in kotlinToLightElementsBySourceFile) {
+            val internalUsages = LinkedHashSet<UsageInfo>()
+            val externalUsages = LinkedHashSet<UsageInfo>()
+
+            if (descriptor.scanEntireFile) {
+                val changeInfo = ContainerChangeInfo(
+                        ContainerInfo.Package(sourceFile.packageFqName),
+                        descriptor.moveTarget.targetContainerFqName?.let { ContainerInfo.Package(it) } ?: ContainerInfo.UnknownPackage
+                )
+                internalUsages += sourceFile.getInternalReferencesToUpdateOnPackageNameChange(changeInfo)
+            }
+            else {
+                kotlinToLightElements.keys.forEach {
                    val packageNameInfo = descriptor.delegate.getContainerChangeInfo(it, descriptor.moveTarget)
-                    val (usagesToProcessLater, usagesToProcessEarly) = it
-                            .getInternalReferencesToUpdateOnPackageNameChange(packageNameInfo)
-                            .partition { it is UnqualifiableMoveRenameUsageInfo }
-                    usages.addAll(usagesToProcessLater)
-                    usagesToProcessBeforeMove.addAll(usagesToProcessEarly)
+                    internalUsages += it.getInternalReferencesToUpdateOnPackageNameChange(packageNameInfo)
                }
            }

-            usages += descriptor.delegate.findUsages(descriptor)
-            collectUsages(kotlinToLightElements, usages)
-            conflictChecker.checkAllConflicts(usages, conflicts)
-            descriptor.delegate.collectConflicts(descriptor, usages, conflicts)
+            internalUsages += descriptor.delegate.findInternalUsages(descriptor)
+            collectUsages(kotlinToLightElements, externalUsages)
+            conflictChecker.checkAllConflicts(externalUsages, internalUsages, conflicts)
+            descriptor.delegate.collectConflicts(descriptor, internalUsages, conflicts)
+
+            usages += externalUsages
+
+            val (usagesToProcessLater, usagesToProcessEarly) = internalUsages.partition { it is UnqualifiableMoveRenameUsageInfo }
+            usages += usagesToProcessLater
+            usagesToProcessBeforeMove += usagesToProcessEarly
+            usages += UsagesToProcessBeforeMoveWrapper(sourceFile, usagesToProcessBeforeMove)
        }

        descriptor.delegate.collectConflicts(descriptor, usagesToProcessBeforeMove, conflicts)
@@ -217,7 +229,12 @@ class MoveKotlinDeclarationsProcessor(
        try {
            val usageList = usages.toList()

-            descriptor.delegate.preprocessUsages(project, usageList)
+            val (usagesToProcessBeforeMoveWrappers, usagesToProcessAfterMove) =
+                    usageList.partition { it is UsagesToProcessBeforeMoveWrapper }
+            val usagesToProcessBeforeMove = usagesToProcessBeforeMoveWrappers.flatMap { (it as UsagesToProcessBeforeMoveWrapper).usages }
+
+            descriptor.delegate.preprocessUsages(project, usagesToProcessBeforeMove)
+            descriptor.delegate.preprocessUsages(project, usagesToProcessAfterMove)

            postProcessMoveUsages(usagesToProcessBeforeMove, shorteningMode = ShorteningMode.NO_SHORTENING)

@@ -268,7 +285,7 @@ class MoveKotlinDeclarationsProcessor(
                }
            }

-            nonCodeUsages = postProcessMoveUsages(usageList, oldToNewElementsMapping).toTypedArray()
+            nonCodeUsages = postProcessMoveUsages(usagesToProcessAfterMove, oldToNewElementsMapping).toTypedArray()
        }
        catch (e: IncorrectOperationException) {
            nonCodeUsages = null
@@ -286,8 +303,4 @@ class MoveKotlinDeclarationsProcessor(
    }

    override fun getCommandName(): String = REFACTORING_NAME
-}
-
-interface D : DeclarationDescriptorWithSource {
-
 }
@@ -140,15 +140,28 @@ class MoveConflictChecker(

    private fun render(declaration: PsiElement) = RefactoringUIUtil.getDescription(declaration, false)

-    fun checkModuleConflictsInUsages(usages: List<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
+    fun checkModuleConflictsInUsages(externalUsages: MutableSet<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
+        val newConflicts = MultiMap<PsiElement, String>()
        val sourceRoot = moveTarget.targetFile ?: return
-        RefactoringConflictsUtil.analyzeModuleConflicts(project, elementsToMove, usages.toTypedArray(), sourceRoot, conflicts)
+        RefactoringConflictsUtil.analyzeModuleConflicts(project, elementsToMove, externalUsages.toTypedArray(), sourceRoot, newConflicts)
+        if (!newConflicts.isEmpty) {
+            val referencedElementsToSkip = newConflicts.keySet().mapNotNullTo(HashSet()) { it.namedUnwrappedElement }
+            externalUsages.removeIf {
+                it is MoveRenameUsageInfo &&
+                it.referencedElement?.namedUnwrappedElement?.let { it in referencedElementsToSkip } ?: false
+            }
+            conflicts.putAllValues(newConflicts)
+        }
    }

-    fun checkModuleConflictsInDeclarations(conflicts: MultiMap<PsiElement, String>) {
+    private fun checkModuleConflictsInDeclarations(
+            internalUsages: MutableSet<UsageInfo>,
+            conflicts: MultiMap<PsiElement, String>
+    ) {
        val sourceRoot = moveTarget.targetFile ?: return
        val targetModule = ModuleUtilCore.findModuleForFile(sourceRoot, project) ?: return
        val resolveScope = GlobalSearchScope.moduleWithDependenciesAndLibrariesScope(targetModule)
+        val referencesToSkip = HashSet<KtReferenceExpression>()
        for (declaration in elementsToMove - doNotGoIn) {
            declaration.forEachDescendantOfType<KtReferenceExpression> { refExpr ->
                val targetDescriptor = refExpr.analyze(BodyResolveMode.PARTIAL)[BindingContext.REFERENCE_TARGET, refExpr] ?: return@forEachDescendantOfType
@@ -172,11 +185,13 @@ class MoveConflictChecker(
                                                        scopeDescription,
                                                        CommonRefactoringUtil.htmlEmphasize(targetModule.name))
                conflicts.putValue(target, CommonRefactoringUtil.capitalize(message))
+                referencesToSkip += refExpr
            }
        }
+        internalUsages.removeIf { it.reference?.element?.let { it in referencesToSkip } ?: false }
    }

-    fun checkVisibilityInUsages(usages: List<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
+    fun checkVisibilityInUsages(usages: Collection<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
        val declarationToContainers = HashMap<KtNamedDeclaration, MutableSet<PsiElement>>()
        for (usage in usages) {
            val element = usage.element
@@ -205,7 +220,7 @@ class MoveConflictChecker(
        }
    }

-    fun checkVisibilityInDeclarations(conflicts: MultiMap<PsiElement, String>) {
+    private fun checkVisibilityInDeclarations(conflicts: MultiMap<PsiElement, String>) {
        val targetContainer = moveTarget.getContainerDescriptor() ?: return
        for (declaration in elementsToMove - doNotGoIn) {
            declaration.forEachDescendantOfType<KtReferenceExpression> { refExpr ->
@@ -227,10 +242,14 @@ class MoveConflictChecker(
        }
    }

-    fun checkAllConflicts(usages: List<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
-        checkModuleConflictsInUsages(usages, conflicts)
-        checkModuleConflictsInDeclarations(conflicts)
-        checkVisibilityInUsages(usages, conflicts)
+    fun checkAllConflicts(
+            externalUsages: MutableSet<UsageInfo>,
+            internalUsages: MutableSet<UsageInfo>,
+            conflicts: MultiMap<PsiElement, String>
+    ) {
+        checkModuleConflictsInUsages(externalUsages, conflicts)
+        checkModuleConflictsInDeclarations(internalUsages, conflicts)
+        checkVisibilityInUsages(externalUsages, conflicts)
        checkVisibilityInDeclarations(conflicts)
    }
 }
@@ -91,7 +91,7 @@ class MoveKotlinFileHandler : MoveFileHandler() {
                        elementsToMove = psiFile.declarations.filterIsInstance<KtNamedDeclaration>(),
                        moveTarget = moveTarget,
                        delegate = MoveDeclarationsDelegate.TopLevel,
-                        updateInternalReferences = false
+                        scanEntireFile = true
                ),
                Mover.Idle
        )
@@ -122,7 +122,6 @@ class MoveKotlinFileHandler : MoveFileHandler() {
            usages += it.findUsages()
            usages += it.getConflictsAsUsages()
        }
-        newParent?.let { usages += findInternalUsages(psiFile, it) }
        return usages
    }

@@ -303,7 +303,7 @@ private fun updateJavaReference(reference: PsiReferenceExpression, oldElement: P
 /**
 * Perform usage postprocessing and return non-code usages
 */
-fun postProcessMoveUsages(usages: List<UsageInfo>,
+fun postProcessMoveUsages(usages: Collection<UsageInfo>,
                          oldToNewElementsMapping: Map<PsiElement, PsiElement> = Collections.emptyMap(),
                          shorteningMode: ShorteningMode = ShorteningMode.DELAYED_SHORTENING
 ): List<NonCodeUsageInfo> {
@@ -1,5 +1,5 @@
 package a2

-import b.internalTargetVal
+import a1.internalTargetVal

-internal val sourceVal = b.internalTargetVal
+internal val sourceVal = internalTargetVal
@@ -1,3 +1,2 @@
 Property a1.internalTargetVal, referenced in file internalSource.kt, will not be accessible from module A
-Property a1.internalTargetVal, referenced in file internalSource.kt, will not be accessible from module A
-Property sourceVal uses property internalTargetVal which will be inaccessible after move
+Property a1.internalTargetVal, referenced in file internalSource.kt, will not be accessible from module A
@@ -1,3 +1,3 @@
 package a

-val val2 = target.val1
+val val2 = val1
@@ -1,2 +1 @@
 Property a.val1, referenced in file test.kt, will not be accessible from module A
-Property a.val1, referenced in file test.kt, will not be accessible from module A
@@ -1,3 +1,3 @@
 package a2

-import b.internalTargetVal
+import a1.internalTargetVal