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
This commit is contained in:
Alexey Sedunov
2017-04-13 15:31:40 +03:00
parent aa0f0ef02d
commit b229eeeb47
14 changed files with 97 additions and 19 deletions
@@ -230,7 +230,9 @@ class MoveKotlinDeclarationsProcessor(
return showConflicts(conflicts, refUsages.get()) return showConflicts(conflicts, refUsages.get())
} }
override fun performRefactoring(usages: Array<out UsageInfo>) { override fun performRefactoring(usages: Array<out UsageInfo>) = doPerformRefactoring(usages.toList())
internal fun doPerformRefactoring(usages: List<UsageInfo>) {
fun moveDeclaration(declaration: KtNamedDeclaration, moveTarget: KotlinMoveTarget): KtNamedDeclaration { fun moveDeclaration(declaration: KtNamedDeclaration, moveTarget: KotlinMoveTarget): KtNamedDeclaration {
val targetContainer = moveTarget.getOrCreateTargetPsi(declaration) val targetContainer = moveTarget.getOrCreateTargetPsi(declaration)
?: throw AssertionError("Couldn't create Kotlin file for: ${declaration::class.java}: ${declaration.text}") ?: 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) = usages.partition { it is KotlinMoveUsage && it.isInternal }
val (oldInternalUsages, externalUsages) = usageList.partition { it is KotlinMoveUsage && it.isInternal }
val newInternalUsages = ArrayList<UsageInfo>() val newInternalUsages = ArrayList<UsageInfo>()
oldInternalUsages.forEach { (it.element as? KtSimpleNameExpression)?.internalUsageInfo = it } oldInternalUsages.forEach { (it.element as? KtSimpleNameExpression)?.internalUsageInfo = it }
@@ -249,7 +250,7 @@ class MoveKotlinDeclarationsProcessor(
val usagesToProcess = ArrayList(externalUsages) val usagesToProcess = ArrayList(externalUsages)
try { try {
descriptor.delegate.preprocessUsages(descriptor, usageList) descriptor.delegate.preprocessUsages(descriptor, usages)
val oldToNewElementsMapping = THashMap<PsiElement, PsiElement>(ElementHashingStrategy) val oldToNewElementsMapping = THashMap<PsiElement, PsiElement>(ElementHashingStrategy)
@@ -257,7 +258,7 @@ class MoveKotlinDeclarationsProcessor(
for ((sourceFile, kotlinToLightElements) in kotlinToLightElementsBySourceFile) { for ((sourceFile, kotlinToLightElements) in kotlinToLightElementsBySourceFile) {
for ((oldDeclaration, oldLightElements) in kotlinToLightElements) { for ((oldDeclaration, oldLightElements) in kotlinToLightElements) {
val elementListener = transaction!!.getElementListener(oldDeclaration) val elementListener = transaction?.getElementListener(oldDeclaration)
val newDeclaration = moveDeclaration(oldDeclaration, descriptor.moveTarget) val newDeclaration = moveDeclaration(oldDeclaration, descriptor.moveTarget)
newDeclarations += newDeclaration newDeclarations += newDeclaration
@@ -265,7 +266,7 @@ class MoveKotlinDeclarationsProcessor(
oldToNewElementsMapping[oldDeclaration] = newDeclaration oldToNewElementsMapping[oldDeclaration] = newDeclaration
oldToNewElementsMapping[sourceFile] = newDeclaration.containingKtFile oldToNewElementsMapping[sourceFile] = newDeclaration.containingKtFile
elementListener.elementMoved(newDeclaration) elementListener?.elementMoved(newDeclaration)
for ((oldElement, newElement) in oldLightElements.asSequence().zip(newDeclaration.toLightElements().asSequence())) { for ((oldElement, newElement) in oldLightElements.asSequence().zip(newDeclaration.toLightElements().asSequence())) {
oldToNewElementsMapping[oldElement] = newElement oldToNewElementsMapping[oldElement] = newElement
} }
@@ -75,10 +75,10 @@ class KotlinMoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() {
project: Project, project: Project,
files: MutableSet<PsiFile>, files: MutableSet<PsiFile>,
infos: Array<UsageInfo>, infos: Array<UsageInfo>,
directory: PsiDirectory, directory: PsiDirectory?,
conflicts: MultiMap<PsiElement, String> conflicts: MultiMap<PsiElement, String>
) { ) {
val psiPackage = directory.getPackage() ?: return val psiPackage = directory?.getPackage() ?: return
val moveTarget = KotlinDirectoryMoveTarget(FqName(psiPackage.qualifiedName), directory) val moveTarget = KotlinDirectoryMoveTarget(FqName(psiPackage.qualifiedName), directory)
for ((index, usageInfo) in infos.withIndex()) { for ((index, usageInfo) in infos.withIndex()) {
if (usageInfo !is FileUsagesWrapper) continue if (usageInfo !is FileUsagesWrapper) continue
@@ -28,7 +28,6 @@ import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFileHandler
import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesUtil import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesUtil
import com.intellij.usageView.UsageInfo import com.intellij.usageView.UsageInfo
import org.jetbrains.kotlin.idea.KotlinLanguage 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.getPackage
import org.jetbrains.kotlin.idea.core.packageMatchesDirectory import org.jetbrains.kotlin.idea.core.packageMatchesDirectory
import org.jetbrains.kotlin.idea.core.quoteIfNeeded import org.jetbrains.kotlin.idea.core.quoteIfNeeded
@@ -147,8 +146,6 @@ class MoveKotlinFileHandler : MoveFileHandler() {
} }
fun retargetUsages(usageInfos: List<UsageInfo>?, moveDeclarationsProcessor: MoveKotlinDeclarationsProcessor) { fun retargetUsages(usageInfos: List<UsageInfo>?, moveDeclarationsProcessor: MoveKotlinDeclarationsProcessor) {
moveDeclarationsProcessor.project.runRefactoringAndKeepDelayedRequests { usageInfos?.let { moveDeclarationsProcessor.doPerformRefactoring(it) }
usageInfos?.let { moveDeclarationsProcessor.execute(it) }
}
} }
} }
@@ -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 {
}
}
}
@@ -0,0 +1,7 @@
package x.y
class Bar
fun Bar.bar(block: () -> Unit) {
}
@@ -0,0 +1,7 @@
package x.y
class Foo
fun Foo.foo(block: Bar.() -> Unit) {
Bar().block()
}
@@ -0,0 +1,7 @@
package a./*rename*/b
class Bar
fun Bar.bar(block: () -> Unit) {
}
@@ -0,0 +1,7 @@
package a.b
class Foo
fun Foo.foo(block: Bar.() -> Unit) {
Bar().block()
}
@@ -0,0 +1,11 @@
package c
import a.b.*
fun Foo.test() {
foo {
bar {
}
}
}
@@ -0,0 +1,7 @@
{
"type": "MARKED_ELEMENT",
"mainFile": "a/b/bar.kt",
"newName": "x.y",
"withRuntime": "true",
"byRef": "true"
}
@@ -37,16 +37,15 @@ import com.intellij.openapi.vfs.VirtualFileVisitor
import com.intellij.psi.* import com.intellij.psi.*
import com.intellij.psi.impl.source.tree.injected.InjectedLanguageUtil import com.intellij.psi.impl.source.tree.injected.InjectedLanguageUtil
import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.search.GlobalSearchScope
import com.intellij.refactoring.BaseRefactoringProcessor
import com.intellij.refactoring.BaseRefactoringProcessor.ConflictsInTestsException import com.intellij.refactoring.BaseRefactoringProcessor.ConflictsInTestsException
import com.intellij.refactoring.rename.PsiElementRenameHandler import com.intellij.refactoring.rename.*
import com.intellij.refactoring.rename.RenameHandlerRegistry
import com.intellij.refactoring.rename.RenameProcessor
import com.intellij.refactoring.rename.RenamePsiElementProcessor
import com.intellij.refactoring.rename.naming.AutomaticRenamerFactory import com.intellij.refactoring.rename.naming.AutomaticRenamerFactory
import com.intellij.refactoring.util.CommonRefactoringUtil.RefactoringErrorHintException import com.intellij.refactoring.util.CommonRefactoringUtil.RefactoringErrorHintException
import com.intellij.testFramework.LightProjectDescriptor import com.intellij.testFramework.LightProjectDescriptor
import com.intellij.testFramework.PlatformTestUtil import com.intellij.testFramework.PlatformTestUtil
import com.intellij.testFramework.UsefulTestCase import com.intellij.testFramework.UsefulTestCase
import org.jetbrains.kotlin.asJava.finder.KtLightPackage
import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime
import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
@@ -423,11 +422,26 @@ fun runRenameProcessor(
isSearchTextOccurrences: Boolean isSearchTextOccurrences: Boolean
) { ) {
if (substitution == null) return 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) { if (renameParamsObject["overloadRenamer.onlyPrimaryElement"]?.asBoolean ?: false) {
with(AutomaticOverloadsRenamer) { substitution.elementFilter = { false } } with(AutomaticOverloadsRenamer) { substitution.elementFilter = { false } }
} }
Extensions.getExtensions(AutomaticRenamerFactory.EP_NAME).forEach { renameProcessor.addRenamerFactory(it) } if (processor is RenameProcessor) {
renameProcessor.run() Extensions.getExtensions(AutomaticRenamerFactory.EP_NAME).forEach { processor.addRenamerFactory(it) }
}
processor.run()
} }
@@ -1068,6 +1068,12 @@ public class RenameTestGenerated extends AbstractRenameTest {
doTest(fileName); 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") @TestMetadata("renamePackageWithQuotation/renamePackageWithQuotation.test")
public void testRenamePackageWithQuotation_RenamePackageWithQuotation() throws Exception { public void testRenamePackageWithQuotation_RenamePackageWithQuotation() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/renamePackageWithQuotation/renamePackageWithQuotation.test"); String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/renamePackageWithQuotation/renamePackageWithQuotation.test");