Skip to content

Commit a80fb6f

Browse files
asedunovasedunov
committed
Copy: Report module accessibility conflicts
1 parent bfb3b38 commit a80fb6f

File tree

30 files changed

+363
-48
lines changed

30 files changed

+363
-48
lines changed

‎generators/src/org/jetbrains/kotlin/generators/tests/GenerateTests.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ import org.jetbrains.kotlin.idea.quickfix.AbstractQuickFixMultiFileTest
116116
import org.jetbrains.kotlin.idea.quickfix.AbstractQuickFixTest
117117
import org.jetbrains.kotlin.idea.refactoring.AbstractNameSuggestionProviderTest
118118
import org.jetbrains.kotlin.idea.refactoring.copy.AbstractCopyTest
119+
import org.jetbrains.kotlin.idea.refactoring.copy.AbstractMultiModuleCopyTest
119120
import org.jetbrains.kotlin.idea.refactoring.inline.AbstractInlineTest
120121
import org.jetbrains.kotlin.idea.refactoring.introduce.AbstractExtractionTest
121122
import org.jetbrains.kotlin.idea.refactoring.move.AbstractMoveTest
@@ -759,6 +760,10 @@ fun main(args: Array<String>) {
759760
model("refactoring/moveMultiModule", extension = "test", singleClass = true)
760761
}
761762

763+
testClass<AbstractMultiModuleCopyTest> {
764+
model("refactoring/copyMultiModule", extension = "test", singleClass = true)
765+
}
766+
762767
testClass<AbstractMultiFileIntentionTest> {
763768
model("multiFileIntentions", extension = "test", singleClass = true, filenameStartsLowerCase = true)
764769
}

‎idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/ProjectRootsUtil.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@ import com.intellij.injected.editor.VirtualFileWindow
2222
import com.intellij.openapi.project.Project
2323
import com.intellij.openapi.roots.FileIndex
2424
import com.intellij.openapi.roots.ProjectFileIndex
25+
import com.intellij.openapi.roots.ProjectRootManager
2526
import com.intellij.openapi.util.Ref
2627
import com.intellij.openapi.vfs.VirtualFile
2728
import com.intellij.psi.PsiDirectory
2829
import com.intellij.psi.PsiElement
30+
import com.intellij.psi.PsiFileSystemItem
2931
import org.jetbrains.kotlin.idea.KotlinModuleFileType
3032
import org.jetbrains.kotlin.idea.caches.resolve.JsProjectDetector
3133
import org.jetbrains.kotlin.idea.core.script.KotlinScriptConfigurationManager
@@ -40,6 +42,11 @@ fun FileIndex.isInSourceContentWithoutInjected(file: VirtualFile): Boolean {
4042
return file !is VirtualFileWindow && isInSourceContent(file)
4143
}
4244

45+
fun VirtualFile.getSourceRoot(project: Project): VirtualFile? = ProjectRootManager.getInstance(project).fileIndex.getSourceRootForFile(this)
46+
47+
val PsiFileSystemItem.sourceRoot: VirtualFile?
48+
get() = virtualFile?.getSourceRoot(project)
49+
4350
object ProjectRootsUtil {
4451
@JvmStatic fun isInContent(project: Project, file: VirtualFile, includeProjectSource: Boolean,
4552
includeLibrarySource: Boolean, includeLibraryClasses: Boolean,

‎idea/src/org/jetbrains/kotlin/idea/refactoring/copy/CopyKotlinDeclarationDialog.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
*/
1616
package org.jetbrains.kotlin.idea.refactoring.copy
1717

18+
import com.intellij.ide.util.DirectoryChooser
1819
import com.intellij.openapi.project.Project
1920
import com.intellij.openapi.roots.JavaProjectRootsUtil
2021
import com.intellij.openapi.ui.DialogWrapper
2122
import com.intellij.openapi.ui.Messages
23+
import com.intellij.openapi.vfs.VirtualFile
2224
import com.intellij.psi.PsiDirectory
2325
import com.intellij.psi.PsiManager
2426
import com.intellij.refactoring.HelpID
@@ -39,6 +41,7 @@ import org.jetbrains.annotations.NonNls
3941
import org.jetbrains.kotlin.idea.core.getPackage
4042
import org.jetbrains.kotlin.idea.refactoring.Pass
4143
import org.jetbrains.kotlin.idea.refactoring.hasIdentifiersOnly
44+
import org.jetbrains.kotlin.idea.util.sourceRoot
4245
import org.jetbrains.kotlin.name.FqNameUnsafe
4346
import org.jetbrains.kotlin.psi.KtNamedDeclaration
4447
import java.awt.BorderLayout
@@ -68,6 +71,9 @@ class CopyKotlinDeclarationDialog(
6871
var targetDirectory: MoveDestination? = null
6972
private set
7073

74+
val targetSourceRoot: VirtualFile?
75+
get() = ((destinationComboBox.comboBox.selectedItem as? DirectoryChooser.ItemWrapper)?.directory ?: originalFile).sourceRoot
76+
7177
init {
7278
informationLabel.text = RefactoringBundle.message("copy.class.copy.0.1", UsageViewUtil.getType(declaration), UsageViewUtil.getLongName(declaration))
7379
informationLabel.font = informationLabel.font.deriveFont(Font.BOLD)

‎idea/src/org/jetbrains/kotlin/idea/refactoring/copy/CopyKotlinDeclarationsHandler.kt

Lines changed: 68 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,33 @@ import com.intellij.openapi.project.Project
2222
import com.intellij.openapi.roots.ProjectRootManager
2323
import com.intellij.openapi.ui.Messages
2424
import com.intellij.openapi.util.Key
25+
import com.intellij.openapi.vfs.VirtualFile
2526
import com.intellij.psi.PsiDirectory
2627
import com.intellij.psi.PsiElement
2728
import com.intellij.psi.PsiFile
2829
import com.intellij.psi.PsiNamedElement
30+
import com.intellij.refactoring.BaseRefactoringProcessor
2931
import com.intellij.refactoring.RefactoringBundle
3032
import com.intellij.refactoring.copy.CopyFilesOrDirectoriesDialog
3133
import com.intellij.refactoring.copy.CopyHandlerDelegateBase
3234
import com.intellij.refactoring.rename.RenameProcessor
3335
import com.intellij.refactoring.util.MoveRenameUsageInfo
3436
import com.intellij.usageView.UsageInfo
3537
import com.intellij.util.IncorrectOperationException
38+
import com.intellij.util.containers.MultiMap
3639
import org.jetbrains.annotations.TestOnly
3740
import org.jetbrains.kotlin.idea.codeInsight.shorten.performDelayedRefactoringRequests
3841
import org.jetbrains.kotlin.idea.core.quoteIfNeeded
42+
import org.jetbrains.kotlin.idea.refactoring.checkConflictsInteractively
3943
import org.jetbrains.kotlin.idea.refactoring.createKotlinFile
4044
import org.jetbrains.kotlin.idea.refactoring.move.*
45+
import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.KotlinDirectoryMoveTarget
46+
import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.MoveConflictChecker
47+
import org.jetbrains.kotlin.idea.refactoring.toPsiDirectory
4148
import org.jetbrains.kotlin.idea.util.application.executeCommand
4249
import org.jetbrains.kotlin.idea.util.application.runReadAction
4350
import org.jetbrains.kotlin.idea.util.application.runWriteAction
51+
import org.jetbrains.kotlin.idea.util.sourceRoot
4452
import org.jetbrains.kotlin.name.FqName
4553
import org.jetbrains.kotlin.psi.KtElement
4654
import org.jetbrains.kotlin.psi.KtFile
@@ -55,7 +63,7 @@ class CopyKotlinDeclarationsHandler : CopyHandlerDelegateBase() {
5563
@set:TestOnly
5664
var Project.newName: String? by UserDataProperty(Key.create("NEW_NAME"))
5765

58-
private fun PsiElement.getElementsToCopy(): List<PsiNamedElement> {
66+
private fun PsiElement.getElementsToCopy(): List<KtElement> {
5967
val declarationOrFile = parentsWithSelf.firstOrNull { it is KtFile || (it is KtNamedDeclaration && it.parent is KtFile) }
6068
return when (declarationOrFile) {
6169
is KtFile -> declarationOrFile.declarations.filterIsInstance<KtNamedDeclaration>().ifEmpty { listOf(declarationOrFile) }
@@ -147,15 +155,16 @@ class CopyKotlinDeclarationsHandler : CopyHandlerDelegateBase() {
147155

148156
val project = initialTargetDirectory.project
149157

150-
if (ProjectRootManager.getInstance(project).fileIndex.getSourceRootForFile(initialTargetDirectory.virtualFile) == null) return
151-
152158
val commandName = "Copy Declarations"
153159

154160
var openInEditor = false
155161
var newName: String? = singleElementToCopy?.name ?: originalFile.name
156162
var targetDirWrapper: AutocreatingPsiDirectoryWrapper = initialTargetDirectory.toDirectoryWrapper()
163+
var targetSourceRoot: VirtualFile? = initialTargetDirectory.sourceRoot ?: return
164+
165+
val isUnitTestMode = ApplicationManager.getApplication().isUnitTestMode
157166

158-
if (!ApplicationManager.getApplication().isUnitTestMode) {
167+
if (!isUnitTestMode) {
159168
if (singleElementToCopy != null && singleElementToCopy is KtNamedDeclaration) {
160169
val dialog = CopyKotlinDeclarationDialog(singleElementToCopy, initialTargetDirectory, project)
161170
dialog.title = commandName
@@ -164,13 +173,15 @@ class CopyKotlinDeclarationsHandler : CopyHandlerDelegateBase() {
164173
openInEditor = dialog.openInEditor
165174
newName = dialog.newName ?: singleElementToCopy.name
166175
targetDirWrapper = dialog.targetDirectory?.toDirectoryWrapper() ?: return
176+
targetSourceRoot = dialog.targetSourceRoot
167177
}
168178
else {
169179
val dialog = CopyFilesOrDirectoriesDialog(arrayOf(originalFile), initialTargetDirectory, project, false)
170180
if (!dialog.showAndGet()) return
171181
openInEditor = dialog.openInEditor()
172182
newName = dialog.newName
173183
targetDirWrapper = dialog.targetDirectory?.toDirectoryWrapper() ?: return
184+
targetSourceRoot = dialog.targetDirectory?.sourceRoot
174185
}
175186
}
176187
else {
@@ -185,60 +196,80 @@ class CopyKotlinDeclarationsHandler : CopyHandlerDelegateBase() {
185196
ContainerInfo.Package(originalFile.packageFqName),
186197
ContainerInfo.Package(FqName(targetPackageName))
187198
)
188-
elementsToCopy.flatMap { elementToCopy ->
189-
(elementToCopy as KtElement).getInternalReferencesToUpdateOnPackageNameChange(changeInfo).filter {
199+
elementsToCopy.flatMapTo(LinkedHashSet()) { elementToCopy ->
200+
elementToCopy.getInternalReferencesToUpdateOnPackageNameChange(changeInfo).filter {
190201
val referencedElement = (it as? MoveRenameUsageInfo)?.referencedElement
191202
referencedElement == null || !elementToCopy.isAncestor(referencedElement)
192203
}
193204
}
194205
}
195206
markInternalUsages(internalUsages)
196207

197-
val restoredInternalUsages = ArrayList<UsageInfo>()
208+
fun doRefactor() {
209+
val restoredInternalUsages = ArrayList<UsageInfo>()
198210

199-
project.executeCommand(commandName) {
200-
try {
201-
val targetDirectory = runWriteAction { targetDirWrapper.getOrCreateDirectory(initialTargetDirectory) }
202-
val targetFileName = if (newName?.contains(".") ?: false) newName!! else newName + "." + originalFile.virtualFile.extension
211+
project.executeCommand(commandName) {
212+
try {
213+
val targetDirectory = runWriteAction { targetDirWrapper.getOrCreateDirectory(initialTargetDirectory) }
214+
val targetFileName = if (newName?.contains(".") ?: false) newName!! else newName + "." + originalFile.virtualFile.extension
203215

204-
val oldToNewElementsMapping = HashMap<PsiElement, PsiElement>()
216+
val oldToNewElementsMapping = HashMap<PsiElement, PsiElement>()
217+
218+
val targetFile: KtFile
219+
if (singleElementToCopy is KtFile) {
220+
targetFile = runWriteAction { targetDirectory.copyFileFrom(targetFileName, singleElementToCopy) as KtFile }
221+
}
222+
else {
223+
targetFile = getOrCreateTargetFile(originalFile, targetDirectory, targetFileName, commandName) ?: return@executeCommand
224+
runWriteAction {
225+
val newElements = elementsToCopy.map { targetFile.add(it.copy()) as KtNamedDeclaration }
226+
elementsToCopy.zip(newElements).toMap(oldToNewElementsMapping)
227+
}
228+
}
205229

206-
val targetFile: KtFile
207-
if (singleElementToCopy is KtFile) {
208-
targetFile = runWriteAction { targetDirectory.copyFileFrom(targetFileName, singleElementToCopy) as KtFile }
209-
}
210-
else {
211-
targetFile = getOrCreateTargetFile(originalFile, targetDirectory, targetFileName, commandName) ?: return@executeCommand
212230
runWriteAction {
213-
val newElements = elementsToCopy.map { targetFile.add(it.copy()) as KtNamedDeclaration }
214-
elementsToCopy.zip(newElements).toMap(oldToNewElementsMapping)
231+
for (newElement in oldToNewElementsMapping.values) {
232+
restoredInternalUsages += restoreInternalUsages(newElement as KtElement, oldToNewElementsMapping, true)
233+
postProcessMoveUsages(restoredInternalUsages, oldToNewElementsMapping)
234+
}
235+
236+
performDelayedRefactoringRequests(project)
215237
}
216-
}
217238

218-
runWriteAction {
219-
for (newElement in oldToNewElementsMapping.values) {
220-
restoredInternalUsages += restoreInternalUsages(newElement as KtElement, oldToNewElementsMapping, true)
221-
postProcessMoveUsages(restoredInternalUsages, oldToNewElementsMapping)
239+
oldToNewElementsMapping.values.singleOrNull()?.let {
240+
RenameProcessor(project, it, newName!!.quoteIfNeeded(), false, false).run()
222241
}
223242

224-
performDelayedRefactoringRequests(project)
243+
if (openInEditor) {
244+
EditorHelper.openFilesInEditor(arrayOf(targetFile))
245+
}
225246
}
226-
227-
oldToNewElementsMapping.values.singleOrNull()?.let {
228-
RenameProcessor(project, it, newName!!.quoteIfNeeded(), false, false).run()
247+
catch (e: IncorrectOperationException) {
248+
Messages.showMessageDialog(project, e.message, RefactoringBundle.message("error.title"), Messages.getErrorIcon())
229249
}
230-
231-
if (openInEditor) {
232-
EditorHelper.openFilesInEditor(arrayOf(targetFile))
250+
finally {
251+
cleanUpInternalUsages(internalUsages + restoredInternalUsages)
233252
}
234253
}
235-
catch (e: IncorrectOperationException) {
236-
Messages.showMessageDialog(project, e.message, RefactoringBundle.message("error.title"), Messages.getErrorIcon())
237-
}
238-
finally {
239-
cleanUpInternalUsages(internalUsages + restoredInternalUsages)
254+
}
255+
256+
val conflicts = MultiMap<PsiElement, String>()
257+
258+
if (!(isUnitTestMode && BaseRefactoringProcessor.ConflictsInTestsException.isTestIgnore())) {
259+
val targetSourceRootPsi = targetSourceRoot?.toPsiDirectory(project)
260+
if (targetSourceRootPsi != null) {
261+
val conflictChecker = MoveConflictChecker(
262+
project,
263+
elementsToCopy,
264+
KotlinDirectoryMoveTarget(FqName.ROOT, targetSourceRootPsi),
265+
originalFile
266+
)
267+
conflictChecker.checkModuleConflictsInDeclarations(internalUsages, conflicts)
268+
conflictChecker.checkVisibilityInDeclarations(conflicts)
240269
}
241270
}
271+
272+
project.checkConflictsInteractively(conflicts, onAccept = ::doRefactor)
242273
}
243274

244275
override fun doClone(element: PsiElement) {

‎idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ class MoveConflictChecker(
219219
}
220220
}
221221

222-
private fun checkModuleConflictsInDeclarations(
222+
fun checkModuleConflictsInDeclarations(
223223
internalUsages: MutableSet<UsageInfo>,
224224
conflicts: MultiMap<PsiElement, String>
225225
) {
@@ -327,7 +327,7 @@ class MoveConflictChecker(
327327
}
328328
}
329329

330-
private fun checkVisibilityInDeclarations(conflicts: MultiMap<PsiElement, String>) {
330+
fun checkVisibilityInDeclarations(conflicts: MultiMap<PsiElement, String>) {
331331
val targetContainer = moveTarget.getContainerDescriptor() ?: return
332332

333333
fun DeclarationDescriptor.targetAwareContainingDescriptor(): DeclarationDescriptor? {

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/after/A/A.iml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<module type="JAVA_MODULE" version="4">
3+
<component name="NewModuleRootManager" inherit-compiler-output="true">
4+
<exclude-output />
5+
<content url="file://$MODULE_DIR$">
6+
<sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
7+
</content>
8+
<orderEntry type="inheritedJdk" />
9+
<orderEntry type="sourceFolder" forTests="false" />
10+
</component>
11+
</module>

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/after/A/src/foo/test.kt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package foo
2+
3+
internal class A {
4+
val a: A = A()
5+
val b: B = B()
6+
}
7+
8+
internal class B {
9+
val a: A = A()
10+
val b: B = B()
11+
}

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/after/B/B.iml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<module type="JAVA_MODULE" version="4">
3+
<component name="NewModuleRootManager" inherit-compiler-output="true">
4+
<exclude-output />
5+
<content url="file://$MODULE_DIR$">
6+
<sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
7+
</content>
8+
<orderEntry type="inheritedJdk" />
9+
<orderEntry type="sourceFolder" forTests="false" />
10+
<orderEntry type="module" module-name="A" />
11+
</component>
12+
</module>

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/after/B/src/bar/A.kt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package bar
2+
3+
import foo.B
4+
5+
internal class A {
6+
val a: A = A()
7+
val b: B = B()
8+
}

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/after/B/src/bar/dummy.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package bar

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/before/A/A.iml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<module type="JAVA_MODULE" version="4">
3+
<component name="NewModuleRootManager" inherit-compiler-output="true">
4+
<exclude-output />
5+
<content url="file://$MODULE_DIR$">
6+
<sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
7+
</content>
8+
<orderEntry type="inheritedJdk" />
9+
<orderEntry type="sourceFolder" forTests="false" />
10+
</component>
11+
</module>

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/before/A/src/foo/test.kt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package foo
2+
3+
internal class <caret>A {
4+
val a: A = A()
5+
val b: B = B()
6+
}
7+
8+
internal class B {
9+
val a: A = A()
10+
val b: B = B()
11+
}

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/before/B/B.iml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<module type="JAVA_MODULE" version="4">
3+
<component name="NewModuleRootManager" inherit-compiler-output="true">
4+
<exclude-output />
5+
<content url="file://$MODULE_DIR$">
6+
<sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
7+
</content>
8+
<orderEntry type="inheritedJdk" />
9+
<orderEntry type="sourceFolder" forTests="false" />
10+
<orderEntry type="module" module-name="A" />
11+
</component>
12+
</module>

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/before/B/src/bar/dummy.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package bar

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/conflicts.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Class A uses class B which will be inaccessible after move

‎idea/testData/refactoring/copyMultiModule/internalReferencesToAnotherModule2/internalReferencesToAnotherModule.test

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"mainFile": "A/src/foo/test.kt",
3+
"targetDirectory": "B/src/bar"
4+
}

0 commit comments

Comments
 (0)