Move: Implement conflict checking for internal members

#KT-13190 Fixed
2017-03-06 22:23:30 +03:00
parent 6f5249ecc7
commit e82b256640
15 changed files with 166 additions and 8 deletions
@@ -23,6 +23,7 @@ import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiMember
 import com.intellij.psi.PsiMethod
 import com.intellij.psi.search.GlobalSearchScope
+import com.intellij.psi.search.searches.ReferencesSearch
 import com.intellij.refactoring.RefactoringBundle
 import com.intellij.refactoring.util.*
 import com.intellij.usageView.UsageInfo
@@ -35,11 +36,9 @@ import org.jetbrains.kotlin.descriptors.impl.MutablePackageFragmentDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.*
 import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
 import org.jetbrains.kotlin.idea.refactoring.getUsageContext
+import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
-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.isInsideOf
+import org.jetbrains.kotlin.psi.psiUtil.*
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
@@ -83,13 +82,13 @@ class MoveConflictChecker(

            is KotlinDirectoryBasedMoveTarget -> {
                val packageFqName = targetContainerFqName ?: return null
-                val targetDir = directory
+                val targetDir = directory?.virtualFile ?: targetFile
                val targetModuleDescriptor = if (targetDir != null) {
-                    val targetModule = ModuleUtilCore.findModuleForPsiElement(targetDir) ?: return null
+                    val targetModule = ModuleUtilCore.findModuleForFile(targetDir, project) ?: return null
                    val moduleFileIndex = ModuleRootManager.getInstance(targetModule).fileIndex
                    val targetModuleInfo = when {
-                        moduleFileIndex.isInSourceContent(targetDir.virtualFile) -> targetModule.productionSourceInfo()
-                        moduleFileIndex.isInTestSourceContent(targetDir.virtualFile) -> targetModule.testSourceInfo()
+                        moduleFileIndex.isInSourceContent(targetDir) -> targetModule.productionSourceInfo()
+                        moduleFileIndex.isInTestSourceContent(targetDir) -> targetModule.testSourceInfo()
                        else -> return null
                    }
                    resolutionFacade.findModuleDescriptor(targetModuleInfo) ?: return null
@@ -242,6 +241,35 @@ class MoveConflictChecker(
        }
    }

+    private fun isToBeMoved(element: PsiElement): Boolean = elementsToMove.any { it.isAncestor(element, false) }
+
+    private fun checkInternalMemberUsages(conflicts: MultiMap<PsiElement, String>) {
+        val sourceRoot = moveTarget.targetFile ?: return
+        val targetModule = ModuleUtilCore.findModuleForFile(sourceRoot, project) ?: return
+
+        val membersToCheck = LinkedHashSet<KtDeclaration>()
+        val memberCollector = object : KtVisitorVoid() {
+            override fun visitClassOrObject(classOrObject: KtClassOrObject) {
+                val declarations = classOrObject.declarations
+                declarations.filterTo(membersToCheck) { it.hasModifier(KtTokens.INTERNAL_KEYWORD) }
+                declarations.forEach { it.accept(this) }
+            }
+        }
+        elementsToMove.forEach { it.accept(memberCollector) }
+
+        for (memberToCheck in membersToCheck) {
+            for (reference in ReferencesSearch.search(memberToCheck)) {
+                val element = reference.element ?: continue
+                val usageModule = ModuleUtilCore.findModuleForPsiElement(element)
+                if (usageModule != targetModule && !isToBeMoved(element)) {
+                    val container = element.getUsageContext()
+                    val message = "${render(container)} uses internal ${render(memberToCheck)} which will be inaccessible after move"
+                    conflicts.putValue(element, message.capitalize())
+                }
+            }
+        }
+    }
+
    fun checkAllConflicts(
            externalUsages: MutableSet<UsageInfo>,
            internalUsages: MutableSet<UsageInfo>,
@@ -251,5 +279,6 @@ class MoveConflictChecker(
        checkModuleConflictsInDeclarations(internalUsages, conflicts)
        checkVisibilityInUsages(externalUsages, conflicts)
        checkVisibilityInDeclarations(conflicts)
+        checkInternalMemberUsages(conflicts)
    }
 }
@@ -0,0 +1,13 @@
+<?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" />
+    <orderEntry type="library" name="KotlinJavaRuntime" level="project" />
+    <orderEntry type="module" module-name="B" />
+  </component>
+</module>
@@ -0,0 +1,5 @@
+package packA1
+
+import packA2.InternalContentUser
+
+class More
@@ -0,0 +1,11 @@
+package packA2
+
+import packA1.InternalContent
+
+class InternalContentUser {
+    fun useInternal(p: InternalContent) = p.internalFun()
+
+    internal fun internalFun() {
+
+    }
+}
@@ -0,0 +1,12 @@
+<?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" />
+    <orderEntry type="library" name="KotlinJavaRuntime" level="project" />
+  </component>
+</module>
@@ -0,0 +1,13 @@
+package packB
+
+class InternalContent {
+    internal fun internalFun() {}
+
+    fun useInternalInside() {
+        internalFun()
+    }
+
+    fun useInternal() {
+        InternalContentUser().internalFun()
+    }
+}
@@ -0,0 +1 @@
+This file is needed to ensure that containing directory is under version control
@@ -0,0 +1,13 @@
+<?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" />
+    <orderEntry type="library" name="KotlinJavaRuntime" level="project" />
+    <orderEntry type="module" module-name="B" />
+  </component>
+</module>
@@ -0,0 +1,17 @@
+package packA1
+
+import packA2.InternalContentUser
+
+class <caret>InternalContent {
+    internal fun internalFun() {}
+
+    fun useInternalInside() {
+        internalFun()
+    }
+
+    fun useInternal() {
+        InternalContentUser().internalFun()
+    }
+}
+
+class More
@@ -0,0 +1,11 @@
+package packA2
+
+import packA1.InternalContent
+
+class InternalContentUser {
+    fun useInternal(p: InternalContent) = p.internalFun()
+
+    internal fun internalFun() {
+
+    }
+}
@@ -0,0 +1,12 @@
+<?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" />
+    <orderEntry type="library" name="KotlinJavaRuntime" level="project" />
+  </component>
+</module>
@@ -0,0 +1 @@
+This file is needed to ensure that containing directory is under version control
@@ -0,0 +1,6 @@
+Class InternalContent uses function internalFun() which will be inaccessible after move
+Class packA1.InternalContent, referenced in file InternalSource.kt, will not be accessible from module A
+Class packA1.InternalContent, referenced in file InternalSource.kt, will not be accessible from module A
+Class packA2.InternalContentUser, referenced in function packA1.InternalContent.useInternal(), will not be accessible in module B
+Function packA2.InternalContentUser.internalFun(), referenced in function packA1.InternalContent.useInternal(), will not be accessible in module B
+Function useInternal(InternalContent) uses internal function internalFun() which will be inaccessible after move
@@ -0,0 +1,8 @@
+{
+  "mainFile": "A/src/packA1/InternalTarget.kt",
+  "type": "MOVE_KOTLIN_TOP_LEVEL_DECLARATIONS",
+  "targetPackage": "packB",
+  "targetSourceRoot": "B/src",
+  "isMultiModule": "true",
+  "withRuntime": "true"
+}
@@ -492,6 +492,12 @@ public class MoveTestGenerated extends AbstractMoveTest {
        doTest(fileName);
    }

+    @TestMetadata("kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/moveInternalToAnotherModule.test")
+    public void testKotlin_moveTopLevelDeclarations_misc_moveInternalToAnotherModule_MoveInternalToAnotherModule() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/moveInternalToAnotherModule.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("kotlin/moveTopLevelDeclarations/misc/moveToUnrelatedModuleConflict/moveToUnrelatedModuleConflict.test")
    public void testKotlin_moveTopLevelDeclarations_misc_moveToUnrelatedModuleConflict_MoveToUnrelatedModuleConflict() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveToUnrelatedModuleConflict/moveToUnrelatedModuleConflict.test");
				`@@ -0,0 +1 @@`
				`This file is needed to ensure that containing directory is under version control`