Move: Warn about accessibility conflicts when moving entire file

#KT-13175 Fixed

ChangeLog.md

@@ -265,6 +265,7 @@ Using 'this' as function argument in constructor of non-final class
 - [`KT-13282`](https://youtrack.jetbrains.com/issue/KT-13282), [`KT-13283`](https://youtrack.jetbrains.com/issue/KT-13283) Rename: Fix name quoting for automatic renamers
 - [`KT-13239`](https://youtrack.jetbrains.com/issue/KT-13239) Rename: Warn about function name conflicts
 - [`KT-13174`](https://youtrack.jetbrains.com/issue/KT-13174) Move: Warn about accessibility conflicts due to moving to unrelated module
+- [`KT-13175`](https://youtrack.jetbrains.com/issue/KT-13175) Move: Warn about accessibility conflicts when moving entire file
 
 ##### New features
 

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

@@ -0,0 +1,100 @@
+/*
+ * Copyright 2010-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations
+
+import com.intellij.openapi.project.Project
+import com.intellij.openapi.util.EmptyRunnable
+import com.intellij.openapi.util.Ref
+import com.intellij.psi.PsiDirectory
+import com.intellij.psi.PsiElement
+import com.intellij.refactoring.move.MoveCallback
+import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesProcessor
+import com.intellij.usageView.UsageInfo
+import com.intellij.usageView.UsageViewUtil
+import com.intellij.util.containers.MultiMap
+import com.intellij.util.text.UniqueNameGenerator
+import org.jetbrains.kotlin.psi.KtFile
+
+class MoveFilesWithDeclarationsProcessor(
+        project: Project,
+        private val sourceFiles: List<KtFile>,
+        targetDirectory: PsiDirectory,
+        private val targetFileName: String?,
+        searchInComments: Boolean,
+        searchInNonJavaFiles: Boolean,
+        moveCallback: MoveCallback?
+) : MoveFilesOrDirectoriesProcessor(project,
+                                    sourceFiles.toTypedArray<PsiElement>(),
+                                    targetDirectory,
+                                    true,
+                                    searchInComments,
+                                    searchInNonJavaFiles,
+                                    MoveCallbackImpl(sourceFiles, targetFileName, moveCallback),
+                                    EmptyRunnable.INSTANCE) {
+    class MoveCallbackImpl(
+            private val sourceFiles: List<KtFile>,
+            private val targetFileName: String?,
+            private val nextCallback: MoveCallback?
+    ) : MoveCallback {
+        override fun refactoringCompleted() {
+            try {
+                if (targetFileName != null) {
+                    sourceFiles.single().name = targetFileName
+                }
+            }
+            finally {
+                nextCallback?.refactoringCompleted()
+            }
+        }
+    }
+
+    override fun getCommandName(): String {
+        return if (targetFileName != null) "Move " + sourceFiles.single().name else "Move"
+    }
+
+    override fun preprocessUsages(refUsages: Ref<Array<UsageInfo>>): Boolean {
+        val usages = refUsages.get()
+
+        val distinctConflictUsages = UsageViewUtil.removeDuplicatedUsages(usages.filterIsInstance<ConflictUsageInfo>().toTypedArray())
+        val conflicts = MultiMap<PsiElement, String>()
+        for (conflictUsage in distinctConflictUsages) {
+            conflicts.putValues(conflictUsage.element, (conflictUsage as ConflictUsageInfo).messages)
+        }
+
+        return showConflicts(conflicts, usages)
+    }
+
+    // Assign a temporary name to file-under-move to avoid naming conflict during the refactoring
+    private fun renameFileTemporarily() {
+        if (targetFileName != null) {
+            val sourceFile = sourceFiles.single()
+            //noinspection ConstantConditions
+            val temporaryName = UniqueNameGenerator.generateUniqueName(
+                    "temp",
+                    "",
+                    ".kt",
+                    sourceFile.containingDirectory!!.files.map { file -> file.name })
+            sourceFile.name = temporaryName
+        }
+    }
+
+    override fun performRefactoring(usages: Array<UsageInfo>) {
+        renameFileTemporarily()
+
+        super.performRefactoring(usages)
+    }
+}

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

@@ -97,6 +97,8 @@ class MoveDeclarationsDescriptor(
         val openInEditor: Boolean = false
 )
 
+class ConflictUsageInfo(element: PsiElement, val messages: Collection<String>) : UsageInfo(element)
+
 class MoveKotlinDeclarationsProcessor(
         val project: Project,
         val descriptor: MoveDeclarationsDescriptor,
@@ -128,6 +130,8 @@ class MoveKotlinDeclarationsProcessor(
 
     private val fakeFile = KtPsiFactory(project).createFile("")
 
+    fun getConflictsAsUsages(): List<UsageInfo> = conflicts.entrySet().map { ConflictUsageInfo(it.key, it.value) }
+
     public override fun findUsages(): Array<UsageInfo> {
         val newContainerName = descriptor.moveTarget.targetContainerFqName?.asString() ?: ""
 

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

@@ -27,7 +27,6 @@ import com.intellij.openapi.roots.JavaProjectRootsUtil;
 import com.intellij.openapi.ui.Messages;
 import com.intellij.openapi.ui.TextFieldWithBrowseButton;
 import com.intellij.openapi.util.Comparing;
-import com.intellij.openapi.util.EmptyRunnable;
 import com.intellij.openapi.util.Pair;
 import com.intellij.openapi.util.Pass;
 import com.intellij.openapi.util.text.StringUtil;
@@ -46,19 +45,15 @@ import com.intellij.refactoring.move.moveClassesOrPackages.AutocreatingSingleSou
 import com.intellij.refactoring.move.moveClassesOrPackages.DestinationFolderComboBox;
 import com.intellij.refactoring.move.moveClassesOrPackages.MoveClassesOrPackagesUtil;
 import com.intellij.refactoring.move.moveClassesOrPackages.MultipleRootsMoveDestination;
-import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesProcessor;
 import com.intellij.refactoring.ui.PackageNameReferenceEditorCombo;
 import com.intellij.refactoring.ui.RefactoringDialog;
 import com.intellij.refactoring.util.CommonRefactoringUtil;
 import com.intellij.ui.ComboboxWithBrowseButton;
 import com.intellij.ui.RecentsManager;
 import com.intellij.ui.ReferenceEditorComboWithBrowseButton;
-import com.intellij.usageView.UsageInfo;
 import com.intellij.util.Function;
 import com.intellij.util.IncorrectOperationException;
-import com.intellij.util.text.UniqueNameGenerator;
 import com.intellij.util.ui.UIUtil;
-import kotlin.collections.ArraysKt;
 import kotlin.collections.CollectionsKt;
 import kotlin.jvm.functions.Function0;
 import kotlin.jvm.functions.Function1;
@@ -714,7 +709,7 @@ public class MoveKotlinTopLevelDeclarationsDialog extends RefactoringDialog {
         saveRefactoringSettings();
 
         List<KtNamedDeclaration> elementsToMove = getSelectedElementsToMove();
-        final List<KtFile> sourceFiles = getSourceFiles(elementsToMove);
+        List<KtFile> sourceFiles = getSourceFiles(elementsToMove);
         final PsiDirectory sourceDirectory = getSourceDirectory(sourceFiles);
 
         for (PsiElement element : elementsToMove) {
@@ -735,7 +730,7 @@ public class MoveKotlinTopLevelDeclarationsDialog extends RefactoringDialog {
                     final MoveDestination moveDestination = sourceRootWithMoveDestination.getSecond();
 
                     PsiDirectory targetDir = moveDestination.getTargetIfExists(sourceDirectory);
-                    final String targetFileName = sourceFiles.size() > 1 ? null : tfFileNameInPackage.getText();
+                    String targetFileName = sourceFiles.size() > 1 ? null : tfFileNameInPackage.getText();
                     List<PsiFile> filesExistingInTargetDir = getFilesExistingInTargetDir(sourceFiles, targetFileName, targetDir);
                     if (filesExistingInTargetDir.isEmpty()) {
                         PsiDirectory targetDirectory = ApplicationUtilsKt.runWriteAction(
@@ -752,60 +747,13 @@ public class MoveKotlinTopLevelDeclarationsDialog extends RefactoringDialog {
                         }
 
                         invokeRefactoring(
-                                new MoveFilesOrDirectoriesProcessor(
+                                new MoveFilesWithDeclarationsProcessor(myProject,
-                                        myProject,
+                                                                       sourceFiles,
-                                        sourceFiles.toArray(new PsiElement[sourceFiles.size()]),
+                                                                       targetDirectory,
-                                        targetDirectory,
+                                                                       targetFileName,
-                                        true,
+                                                                       isSearchInComments(),
-                                        isSearchInComments(),
+                                                                       isSearchInNonJavaFiles(),
-                                        isSearchInNonJavaFiles(),
+                                                                       moveCallback)
-                                        new MoveCallback() {
-                                            @Override
-                                            public void refactoringCompleted() {
-                                                try {
-                                                    if (targetFileName != null) {
-                                                        CollectionsKt.single(sourceFiles).setName(targetFileName);
-                                                    }
-                                                }
-                                                finally {
-                                                    if (moveCallback != null) {
-                                                        moveCallback.refactoringCompleted();
-                                                    }
-                                                }
-                                            }
-                                        },
-                                        EmptyRunnable.INSTANCE
-                                ) {
-                                    @Override
-                                    protected String getCommandName() {
-                                        return targetFileName != null ? "Move " + CollectionsKt.single(sourceFiles).getName() : "Move";
-                                    }
-
-                                    @Override
-                                    protected void performRefactoring(@NotNull UsageInfo[] usages) {
-                                        if (targetFileName != null) {
-                                            KtFile sourceFile = CollectionsKt.single(sourceFiles);
-                                            //noinspection ConstantConditions
-                                            String temporaryName = UniqueNameGenerator.generateUniqueName(
-                                                    "temp",
-                                                    "",
-                                                    ".kt",
-                                                    ArraysKt.map(
-                                                            sourceFile.getContainingDirectory().getFiles(),
-                                                            new Function1<PsiFile, String>() {
-                                                                @Override
-                                                                public String invoke(PsiFile file) {
-                                                                    return file.getName();
-                                                                }
-                                                            }
-                                                    )
-                                            );
-                                            sourceFile.setName(temporaryName);
-                                        }
-
-                                        super.performRefactoring(usages);
-                                    }
-                                }
                         );
 
                         return;

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

@@ -16,6 +16,7 @@
 
 package org.jetbrains.kotlin.idea.refactoring.move.moveFilesOrDirectories
 
+import com.intellij.openapi.module.ModuleUtilCore
 import com.intellij.openapi.roots.JavaProjectRootsUtil
 import com.intellij.psi.*
 import com.intellij.psi.impl.light.LightElement
@@ -26,10 +27,10 @@ import org.jetbrains.kotlin.idea.KotlinLanguage
 import org.jetbrains.kotlin.idea.codeInsight.shorten.runWithElementsToShortenIsEmptyIgnored
 import org.jetbrains.kotlin.idea.core.getPackage
 import org.jetbrains.kotlin.idea.core.packageMatchesDirectory
+import org.jetbrains.kotlin.idea.core.quoteIfNeeded
 import org.jetbrains.kotlin.idea.refactoring.hasIdentifiersOnly
 import org.jetbrains.kotlin.idea.refactoring.move.*
 import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.*
-import org.jetbrains.kotlin.idea.core.quoteIfNeeded
 import org.jetbrains.kotlin.name.FqNameUnsafe
 import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.psi.KtNamedDeclaration
@@ -59,7 +60,8 @@ class MoveKotlinFileHandler : MoveFileHandler() {
                                                                                ContainerInfo.UnknownPackage)
 
         val newPackageName = FqNameUnsafe(newPackage.qualifiedName)
-        if (oldPackageName.asString() == newPackageName.asString()) return null
+        if (oldPackageName.asString() == newPackageName.asString()
+            && ModuleUtilCore.findModuleForPsiElement(this) == ModuleUtilCore.findModuleForPsiElement(newParent)) return null
         if (!newPackageName.hasIdentifiersOnly()) return null
 
         return ContainerChangeInfo(ContainerInfo.Package(oldPackageName), ContainerInfo.Package(newPackageName.toSafe()))
@@ -114,7 +116,10 @@ class MoveKotlinFileHandler : MoveFileHandler() {
         if (psiFile !is KtFile) return emptyList()
 
         val usages = ArrayList<UsageInfo>()
-        initMoveProcessor(psiFile, newParent)?.findUsages()?.let { usages += it }
+        initMoveProcessor(psiFile, newParent)?.let {
+            usages += it.findUsages()
+            usages += it.getConflictsAsUsages()
+        }
         newParent?.let { usages += findInternalUsages(psiFile, it) }
         return usages
     }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/after/A/A.iml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/after/A/src/a1/internalTarget.kt

@@ -0,0 +1,3 @@
+package a1
+
+internal val internalTargetVal = 0

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/after/A/src/a2/internalSource.kt

@@ -0,0 +1,5 @@
+package a2
+
+import a1.internalTargetVal
+
+internal val sourceVal = internalTargetVal

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/after/B/B.iml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/after/B/src/b/test.kt

@@ -0,0 +1,3 @@
+package b
+
+class X

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/before/A/A.iml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/before/A/src/a1/internalTarget.kt

@@ -0,0 +1,3 @@
+package a1
+
+internal val <caret>internalTargetVal = 0

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/before/A/src/a2/internalSource.kt

@@ -0,0 +1,5 @@
+package a2
+
+import a1.internalTargetVal
+
+internal val sourceVal = internalTargetVal

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/before/B/B.iml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/before/B/src/b/test.kt

@@ -0,0 +1,3 @@
+package b
+
+class X

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/conflicts.txt

@@ -0,0 +1,3 @@
+Property a1.internalTargetVal, referenced in file internalSource.kt, will not be accessible from module A
+Property a1.internalTargetVal, referenced in file internalSource.kt, will not be accessible from module A
+Property sourceVal uses property internalTargetVal which will be inaccessible after move

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/moveFileWithDeclarationsToUnrelatedModuleConflict.test

@@ -0,0 +1,7 @@
+{
+  "mainFile": "A/src/a1/internalTarget.kt",
+  "type": "MOVE_FILES_WITH_DECLARATIONS",
+  "targetDirectory": "B/src/b",
+  "isMultiModule": "true",
+  "withRuntime": "true"
+}

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

@@ -273,6 +273,24 @@ enum class MoveAction {
         }
     },
 
+    MOVE_FILES_WITH_DECLARATIONS {
+        override fun runRefactoring(rootDir: VirtualFile, mainFile: PsiFile, elementAtCaret: PsiElement?, config: JsonObject) {
+            val project = mainFile.project
+
+            val targetDirPath = config.getString("targetDirectory")
+            val targetDir = rootDir.findFileByRelativePath(targetDirPath)!!.toPsiDirectory(project)!!
+            MoveFilesWithDeclarationsProcessor(
+                    project,
+                    listOf(mainFile as KtFile),
+                    targetDir,
+                    mainFile.name,
+                    searchInComments = true,
+                    searchInNonJavaFiles = true,
+                    moveCallback = null
+            ).run()
+        }
+    },
+
     MOVE_KOTLIN_TOP_LEVEL_DECLARATIONS {
         override fun runRefactoring(rootDir: VirtualFile, mainFile: PsiFile, elementAtCaret: PsiElement?, config: JsonObject) {
             val project = mainFile.project

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

@@ -425,6 +425,12 @@ public class MoveTestGenerated extends AbstractMoveTest {
         doTest(fileName);
     }
 
+    @TestMetadata("kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/moveFileWithDeclarationsToUnrelatedModuleConflict.test")
+    public void testKotlin_moveTopLevelDeclarations_misc_moveFileWithDeclarationsToUnrelatedModuleConflict_MoveFileWithDeclarationsToUnrelatedModuleConflict() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFileWithDeclarationsToUnrelatedModuleConflict/moveFileWithDeclarationsToUnrelatedModuleConflict.test");
+        doTest(fileName);
+    }
+
     @TestMetadata("kotlin/moveTopLevelDeclarations/misc/moveFromDefaultPackage/moveFromDefaultPackage.test")
     public void testKotlin_moveTopLevelDeclarations_misc_moveFromDefaultPackage_MoveFromDefaultPackage() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFromDefaultPackage/moveFromDefaultPackage.test");