Move: Add conflict checking for "Move directory with classes" case

2017-04-07 21:14:39 +03:00
parent 87a12ee1c7
commit e21ff23e26
18 changed files with 161 additions and 20 deletions
@@ -43,19 +43,7 @@ class KotlinChangePackageRefactoring(val file: KtFile) {
                MoveDeclarationsDescriptor(
                        project = project,
                        elementsToMove = file.declarations.filterIsInstance<KtNamedDeclaration>(),
-                        moveTarget = object: KotlinDirectoryBasedMoveTarget {
-                            override val targetContainerFqName = newFqName
-
-                            override val directory: PsiDirectory = file.containingDirectory!!
-
-                            override val targetFile: VirtualFile? = directory.virtualFile
-
-                            override fun getOrCreateTargetPsi(originalPsi: PsiElement) = originalPsi.containingFile as? KtFile
-
-                            override fun getTargetPsiIfExists(originalPsi: PsiElement) = null
-
-                            override fun verify(file: PsiFile) = null
-                        },
+                        moveTarget = KotlinDirectoryMoveTarget(newFqName, file.containingDirectory!!),
                        delegate = MoveDeclarationsDelegate.TopLevel,
                        scanEntireFile = true
                ),
@@ -98,3 +98,16 @@ class KotlinMoveTargetForDeferredFile(
    // No additional verification is needed
    override fun verify(file: PsiFile): String? = null
 }
+
+class KotlinDirectoryMoveTarget(
+        override val targetContainerFqName: FqName,
+        override val directory: PsiDirectory
+) : KotlinDirectoryBasedMoveTarget {
+    override val targetFile: VirtualFile? = directory.virtualFile
+
+    override fun getOrCreateTargetPsi(originalPsi: PsiElement) = originalPsi.containingFile as? KtFile
+
+    override fun getTargetPsiIfExists(originalPsi: PsiElement) = null
+
+    override fun verify(file: PsiFile) = null
+}
@@ -25,18 +25,21 @@ import com.intellij.refactoring.move.moveClassesOrPackages.MoveDirectoryWithClas
 import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesUtil
 import com.intellij.usageView.UsageInfo
 import com.intellij.util.Function
+import com.intellij.util.containers.MultiMap
 import org.jetbrains.kotlin.idea.core.getPackage
-import org.jetbrains.kotlin.idea.refactoring.invokeOnceOnCommandFinish
-import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.MoveKotlinDeclarationsProcessor
 import org.jetbrains.kotlin.idea.core.quoteIfNeeded
+import org.jetbrains.kotlin.idea.refactoring.invokeOnceOnCommandFinish
+import org.jetbrains.kotlin.idea.refactoring.move.KotlinMoveUsage
+import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.KotlinDirectoryMoveTarget
+import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.MoveConflictChecker
+import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.MoveKotlinDeclarationsProcessor
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.KtFile
-import java.util.ArrayList
-import java.util.HashMap
+import java.util.*

 class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() {
-    private class FileUsagesWrapper(
-            val psiFile: PsiFile,
+    private data class FileUsagesWrapper(
+            val psiFile: KtFile,
            val usages: List<UsageInfo>,
            val moveDeclarationsProcessor: MoveKotlinDeclarationsProcessor?
    ) : UsageInfo(psiFile)
@@ -69,6 +72,39 @@ class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() {
                .mapTo(result) { FileUsagesWrapper(it, fileHandler.findUsages(it, null, searchInComments, searchInNonJavaFiles), null) }
    }

+    override fun preprocessUsages(
+            project: Project,
+            files: MutableSet<PsiFile>,
+            infos: Array<UsageInfo>,
+            directory: PsiDirectory,
+            conflicts: MultiMap<PsiElement, String>
+    ) {
+        val psiPackage = directory.getPackage() ?: return
+        val moveTarget = KotlinDirectoryMoveTarget(FqName(psiPackage.qualifiedName), directory)
+        for ((index, usageInfo) in infos.withIndex()) {
+            if (usageInfo !is FileUsagesWrapper) continue
+
+            val elementsToMove = usageInfo.psiFile.declarations
+            if (elementsToMove.isEmpty()) continue
+
+            val (internalUsages, externalUsages) = usageInfo.usages.partition { it is KotlinMoveUsage && it.isInternal }
+            val internalUsageSet = internalUsages.toMutableSet()
+            val externalUsageSet = externalUsages.toMutableSet()
+
+            val conflictChecker = MoveConflictChecker(
+                    project,
+                    elementsToMove,
+                    moveTarget,
+                    elementsToMove.first(),
+                    allElementsToMove = files
+            )
+            conflictChecker.checkAllConflicts(externalUsageSet, internalUsageSet, conflicts)
+            if (externalUsageSet.size != externalUsages.size || internalUsageSet.size != internalUsages.size) {
+                infos[index] = usageInfo.copy(usages = (externalUsageSet + internalUsageSet).toList())
+            }
+        }
+    }
+
    override fun beforeMove(psiFile: PsiFile) {

    }
@@ -113,7 +149,7 @@ class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() {
                    val moveDeclarationsProcessor = moveContext.moveDeclarationsProcessor ?: return@body
                    val movedFile = moveContext.newParent.findFile(file.name) ?: return@body

-                    usagesToProcess += FileUsagesWrapper(movedFile, it.usages, moveDeclarationsProcessor)
+                    usagesToProcess += FileUsagesWrapper(movedFile as KtFile, it.usages, moveDeclarationsProcessor)
                }
            usagesToProcess.forEach { fileHandler.retargetUsages(it.usages, it.moveDeclarationsProcessor!!) }
        }
@@ -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,8 @@
+package test.pack
+
+import test2.J
+
+class Bar {
+    internal val foo = Foo()
+    val j = J()
+}
@@ -0,0 +1,7 @@
+package test.pack
+
+import test2.J
+
+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,7 @@
+package test2;
+
+import test2.pack.Foo;
+
+public class J {
+    Foo foo = new Foo();
+}
@@ -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,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,7 @@
+package test2;
+
+import test2.pack.Foo;
+
+public class J {
+    Foo foo = new Foo();
+}
@@ -0,0 +1,8 @@
+package test2.pack
+
+import test2.J
+
+class Bar {
+    internal val foo = Foo()
+    val j = J()
+}
@@ -0,0 +1,7 @@
+package test2.pack
+
+import test2.J
+
+internal class Foo {
+    val j = J()
+}
@@ -0,0 +1,4 @@
+Class test2.J, referenced in property test2.pack.Bar.j, will not be accessible in module A
+Class test2.J, referenced in property test2.pack.Foo.j, will not be accessible in module A
+Class test2.pack.Foo, referenced in field J.foo, will not be accessible from module B
+Class test2.pack.Foo, referenced in field J.foo, will not be accessible from module B
@@ -0,0 +1,6 @@
+{
+  "mainFile": "B/src/test2/pack/Bar.kt",
+  "type": "MOVE_DIRECTORY_WITH_CLASSES",
+  "sourceDir": "B/src/test2/pack",
+  "targetDir": "A/src/test"
+}
@@ -42,6 +42,12 @@ public class MultiModuleMoveTestGenerated extends AbstractMultiModuleMoveTest {
        doTest(fileName);
    }

+    @TestMetadata("moveDirectoryToUnrelatedModuleConflict/moveDirectoryToUnrelatedModuleConflict.test")
+    public void testMoveDirectoryToUnrelatedModuleConflict_MoveDirectoryToUnrelatedModuleConflict() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/moveMultiModule/moveDirectoryToUnrelatedModuleConflict/moveDirectoryToUnrelatedModuleConflict.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("moveFileWithDeclarationsToUnrelatedModuleConflict/moveFileWithDeclarationsToUnrelatedModuleConflict.test")
    public void testMoveFileWithDeclarationsToUnrelatedModuleConflict_MoveFileWithDeclarationsToUnrelatedModuleConflict() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/moveMultiModule/moveFileWithDeclarationsToUnrelatedModuleConflict/moveFileWithDeclarationsToUnrelatedModuleConflict.test");