diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/kotlinRefactoringUtil.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/kotlinRefactoringUtil.kt index daf216ec918..6cfa6460e88 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/kotlinRefactoringUtil.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/kotlinRefactoringUtil.kt @@ -149,7 +149,8 @@ fun PsiElement.getUsageContext(): PsiElement { this, KtPropertyAccessor::class.java, KtProperty::class.java, - KtFunction::class.java, + KtNamedFunction::class.java, + KtConstructor::class.java, KtClassOrObject::class.java ) ?: containingFile else -> ConflictsUtil.getContainer(this) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt index d2505ad7946..c10a38dd99c 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt @@ -96,7 +96,8 @@ class MoveDeclarationsDescriptor @JvmOverloads constructor( val deleteSourceFiles: Boolean = false, val moveCallback: MoveCallback? = null, val openInEditor: Boolean = false, - val allElementsToMove: List? = null + val allElementsToMove: List? = null, + val analyzeConflicts: Boolean = true ) class ConflictUsageInfo(element: PsiElement, val messages: Collection) : UsageInfo(element) @@ -226,8 +227,10 @@ class MoveKotlinDeclarationsProcessor( internalUsages += descriptor.delegate.findInternalUsages(descriptor) collectUsages(kotlinToLightElements, externalUsages) - conflictChecker.checkAllConflicts(externalUsages, internalUsages, conflicts) - descriptor.delegate.collectConflicts(descriptor, internalUsages, conflicts) + if (descriptor.analyzeConflicts) { + conflictChecker.checkAllConflicts(externalUsages, internalUsages, conflicts) + descriptor.delegate.collectConflicts(descriptor, internalUsages, conflicts) + } usages += internalUsages usages += externalUsages 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 f5ce7703948..5239506bf37 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 @@ -48,6 +48,8 @@ import org.jetbrains.kotlin.idea.refactoring.getUsageContext import org.jetbrains.kotlin.idea.refactoring.move.KotlinMoveUsage import org.jetbrains.kotlin.idea.search.and import org.jetbrains.kotlin.idea.search.not +import org.jetbrains.kotlin.idea.util.projectStructure.getModule +import org.jetbrains.kotlin.idea.util.projectStructure.module import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.contains @@ -164,11 +166,11 @@ class MoveConflictChecker( // Based on RefactoringConflictsUtil.analyzeModuleConflicts fun analyzeModuleConflictsInUsages(project: Project, usages: Collection, - sourceRoot: VirtualFile, + targetFile: VirtualFile, conflicts: MultiMap) { - val targetModule = ModuleUtilCore.findModuleForFile(sourceRoot, project) ?: return + val targetModule = targetFile.getModule(project) ?: return - val isInTestSources = ModuleRootManager.getInstance(targetModule).fileIndex.isInTestSourceContent(sourceRoot) + val isInTestSources = ModuleRootManager.getInstance(targetModule).fileIndex.isInTestSourceContent(targetFile) NextUsage@ for (usage in usages) { val element = usage.element ?: continue if (PsiTreeUtil.getParentOfType(element, PsiImportStatement::class.java, false) != null) continue @@ -177,9 +179,7 @@ class MoveConflictChecker( val resolveScope = element.resolveScope if (resolveScope.isSearchInModuleContent(targetModule, isInTestSources)) continue - val usageFile = element.containingFile - val usageVFile = usageFile.virtualFile ?: continue - val usageModule = ModuleUtilCore.findModuleForFile(usageVFile, project) ?: continue + val usageModule = element.module ?: continue val scopeDescription = RefactoringUIUtil.getDescription(element.getUsageContext(), true) val referencedElement = (if (usage is MoveRenameUsageInfo) usage.referencedElement else usage.element) ?: error(usage) val message = if (usageModule == targetModule && isInTestSources) { @@ -200,9 +200,9 @@ class MoveConflictChecker( fun checkModuleConflictsInUsages(externalUsages: MutableSet, conflicts: MultiMap) { val newConflicts = MultiMap() - val sourceRoot = moveTarget.targetFile ?: return + val targetFile = moveTarget.targetFile ?: return - analyzeModuleConflictsInUsages(project, externalUsages, sourceRoot, newConflicts) + analyzeModuleConflictsInUsages(project, externalUsages, targetFile, newConflicts) if (!newConflicts.isEmpty) { val referencedElementsToSkip = newConflicts.keySet().mapNotNullTo(HashSet()) { it.namedUnwrappedElement } externalUsages.removeIf { @@ -242,8 +242,8 @@ class MoveConflictChecker( internalUsages: MutableSet, conflicts: MultiMap ) { - val sourceRoot = moveTarget.targetFile ?: return - val targetModule = ModuleUtilCore.findModuleForFile(sourceRoot, project) ?: return + val targetFile = moveTarget.targetFile ?: return + val targetModule = targetFile.getModule(project) ?: return val resolveScope = targetModule.getScopeWithPlatformAwareDependencies() fun isInScope(targetElement: PsiElement, targetDescriptor: DeclarationDescriptor): Boolean { @@ -255,7 +255,7 @@ class MoveConflictChecker( val renderedImportableTarget = DESCRIPTOR_RENDERER_FOR_COMPARISON.render(importableDescriptor) val renderedTarget by lazy { DESCRIPTOR_RENDERER_FOR_COMPARISON.render(targetDescriptor) } - val targetModuleInfo = getModuleInfoByVirtualFile(project, sourceRoot) + val targetModuleInfo = getModuleInfoByVirtualFile(project, targetFile) val dummyFile = KtPsiFactory(targetElement.project).createFile("dummy.kt", "").apply { moduleInfo = targetModuleInfo targetPlatform = TargetPlatformDetector.getPlatform(targetModule) @@ -288,6 +288,8 @@ class MoveConflictChecker( val referencesToSkip = HashSet() for (declaration in elementsToMove - doNotGoIn) { + if (declaration.module == targetModule) continue + declaration.forEachDescendantOfType { refExpr -> val targetDescriptor = refExpr.analyze(BodyResolveMode.PARTIAL)[BindingContext.REFERENCE_TARGET, refExpr] ?: return@forEachDescendantOfType @@ -328,15 +330,19 @@ class MoveConflictChecker( val referencedElement = usage.referencedElement?.namedUnwrappedElement as? KtNamedDeclaration ?: continue val referencedDescriptor = resolutionFacade.resolveToDescriptor(referencedElement) + if (referencedDescriptor is DeclarationDescriptorWithVisibility + && referencedDescriptor.visibility == Visibilities.PUBLIC) continue + val container = element.getUsageContext() if (!declarationToContainers.getOrPut(referencedElement) { HashSet() }.add(container)) continue + val targetContainer = moveTarget.getContainerDescriptor() ?: continue + val referencingDescriptor = when (container) { is KtDeclaration -> container.unsafeResolveToDescriptor() is PsiMember -> container.getJavaMemberDescriptor() else -> null } ?: continue - val targetContainer = moveTarget.getContainerDescriptor() ?: continue val descriptorToCheck = referencedDescriptor.asPredicted(targetContainer) ?: continue if (referencedDescriptor.isVisibleIn(referencingDescriptor) && !descriptorToCheck.isVisibleIn(referencingDescriptor)) { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt index b3c6b3896f6..de3600c69ae 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt @@ -32,6 +32,8 @@ import org.jetbrains.kotlin.idea.refactoring.invokeOnceOnCommandFinish import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.KotlinDirectoryMoveTarget import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.MoveKotlinDeclarationsProcessor import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.analyzeConflictsInFile +import org.jetbrains.kotlin.idea.runSynchronouslyWithProgress +import org.jetbrains.kotlin.idea.util.application.runReadAction import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtFile import java.util.* @@ -68,7 +70,7 @@ class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() { project: Project) { filesToMove .filterIsInstance() - .mapTo(result) { FileUsagesWrapper(it, fileHandler.findUsages(it, null, searchInComments, searchInNonJavaFiles), null) } + .mapTo(result) { FileUsagesWrapper(it, fileHandler.findUsages(it, null, false), null) } } override fun preprocessUsages( @@ -83,8 +85,12 @@ class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() { for ((index, usageInfo) in infos.withIndex()) { if (usageInfo !is FileUsagesWrapper) continue - analyzeConflictsInFile(usageInfo.psiFile, usageInfo.usages, moveTarget, files, conflicts) { - infos[index] = usageInfo.copy(usages = it) + project.runSynchronouslyWithProgress("Analyzing conflicts in ${usageInfo.psiFile.name}", false) { + runReadAction { + analyzeConflictsInFile(usageInfo.psiFile, usageInfo.usages, moveTarget, files, conflicts) { + infos[index] = usageInfo.copy(usages = it) + } + } } } } @@ -103,7 +109,7 @@ class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() { ): Boolean { if (file !is KtFile) return false - val moveDeclarationsProcessor = fileHandler.initMoveProcessor(file, moveDestination) + val moveDeclarationsProcessor = fileHandler.initMoveProcessor(file, moveDestination, false) val moveContextMap = getOrCreateMoveContextMap() moveContextMap[file] = MoveContext(moveDestination, moveDeclarationsProcessor) if (moveDeclarationsProcessor != null) { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/MoveKotlinFileHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/MoveKotlinFileHandler.kt index cbc13a9c125..9dd34706fa2 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/MoveKotlinFileHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/MoveKotlinFileHandler.kt @@ -73,7 +73,7 @@ class MoveKotlinFileHandler : MoveFileHandler() { return ContainerChangeInfo(ContainerInfo.Package(oldPackageName), ContainerInfo.Package(newPackageName.toSafe())) } - fun initMoveProcessor(psiFile: PsiFile, newParent: PsiDirectory?): MoveKotlinDeclarationsProcessor? { + fun initMoveProcessor(psiFile: PsiFile, newParent: PsiDirectory?, withConflicts: Boolean): MoveKotlinDeclarationsProcessor? { if (psiFile !is KtFile) return null val packageNameInfo = psiFile.getPackageNameInfo(newParent, false) ?: return null @@ -98,7 +98,8 @@ class MoveKotlinFileHandler : MoveFileHandler() { moveTarget = moveTarget, delegate = MoveDeclarationsDelegate.TopLevel, scanEntireFile = true, - allElementsToMove = psiFile.allElementsToMove + allElementsToMove = psiFile.allElementsToMove, + analyzeConflicts = withConflicts ), Mover.Idle ) @@ -114,11 +115,19 @@ class MoveKotlinFileHandler : MoveFileHandler() { newParent: PsiDirectory?, searchInComments: Boolean, searchInNonJavaFiles: Boolean + ): List { + return findUsages(psiFile, newParent, true) + } + + fun findUsages( + psiFile: PsiFile, + newParent: PsiDirectory?, + withConflicts: Boolean ): List { if (psiFile !is KtFile) return emptyList() val usages = arrayListOf(FileInfo(psiFile)) - initMoveProcessor(psiFile, newParent)?.let { + initMoveProcessor(psiFile, newParent, withConflicts)?.let { usages += it.findUsages() usages += it.getConflictsAsUsages() } @@ -127,7 +136,7 @@ class MoveKotlinFileHandler : MoveFileHandler() { override fun prepareMovedFile(file: PsiFile, moveDestination: PsiDirectory, oldToNewMap: MutableMap) { if (file !is KtFile) return - val moveProcessor = initMoveProcessor(file, moveDestination) ?: return + val moveProcessor = initMoveProcessor(file, moveDestination, false) ?: return val moveContext = MoveContext(file, moveProcessor) oldToNewMap[moveContext] = moveContext val packageNameInfo = file.getPackageNameInfo(moveDestination, true) ?: return diff --git a/idea/testData/intentions/moveOutOfCompanion/moveFunctionWithExternalRefs.kt b/idea/testData/intentions/moveOutOfCompanion/moveFunctionWithExternalRefs.kt index 050c44335cd..1f289152f1f 100644 --- a/idea/testData/intentions/moveOutOfCompanion/moveFunctionWithExternalRefs.kt +++ b/idea/testData/intentions/moveOutOfCompanion/moveFunctionWithExternalRefs.kt @@ -1,5 +1,5 @@ // WITH_RUNTIME -// SHOULD_FAIL_WITH: 'foo' in class B will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance, 'foo' in lambda <anonymous>() will require class instance +// SHOULD_FAIL_WITH: 'foo' in class B will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance class A { companion object { class B { diff --git a/idea/testData/intentions/moveOutOfCompanion/movePropertyWithExternalRefs.kt b/idea/testData/intentions/moveOutOfCompanion/movePropertyWithExternalRefs.kt index e583ca5ffe8..6b57efdd7b5 100644 --- a/idea/testData/intentions/moveOutOfCompanion/movePropertyWithExternalRefs.kt +++ b/idea/testData/intentions/moveOutOfCompanion/movePropertyWithExternalRefs.kt @@ -1,5 +1,5 @@ // WITH_RUNTIME -// SHOULD_FAIL_WITH: 'foo' in class B will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance, 'foo' in lambda <anonymous>() will require class instance +// SHOULD_FAIL_WITH: 'foo' in class B will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance class A { companion object { class B {