From 6fb788d446d311df607bae59c03b38e236aa2f21 Mon Sep 17 00:00:00 2001 From: Igor Yakovlev Date: Thu, 19 Dec 2019 01:03:57 +0300 Subject: [PATCH] Fix move refactoring for several class files Fixed #KT-35235 (also fixed #KT-35427) --- .../MoveToKotlinFileProcessor.kt | 11 +- .../ui/MoveKotlinTopLevelDeclarationsModel.kt | 164 ++++++++++-------- 2 files changed, 93 insertions(+), 82 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveToKotlinFileProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveToKotlinFileProcessor.kt index 52d696eca84..6521ace305d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveToKotlinFileProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveToKotlinFileProcessor.kt @@ -76,18 +76,13 @@ class MoveToKotlinFileProcessor @JvmOverloads constructor( } override fun performRefactoring(usages: Array) { - val needTemporaryRename = targetDirectory.findFile(targetFileName) != null + val needTemporaryRename = targetDirectory.findFile(sourceFile.name) != null if (needTemporaryRename) { renameFileTemporarily() } - try { - super.performRefactoring(usages) - } finally { - if (needTemporaryRename) { - sourceFile.name = targetFileName - } - } + super.performRefactoring(usages) + sourceFile.name = targetFileName } companion object { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinTopLevelDeclarationsModel.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinTopLevelDeclarationsModel.kt index 9720f90d0e2..19a76a055ce 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinTopLevelDeclarationsModel.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinTopLevelDeclarationsModel.kt @@ -18,6 +18,7 @@ import com.intellij.refactoring.move.MoveCallback import com.intellij.refactoring.move.moveClassesOrPackages.AutocreatingSingleSourceRootMoveDestination import com.intellij.refactoring.move.moveClassesOrPackages.MultipleRootsMoveDestination import com.intellij.util.IncorrectOperationException +import org.jetbrains.kotlin.backend.common.onlyIf import org.jetbrains.kotlin.idea.KotlinFileType import org.jetbrains.kotlin.idea.core.util.toPsiFile import org.jetbrains.kotlin.idea.refactoring.KotlinRefactoringBundle @@ -51,8 +52,11 @@ internal class MoveKotlinTopLevelDeclarationsModel( val moveCallback: MoveCallback? ) : Model { + private inline fun List.mapToSingleOrNull(transform: (T) -> K?): K? = + mapTo(mutableSetOf(), transform).singleOrNull() + private val sourceDirectory by lazy { - sourceFiles.singleOrNull { it.parent !== null }?.parent ?: throw ConfigurationException("Can't determine sources directory") + sourceFiles.mapToSingleOrNull { it.parent } ?: throw ConfigurationException("Can't determine sources directory") } private val sourceFiles = elementsToMove.map { it.containingKtFile }.distinct() @@ -81,48 +85,54 @@ internal class MoveKotlinTopLevelDeclarationsModel( private fun getFilesExistingInTargetDir( targetFileName: String?, - targetDirectory: PsiDirectory? - ): List { - targetDirectory ?: return emptyList() - - val fileNames = targetFileName?.let { listOf(it) } ?: sourceFiles.map { it.name } - - return fileNames - .distinct() - .mapNotNull { targetDirectory.findFile(it) } + targetDirectory: PsiDirectory + ): Set { + return if (targetFileName != null) { + targetDirectory.findFile(targetFileName)?.let { setOf(it) }.orEmpty() + } else { + sourceFiles.mapNotNullTo(mutableSetOf()) { targetDirectory.findFile(it.name) } + } } private fun selectMoveTargetToPackage(): KotlinMoveTarget { - - if (sourceFiles.size > 1) throw ConfigurationException("Can't move from multiply source files") - - checkTargetFileName(fileNameInPackage) + require(sourceFiles.isNotEmpty()) val (targetDir, moveDestination) = selectPackageBasedTargetDirAndDestination() - val targetDirectory = moveDestination.getTargetIfExists(sourceDirectory) - val filesExistingInTargetDir = getFilesExistingInTargetDir(fileNameInPackage, targetDirectory) + val destination = sourceFiles + .mapToSingleOrNull { moveDestination.getTargetIfExists(it) } + ?: throw ConfigurationException("Can't get target for all source elements") + + val singleSourceFileMode = sourceFiles.size == 1 + + val targetFileName = if (singleSourceFileMode) fileNameInPackage.also(::checkTargetFileName) else null + val filesExistingInTargetDir = getFilesExistingInTargetDir(targetFileName, targetDirectory) + if (filesExistingInTargetDir.isNotEmpty()) { - if (filesExistingInTargetDir.size > 1) { + if (singleSourceFileMode) { + val singeTargetFile = filesExistingInTargetDir.single() as? KtFile + if (singeTargetFile != null) { + return KotlinMoveTargetForExistingElement(singeTargetFile) + } + } else { val filePathsToReport = filesExistingInTargetDir.joinToString( separator = "\n", prefix = "Cannot perform refactoring since the following files already exist:\n\n" ) { it.virtualFile.path } throw ConfigurationException(filePathsToReport) } - - (filesExistingInTargetDir[0] as? KtFile)?.let { - return KotlinMoveTargetForExistingElement(it) - } } - // All source files must be in the same directory return KotlinMoveTargetForDeferredFile( FqName(targetPackage), - moveDestination.getTargetIfExists(sourceFiles[0]), + destination, targetDir - ) { getOrCreateKotlinFile(fileNameInPackage, moveDestination.getTargetDirectory(it)) } + ) { + val deferredFileName = if (singleSourceFileMode) fileNameInPackage else it.name + val deferredFileDirectory = moveDestination.getTargetDirectory(it) + getOrCreateKotlinFile(deferredFileName, deferredFileDirectory) + } } private fun selectMoveTargetToFile(): KotlinMoveTarget { @@ -137,7 +147,7 @@ internal class MoveKotlinTopLevelDeclarationsModel( checkTargetFileName(targetFile.name) val jetFile = targetFile.toPsiFile(project) as? KtFile - if (jetFile !== null) { + if (jetFile != null) { if (sourceFiles.size == 1 && sourceFiles.contains(jetFile)) { throw ConfigurationException("Can't move to the original file") } @@ -173,13 +183,11 @@ internal class MoveKotlinTopLevelDeclarationsModel( private fun selectMoveTarget() = if (isMoveToPackage) selectMoveTargetToPackage() else selectMoveTargetToFile() - private fun verifyBeforeRun(target: KotlinMoveTarget) { + private fun verifyBeforeRun() { if (elementsToMove.isEmpty()) throw ConfigurationException("At least one member must be selected") - - for (element in elementsToMove) { - target.verify(element.containingFile)?.let { throw ConfigurationException(it) } - } + if (sourceFiles.isEmpty()) throw ConfigurationException("None elements were selected") + if (sourceFiles.size == 1 && fileNameInPackage.isBlank()) throw ConfigurationException("File name may not be empty") if (isMoveToPackage) { if (targetPackage.isNotEmpty() && !PsiNameHelper.getInstance(project).isQualifiedName(targetPackage)) { @@ -187,68 +195,76 @@ internal class MoveKotlinTopLevelDeclarationsModel( } } else { val targetFile = File(targetFilePath).toPsiFile(project) - if (targetFile !== null && targetFile !is KtFile) { + if (targetFile != null && targetFile !is KtFile) { throw ConfigurationException(KotlinRefactoringBundle.message("refactoring.move.non.kotlin.file")) } } - - if (sourceFiles.size == 1 && fileNameInPackage.isEmpty()) { - throw ConfigurationException("File name may not be empty") - } } - private val verifiedMoveTarget get() = selectMoveTarget().also { verifyBeforeRun(it) } - @Throws(ConfigurationException::class) override fun computeModelResult() = computeModelResult(throwOnConflicts = false) @Throws(ConfigurationException::class) override fun computeModelResult(throwOnConflicts: Boolean): BaseRefactoringProcessor { - val target = verifiedMoveTarget + verifyBeforeRun() if (isFullFileMove && isMoveToPackage) { - val (_, moveDestination) = selectPackageBasedTargetDirAndDestination() + tryMoveFile(throwOnConflicts)?.let { return it } + } - val targetDir = moveDestination.getTargetIfExists(sourceDirectory) - val targetFileName = if (sourceFiles.size > 1) null else fileNameInPackage + return moveDeclaration(throwOnConflicts) + } - val filesExistingInTargetDir = getFilesExistingInTargetDir(targetFileName, targetDir) + private fun tryMoveFile(throwOnConflicts: Boolean): BaseRefactoringProcessor? { - if (filesExistingInTargetDir.isEmpty() || - (filesExistingInTargetDir.size == 1 && sourceFiles.contains(filesExistingInTargetDir[0])) - ) { - val targetDirectory = project.executeCommand(RefactoringBundle.message("move.title"), null) { - runWriteAction { - moveDestination.getTargetDirectory(sourceDirectory) - } - } + val targetFileName = if (sourceFiles.size > 1) null else fileNameInPackage + if (targetFileName != null) checkTargetFileName(targetFileName) - sourceFiles.forEach { it.updatePackageDirective = isUpdatePackageDirective } + val moveDestination = selectPackageBasedTargetDirAndDestination().destination + val targetDir = moveDestination.getTargetIfExists(sourceDirectory) - return if (sourceFiles.size == 1 && targetFileName !== null) - MoveToKotlinFileProcessor( - project, - sourceFiles[0], - targetDirectory, - targetFileName, - searchInComments = isSearchInComments, - searchInNonJavaFiles = isSearchInNonJavaFiles, - moveCallback = moveCallback, - throwOnConflicts = throwOnConflicts - ) - else - KotlinAwareMoveFilesOrDirectoriesProcessor( - project, - sourceFiles, - targetDirectory, - isSearchReferences, - searchInComments = isSearchInComments, - searchInNonJavaFiles = isSearchInNonJavaFiles, - moveCallback = moveCallback, - throwOnConflicts = throwOnConflicts - ) - } + val filesExistingInTargetDir = getFilesExistingInTargetDir(targetFileName, targetDir) + + val moveAsFile = filesExistingInTargetDir.isEmpty() || + filesExistingInTargetDir.singleOrNull()?.let { sourceFiles.contains(it) } == true + + if (!moveAsFile) return null + + val targetDirectory = project.executeCommand(RefactoringBundle.message("move.title"), null) { + runWriteAction { moveDestination.getTargetDirectory(sourceDirectory) } + } + + sourceFiles.forEach { it.updatePackageDirective = isUpdatePackageDirective } + + return if (targetFileName != null) + MoveToKotlinFileProcessor( + project, + sourceFiles.first(), + targetDirectory, + targetFileName, + searchInComments = isSearchInComments, + searchInNonJavaFiles = isSearchInNonJavaFiles, + moveCallback = moveCallback, + throwOnConflicts = throwOnConflicts + ) + else + KotlinAwareMoveFilesOrDirectoriesProcessor( + project, + sourceFiles, + targetDirectory, + isSearchReferences, + searchInComments = isSearchInComments, + searchInNonJavaFiles = isSearchInNonJavaFiles, + moveCallback = moveCallback, + throwOnConflicts = throwOnConflicts + ) + } + + private fun moveDeclaration(throwOnConflicts: Boolean): BaseRefactoringProcessor { + val target = selectMoveTarget() + for (element in elementsToMove) { + target.verify(element.containingFile)?.let { throw ConfigurationException(it) } } val options = MoveDeclarationsDescriptor(