From b229eeeb476b49a42be95eed2e5c38ad2cbf9be0 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Thu, 13 Apr 2017 15:31:40 +0300 Subject: [PATCH] Move: Prevent running refactoring helpers on partial move It may lead to some imports being wringly removed due to their references being unresolved yet Also fix NPE on package rename via directory move --- .../MoveKotlinDeclarationsProcessor.kt | 13 +++++---- .../KotlinMoveDirectoryWithClassesHelper.kt | 4 +-- .../MoveKotlinFileHandler.kt | 5 +--- .../after/a/dummy.txt | 0 .../after/c/usages.kt | 14 ++++++++++ .../after/x/y/bar.kt | 7 +++++ .../after/x/y/foo.kt | 7 +++++ .../before/a/b/bar.kt | 7 +++++ .../before/a/b/foo.kt | 7 +++++ .../before/a/dummy.txt | 0 .../before/c/usages.kt | 11 ++++++++ .../renamePackageFqNameStarImport.test | 7 +++++ .../refactoring/rename/AbstractRenameTest.kt | 28 ++++++++++++++----- .../rename/RenameTestGenerated.java | 6 ++++ 14 files changed, 97 insertions(+), 19 deletions(-) create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/a/dummy.txt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/c/usages.kt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/bar.kt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/foo.kt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/bar.kt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/foo.kt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/dummy.txt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/c/usages.kt create mode 100644 idea/testData/refactoring/rename/renamePackageFqNameStarImport/renamePackageFqNameStarImport.test diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt index 4314c08e6b5..f29e7b44642 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/MoveKotlinDeclarationsProcessor.kt @@ -230,7 +230,9 @@ class MoveKotlinDeclarationsProcessor( return showConflicts(conflicts, refUsages.get()) } - override fun performRefactoring(usages: Array) { + override fun performRefactoring(usages: Array) = doPerformRefactoring(usages.toList()) + + internal fun doPerformRefactoring(usages: List) { fun moveDeclaration(declaration: KtNamedDeclaration, moveTarget: KotlinMoveTarget): KtNamedDeclaration { val targetContainer = moveTarget.getOrCreateTargetPsi(declaration) ?: throw AssertionError("Couldn't create Kotlin file for: ${declaration::class.java}: ${declaration.text}") @@ -240,8 +242,7 @@ class MoveKotlinDeclarationsProcessor( } } - val usageList = usages.toList() - val (oldInternalUsages, externalUsages) = usageList.partition { it is KotlinMoveUsage && it.isInternal } + val (oldInternalUsages, externalUsages) = usages.partition { it is KotlinMoveUsage && it.isInternal } val newInternalUsages = ArrayList() oldInternalUsages.forEach { (it.element as? KtSimpleNameExpression)?.internalUsageInfo = it } @@ -249,7 +250,7 @@ class MoveKotlinDeclarationsProcessor( val usagesToProcess = ArrayList(externalUsages) try { - descriptor.delegate.preprocessUsages(descriptor, usageList) + descriptor.delegate.preprocessUsages(descriptor, usages) val oldToNewElementsMapping = THashMap(ElementHashingStrategy) @@ -257,7 +258,7 @@ class MoveKotlinDeclarationsProcessor( for ((sourceFile, kotlinToLightElements) in kotlinToLightElementsBySourceFile) { for ((oldDeclaration, oldLightElements) in kotlinToLightElements) { - val elementListener = transaction!!.getElementListener(oldDeclaration) + val elementListener = transaction?.getElementListener(oldDeclaration) val newDeclaration = moveDeclaration(oldDeclaration, descriptor.moveTarget) newDeclarations += newDeclaration @@ -265,7 +266,7 @@ class MoveKotlinDeclarationsProcessor( oldToNewElementsMapping[oldDeclaration] = newDeclaration oldToNewElementsMapping[sourceFile] = newDeclaration.containingKtFile - elementListener.elementMoved(newDeclaration) + elementListener?.elementMoved(newDeclaration) for ((oldElement, newElement) in oldLightElements.asSequence().zip(newDeclaration.toLightElements().asSequence())) { oldToNewElementsMapping[oldElement] = newElement } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt index 0597c6695d2..b3c6b3896f6 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveFilesOrDirectories/KotlinMoveDirectoryWithClassesHelper.kt @@ -75,10 +75,10 @@ class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() { project: Project, files: MutableSet, infos: Array, - directory: PsiDirectory, + directory: PsiDirectory?, conflicts: MultiMap ) { - val psiPackage = directory.getPackage() ?: return + val psiPackage = directory?.getPackage() ?: return val moveTarget = KotlinDirectoryMoveTarget(FqName(psiPackage.qualifiedName), directory) for ((index, usageInfo) in infos.withIndex()) { if (usageInfo !is FileUsagesWrapper) continue 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 2e00cf2974c..0bf63f8071d 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 @@ -28,7 +28,6 @@ import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFileHandler import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesUtil import com.intellij.usageView.UsageInfo import org.jetbrains.kotlin.idea.KotlinLanguage -import org.jetbrains.kotlin.idea.codeInsight.shorten.runRefactoringAndKeepDelayedRequests import org.jetbrains.kotlin.idea.core.getPackage import org.jetbrains.kotlin.idea.core.packageMatchesDirectory import org.jetbrains.kotlin.idea.core.quoteIfNeeded @@ -147,8 +146,6 @@ class MoveKotlinFileHandler : MoveFileHandler() { } fun retargetUsages(usageInfos: List?, moveDeclarationsProcessor: MoveKotlinDeclarationsProcessor) { - moveDeclarationsProcessor.project.runRefactoringAndKeepDelayedRequests { - usageInfos?.let { moveDeclarationsProcessor.execute(it) } - } + usageInfos?.let { moveDeclarationsProcessor.doPerformRefactoring(it) } } } diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/a/dummy.txt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/a/dummy.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/c/usages.kt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/c/usages.kt new file mode 100644 index 00000000000..2baff72e207 --- /dev/null +++ b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/c/usages.kt @@ -0,0 +1,14 @@ +package c + +import a.b.* +import x.y.Foo +import x.y.bar +import x.y.foo + +fun Foo.test() { + foo { + bar { + + } + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/bar.kt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/bar.kt new file mode 100644 index 00000000000..613d45c5ce0 --- /dev/null +++ b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/bar.kt @@ -0,0 +1,7 @@ +package x.y + +class Bar + +fun Bar.bar(block: () -> Unit) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/foo.kt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/foo.kt new file mode 100644 index 00000000000..68db26b8247 --- /dev/null +++ b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/after/x/y/foo.kt @@ -0,0 +1,7 @@ +package x.y + +class Foo + +fun Foo.foo(block: Bar.() -> Unit) { + Bar().block() +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/bar.kt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/bar.kt new file mode 100644 index 00000000000..80faa3b57af --- /dev/null +++ b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/bar.kt @@ -0,0 +1,7 @@ +package a./*rename*/b + +class Bar + +fun Bar.bar(block: () -> Unit) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/foo.kt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/foo.kt new file mode 100644 index 00000000000..e43dd1c903d --- /dev/null +++ b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/b/foo.kt @@ -0,0 +1,7 @@ +package a.b + +class Foo + +fun Foo.foo(block: Bar.() -> Unit) { + Bar().block() +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/dummy.txt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/a/dummy.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/c/usages.kt b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/c/usages.kt new file mode 100644 index 00000000000..fcbe13fe898 --- /dev/null +++ b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/before/c/usages.kt @@ -0,0 +1,11 @@ +package c + +import a.b.* + +fun Foo.test() { + foo { + bar { + + } + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/renamePackageFqNameStarImport/renamePackageFqNameStarImport.test b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/renamePackageFqNameStarImport.test new file mode 100644 index 00000000000..c3090416aef --- /dev/null +++ b/idea/testData/refactoring/rename/renamePackageFqNameStarImport/renamePackageFqNameStarImport.test @@ -0,0 +1,7 @@ +{ + "type": "MARKED_ELEMENT", + "mainFile": "a/b/bar.kt", + "newName": "x.y", + "withRuntime": "true", + "byRef": "true" +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/AbstractRenameTest.kt b/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/AbstractRenameTest.kt index 0260cdf38c8..417ab91f152 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/AbstractRenameTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/AbstractRenameTest.kt @@ -37,16 +37,15 @@ import com.intellij.openapi.vfs.VirtualFileVisitor import com.intellij.psi.* import com.intellij.psi.impl.source.tree.injected.InjectedLanguageUtil import com.intellij.psi.search.GlobalSearchScope +import com.intellij.refactoring.BaseRefactoringProcessor import com.intellij.refactoring.BaseRefactoringProcessor.ConflictsInTestsException -import com.intellij.refactoring.rename.PsiElementRenameHandler -import com.intellij.refactoring.rename.RenameHandlerRegistry -import com.intellij.refactoring.rename.RenameProcessor -import com.intellij.refactoring.rename.RenamePsiElementProcessor +import com.intellij.refactoring.rename.* import com.intellij.refactoring.rename.naming.AutomaticRenamerFactory import com.intellij.refactoring.util.CommonRefactoringUtil.RefactoringErrorHintException import com.intellij.testFramework.LightProjectDescriptor import com.intellij.testFramework.PlatformTestUtil import com.intellij.testFramework.UsefulTestCase +import org.jetbrains.kotlin.asJava.finder.KtLightPackage import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor @@ -423,11 +422,26 @@ fun runRenameProcessor( isSearchTextOccurrences: Boolean ) { if (substitution == null) return - val renameProcessor = RenameProcessor(project, substitution, newName, isSearchInComments, isSearchTextOccurrences) + + fun createProcessor(): BaseRefactoringProcessor { + if (substitution is PsiPackage && substitution !is KtLightPackage) { + val oldName = substitution.qualifiedName + if (StringUtil.getPackageName(oldName) != StringUtil.getPackageName(newName)) { + return RenamePsiPackageProcessor.createRenameMoveProcessor(newName, substitution, isSearchInComments, isSearchTextOccurrences) + } + } + + return RenameProcessor(project, substitution, newName, isSearchInComments, isSearchTextOccurrences) + } + + val processor = createProcessor() + if (renameParamsObject["overloadRenamer.onlyPrimaryElement"]?.asBoolean ?: false) { with(AutomaticOverloadsRenamer) { substitution.elementFilter = { false } } } - Extensions.getExtensions(AutomaticRenamerFactory.EP_NAME).forEach { renameProcessor.addRenamerFactory(it) } - renameProcessor.run() + if (processor is RenameProcessor) { + Extensions.getExtensions(AutomaticRenamerFactory.EP_NAME).forEach { processor.addRenamerFactory(it) } + } + processor.run() } diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java index 8143158cf86..cf0da056a2f 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java @@ -1068,6 +1068,12 @@ public class RenameTestGenerated extends AbstractRenameTest { doTest(fileName); } + @TestMetadata("renamePackageFqNameStarImport/renamePackageFqNameStarImport.test") + public void testRenamePackageFqNameStarImport_RenamePackageFqNameStarImport() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/renamePackageFqNameStarImport/renamePackageFqNameStarImport.test"); + doTest(fileName); + } + @TestMetadata("renamePackageWithQuotation/renamePackageWithQuotation.test") public void testRenamePackageWithQuotation_RenamePackageWithQuotation() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/renamePackageWithQuotation/renamePackageWithQuotation.test");