Move: More accurate visibility analysis

#KT-10553 Fixed

compiler/frontend/src/org/jetbrains/kotlin/resolve/source/PsiSourceElement.kt

@@ -27,7 +27,7 @@ interface PsiSourceElement : SourceElement {
     override fun getContainingFile(): SourceFile = psi?.containingFile?.let { PsiSourceFile(it) } ?: SourceFile.NO_SOURCE_FILE
 }
 
-class PsiSourceFile(private val psiFile: PsiFile): SourceFile {
+class PsiSourceFile(val psiFile: PsiFile): SourceFile {
     override fun equals(other: Any?): Boolean = other is PsiSourceFile && psiFile == other.psiFile
     override fun hashCode(): Int = psiFile.hashCode()
 }

idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/ProjectResolutionFacade.kt

@@ -95,7 +95,9 @@ internal class ResolutionFacadeImpl(
     //TODO: ideally we would like to store moduleDescriptor once and for all
     // but there are some usages that use resolutionFacade and mutate the psi leading to recomputation of underlying structures
     override val moduleDescriptor: ModuleDescriptor
-        get() = projectFacade.findModuleDescriptor(moduleInfo)
+        get() = findModuleDescriptor(moduleInfo)
+
+    fun findModuleDescriptor(ideaModuleInfo: IdeaModuleInfo) = projectFacade.findModuleDescriptor(ideaModuleInfo)
 
     override fun analyze(element: KtElement, bodyResolveMode: BodyResolveMode): BindingContext {
         val resolveElementCache = getFrontendService(element, ResolveElementCache::class.java)
@@ -129,3 +131,7 @@ internal class ResolutionFacadeImpl(
     }
 
 }
+
+fun ResolutionFacade.findModuleDescriptor(ideaModuleInfo: IdeaModuleInfo): ModuleDescriptor? {
+    return (this as? ResolutionFacadeImpl)?.findModuleDescriptor(ideaModuleInfo)
+}

idea/src/org/jetbrains/kotlin/idea/findUsages/KotlinElementDescriptionProvider.kt

@@ -39,7 +39,7 @@ class KotlinElementDescriptionProvider : ElementDescriptionProvider {
             is KtClass -> if (targetElement.isInterface()) "interface" else "class"
             is KtObjectDeclaration -> "object"
             is KtNamedFunction -> "function"
-            is KtSecondaryConstructor -> "constructor"
+            is KtPrimaryConstructor, is KtSecondaryConstructor -> "constructor"
             is KtProperty -> if (targetElement.isLocal) "variable" else "property"
             is KtTypeParameter -> "type parameter"
             is KtParameter -> "parameter"

idea/src/org/jetbrains/kotlin/idea/refactoring/move/changePackage/KotlinChangePackageRefactoring.kt

@@ -16,6 +16,7 @@
 
 package org.jetbrains.kotlin.idea.refactoring.move.changePackage
 
+import com.intellij.psi.PsiDirectory
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiFile
 import org.jetbrains.kotlin.idea.codeInsight.shorten.runWithElementsToShortenIsEmptyIgnored
@@ -40,9 +41,11 @@ class KotlinChangePackageRefactoring(val file: KtFile) {
                 project,
                 MoveDeclarationsDescriptor(
                         elementsToMove = file.declarations.filterIsInstance<KtNamedDeclaration>(),
-                        moveTarget = object: KotlinMoveTarget {
+                        moveTarget = object: KotlinDirectoryBasedMoveTarget {
                             override val targetContainerFqName = newFqName
 
+                            override val directory: PsiDirectory = file.containingDirectory!!
+
                             override fun getOrCreateTargetPsi(originalPsi: PsiElement) = originalPsi.containingFile as? KtFile
 
                             override fun getTargetPsiIfExists(originalPsi: PsiElement) = null

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/KotlinMoveTarget.kt

@@ -16,6 +16,7 @@
 
 package org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations
 
+import com.intellij.psi.PsiDirectory
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiFile
 import org.jetbrains.kotlin.name.FqName
@@ -33,6 +34,10 @@ interface KotlinMoveTarget {
     fun verify(file: PsiFile): String?
 }
 
+interface KotlinDirectoryBasedMoveTarget : KotlinMoveTarget {
+    val directory: PsiDirectory?
+}
+
 object EmptyKotlinMoveTarget: KotlinMoveTarget {
     override val targetContainerFqName = null
     override fun getOrCreateTargetPsi(originalPsi: PsiElement) = null
@@ -53,8 +58,9 @@ class KotlinMoveTargetForExistingElement(val targetElement: KtElement): KotlinMo
 
 class KotlinMoveTargetForDeferredFile(
         override val targetContainerFqName: FqName,
+        override val directory: PsiDirectory?,
         private val createFile: (KtFile) -> KtFile?
-): KotlinMoveTarget {
+): KotlinDirectoryBasedMoveTarget {
     private val createdFiles = HashMap<KtFile, KtFile?>()
 
     override fun getOrCreateTargetPsi(originalPsi: PsiElement): KtElement? {

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveDeclarationToSeparateFileIntention.kt

@@ -86,7 +86,7 @@ class MoveDeclarationToSeparateFileIntention :
             }
             return
         }
-        val moveTarget = KotlinMoveTargetForDeferredFile(packageName) {
+        val moveTarget = KotlinMoveTargetForDeferredFile(packageName, directory) {
             createKotlinFile(targetFileName, directory, packageName.asString())
         }
         val moveOptions = MoveDeclarationsDescriptor(

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveDeclarationsDelegate.kt

@@ -29,13 +29,11 @@ import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.refactoring.move.*
 import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
 import org.jetbrains.kotlin.lexer.KtTokens
-import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
 import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelector
 
 sealed class MoveDeclarationsDelegate {
-    abstract fun getOriginalContainerFqName(descriptor: MoveDeclarationsDescriptor, originalFile: KtFile): FqName
     abstract fun getContainerChangeInfo(originalDeclaration: KtNamedDeclaration, moveTarget: KotlinMoveTarget): ContainerChangeInfo
     abstract fun findUsages(descriptor: MoveDeclarationsDescriptor): List<UsageInfo>
     abstract fun collectConflicts(usages: MutableList<UsageInfo>, conflicts: MultiMap<PsiElement, String>)
@@ -43,8 +41,6 @@ sealed class MoveDeclarationsDelegate {
     abstract fun preprocessUsages(project: Project, usages: List<UsageInfo>)
 
     object TopLevel : MoveDeclarationsDelegate() {
-        override fun getOriginalContainerFqName(descriptor: MoveDeclarationsDescriptor, originalFile: KtFile) = originalFile.packageFqName
-
         override fun getContainerChangeInfo(originalDeclaration: KtNamedDeclaration, moveTarget: KotlinMoveTarget): ContainerChangeInfo {
             return ContainerChangeInfo(ContainerInfo.Package(originalDeclaration.getContainingKtFile().packageFqName),
                                        ContainerInfo.Package(moveTarget.targetContainerFqName!!))
@@ -69,10 +65,6 @@ sealed class MoveDeclarationsDelegate {
             val newClassName: String? = null,
             val outerInstanceParameterName: String? = null
     ) : MoveDeclarationsDelegate() {
-        override fun getOriginalContainerFqName(descriptor: MoveDeclarationsDescriptor, originalFile: KtFile): FqName {
-            return descriptor.elementsToMove.first().containingClassOrObject!!.fqName!!
-        }
-
         override fun getContainerChangeInfo(originalDeclaration: KtNamedDeclaration, moveTarget: KotlinMoveTarget): ContainerChangeInfo {
             val originalInfo = ContainerInfo.Class(originalDeclaration.containingClassOrObject!!.fqName!!)
             val movingToClass = (moveTarget as? KotlinMoveTargetForExistingElement)?.targetElement is KtClassOrObject

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt

@@ -17,10 +17,15 @@
 package org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations
 
 import com.intellij.ide.util.EditorHelper
+import com.intellij.openapi.module.ModuleUtilCore
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.roots.ModuleRootManager
 import com.intellij.openapi.util.Ref
 import com.intellij.openapi.util.text.StringUtil
-import com.intellij.psi.*
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiMember
+import com.intellij.psi.PsiNamedElement
+import com.intellij.psi.PsiReference
 import com.intellij.psi.search.searches.ReferencesSearch
 import com.intellij.refactoring.BaseRefactoringProcessor
 import com.intellij.refactoring.move.MoveCallback
@@ -37,35 +42,37 @@ import com.intellij.usageView.UsageViewDescriptor
 import com.intellij.usageView.UsageViewUtil
 import com.intellij.util.IncorrectOperationException
 import com.intellij.util.SmartList
-import com.intellij.util.VisibilityUtil
 import com.intellij.util.containers.MultiMap
 import gnu.trove.THashMap
 import gnu.trove.TObjectHashingStrategy
 import org.jetbrains.kotlin.asJava.KtLightElement
 import org.jetbrains.kotlin.asJava.namedUnwrappedElement
 import org.jetbrains.kotlin.asJava.toLightElements
-import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
+import org.jetbrains.kotlin.caches.resolve.KotlinCacheService
-import org.jetbrains.kotlin.idea.codeInsight.KotlinFileReferencesResolver
+import org.jetbrains.kotlin.descriptors.*
+import org.jetbrains.kotlin.descriptors.impl.MutablePackageFragmentDescriptor
+import org.jetbrains.kotlin.idea.caches.resolve.*
 import org.jetbrains.kotlin.idea.codeInsight.shorten.addToShorteningWaitSet
 import org.jetbrains.kotlin.idea.core.deleteSingle
-import org.jetbrains.kotlin.idea.core.getPackage
 import org.jetbrains.kotlin.idea.refactoring.KotlinRefactoringBundle
 import org.jetbrains.kotlin.idea.refactoring.fqName.getKotlinFqName
 import org.jetbrains.kotlin.idea.refactoring.getUsageContext
-import org.jetbrains.kotlin.idea.refactoring.move.*
+import org.jetbrains.kotlin.idea.refactoring.move.MoveRenameUsageInfoForExtension
+import org.jetbrains.kotlin.idea.refactoring.move.createMoveUsageInfoIfPossible
+import org.jetbrains.kotlin.idea.refactoring.move.getInternalReferencesToUpdateOnPackageNameChange
 import org.jetbrains.kotlin.idea.refactoring.move.moveFilesOrDirectories.MoveKotlinClassHandler
+import org.jetbrains.kotlin.idea.refactoring.move.postProcessMoveUsages
 import org.jetbrains.kotlin.idea.references.KtSimpleNameReference.ShorteningMode
 import org.jetbrains.kotlin.idea.search.projectScope
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.psi.psiUtil.getElementTextWithContext
+import org.jetbrains.kotlin.psi.psiUtil.*
-import org.jetbrains.kotlin.psi.psiUtil.isAncestor
-import org.jetbrains.kotlin.psi.psiUtil.isInsideOf
-import org.jetbrains.kotlin.psi.psiUtil.isPrivate
 import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.DescriptorUtils
+import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
 import org.jetbrains.kotlin.utils.keysToMap
 import java.util.*
 
-interface Mover: (KtNamedDeclaration, KtElement) -> KtNamedDeclaration {
+interface Mover : (KtNamedDeclaration, KtElement) -> KtNamedDeclaration {
     object Default : Mover {
         override fun invoke(originalElement: KtNamedDeclaration, targetContainer: KtElement): KtNamedDeclaration {
             return when (targetContainer) {
@@ -108,6 +115,8 @@ class MoveKotlinDeclarationsProcessor(
             .mapValues { it.value.keysToMap { it.toLightElements() } }
     private val conflicts = MultiMap<PsiElement, String>()
 
+    private val resolutionFacade by lazy { KotlinCacheService.getInstance(project).getResolutionFacade(elementsToMove) }
+
     override fun createUsageViewDescriptor(usages: Array<out UsageInfo>): UsageViewDescriptor {
         val targetContainerFqName = descriptor.moveTarget.targetContainerFqName?.let {
             if (it.isRoot) UsageViewBundle.message("default.package.presentable.name") else it.asString()
@@ -117,6 +126,8 @@ class MoveKotlinDeclarationsProcessor(
 
     private val usagesToProcessBeforeMove = SmartList<UsageInfo>()
 
+    private val fakeFile = KtPsiFactory(project).createFile("")
+
     public override fun findUsages(): Array<UsageInfo> {
         val newContainerName = descriptor.moveTarget.targetContainerFqName?.asString() ?: ""
 
@@ -155,78 +166,138 @@ class MoveKotlinDeclarationsProcessor(
             }
         }
 
-        /*
+        fun PackageFragmentDescriptor.withSource(sourceFile: KtFile): PackageFragmentDescriptor {
-            There must be a conflict if all of the following conditions are satisfied:
+            return object : PackageFragmentDescriptor by this {
-                declaration is package-private
+                override fun getOriginal() = this
-                usage does not belong to target package
+                override fun getSource() = KotlinSourceElement(sourceFile)
-                usage is not being moved together with declaration
+            }
-         */
+        }
 
-        fun collectConflictsInUsages(usages: List<UsageInfo>) {
+        fun DeclarationDescriptor.asPredicted(newContainer: DeclarationDescriptor): DeclarationDescriptor? {
+            val originalVisibility = (this as? DeclarationDescriptorWithVisibility)?.visibility ?: return null
+            val visibility = if (originalVisibility == Visibilities.PROTECTED && newContainer is PackageFragmentDescriptor) {
+                Visibilities.PUBLIC
+            } else {
+                originalVisibility
+            }
+            return when (this) {
+                // We rely on visibility not depending on more specific type of CallableMemberDescriptor
+                is CallableMemberDescriptor -> object : CallableMemberDescriptor by this {
+                    override fun getOriginal() = this
+                    override fun getContainingDeclaration() = newContainer
+                    override fun getVisibility(): Visibility = visibility
+                    override fun getSource() = SourceElement { DescriptorUtils.getContainingSourceFile(newContainer) }
+                }
+                is ClassDescriptor -> object: ClassDescriptor by this {
+                    override fun getOriginal() = this
+                    override fun getContainingDeclaration() = newContainer
+                    override fun getVisibility(): Visibility = visibility
+                    override fun getSource() = SourceElement { DescriptorUtils.getContainingSourceFile(newContainer) }
+                }
+                else -> null
+            }
+        }
+
+        fun KotlinMoveTarget.getContainerDescriptor(): DeclarationDescriptor? {
+            return when (this) {
+                is KotlinMoveTargetForExistingElement -> {
+                    val targetElement = targetElement
+                    when (targetElement) {
+                        is KtNamedDeclaration -> resolutionFacade.resolveToDescriptor(targetElement)
+
+                        is KtFile -> {
+                            val packageFragment = resolutionFacade.analyze(targetElement)[BindingContext.FILE_TO_PACKAGE_FRAGMENT, targetElement]
+                            packageFragment?.withSource(targetElement)
+                        }
+
+                        else -> null
+                    }
+                }
+
+                is KotlinDirectoryBasedMoveTarget -> {
+                    val packageFqName = targetContainerFqName ?: return null
+                    val targetDir = directory
+                    val targetModuleDescriptor = if (targetDir != null) {
+                        val targetModule = ModuleUtilCore.findModuleForPsiElement(targetDir) ?: return null
+                        val moduleFileIndex = ModuleRootManager.getInstance(targetModule).fileIndex
+                        val targetModuleInfo = when {
+                            moduleFileIndex.isInSourceContent(targetDir.virtualFile) -> targetModule.productionSourceInfo()
+                            moduleFileIndex.isInTestSourceContent(targetDir.virtualFile) -> targetModule.testSourceInfo()
+                            else -> return null
+                        }
+                        resolutionFacade.findModuleDescriptor(targetModuleInfo) ?: return null
+                    }
+                    else {
+                        resolutionFacade.moduleDescriptor
+                    }
+                    MutablePackageFragmentDescriptor(targetModuleDescriptor, packageFqName).withSource(fakeFile)
+                }
+
+                else -> null
+            }
+        }
+
+        fun DeclarationDescriptor.isVisibleIn(where: DeclarationDescriptor): Boolean {
+            return when {
+                this !is DeclarationDescriptorWithVisibility -> true
+                !Visibilities.isVisibleWithIrrelevantReceiver(this, where) -> false
+                this is ConstructorDescriptor -> Visibilities.isVisibleWithIrrelevantReceiver(containingDeclaration, where)
+                else -> true
+            }
+        }
+
+        fun render(declaration: PsiElement): String {
+            val text = RefactoringUIUtil.getDescription(declaration, false)
+            return if (declaration is KtFunction) "$text()" else text
+        }
+
+        fun checkVisibilityInUsages(usages: List<UsageInfo>) {
             val declarationToContainers = HashMap<KtNamedDeclaration, MutableSet<PsiElement>>()
             for (usage in usages) {
                 val element = usage.element
                 if (element == null || usage !is MoveRenameUsageInfo || usage is NonCodeUsageInfo) continue
 
-                val declaration = usage.referencedElement?.namedUnwrappedElement as? KtNamedDeclaration
-                if (declaration == null || !declaration.isPrivate()) continue
-
                 if (element.isInsideOf(elementsToMove)) continue
 
+                val referencedElement = usage.referencedElement?.namedUnwrappedElement as? KtNamedDeclaration ?: continue
+                val referencedDescriptor = resolutionFacade.resolveToDescriptor(referencedElement)
+
                 val container = element.getUsageContext()
-                if (!declarationToContainers.getOrPut(declaration) { HashSet<PsiElement>() }.add(container)) continue
+                if (!declarationToContainers.getOrPut(referencedElement) { HashSet<PsiElement>() }.add(container)) continue
 
-                // todo: ok for now, will be replaced by proper visibility analysis
+                val referencingDescriptor = when (container) {
-                val currentPackage = element.containingFile?.containingDirectory?.getPackage()
+                    is KtDeclaration -> container.resolveToDescriptor()
-                if (currentPackage?.qualifiedName == newContainerName) continue
+                    is PsiMember -> container.getJavaMemberDescriptor()
+                    else -> null
+                } ?: continue
+                val targetContainer = descriptor.moveTarget.getContainerDescriptor() ?: continue
+                val descriptorToCheck = referencedDescriptor.asPredicted(targetContainer) ?: continue
 
-                conflicts.putValue(
+                if (!descriptorToCheck.isVisibleIn(referencingDescriptor)) {
-                        declaration,
+                    val message = "${render(container)} uses ${render(referencedElement)} which will be inaccessible after move"
-                        KotlinRefactoringBundle.message(
+                    conflicts.putValue(element, message.capitalize())
-                                "package.private.0.will.no.longer.be.accessible.from.1",
+                }
-                                RefactoringUIUtil.getDescription(declaration, true),
-                                RefactoringUIUtil.getDescription(container, true)
-                        )
-                )
             }
         }
 
-        fun collectConflictsInDeclarations() {
+        fun checkVisibilityInDeclarations() {
-            if (newContainerName == UNKNOWN_PACKAGE_FQ_NAME.asString()) return
+            val targetContainer = descriptor.moveTarget.getContainerDescriptor() ?: return
-
-            val declarationToReferenceTargets = HashMap<KtNamedDeclaration, MutableSet<PsiElement>>()
             for (declaration in elementsToMove) {
-                val referenceToContext = KotlinFileReferencesResolver.resolve(element = declaration, resolveQualifiers = false)
+                declaration.forEachDescendantOfType<KtReferenceExpression> { refExpr ->
-                for ((refExpr, bindingContext) in referenceToContext) {
+                    refExpr.references
-                    val refTarget = bindingContext[BindingContext.REFERENCE_TARGET, refExpr]?.let { descriptor ->
+                            .forEach { ref ->
-                        DescriptorToSourceUtilsIde.getAnyDeclaration(declaration.project, descriptor)
+                                val target = ref.resolve() ?: return@forEach
-                    }
+                                if (target.isInsideOf(elementsToMove)) return@forEach
-                    if (refTarget == null || refTarget.isInsideOf(elementsToMove)) continue
+                                val targetDescriptor = when (target) {
-
+                                    is KtDeclaration -> target.resolveToDescriptor()
-                    val packagePrivate = when(refTarget) {
+                                    is PsiMember -> target.getJavaMemberDescriptor()
-                        is KtModifierListOwner ->
+                                    else -> null
-                            refTarget.isPrivate()
+                                } ?: return@forEach
-                        is PsiModifierListOwner ->
+                                if (!targetDescriptor.isVisibleIn(targetContainer)) {
-                            VisibilityUtil.getVisibilityModifier(refTarget.modifierList) == PsiModifier.PACKAGE_LOCAL
+                                    val message = "${render(declaration)} uses ${render(target)} which will be inaccessible after move"
-                        else -> false
+                                    conflicts.putValue(refExpr, message.capitalize())
-                    }
+                                }
-
+                            }
-                    if (!packagePrivate) continue
-
-                    if (!declarationToReferenceTargets.getOrPut(declaration) { HashSet<PsiElement>() }.add(refTarget)) continue
-
-                    // todo: ok for now, will be replaced by proper visibility analysis
-                    val currentPackage = declaration.containingFile?.containingDirectory?.getPackage()
-                    if (currentPackage?.qualifiedName == newContainerName) continue
-
-                    conflicts.putValue(
-                            declaration,
-                            KotlinRefactoringBundle.message(
-                                    "0.uses.package.private.1",
-                                    RefactoringUIUtil.getDescription(declaration, true),
-                                    RefactoringUIUtil.getDescription(refTarget, true)
-                            ).capitalize()
-                    )
                 }
             }
         }
@@ -244,14 +315,10 @@ class MoveKotlinDeclarationsProcessor(
                 }
             }
 
-            // No need to find and process usages if package is not changed
-            val originalContainerName = descriptor.delegate.getOriginalContainerFqName(descriptor, sourceFile).asString()
-            if (originalContainerName == newContainerName) return UsageInfo.EMPTY_ARRAY
-
             usages += descriptor.delegate.findUsages(descriptor)
             collectUsages(kotlinToLightElements, usages)
-            collectConflictsInUsages(usages)
+            checkVisibilityInUsages(usages)
-            collectConflictsInDeclarations()
+            checkVisibilityInDeclarations()
             descriptor.delegate.collectConflicts(usages, conflicts)
         }
 
@@ -353,3 +420,7 @@ class MoveKotlinDeclarationsProcessor(
 
     override fun getCommandName(): String = REFACTORING_NAME
 }
+
+interface D : DeclarationDescriptorWithSource {
+
+}

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinNestedClassesToUpperLevelDialog.java

@@ -370,6 +370,7 @@ public class MoveKotlinNestedClassesToUpperLevelDialog extends MoveDialogBase {
             );
             moveTarget = new KotlinMoveTargetForDeferredFile(
                     targetPackageFqName,
+                    targetDir,
                     new Function1<KtFile, KtFile>() {
                         @Override
                         public KtFile invoke(@NotNull KtFile originalFile) {

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/ui/MoveKotlinTopLevelDeclarationsDialog.java

@@ -489,7 +489,7 @@ public class MoveKotlinTopLevelDeclarationsDialog extends RefactoringDialog {
         PsiDirectory sourceDirectory = getSourceDirectory(sourceFiles);
 
         if (isMoveToPackage()) {
-            final MoveDestination moveDestination = selectPackageBasedMoveDestination(true);
+            MoveDestination moveDestination = selectPackageBasedMoveDestination(true);
             if (moveDestination == null) return null;
 
             final String targetFileName = sourceFiles.size() > 1 ? null : tfFileNameInPackage.getText();
@@ -532,14 +532,16 @@ public class MoveKotlinTopLevelDeclarationsDialog extends RefactoringDialog {
                 if (ret != Messages.YES) return null;
             }
 
+            // All source files must be in the same directory
+            final PsiDirectory targetDir = moveDestination.getTargetDirectory(sourceFiles.get(0));
             return new KotlinMoveTargetForDeferredFile(
                     new FqName(getTargetPackage()),
+                    targetDir,
                     new Function1<KtFile, KtFile>() {
                         @Override
                         public KtFile invoke(@NotNull KtFile originalFile) {
                             return JetRefactoringUtilKt.getOrCreateKotlinFile(
-                                    targetFileName != null ? targetFileName : originalFile.getName(),
+                                    targetFileName != null ? targetFileName : originalFile.getName(), targetDir
-                                    moveDestination.getTargetDirectory(originalFile)
                             );
                         }
                     }
@@ -570,6 +572,7 @@ public class MoveKotlinTopLevelDeclarationsDialog extends RefactoringDialog {
 
         return new KotlinMoveTargetForDeferredFile(
                 new FqName(psiPackage.getQualifiedName()),
+                psiDirectory,
                 new Function1<KtFile, KtFile>() {
                     @Override
                     public KtFile invoke(@NotNull KtFile originalFile) {

idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/MoveKotlinFileHandler.kt

@@ -74,7 +74,7 @@ class MoveKotlinFileHandler : MoveFileHandler() {
         val moveTarget = when (newPackage) {
             ContainerInfo.UnknownPackage -> EmptyKotlinMoveTarget
 
-            else -> KotlinMoveTargetForDeferredFile(newPackage.fqName!!) {
+            else -> KotlinMoveTargetForDeferredFile(newPackage.fqName!!, newParent) {
                 MoveFilesOrDirectoriesUtil.doMoveFile(psiFile, newParent)
                 newParent?.findFile(psiFile.name) as? KtFile
             }

idea/testData/refactoring/move/kotlin/moveNestedClass/deepPrivateClass/after/main.kt

@@ -0,0 +1,9 @@
+class A {
+    private class B {
+        private class D
+
+        private class C {
+            private val d = D()
+        }
+    }
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/deepPrivateClass/before/main.kt

@@ -0,0 +1,9 @@
+class A {
+    private class B {
+        private class D
+
+        private class <caret>C {
+            private val d = D()
+        }
+    }
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/deepPrivateClass/conflicts.txt

@@ -0,0 +1 @@
+Class C uses class D which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveNestedClass/deepPrivateClass/deepPrivateClass.test

@@ -0,0 +1,6 @@
+{
+  "mainFile": "main.kt",
+  "type": "MOVE_KOTLIN_NESTED_CLASS",
+  "targetClass": "A",
+  "withRuntime": "true"
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/deepProtectedClass/after/main.kt

@@ -0,0 +1,13 @@
+class A {
+    open class B {
+        protected class D
+
+        protected class C {
+            private val d = D()
+        }
+    }
+}
+
+class X : A.B() {
+    private val c = A.B.C()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/deepProtectedClass/before/main.kt

@@ -0,0 +1,13 @@
+class A {
+    open class B {
+        protected class D
+
+        protected class <caret>C {
+            private val d = D()
+        }
+    }
+}
+
+class X : A.B() {
+    private val c = A.B.C()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/deepProtectedClass/conflicts.txt

@@ -0,0 +1,2 @@
+Class C uses class D which will be inaccessible after move
+Property c uses class C which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveNestedClass/deepProtectedClass/deepProtectedClass.test

@@ -0,0 +1,6 @@
+{
+  "mainFile": "main.kt",
+  "type": "MOVE_KOTLIN_NESTED_CLASS",
+  "targetClass": "A",
+  "withRuntime": "true"
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/privateClass/after/main.kt

@@ -0,0 +1,9 @@
+class A {
+    private val a = B()
+
+    private class B {
+        private val c = C()
+    }
+
+    private class C()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/privateClass/before/main.kt

@@ -0,0 +1,9 @@
+class A {
+    private val a = B()
+
+    private class <caret>B {
+        private val c = C()
+    }
+
+    private class C()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/privateClass/conflicts.txt

@@ -0,0 +1,2 @@
+Class B uses constructor C() which will be inaccessible after move
+Property a uses class B which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveNestedClass/privateClass/privateClass.test

@@ -0,0 +1,5 @@
+{
+  "mainFile": "main.kt",
+  "type": "MOVE_KOTLIN_NESTED_CLASS",
+  "withRuntime": "true"
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClass/after/main.kt

@@ -0,0 +1,13 @@
+open class A {
+    private val a = B()
+
+    protected class B {
+        private val c = C()
+    }
+
+    protected class C()
+}
+
+class X : A() {
+    private val b = B()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClass/before/main.kt

@@ -0,0 +1,13 @@
+open class A {
+    private val a = B()
+
+    protected class <caret>B {
+        private val c = C()
+    }
+
+    protected class C()
+}
+
+class X : A() {
+    private val b = B()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClass/conflicts.txt

@@ -0,0 +1 @@
+Class B uses constructor C() which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClass/protectedClass.test

@@ -0,0 +1,5 @@
+{
+  "mainFile": "main.kt",
+  "type": "MOVE_KOTLIN_NESTED_CLASS",
+  "withRuntime": "true"
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClassNoConflicts/after/B.kt

@@ -0,0 +1,3 @@
+class B {
+    private val c = A.C()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClassNoConflicts/after/main.kt

@@ -0,0 +1,9 @@
+open class A {
+    private val a = B()
+
+    class C()
+}
+
+class X : A() {
+    private val b = B()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClassNoConflicts/before/main.kt

@@ -0,0 +1,13 @@
+open class A {
+    private val a = B()
+
+    protected class <caret>B {
+        private val c = C()
+    }
+
+    class C()
+}
+
+class X : A() {
+    private val b = B()
+}

idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClassNoConflicts/protectedClass.test

@@ -0,0 +1,5 @@
+{
+  "mainFile": "main.kt",
+  "type": "MOVE_KOTLIN_NESTED_CLASS",
+  "withRuntime": "true"
+}

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveClassToPackageWithConflicts/conflicts.txt

@@ -1,4 +1,4 @@
-Class a.Test uses package-private class a.Foo
+Class Test uses class Foo which will be inaccessible after move
-Class a.Test uses package-private function a.foo
+Class Test uses function foo() which will be inaccessible after move
-Package-private class a.Test will no longer be accessible from method J.bar()
+Method bar() uses class Test which will be inaccessible after move
-Package-private class a.Test will no longer be accessible from variable a.bar.t
+Variable t uses class Test which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToPackageWithConflicts/conflicts.txt

@@ -1,4 +1,4 @@
-Function a.test uses package-private class a.Foo
+Function bar() uses function test() which will be inaccessible after move
-Function a.test uses package-private function a.foo
+Function test() uses class Foo which will be inaccessible after move
-Package-private function a.test will no longer be accessible from function a.bar
+Function test() uses function foo() which will be inaccessible after move
-Package-private function a.test will no longer be accessible from method J.bar()
+Method bar() uses function test() which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveObjectToPackageWithConflicts/conflicts.txt

@@ -1,4 +1,4 @@
-Object a.Test uses package-private class a.Foo
+Method bar() uses object Test which will be inaccessible after move
-Object a.Test uses package-private function a.foo
+Object Test uses class Foo which will be inaccessible after move
-Package-private object a.Test will no longer be accessible from method J.bar()
+Object Test uses function foo() which will be inaccessible after move
-Package-private object a.Test will no longer be accessible from variable a.bar.t
+Variable t uses object Test which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateFun/after/main.kt

@@ -0,0 +1,9 @@
+package a
+
+private fun foo() {
+    bar()
+}
+
+private fun bar() {
+    foo()
+}

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateFun/after/utils.kt

@@ -0,0 +1,3 @@
+package a
+
+object Utils

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateFun/before/main.kt

@@ -0,0 +1,9 @@
+package a
+
+private fun <caret>foo() {
+    bar()
+}
+
+private fun bar() {
+    foo()
+}

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateFun/before/utils.kt

@@ -0,0 +1,3 @@
+package a
+
+object Utils

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateFun/conflicts.txt

@@ -0,0 +1,2 @@
+Function bar() uses function foo() which will be inaccessible after move
+Function foo() uses function bar() which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateFun/movePrivateFun.test

@@ -0,0 +1,5 @@
+{
+  "mainFile": "main.kt",
+  "type": "MOVE_KOTLIN_TOP_LEVEL_DECLARATIONS",
+  "targetFile": "utils.kt"
+}

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateProperty/after/main.kt

@@ -0,0 +1,7 @@
+package a
+
+private val foo: Int
+    get() = bar
+
+private val bar: Int
+    get() = foo

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateProperty/after/utils.kt

@@ -0,0 +1,3 @@
+package a
+
+object Utils

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateProperty/before/main.kt

@@ -0,0 +1,7 @@
+package a
+
+private val <caret>foo: Int
+    get() = bar
+
+private val bar: Int
+    get() = foo

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateProperty/before/utils.kt

@@ -0,0 +1,3 @@
+package a
+
+object Utils

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateProperty/conflicts.txt

@@ -0,0 +1,2 @@
+Property bar uses property foo which will be inaccessible after move
+Property foo uses property bar which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateProperty/movePrivateProperty.test

@@ -0,0 +1,5 @@
+{
+  "mainFile": "main.kt",
+  "type": "MOVE_KOTLIN_TOP_LEVEL_DECLARATIONS",
+  "targetFile": "utils.kt"
+}

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToPackageWithConflicts/conflicts.txt

@@ -1,4 +1,4 @@
-Package-private property a.test will no longer be accessible from function a.bar
+Function bar() uses property test which will be inaccessible after move
-Package-private property a.test will no longer be accessible from method J.bar()
+Method bar() uses property test which will be inaccessible after move
-Property a.test uses package-private class a.Foo
+Property test uses class Foo which will be inaccessible after move
-Property a.test uses package-private function a.foo
+Property test uses function foo() which will be inaccessible after move

idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt

@@ -49,6 +49,7 @@ import org.jetbrains.kotlin.idea.stubindex.KotlinFullClassNameIndex
 import org.jetbrains.kotlin.idea.test.ConfigLibraryUtil
 import org.jetbrains.kotlin.idea.test.KotlinMultiFileTestCase
 import org.jetbrains.kotlin.idea.test.PluginTestCaseBase
+import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.KtClassOrObject
 import org.jetbrains.kotlin.psi.KtFile
@@ -249,8 +250,8 @@ enum class MoveAction {
             val elementToMove = elementAtCaret!!.getNonStrictParentOfType<KtNamedDeclaration>()!!
 
             val moveTarget = config.getNullableString("targetPackage")?.let { packageName ->
-                KotlinMoveTargetForDeferredFile(FqName(packageName)) {
+                val moveDestination = MultipleRootsMoveDestination(PackageWrapper(mainFile.getManager(), packageName))
-                    val moveDestination = MultipleRootsMoveDestination(PackageWrapper(mainFile.getManager(), packageName))
+                KotlinMoveTargetForDeferredFile(FqName(packageName), moveDestination.getTargetIfExists(mainFile)) {
                     createKotlinFile(guessNewFileName(listOf(elementToMove))!!, moveDestination.getTargetDirectory(mainFile))
                 }
             } ?: config.getString("targetFile").let { filePath ->
@@ -296,8 +297,9 @@ enum class MoveAction {
                     else {
                         val fileName = (delegate.newClassName ?: elementToMove.name!!) + ".kt"
                         val targetPackageFqName = (mainFile as KtFile).packageFqName
-                        KotlinMoveTargetForDeferredFile(targetPackageFqName) {
+                        val targetDir = mainFile.containingDirectory!!
-                            createKotlinFile(fileName, mainFile.containingDirectory!!, targetPackageFqName.asString())
+                        KotlinMoveTargetForDeferredFile(targetPackageFqName, targetDir) {
+                            createKotlinFile(fileName, targetDir, targetPackageFqName.asString())
                         }
                     }
             val descriptor = MoveDeclarationsDescriptor(listOf(elementToMove), moveTarget, delegate)

idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java

@@ -293,6 +293,18 @@ public class MoveTestGenerated extends AbstractMoveTest {
         doTest(fileName);
     }
 
+    @TestMetadata("kotlin/moveNestedClass/deepPrivateClass/deepPrivateClass.test")
+    public void testKotlin_moveNestedClass_deepPrivateClass_DeepPrivateClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveNestedClass/deepPrivateClass/deepPrivateClass.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("kotlin/moveNestedClass/deepProtectedClass/deepProtectedClass.test")
+    public void testKotlin_moveNestedClass_deepProtectedClass_DeepProtectedClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveNestedClass/deepProtectedClass/deepProtectedClass.test");
+        doTest(fileName);
+    }
+
     @TestMetadata("kotlin/moveNestedClass/innerToTopLevelNoThis/innerToTopLevelNoThis.test")
     public void testKotlin_moveNestedClass_innerToTopLevelNoThis_InnerToTopLevelNoThis() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveNestedClass/innerToTopLevelNoThis/innerToTopLevelNoThis.test");
@@ -347,6 +359,24 @@ public class MoveTestGenerated extends AbstractMoveTest {
         doTest(fileName);
     }
 
+    @TestMetadata("kotlin/moveNestedClass/privateClass/privateClass.test")
+    public void testKotlin_moveNestedClass_privateClass_PrivateClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveNestedClass/privateClass/privateClass.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("kotlin/moveNestedClass/protectedClass/protectedClass.test")
+    public void testKotlin_moveNestedClass_protectedClass_ProtectedClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClass/protectedClass.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("kotlin/moveNestedClass/protectedClassNoConflicts/protectedClass.test")
+    public void testKotlin_moveNestedClass_protectedClassNoConflicts_ProtectedClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveNestedClass/protectedClassNoConflicts/protectedClass.test");
+        doTest(fileName);
+    }
+
     @TestMetadata("kotlin/movePackage/movePackage/movePackage.test")
     public void testKotlin_movePackage_movePackage_MovePackage() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/movePackage/movePackage/movePackage.test");
@@ -503,6 +533,18 @@ public class MoveTestGenerated extends AbstractMoveTest {
         doTest(fileName);
     }
 
+    @TestMetadata("kotlin/moveTopLevelDeclarations/movePrivateFun/movePrivateFun.test")
+    public void testKotlin_moveTopLevelDeclarations_movePrivateFun_MovePrivateFun() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateFun/movePrivateFun.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("kotlin/moveTopLevelDeclarations/movePrivateProperty/movePrivateProperty.test")
+    public void testKotlin_moveTopLevelDeclarations_movePrivateProperty_MovePrivateProperty() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePrivateProperty/movePrivateProperty.test");
+        doTest(fileName);
+    }
+
     @TestMetadata("kotlin/moveTopLevelDeclarations/movePropertyToFile/movePropertyToFile.test")
     public void testKotlin_moveTopLevelDeclarations_movePropertyToFile_MovePropertyToFile() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToFile/movePropertyToFile.test");