From e82b256640b3cfb39ece72d2a7e9ed8f55cb1ee7 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Mon, 6 Mar 2017 22:23:30 +0300 Subject: [PATCH] Move: Implement conflict checking for internal members #KT-13190 Fixed --- .../moveDeclarations/moveConflictUtils.kt | 45 +++++++++++++++---- .../moveInternalToAnotherModule/after/A/A.iml | 13 ++++++ .../after/A/src/packA1/InternalTarget.kt | 5 +++ .../after/A/src/packA2/InternalSource.kt | 11 +++++ .../moveInternalToAnotherModule/after/B/B.iml | 12 +++++ .../after/B/src/packB/InternalContent.kt | 13 ++++++ .../after/B/src/packB/dummy.txt | 1 + .../before/A/A.iml | 13 ++++++ .../before/A/src/packA1/InternalTarget.kt | 17 +++++++ .../before/A/src/packA2/InternalSource.kt | 11 +++++ .../before/B/B.iml | 12 +++++ .../before/B/src/packB/dummy.txt | 1 + .../moveInternalToAnotherModule/conflicts.txt | 6 +++ .../moveInternalToAnotherModule.test | 8 ++++ .../refactoring/move/MoveTestGenerated.java | 6 +++ 15 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/A.iml create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA1/InternalTarget.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA2/InternalSource.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/B.iml create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/InternalContent.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/dummy.txt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/A.iml create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA1/InternalTarget.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA2/InternalSource.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/B.iml create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/src/packB/dummy.txt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/conflicts.txt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/moveInternalToAnotherModule.test diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt index 92fe5bda77e..977788b6f2f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt @@ -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) { + val sourceRoot = moveTarget.targetFile ?: return + val targetModule = ModuleUtilCore.findModuleForFile(sourceRoot, project) ?: return + + val membersToCheck = LinkedHashSet() + 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, internalUsages: MutableSet, @@ -251,5 +279,6 @@ class MoveConflictChecker( checkModuleConflictsInDeclarations(internalUsages, conflicts) checkVisibilityInUsages(externalUsages, conflicts) checkVisibilityInDeclarations(conflicts) + checkInternalMemberUsages(conflicts) } } diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/A.iml b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/A.iml new file mode 100644 index 00000000000..7c331a99b04 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/A.iml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA1/InternalTarget.kt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA1/InternalTarget.kt new file mode 100644 index 00000000000..6b49e2e1bbd --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA1/InternalTarget.kt @@ -0,0 +1,5 @@ +package packA1 + +import packA2.InternalContentUser + +class More \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA2/InternalSource.kt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA2/InternalSource.kt new file mode 100644 index 00000000000..d02826f3ff5 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/A/src/packA2/InternalSource.kt @@ -0,0 +1,11 @@ +package packA2 + +import packA1.InternalContent + +class InternalContentUser { + fun useInternal(p: InternalContent) = p.internalFun() + + internal fun internalFun() { + + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/B.iml b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/B.iml new file mode 100644 index 00000000000..245d3429faa --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/B.iml @@ -0,0 +1,12 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/InternalContent.kt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/InternalContent.kt new file mode 100644 index 00000000000..7767edc8ef6 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/InternalContent.kt @@ -0,0 +1,13 @@ +package packB + +class InternalContent { + internal fun internalFun() {} + + fun useInternalInside() { + internalFun() + } + + fun useInternal() { + InternalContentUser().internalFun() + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/dummy.txt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/dummy.txt new file mode 100644 index 00000000000..0f8d237a250 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/after/B/src/packB/dummy.txt @@ -0,0 +1 @@ +This file is needed to ensure that containing directory is under version control \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/A.iml b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/A.iml new file mode 100644 index 00000000000..7c331a99b04 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/A.iml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA1/InternalTarget.kt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA1/InternalTarget.kt new file mode 100644 index 00000000000..ca482ad44d1 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA1/InternalTarget.kt @@ -0,0 +1,17 @@ +package packA1 + +import packA2.InternalContentUser + +class InternalContent { + internal fun internalFun() {} + + fun useInternalInside() { + internalFun() + } + + fun useInternal() { + InternalContentUser().internalFun() + } +} + +class More \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA2/InternalSource.kt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA2/InternalSource.kt new file mode 100644 index 00000000000..d02826f3ff5 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/A/src/packA2/InternalSource.kt @@ -0,0 +1,11 @@ +package packA2 + +import packA1.InternalContent + +class InternalContentUser { + fun useInternal(p: InternalContent) = p.internalFun() + + internal fun internalFun() { + + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/B.iml b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/B.iml new file mode 100644 index 00000000000..245d3429faa --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/B.iml @@ -0,0 +1,12 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/src/packB/dummy.txt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/src/packB/dummy.txt new file mode 100644 index 00000000000..0f8d237a250 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/before/B/src/packB/dummy.txt @@ -0,0 +1 @@ +This file is needed to ensure that containing directory is under version control \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/conflicts.txt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/conflicts.txt new file mode 100644 index 00000000000..b06de95e0ae --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/conflicts.txt @@ -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 \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/moveInternalToAnotherModule.test b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/moveInternalToAnotherModule.test new file mode 100644 index 00000000000..2fbd7cec929 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveInternalToAnotherModule/moveInternalToAnotherModule.test @@ -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" +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java index 4abe10cee41..aa797996d3d 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java @@ -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");