From 89801a1ac7b0944cfeca532c42bb54e2ab6c8d08 Mon Sep 17 00:00:00 2001 From: Andrei Klunnyi Date: Fri, 16 Apr 2021 18:17:39 +0200 Subject: [PATCH] KT-44841 Move sealed: no warning on splitting hierarchy (JPS) To provide similar behavior for both Gradle and JPS projects target and source directories are now taken into account. Prior to this commit AbstractMoveTest.kt contained a hack allowing not to create empty target directories. It breaks updated logic of the move check and therefore is removed. To commit empty target directories under 'before' to git one needs to put something in them (git ignores dirs otherwise). Hence .gitignore-s. ^KT-44841 Fixed --- .../move/moveDeclarations/moveConflictUtils.kt | 11 +++++++---- .../ui/MoveKotlinNestedClassesToUpperLevelModel.kt | 1 - .../after/A/src/another/.gitignore | 0 .../before/A/src/another/.gitignore | 0 .../after/B/src/target/.gitignore | 0 .../before/B/src/target/.gitignore | 0 .../kotlin/idea/refactoring/move/AbstractMoveTest.kt | 8 +------- 7 files changed, 8 insertions(+), 12 deletions(-) create mode 100644 idea/testData/refactoring/moveMultiModule/moveSealedCheckTargetPackageHasNoMembers/after/A/src/another/.gitignore create mode 100644 idea/testData/refactoring/moveMultiModule/moveSealedCheckTargetPackageHasNoMembers/before/A/src/another/.gitignore create mode 100644 idea/testData/refactoring/moveMultiModule/moveToUnrelatedModuleConflict/after/B/src/target/.gitignore create mode 100644 idea/testData/refactoring/moveMultiModule/moveToUnrelatedModuleConflict/before/B/src/target/.gitignore 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 8d602dbc9c8..23322dbac3c 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 @@ -733,10 +733,11 @@ class MoveConflictChecker( val targetModule = moveTarget.getTargetModule(project) ?: return null val targetPackage = moveTarget.getTargetPackage() ?: return null + val targetDir = moveTarget.targetFile?.takeIf { it.isDirectory } ?: moveTarget.targetFile?.parent val className = classToMove.nameAsSafeName.asString() - if (otherHierarchyMembers.none { it.residesIn(targetModule, targetPackage) }) { + if (otherHierarchyMembers.none { it.residesIn(targetModule, targetPackage, targetDir) }) { val hierarchyMembers = buildList { add(classToMove); addAll(otherHierarchyMembers) }.toNamesList() return KotlinBundle.message( "text.sealed.broken.hierarchy.none.in.target", @@ -749,9 +750,10 @@ class MoveConflictChecker( val moduleToMoveFrom = classToMove.module ?: return null val packageToMoveFrom = classToMoveDesc.findPsiPackage(moduleToMoveFrom) ?: return null + val directoryToMoveFrom = classToMove.containingKtFile.containingDirectory?.virtualFile val membersRemainingInOriginalPackage = - otherHierarchyMembers.filter { it.residesIn(moduleToMoveFrom, packageToMoveFrom) && !isToBeMoved(it) }.toList() + otherHierarchyMembers.filter { it.residesIn(moduleToMoveFrom, packageToMoveFrom, directoryToMoveFrom) && !isToBeMoved(it) }.toList() if ((targetPackage != packageToMoveFrom || targetModule != moduleToMoveFrom) && membersRemainingInOriginalPackage.any { !isToBeMoved(it) } @@ -765,10 +767,11 @@ class MoveConflictChecker( return null } - private fun KtClassOrObject.residesIn(targetModule: Module, targetPackage: PsiPackage): Boolean { + private fun KtClassOrObject.residesIn(targetModule: Module, targetPackage: PsiPackage, targetDir: VirtualFile?): Boolean { val myModule = module ?: return false val myPackage = descriptor?.findPsiPackage(myModule) - return myPackage == targetPackage && myModule == targetModule + val myDirectory = containingKtFile.containingDirectory?.virtualFile + return myPackage == targetPackage && myModule == targetModule && myDirectory == targetDir } private fun DeclarationDescriptor.findPsiPackage(module: Module): PsiPackage? { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinNestedClassesToUpperLevelModel.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinNestedClassesToUpperLevelModel.kt index 7562e42ba37..0ea1d2b24e6 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinNestedClassesToUpperLevelModel.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinNestedClassesToUpperLevelModel.kt @@ -177,7 +177,6 @@ internal abstract class MoveKotlinNestedClassesToUpperLevelModel( val target = KotlinMoveTargetForDeferredFile( targetPackageFqName, target, - targetFile = null ) { createKotlinFile(targetFileName, target, targetPackageFqName.asString()) } target to MoveRefactoringDestination.FILE diff --git a/idea/testData/refactoring/moveMultiModule/moveSealedCheckTargetPackageHasNoMembers/after/A/src/another/.gitignore b/idea/testData/refactoring/moveMultiModule/moveSealedCheckTargetPackageHasNoMembers/after/A/src/another/.gitignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/testData/refactoring/moveMultiModule/moveSealedCheckTargetPackageHasNoMembers/before/A/src/another/.gitignore b/idea/testData/refactoring/moveMultiModule/moveSealedCheckTargetPackageHasNoMembers/before/A/src/another/.gitignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/testData/refactoring/moveMultiModule/moveToUnrelatedModuleConflict/after/B/src/target/.gitignore b/idea/testData/refactoring/moveMultiModule/moveToUnrelatedModuleConflict/after/B/src/target/.gitignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/testData/refactoring/moveMultiModule/moveToUnrelatedModuleConflict/before/B/src/target/.gitignore b/idea/testData/refactoring/moveMultiModule/moveToUnrelatedModuleConflict/before/B/src/target/.gitignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt index 7b1295df63d..8c16374171c 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt @@ -226,13 +226,7 @@ enum class MoveAction : AbstractMultifileRefactoringTest.RefactoringAction { AutocreatingSingleSourceRootMoveDestination(packageWrapper, rootDir.findFileByRelativePath(it)!!) } ?: MultipleRootsMoveDestination(packageWrapper) val targetDir = moveDestination.getTargetIfExists(mainFile) - val targetVirtualFile = if (targetSourceRootPath != null) { - rootDir.findFileByRelativePath(targetSourceRootPath)!! - } else { - targetDir?.virtualFile - } - - KotlinMoveTargetForDeferredFile(FqName(packageName), targetDir, targetVirtualFile) { + KotlinMoveTargetForDeferredFile(FqName(packageName), targetDir) { createKotlinFile(guessNewFileName(elementsToMove)!!, moveDestination.getTargetDirectory(mainFile)) } } ?: config.getString("targetFile").let { filePath ->