From 358b5fe548ce7f8c283a4f547412ea04978e82b4 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Fri, 7 Apr 2017 17:35:43 +0300 Subject: [PATCH] Move: Force package name change when moving Java files Package statement in Java file is updated too late when moving the entire containing directory, so we force it with intercepting MoveFileHandler implementation --- idea/src/META-INF/plugin.xml | 3 ++ .../MoveFilesWithDeclarationsProcessor.kt | 14 +++++ .../FqNameFixingMoveJavaFIleHandler.kt | 54 +++++++++++++++++++ .../MoveKotlinFileHandler.kt | 8 +-- .../after/test/Bar.java | 4 ++ .../after/test/Foo.kt | 9 ++++ .../after/test2/FooUsage.kt | 5 ++ .../after/test2/dummy.txt | 0 .../after/test2/pack/J.java | 12 +++++ .../after/test2/pack/test.kt | 17 ++++++ .../before/test/Bar.java | 4 ++ .../before/test/Foo.kt | 6 +++ .../before/test/FooUsage.kt | 5 ++ .../before/test/pack/J.java | 11 ++++ .../before/test/pack/test.kt | 16 ++++++ .../before/test2/dummy.txt | 0 ...ingToPackageFragementWithUnmatchedDir.test | 5 ++ .../idea/refactoring/move/AbstractMoveTest.kt | 9 ++-- .../refactoring/move/MoveTestGenerated.java | 6 +++ 19 files changed, 180 insertions(+), 8 deletions(-) create mode 100644 idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/FqNameFixingMoveJavaFIleHandler.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Bar.java create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Foo.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/FooUsage.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/dummy.txt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/J.java create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/test.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Bar.java create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Foo.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/FooUsage.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/J.java create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/test.kt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test2/dummy.txt create mode 100644 idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir.test diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 7c6c492fa53..dda66a7fef4 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -368,6 +368,9 @@ implementation="org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.MoveKotlinDeclarationsHandler" order="first,before kotlin.moveFilesOrDirectories"/> + diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveFilesWithDeclarationsProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveFilesWithDeclarationsProcessor.kt index f29ecccc97a..69422ea3b92 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveFilesWithDeclarationsProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveFilesWithDeclarationsProcessor.kt @@ -22,6 +22,7 @@ import com.intellij.openapi.util.Ref import com.intellij.psi.PsiDirectory import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile +import com.intellij.psi.PsiJavaFile import com.intellij.refactoring.move.MoveCallback import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesProcessor import com.intellij.usageView.UsageInfo @@ -82,6 +83,17 @@ class MoveFilesWithDeclarationsProcessor @JvmOverloads constructor ( sourceFile.name = temporaryName } + private fun markShouldFixFqName(value: Boolean) { + fun PsiElement.doMark(value: Boolean) { + when (this) { + is PsiJavaFile -> shouldFixFqName = value + is PsiDirectory -> children.forEach { it.doMark(value) } + } + } + + elementsToMove.forEach { it.doMark(value) } + } + override fun performRefactoring(usages: Array) { val needTemporaryRename = targetFileName != null && targetDirectory.findFile(targetFileName) != null if (needTemporaryRename) { @@ -89,9 +101,11 @@ class MoveFilesWithDeclarationsProcessor @JvmOverloads constructor ( } try { + markShouldFixFqName(true) super.performRefactoring(usages) } finally { + markShouldFixFqName(false) if (needTemporaryRename) { (elementsToMove.single() as PsiFile).name = targetFileName!! } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/FqNameFixingMoveJavaFIleHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/FqNameFixingMoveJavaFIleHandler.kt new file mode 100644 index 00000000000..478c63e76dd --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/FqNameFixingMoveJavaFIleHandler.kt @@ -0,0 +1,54 @@ +/* + * Copyright 2010-2017 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.moveFilesOrDirectories + +import com.intellij.openapi.util.Key +import com.intellij.psi.* +import com.intellij.psi.util.FileTypeUtils +import com.intellij.psi.util.PsiUtil +import com.intellij.refactoring.move.moveClassesOrPackages.MoveJavaFileHandler +import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFileHandler +import com.intellij.usageView.UsageInfo +import org.jetbrains.kotlin.psi.NotNullableUserDataProperty + +internal var PsiJavaFile.shouldFixFqName: Boolean by NotNullableUserDataProperty(Key.create("SHOULD_FIX_FQ_NAME"), false) + +class FqNameFixingMoveJavaFileHandler : MoveFileHandler() { + private val delegate = MoveJavaFileHandler() + + override fun canProcessElement(element: PsiFile) = + delegate.canProcessElement(element) + + override fun findUsages(psiFile: PsiFile, newParent: PsiDirectory?, searchInComments: Boolean, searchInNonJavaFiles: Boolean) = + delegate.findUsages(psiFile, newParent, searchInComments, searchInNonJavaFiles) + + override fun prepareMovedFile(file: PsiFile, moveDestination: PsiDirectory, oldToNewMap: MutableMap) { + delegate.prepareMovedFile(file, moveDestination, oldToNewMap) + if (file is PsiJavaFile && file.shouldFixFqName) { + val newPackage = JavaDirectoryService.getInstance().getPackage(moveDestination) ?: return + if (!FileTypeUtils.isInServerPageFile(file) && !PsiUtil.isModuleFile(file)) { + file.packageName = newPackage.qualifiedName + } + } + } + + override fun updateMovedFile(file: PsiFile) = + delegate.updateMovedFile(file) + + override fun retargetUsages(usageInfos: MutableList, oldToNewMap: MutableMap) = + delegate.retargetUsages(usageInfos, oldToNewMap) +} \ No newline at end of file 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 597ec8e165a..938d67f4348 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 @@ -123,16 +123,16 @@ class MoveKotlinFileHandler : MoveFileHandler() { } override fun prepareMovedFile(file: PsiFile, moveDestination: PsiDirectory, oldToNewMap: MutableMap) { + if (file !is KtFile) return val moveProcessor = initMoveProcessor(file, moveDestination) ?: return val moveContext = MoveContext(file, moveProcessor) oldToNewMap[moveContext] = moveContext + val packageNameInfo = file.getPackageNameInfo(moveDestination, true) ?: return + file.packageDirective?.fqName = packageNameInfo.newContainer.fqName!!.quoteIfNeeded() } override fun updateMovedFile(file: PsiFile) { - if (file !is KtFile) return - val newDirectory = file.parent ?: return - val packageNameInfo = file.getPackageNameInfo(newDirectory, true) ?: return - file.packageDirective?.fqName = packageNameInfo.newContainer.fqName!!.quoteIfNeeded() + } override fun retargetUsages(usageInfos: List?, oldToNewMap: Map) { diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Bar.java b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Bar.java new file mode 100644 index 00000000000..9e4ce9d2ec7 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Bar.java @@ -0,0 +1,4 @@ +package test; + +public class Bar { +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Foo.kt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Foo.kt new file mode 100644 index 00000000000..f31405e81e0 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test/Foo.kt @@ -0,0 +1,9 @@ +package test.pack + +import test2.pack.A +import test2.pack.J + +class Foo { + val a = A() + val j = J() +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/FooUsage.kt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/FooUsage.kt new file mode 100644 index 00000000000..53385992ea8 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/FooUsage.kt @@ -0,0 +1,5 @@ +package test2 + +import test.pack.Foo + +val foo = Foo() diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/dummy.txt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/dummy.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/J.java b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/J.java new file mode 100644 index 00000000000..09410d2143d --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/J.java @@ -0,0 +1,12 @@ +package test2.pack; + +import test.*; +import test.pack.Foo; + +public class J { + A a = new A(); + B b = new B(); + C c = new C(); + Foo foo = new Foo(); + Bar bar = new Bar(); +} diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/test.kt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/test.kt new file mode 100644 index 00000000000..b769a989ef0 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/after/test2/pack/test.kt @@ -0,0 +1,17 @@ +package test2.pack + +import test.Bar +import test.pack.Foo + +class A { + val b = B() +} + +class B { + val a = A() +} + +class C { + internal val foo = Foo() + internal val bar = Bar() +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Bar.java b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Bar.java new file mode 100644 index 00000000000..9e4ce9d2ec7 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Bar.java @@ -0,0 +1,4 @@ +package test; + +public class Bar { +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Foo.kt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Foo.kt new file mode 100644 index 00000000000..77ad2aedb99 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/Foo.kt @@ -0,0 +1,6 @@ +package test.pack + +class Foo { + val a = A() + val j = J() +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/FooUsage.kt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/FooUsage.kt new file mode 100644 index 00000000000..86071328878 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/FooUsage.kt @@ -0,0 +1,5 @@ +package test + +import test.pack.Foo + +val foo = Foo() diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/J.java b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/J.java new file mode 100644 index 00000000000..b70ad2d0bd8 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/J.java @@ -0,0 +1,11 @@ +package test.pack; + +import test.*; + +public class J { + A a = new A(); + B b = new B(); + C c = new C(); + Foo foo = new Foo(); + Bar bar = new Bar(); +} diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/test.kt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/test.kt new file mode 100644 index 00000000000..6c593dba120 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test/pack/test.kt @@ -0,0 +1,16 @@ +package test.pack + +import test.Bar + +class A { + val b = B() +} + +class B { + val a = A() +} + +class C { + internal val foo = Foo() + internal val bar = Bar() +} \ No newline at end of file diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test2/dummy.txt b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/before/test2/dummy.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir.test b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir.test new file mode 100644 index 00000000000..5c9db35c64f --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir.test @@ -0,0 +1,5 @@ +{ + "filesToMove": ["test/FooUsage.kt", "test/pack"], + "type": "MOVE_FILES_WITH_DECLARATIONS", + "targetDirectory": "test2" +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt index 9a1753fbf5d..44c503e918d 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/AbstractMoveTest.kt @@ -271,16 +271,17 @@ enum class MoveAction { MOVE_FILES_WITH_DECLARATIONS { override fun runRefactoring(rootDir: VirtualFile, mainFile: PsiFile, elementsAtCaret: List, config: JsonObject) { val project = mainFile.project - val psiFilesToMove = config.getAsJsonArray("filesToMove").map { - rootDir.findFileByRelativePath(it.asString)!!.toPsiFile(project) as KtFile + val elementsToMove = config.getAsJsonArray("filesToMove").map { + val virtualFile = rootDir.findFileByRelativePath(it.asString)!! + if (virtualFile.isDirectory) virtualFile.toPsiDirectory(project)!! else virtualFile.toPsiFile(project)!! } val targetDirPath = config.getString("targetDirectory") val targetDir = rootDir.findFileByRelativePath(targetDirPath)!!.toPsiDirectory(project)!! MoveFilesWithDeclarationsProcessor( project, - psiFilesToMove, + elementsToMove, targetDir, - psiFilesToMove.singleOrNull()?.name, + (elementsToMove.singleOrNull() as? KtFile)?.name, searchInComments = true, searchInNonJavaFiles = true, moveCallback = null diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java index 771f3661225..0cc3a889fbb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java @@ -258,6 +258,12 @@ public class MoveTestGenerated extends AbstractMoveTest { doTest(fileName); } + @TestMetadata("kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir.test") + public void testKotlin_moveFile_moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir_MoveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveFile/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir/moveFileAndDirWithJavaFileReferringToPackageFragementWithUnmatchedDir.test"); + doTest(fileName); + } + @TestMetadata("kotlin/moveFile/moveFileToFile/moveFileToFile.test") public void testKotlin_moveFile_moveFileToFile_MoveFileToFile() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveFile/moveFileToFile/moveFileToFile.test");