Move: Check for conflicts using entire collections of elements to move

2017-04-07 19:49:14 +03:00
parent 358b5fe548
commit 87a12ee1c7
19 changed files with 194 additions and 23 deletions
@@ -27,9 +27,11 @@ import com.intellij.refactoring.move.MoveCallback
 import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesProcessor
 import com.intellij.usageView.UsageInfo
 import com.intellij.usageView.UsageViewDescriptor
-import com.intellij.usageView.UsageViewUtil
 import com.intellij.util.containers.MultiMap
 import com.intellij.util.text.UniqueNameGenerator
+import org.jetbrains.kotlin.idea.refactoring.move.moveFilesOrDirectories.allElementsToMove
+import org.jetbrains.kotlin.idea.refactoring.move.moveFilesOrDirectories.shouldFixFqName
+import org.jetbrains.kotlin.psi.KtFile

 class MoveFilesWithDeclarationsProcessor @JvmOverloads constructor (
        project: Project,
@@ -56,14 +58,23 @@ class MoveFilesWithDeclarationsProcessor @JvmOverloads constructor (
        return MoveFilesWithDeclarationsViewDescriptor(elementsToMove.toTypedArray<PsiElement>(), targetDirectory)
    }

+    override fun findUsages(): Array<UsageInfo> {
+        try {
+            markScopeToMove(elementsToMove)
+            return super.findUsages()
+        }
+        finally {
+            markScopeToMove(null)
+        }
+    }
+
    override fun preprocessUsages(refUsages: Ref<Array<UsageInfo>>): Boolean {
        val usages = refUsages.get()

        val (conflictUsages, usagesToProcess) = usages.partition { it is ConflictUsageInfo }

-        val distinctConflictUsages = UsageViewUtil.removeDuplicatedUsages(conflictUsages.toTypedArray())
        val conflicts = MultiMap<PsiElement, String>()
-        for (conflictUsage in distinctConflictUsages) {
+        for (conflictUsage in conflictUsages) {
            conflicts.putValues(conflictUsage.element, (conflictUsage as ConflictUsageInfo).messages)
        }

@@ -83,15 +94,23 @@ class MoveFilesWithDeclarationsProcessor @JvmOverloads constructor (
        sourceFile.name = temporaryName
    }

-    private fun markShouldFixFqName(value: Boolean) {
-        fun PsiElement.doMark(value: Boolean) {
+    private fun markPsiFiles(mark: PsiFile.() -> Unit) {
+        fun PsiElement.doMark(mark: PsiFile.() -> Unit) {
            when (this) {
-                is PsiJavaFile -> shouldFixFqName = value
-                is PsiDirectory -> children.forEach { it.doMark(value) }
+                is PsiFile -> mark()
+                is PsiDirectory -> children.forEach { it.doMark(mark) }
            }
        }

-        elementsToMove.forEach { it.doMark(value) }
+        elementsToMove.forEach { it.doMark(mark) }
+    }
+
+    private fun markShouldFixFqName(value: Boolean) {
+        markPsiFiles { (this as? PsiJavaFile)?.shouldFixFqName = value }
+    }
+
+    private fun markScopeToMove(allElementsToMove: List<PsiElement>?) {
+        markPsiFiles { (this as? KtFile)?.allElementsToMove = allElementsToMove }
    }

    override fun performRefactoring(usages: Array<UsageInfo>) {
@@ -85,7 +85,7 @@ interface Mover : (KtNamedDeclaration, KtElement) -> KtNamedDeclaration {

 internal var KtSimpleNameExpression.internalUsageInfo: UsageInfo? by CopyableUserDataProperty(Key.create("INTERNAL_USAGE_INFO"))

-class MoveDeclarationsDescriptor(
+class MoveDeclarationsDescriptor @JvmOverloads constructor(
        val project: Project,
        val elementsToMove: Collection<KtNamedDeclaration>,
        val moveTarget: KotlinMoveTarget,
@@ -95,7 +95,8 @@ class MoveDeclarationsDescriptor(
        val scanEntireFile: Boolean = false,
        val deleteSourceFiles: Boolean = false,
        val moveCallback: MoveCallback? = null,
-        val openInEditor: Boolean = false
+        val openInEditor: Boolean = false,
+        val allElementsToMove: List<PsiElement>? = null
 )

 class ConflictUsageInfo(element: PsiElement, val messages: Collection<String>) : UsageInfo(element)
@@ -188,7 +189,13 @@ class MoveKotlinDeclarationsProcessor(
        }

        val usages = ArrayList<UsageInfo>()
-        val conflictChecker = MoveConflictChecker(project, elementsToMove, descriptor.moveTarget, elementsToMove.first())
+        val conflictChecker = MoveConflictChecker(
+                project,
+                elementsToMove,
+                descriptor.moveTarget,
+                elementsToMove.first(),
+                allElementsToMove = descriptor.allElementsToMove
+        )
        for ((sourceFile, kotlinToLightElements) in kotlinToLightElementsBySourceFile) {
            val internalUsages = LinkedHashSet<UsageInfo>()
            val externalUsages = LinkedHashSet<UsageInfo>()
@@ -18,12 +18,12 @@ package org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations

 import com.intellij.openapi.module.ModuleUtilCore
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.roots.ModuleRootManager
 import com.intellij.openapi.vfs.VirtualFile
-import com.intellij.psi.PsiElement
-import com.intellij.psi.PsiMember
-import com.intellij.psi.PsiMethod
+import com.intellij.psi.*
 import com.intellij.psi.search.GlobalSearchScope
 import com.intellij.psi.search.searches.ReferencesSearch
+import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.refactoring.RefactoringBundle
 import com.intellij.refactoring.util.*
 import com.intellij.usageView.UsageInfo
@@ -40,7 +40,10 @@ import org.jetbrains.kotlin.idea.project.TargetPlatformDetector
 import org.jetbrains.kotlin.idea.refactoring.getUsageContext
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.psi.psiUtil.*
+import org.jetbrains.kotlin.psi.psiUtil.contains
+import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType
+import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
+import org.jetbrains.kotlin.psi.psiUtil.isAncestor
 import org.jetbrains.kotlin.renderer.ClassifierNamePolicy
 import org.jetbrains.kotlin.renderer.DescriptorRenderer
 import org.jetbrains.kotlin.renderer.ParameterNameRenderingPolicy
@@ -60,12 +63,15 @@ class MoveConflictChecker(
        private val elementsToMove: Collection<KtElement>,
        private val moveTarget: KotlinMoveTarget,
        contextElement: KtElement,
-        private val doNotGoIn: Collection<KtElement> = emptyList()
+        private val doNotGoIn: Collection<KtElement> = emptyList(),
+        allElementsToMove: Collection<PsiElement>? = null
 ) {
    private val resolutionFacade = contextElement.getResolutionFacade()

    private val fakeFile = KtPsiFactory(project).createFile("")

+    private val allElementsToMove = allElementsToMove ?: elementsToMove
+
    private fun PackageFragmentDescriptor.withSource(sourceFile: KtFile): PackageFragmentDescriptor {
        return object : PackageFragmentDescriptor by this {
            override fun getOriginal() = this
@@ -144,10 +150,49 @@ class MoveConflictChecker(

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

+    // Based on RefactoringConflictsUtil.analyzeModuleConflicts
+    fun analyzeModuleConflictsInUsages(project: Project,
+                                       usages: Collection<UsageInfo>,
+                                       sourceRoot: VirtualFile,
+                                       conflicts: MultiMap<PsiElement, String>) {
+        val targetModule = ModuleUtilCore.findModuleForFile(sourceRoot, project) ?: return
+
+        val isInTestSources = ModuleRootManager.getInstance(targetModule).fileIndex.isInTestSourceContent(sourceRoot)
+        NextUsage@ for (usage in usages) {
+            val element = usage.element ?: continue
+            if (PsiTreeUtil.getParentOfType(element, PsiImportStatement::class.java, false) != null) continue
+            if (isToBeMoved(element)) continue@NextUsage
+
+            val resolveScope = element.resolveScope
+            if (resolveScope.isSearchInModuleContent(targetModule, isInTestSources)) continue
+
+            val usageFile = element.containingFile
+            val usageVFile = usageFile.virtualFile ?: continue
+            val usageModule = ModuleUtilCore.findModuleForFile(usageVFile, project) ?: continue
+            val container = if (usageFile is PsiJavaFile) ConflictsUtil.getContainer(element) else usageFile
+            val scopeDescription = RefactoringUIUtil.getDescription(container, true)
+            val referencedElement = (if (usage is MoveRenameUsageInfo) usage.referencedElement else usage.element) ?: error(usage)
+            val message = if (usageModule == targetModule && isInTestSources) {
+                RefactoringBundle.message("0.referenced.in.1.will.not.be.accessible.from.production.of.module.2",
+                                          RefactoringUIUtil.getDescription(referencedElement, true),
+                                          scopeDescription,
+                                          CommonRefactoringUtil.htmlEmphasize(usageModule.name))
+            }
+            else {
+                RefactoringBundle.message("0.referenced.in.1.will.not.be.accessible.from.module.2",
+                                          RefactoringUIUtil.getDescription(referencedElement, true),
+                                          scopeDescription,
+                                          CommonRefactoringUtil.htmlEmphasize(usageModule.name))
+            }
+            conflicts.putValue(referencedElement, CommonRefactoringUtil.capitalize(message))
+        }
+    }
+
    fun checkModuleConflictsInUsages(externalUsages: MutableSet<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
        val newConflicts = MultiMap<PsiElement, String>()
        val sourceRoot = moveTarget.targetFile ?: return
-        RefactoringConflictsUtil.analyzeModuleConflicts(project, elementsToMove, externalUsages.toTypedArray(), sourceRoot, newConflicts)
+
+        analyzeModuleConflictsInUsages(project, externalUsages, sourceRoot, newConflicts)
        if (!newConflicts.isEmpty) {
            val referencedElementsToSkip = newConflicts.keySet().mapNotNullTo(HashSet()) { it.namedUnwrappedElement }
            externalUsages.removeIf {
@@ -228,7 +273,7 @@ class MoveConflictChecker(

                val target = DescriptorToSourceUtilsIde.getAnyDeclaration(project, targetDescriptor) ?: return@forEachDescendantOfType

-                if (target.isInsideOf(elementsToMove)) return@forEachDescendantOfType
+                if (isToBeMoved(target)) return@forEachDescendantOfType

                if (isInScope(target, targetDescriptor)) return@forEachDescendantOfType
                if (target is KtTypeParameter) return@forEachDescendantOfType
@@ -256,7 +301,7 @@ class MoveConflictChecker(
            val element = usage.element
            if (element == null || usage !is MoveRenameUsageInfo || usage is NonCodeUsageInfo) continue

-            if (element.isInsideOf(elementsToMove)) continue
+            if (isToBeMoved(element)) continue

            val referencedElement = usage.referencedElement?.namedUnwrappedElement as? KtNamedDeclaration ?: continue
            val referencedDescriptor = resolutionFacade.resolveToDescriptor(referencedElement)
@@ -286,7 +331,7 @@ class MoveConflictChecker(
                refExpr.references
                        .forEach { ref ->
                            val target = ref.resolve() ?: return@forEach
-                            if (target.isInsideOf(elementsToMove)) return@forEach
+                            if (isToBeMoved(target)) return@forEach
                            val targetDescriptor = when (target) {
                                                       is KtDeclaration -> target.resolveToDescriptor()
                                                       is PsiMember -> target.getJavaMemberDescriptor()
@@ -298,7 +343,7 @@ class MoveConflictChecker(
                                val dispatchReceiver = resolvedCall.dispatchReceiver
                                if (dispatchReceiver is ExpressionReceiver && dispatchReceiver.expression is KtSuperExpression) return@forEach
                                val receiverClass = resolvedCall.dispatchReceiver?.type?.constructor?.declarationDescriptor?.source?.getPsi()
-                                if (receiverClass != null && receiverClass.isInsideOf(elementsToMove)) return@forEach
+                                if (receiverClass != null && isToBeMoved(receiverClass)) return@forEach
                            }

                            if (!targetDescriptor.isVisibleIn(targetContainer)) {
@@ -310,7 +355,7 @@ class MoveConflictChecker(
        }
    }

-    private fun isToBeMoved(element: PsiElement): Boolean = elementsToMove.any { it.isAncestor(element, false) }
+    private fun isToBeMoved(element: PsiElement): Boolean = allElementsToMove.any { it.isAncestor(element, false) }

    private fun checkInternalMemberUsages(conflicts: MultiMap<PsiElement, String>) {
        val sourceRoot = moveTarget.targetFile ?: return
@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.refactoring.move.moveFilesOrDirectories

 import com.intellij.openapi.module.ModuleUtilCore
 import com.intellij.openapi.roots.JavaProjectRootsUtil
+import com.intellij.openapi.util.Key
 import com.intellij.psi.PsiCompiledElement
 import com.intellij.psi.PsiDirectory
 import com.intellij.psi.PsiElement
@@ -39,6 +40,9 @@ import org.jetbrains.kotlin.idea.refactoring.move.updatePackageDirective
 import org.jetbrains.kotlin.name.FqNameUnsafe
 import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.psi.KtNamedDeclaration
+import org.jetbrains.kotlin.psi.UserDataProperty
+
+internal var KtFile.allElementsToMove: List<PsiElement>? by UserDataProperty(Key.create("SCOPE_TO_MOVE"))

 class MoveKotlinFileHandler : MoveFileHandler() {
    internal class FileInfo(file: KtFile) : UsageInfo(file)
@@ -94,7 +98,8 @@ class MoveKotlinFileHandler : MoveFileHandler() {
                        elementsToMove = psiFile.declarations.filterIsInstance<KtNamedDeclaration>(),
                        moveTarget = moveTarget,
                        delegate = MoveDeclarationsDelegate.TopLevel,
-                        scanEntireFile = true
+                        scanEntireFile = true,
+                        allElementsToMove = psiFile.allElementsToMove
                ),
                Mover.Idle
        )
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,6 @@
+package test;
+
+public class J {
+    Foo foo = new Foo();
+    Bar bar = new Bar();
+}
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,6 @@
+package test2
+
+class Bar {
+    internal val foo = Foo()
+    val j = J()
+}
@@ -0,0 +1,5 @@
+package test2
+
+internal class Foo {
+    val j = J()
+}
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,6 @@
+package test
+
+class Bar {
+    internal val foo = Foo()
+    val j = J()
+}
@@ -0,0 +1,5 @@
+package test
+
+internal class Foo {
+    val j = J()
+}
@@ -0,0 +1,6 @@
+package test;
+
+public class J {
+    Foo foo = new Foo();
+    Bar bar = new Bar();
+}
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,6 @@
+Class test.Bar, referenced in field J.bar, will not be accessible from module A
+Class test.Bar, referenced in field J.bar, will not be accessible from module A
+Class test.Foo, referenced in field J.foo, will not be accessible from module A
+Class test.Foo, referenced in field J.foo, will not be accessible from module A
+Class test.J, referenced in property test.Bar.j, will not be accessible in module B
+Class test.J, referenced in property test.Foo.j, will not be accessible in module B
@@ -0,0 +1,5 @@
+{
+  "filesToMove": ["A/src/test/Foo.kt", "A/src/test/Bar.kt"],
+  "type": "MOVE_FILES_WITH_DECLARATIONS",
+  "targetDirectory": "B/src/test2"
+}
@@ -66,6 +66,12 @@ public class MultiModuleMoveTestGenerated extends AbstractMultiModuleMoveTest {
        doTest(fileName);
    }

+    @TestMetadata("moveMultipleFilesToUnrelatedModuleConflict/moveMultipleFilesToUnrelatedModuleConflict.test")
+    public void testMoveMultipleFilesToUnrelatedModuleConflict_MoveMultipleFilesToUnrelatedModuleConflict() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/moveMultiModule/moveMultipleFilesToUnrelatedModuleConflict/moveMultipleFilesToUnrelatedModuleConflict.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("moveToModuleWithoutLibConflict/moveToModuleWithoutLibConflict.test")
    public void testMoveToModuleWithoutLibConflict_MoveToModuleWithoutLibConflict() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/moveMultiModule/moveToModuleWithoutLibConflict/moveToModuleWithoutLibConflict.test");