From 9a1d5ea1699abc76bf13dbb3c0fb0a8a6a7e9200 Mon Sep 17 00:00:00 2001 From: Bart van Helvert Date: Fri, 9 Aug 2024 10:12:52 +0200 Subject: [PATCH] [kotlin] Rework internal usage processing and remove isUpdatable Removes isUpdatable filters after conflict checking and instead of filtering, the refactoring now marks usages whether they are updatable during the marking of internal usages. Because of recent changes that made it so we only call find usages on top level moved elements, this means external usages are also always updatable. #KTIJ-30733 Fixed GitOrigin-RevId: 5685845ae45c4639f81036ab8e4c72a1946e26f0 --- .../refactoring/move/MoveTestGenerated.java | 5 + .../after/RandomJavaFile.java | 3 + .../after/one/.keep | 0 .../zero/destructuring/MovingComponent.kt | 6 + .../after/zero/refComponent.kt | 7 + .../before/RandomJavaFile.java | 3 + .../before/one/.keep | 0 .../one/destructuring/MovingComponent.kt | 6 + .../before/zero/refComponent.kt | 7 + ...movePackageWithDestructuringReference.test | 8 + .../shortenCompanionObject2/after/b/boo.k2.kt | 18 -- .../copy/CopyKotlinDeclarationsHandler.kt | 7 +- .../K2ChangePackageRefactoringProcessor.kt | 7 +- .../K2MoveDeclarationsRefactoringProcessor.kt | 7 +- .../K2MoveDirectoryWithClassesHelper.kt | 6 +- ...eFilesOrDirectoriesRefactoringProcessor.kt | 24 +- .../move/processor/K2MoveRenameUsageInfo.kt | 233 ++++++++---------- .../move/processor/moveConflictUtil.kt | 9 +- .../move/processor/moveUsageUtil.kt | 2 +- .../AbstractK2MoveFileOrDirectoriesTest.kt | 19 +- .../K2MoveFileOrDirectoriesTestGenerated.java | 5 + 21 files changed, 176 insertions(+), 206 deletions(-) create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/RandomJavaFile.java create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/one/.keep create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/destructuring/MovingComponent.kt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/refComponent.kt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/RandomJavaFile.java create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/one/.keep create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/one/destructuring/MovingComponent.kt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/zero/refComponent.kt create mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test delete mode 100644 plugins/kotlin/idea/tests/testData/refactoring/moveTopLevel/kotlin/shortenCompanionObject2/after/b/boo.k2.kt diff --git a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java index 4f4b6040aef5..901e435f776b 100644 --- a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java +++ b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/refactoring/move/MoveTestGenerated.java @@ -144,6 +144,11 @@ public abstract class MoveTestGenerated extends AbstractMoveTest { runTest("testData/refactoring/moveFile/java/moveFileToAnotherPackage/moveFileToAnotherPackage.test"); } + @TestMetadata("java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test") + public void testJava_movePackageWithDestructuringReference_MovePackageWithDestructuringReference() throws Exception { + runTest("testData/refactoring/moveFile/java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test"); + } + @TestMetadata("kotlin/addExtensionImport/addExtensionImport.test") public void testKotlin_addExtensionImport_AddExtensionImport() throws Exception { runTest("testData/refactoring/moveFile/kotlin/addExtensionImport/addExtensionImport.test"); diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/RandomJavaFile.java b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/RandomJavaFile.java new file mode 100644 index 000000000000..633853cd608e --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/RandomJavaFile.java @@ -0,0 +1,3 @@ +public class RandomJavaFile { + // Just here to invoke Java refactoring +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/one/.keep b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/one/.keep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/destructuring/MovingComponent.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/destructuring/MovingComponent.kt new file mode 100644 index 000000000000..76146d3576d5 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/destructuring/MovingComponent.kt @@ -0,0 +1,6 @@ +package zero.destructuring + +class MovingComponent { + operator fun component1() = 1 + operator fun component2() = 2 +} diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/refComponent.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/refComponent.kt new file mode 100644 index 000000000000..cc3a5d02e20a --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/after/zero/refComponent.kt @@ -0,0 +1,7 @@ +package zero + +import zero.destructuring.MovingComponent + +fun referComponent() { + val (x, y) = MovingComponent() +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/RandomJavaFile.java b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/RandomJavaFile.java new file mode 100644 index 000000000000..633853cd608e --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/RandomJavaFile.java @@ -0,0 +1,3 @@ +public class RandomJavaFile { + // Just here to invoke Java refactoring +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/one/.keep b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/one/.keep new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/one/destructuring/MovingComponent.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/one/destructuring/MovingComponent.kt new file mode 100644 index 000000000000..4c6913a4f06b --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/one/destructuring/MovingComponent.kt @@ -0,0 +1,6 @@ +package one.destructuring + +class MovingComponent { + operator fun component1() = 1 + operator fun component2() = 2 +} diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/zero/refComponent.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/zero/refComponent.kt new file mode 100644 index 000000000000..3606b9a0fb3e --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/before/zero/refComponent.kt @@ -0,0 +1,7 @@ +package zero + +import one.destructuring.MovingComponent + +fun referComponent() { + val (x, y) = MovingComponent() +} \ No newline at end of file diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test new file mode 100644 index 000000000000..6c76f9f34ce7 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test @@ -0,0 +1,8 @@ +{ + "mainFile": "RandomJavaFile.java", + "filesToMove": ["one/destructuring"], + "type": "MOVE_FILES", + "targetPackage": "zero", + "enabledInK1": "false", + "enabledInK2": "true" +} diff --git a/plugins/kotlin/idea/tests/testData/refactoring/moveTopLevel/kotlin/shortenCompanionObject2/after/b/boo.k2.kt b/plugins/kotlin/idea/tests/testData/refactoring/moveTopLevel/kotlin/shortenCompanionObject2/after/b/boo.k2.kt deleted file mode 100644 index 9eace15a4929..000000000000 --- a/plugins/kotlin/idea/tests/testData/refactoring/moveTopLevel/kotlin/shortenCompanionObject2/after/b/boo.k2.kt +++ /dev/null @@ -1,18 +0,0 @@ -package b - -import b.Factory.Companion.invoke -import java.util.function.IntPredicate - -interface Factory { - operator fun invoke(i: Int): IntPredicate - - companion object { - inline operator fun invoke(crossinline f: (Int) -> IntPredicate) = object : Factory { - override fun invoke(i: Int) = f(i) - } - } -} - -fun foo(): Factory = Factory { k -> - IntPredicate { n -> n % k == 0 } -} \ No newline at end of file diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/copy/CopyKotlinDeclarationsHandler.kt b/plugins/kotlin/refactorings/kotlin.refactorings.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/copy/CopyKotlinDeclarationsHandler.kt index 5c7b15de02e9..4f399591873d 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/copy/CopyKotlinDeclarationsHandler.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/copy/CopyKotlinDeclarationsHandler.kt @@ -35,7 +35,6 @@ import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsage import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.retargetInternalUsages import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.retargetInternalUsagesForCopyFile import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.unMarkAllUsages -import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.unMarkNonUpdatableUsages import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.conflict.checkModuleDependencyConflictsForInternalUsages import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.conflict.checkVisibilityConflictsForInternalUsages import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.createCopyTarget @@ -165,7 +164,9 @@ class CopyKotlinDeclarationsHandler : AbstractCopyKotlinDeclarationsHandler() { val targetData = getTargetData(sourceData) ?: return for (element in elementsToCopy) { - markInternalUsages(element) + analyzeInModalWindow(elementsToCopy.first(), RefactoringBundle.message("refactoring.preprocess.usages.progress")) { + markInternalUsages(element, element) + } } val conflicts: MultiMap = @@ -173,8 +174,6 @@ class CopyKotlinDeclarationsHandler : AbstractCopyKotlinDeclarationsHandler() { collectConflicts(sourceData, targetData) } - unMarkNonUpdatableUsages(elementsToCopy) - project.checkConflictsInteractively(conflicts) { try { ApplicationManagerEx.getApplicationEx().runWriteActionWithCancellableProgressInDispatchThread(copyCommandName, project, null) { diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2ChangePackageRefactoringProcessor.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2ChangePackageRefactoringProcessor.kt index 1cb07b9cb75e..6d94fe39d378 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2ChangePackageRefactoringProcessor.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2ChangePackageRefactoringProcessor.kt @@ -14,7 +14,6 @@ import org.jetbrains.kotlin.analysis.api.permissions.KaAllowAnalysisOnEdt import org.jetbrains.kotlin.analysis.api.permissions.allowAnalysisOnEdt import org.jetbrains.kotlin.idea.base.resources.KotlinBundle import org.jetbrains.kotlin.idea.k2.refactoring.move.descriptor.K2ChangePackageDescriptor -import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.unMarkNonUpdatableUsages class K2ChangePackageRefactoringProcessor(private val descriptor: K2ChangePackageDescriptor) : BaseRefactoringProcessor(descriptor.project) { override fun getCommandName(): String = KotlinBundle.message( @@ -42,11 +41,7 @@ class K2ChangePackageRefactoringProcessor(private val descriptor: K2ChangePackag usages.filterIsInstance() ) } - val toContinue = showConflicts(conflicts, usages) - if (!toContinue) return false - unMarkNonUpdatableUsages(descriptor.files) - refUsages.set(K2MoveRenameUsageInfo.filterUpdatable(descriptor.files, usages).toTypedArray()) - return true + return showConflicts(conflicts, usages) } @OptIn(KaAllowAnalysisOnEdt::class) diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDeclarationsRefactoringProcessor.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDeclarationsRefactoringProcessor.kt index e4a1c198ccbc..08c14cd15b94 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDeclarationsRefactoringProcessor.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDeclarationsRefactoringProcessor.kt @@ -16,7 +16,6 @@ import org.jetbrains.kotlin.analysis.api.permissions.allowAnalysisOnEdt import org.jetbrains.kotlin.idea.base.psi.deleteSingle import org.jetbrains.kotlin.idea.base.resources.KotlinBundle import org.jetbrains.kotlin.idea.k2.refactoring.move.descriptor.K2MoveOperationDescriptor -import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.unMarkNonUpdatableUsages class K2MoveDeclarationsRefactoringProcessor( private val operationDescriptor: K2MoveOperationDescriptor.Declarations @@ -57,11 +56,7 @@ class K2MoveDeclarationsRefactoringProcessor( } } } - val toContinue = showConflicts(conflicts, usages) - if (!toContinue) return false - unMarkNonUpdatableUsages(operationDescriptor.sourceElements) - refUsages.set(K2MoveRenameUsageInfo.filterUpdatable(operationDescriptor.sourceElements, usages).toTypedArray()) - return true + return showConflicts(conflicts, usages) } @OptIn(KaAllowAnalysisOnEdt::class) diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDirectoryWithClassesHelper.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDirectoryWithClassesHelper.kt index 4ec33eee5cde..857c64feaeff 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDirectoryWithClassesHelper.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveDirectoryWithClassesHelper.kt @@ -14,7 +14,6 @@ import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectori import com.intellij.usageView.UsageInfo import com.intellij.util.Function import com.intellij.util.containers.MultiMap -import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.unMarkNonUpdatableUsages import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtFile @@ -65,12 +64,9 @@ class K2MoveDirectoryWithClassesHelper : MoveDirectoryWithClassesHelper() { targetDirectory: PsiDirectory?, conflicts: MultiMap ) { - val movedFiles = files.filterIsInstance() - unMarkNonUpdatableUsages(movedFiles) - // processing kotlin usages from Java declarations will result in non-deterministic retargeting of the references // to fix this, all usages are sorted by start offset - infos.set(K2MoveRenameUsageInfo.filterUpdatable(movedFiles, infos.get()).sortedBy { it.element?.startOffset ?: -1 }.toTypedArray()) + infos.get().sortedBy { it.element?.startOffset ?: -1 } if (targetDirectory != null) { // TODO probably this should never be null but it happens when there are multiple source roots moveFileHandler.detectConflicts(conflicts, files.filterIsInstance().toTypedArray(), infos.get(), targetDirectory) } diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveFilesOrDirectoriesRefactoringProcessor.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveFilesOrDirectoriesRefactoringProcessor.kt index 8ad22e839707..1b3d198f1588 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveFilesOrDirectoriesRefactoringProcessor.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveFilesOrDirectoriesRefactoringProcessor.kt @@ -3,7 +3,6 @@ package org.jetbrains.kotlin.idea.k2.refactoring.move.processor import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.util.Key -import com.intellij.openapi.util.Ref import com.intellij.openapi.util.registry.Registry import com.intellij.psi.PsiDirectory import com.intellij.psi.PsiElement @@ -18,7 +17,6 @@ import org.jetbrains.kotlin.analysis.api.permissions.allowAnalysisOnEdt import org.jetbrains.kotlin.idea.core.getFqNameWithImplicitPrefix import org.jetbrains.kotlin.idea.core.getFqNameWithImplicitPrefixOrRoot import org.jetbrains.kotlin.idea.k2.refactoring.move.descriptor.K2MoveOperationDescriptor -import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.unMarkNonUpdatableUsages import org.jetbrains.kotlin.psi.CopyablePsiUserDataProperty import org.jetbrains.kotlin.psi.KtFile @@ -31,27 +29,7 @@ class K2MoveFilesOrDirectoriesRefactoringProcessor(descriptor: K2MoveOperationDe descriptor.searchForText, descriptor.moveCallBack, Runnable { } -) { - private fun PsiElement.allFiles(): List = when (this) { - is PsiDirectory -> children.flatMap { it.allFiles() } - is KtFile -> listOf(this) - else -> emptyList() - } - - override fun preprocessUsages(refUsages: Ref>): Boolean { - val toContinue = super.preprocessUsages(refUsages) - if (!toContinue) return false - - // after conflict checking, we don't need non-updatable usages anymore - val elementsToMove = myElementsToMove.toList() - unMarkNonUpdatableUsages(elementsToMove) - val updatableUsages = K2MoveRenameUsageInfo.filterUpdatable(elementsToMove, refUsages.get()) - refUsages.set(updatableUsages.toTypedArray()) - val usagesByFile = updatableUsages.groupBy { (it as? K2MoveRenameUsageInfo)?.referencedElement?.containingFile } - usagesByFile.forEach { file, usages -> myFoundUsages.replace(file, usages) } - return true - } -} +) class K2MoveFilesHandler : MoveFileHandler() { /** diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveRenameUsageInfo.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveRenameUsageInfo.kt index 23d1aedcf9a4..8a27248f4c6c 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveRenameUsageInfo.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/K2MoveRenameUsageInfo.kt @@ -13,17 +13,15 @@ import com.intellij.refactoring.move.moveMembers.MoveMembersOptions import com.intellij.refactoring.move.moveMembers.MoveMembersProcessor import com.intellij.refactoring.util.MoveRenameUsageInfo import com.intellij.usageView.UsageInfo +import org.jetbrains.kotlin.analysis.api.KaSession import org.jetbrains.kotlin.analysis.api.analyze -import org.jetbrains.kotlin.analysis.api.permissions.KaAllowAnalysisOnEdt -import org.jetbrains.kotlin.analysis.api.permissions.allowAnalysisOnEdt import org.jetbrains.kotlin.analysis.api.symbols.* -import org.jetbrains.kotlin.analysis.api.symbols.markers.KaDeclarationContainerSymbol import org.jetbrains.kotlin.analysis.api.types.KaFunctionType import org.jetbrains.kotlin.asJava.toLightElements import org.jetbrains.kotlin.idea.base.analysis.api.utils.shortenReferences -import org.jetbrains.kotlin.idea.base.psi.canBeUsedInImport -import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.internalUsageInfo +import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.updatableUsageInfo import org.jetbrains.kotlin.idea.references.KtConstructorDelegationReference +import org.jetbrains.kotlin.idea.references.KtInvokeFunctionReference import org.jetbrains.kotlin.idea.references.KtReference import org.jetbrains.kotlin.idea.references.KtSimpleNameReference import org.jetbrains.kotlin.idea.references.mainReference @@ -41,14 +39,6 @@ sealed class K2MoveRenameUsageInfo( reference: PsiReference, referencedElement: PsiNamedElement ) : MoveRenameUsageInfo(element, reference, referencedElement) { - /** - * Returns whether this usage info is actually required for updating. - * Sometimes it can depend on the language of the usage whether the usage info is updatable. - * In Kotlin, for example, object members can be imported and might require updating, but in Java these are regular instance methods and - * references to these methods can't be updated. - */ - abstract fun isUpdatable(movedElements: List): Boolean - abstract fun retarget(to: PsiNamedElement): PsiElement? /** @@ -62,10 +52,6 @@ sealed class K2MoveRenameUsageInfo( private val oldContainingFqn: String?, private val lightElementIndex: Int, ) : K2MoveRenameUsageInfo(element, reference, referencedElement) { - override fun isUpdatable(movedElements: List): Boolean { - return true // TODO write better updatable check for light references - } - override fun retarget(to: PsiNamedElement): PsiElement? { if (to !is KtNamedDeclaration) error("Usage must reference a Kotlin element") val element = element ?: return element @@ -129,71 +115,6 @@ sealed class K2MoveRenameUsageInfo( referencedElement: PsiNamedElement, val isInternal: Boolean ) : K2MoveRenameUsageInfo(element, reference, referencedElement) { - @OptIn(KaAllowAnalysisOnEdt::class) - override fun isUpdatable(movedElements: List): Boolean = allowAnalysisOnEdt { - val refExpr = element as? KtSimpleNameExpression ?: return false - if (!refExpr.canBeUsedInImport()) return false - if (refExpr.parentOfType(withSelf = false) != null) return true - if (refExpr.isUnqualifiable()) return true - val refChain = (refExpr.getTopmostParentQualifiedExpressionForReceiver() ?: refExpr) - .collectDescendantsOfType() - .filter { it.canBeUsedInImport() && it.isNameDeterminantInQualifiedChain() } - return if (isInternal) { - // for internal usages, update the first name determinant in the call chain - refChain.firstOrNull() == refExpr - } else { - // for external usages, update the first reference to a moved element - refChain.firstOrNull { simpleNameExpr -> simpleNameExpr.mainReference.resolve() in movedElements } == refExpr - } - } - - private fun KtSimpleNameExpression.getTopmostParentQualifiedExpressionForReceiver(): KtExpression? { - return generateSequence(this) { - it.parent as? KtQualifiedExpression ?: it.parent as? KtCallExpression - }.lastOrNull() - } - - @OptIn(KaAllowAnalysisOnEdt::class) - private fun KtSimpleNameExpression.isNameDeterminantInQualifiedChain(): Boolean = allowAnalysisOnEdt { - analyze(this) { - val resolvedSymbol = mainReference.resolveToSymbol() - if (resolvedSymbol is KaClassSymbol && resolvedSymbol.classKind == KaClassKind.COMPANION_OBJECT) return true - if (resolvedSymbol is KaConstructorSymbol) return true - val containingSymbol = resolvedSymbol?.containingDeclaration - if (resolvedSymbol is KaPackageSymbol) return false // ignore packages - if (containingSymbol == null) return true // top levels are static - if (containingSymbol is KaClassSymbol) { - val classKind = containingSymbol.classKind - if (classKind == KaClassKind.OBJECT || classKind == KaClassKind.COMPANION_OBJECT) return true - } - if (containingSymbol is KaDeclarationContainerSymbol) { - if (resolvedSymbol in containingSymbol.staticMemberScope.declarations) return true - } - return false - } - } - - private fun KtSimpleNameExpression.isUnqualifiable(): Boolean { - // example: a.foo() where foo is an extension function - fun KtSimpleNameExpression.isExtensionReference(): Boolean { - return analyze(this) { - val callable = mainReference.resolveToSymbol() as? KaCallableSymbol - if (callable?.isExtension == true) return true - if (callable is KaPropertySymbol) { - val returnType = callable.returnType - returnType is KaFunctionType && returnType.receiverType != null - } else false - } - } - - // example: ::foo - fun KtSimpleNameExpression.isCallableReferenceExpressionWithoutQualifier(): Boolean { - val parent = parent - return parent is KtCallableReferenceExpression && parent.receiverExpression == null - } - return isExtensionReference() || isCallableReferenceExpressionWithoutQualifier() - } - fun refresh(element: KtElement, referencedElement: PsiNamedElement): K2MoveRenameUsageInfo { val reference = element.mainReference ?: return this return Source(element, reference, referencedElement, isInternal) @@ -212,7 +133,7 @@ sealed class K2MoveRenameUsageInfo( companion object { fun find(declaration: KtNamedDeclaration): List { - markInternalUsages(declaration) + markInternalUsages(declaration, declaration) return preProcessUsages(findExternalUsages(declaration)) } @@ -229,58 +150,111 @@ sealed class K2MoveRenameUsageInfo( * @see restoreInternalUsages * @see K2MoveRenameUsageInfo.Source.refresh */ - internal var KtElement.internalUsageInfo: K2MoveRenameUsageInfo? by CopyablePsiUserDataProperty(Key.create("INTERNAL_USAGE_INFO")) + internal val KtElement.internalUsageInfo get() = updatableUsageInfo ?: nonUpdatableUsageInfo /** - * Finds any usage inside [containing]. We need these usages because when moving [containing] to a different package references - * that where previously imported by default might now require an explicit import. + * Internal usage info that can be retargeted. */ - fun markInternalUsages(containing: KtElement) { - containing.forEachDescendantOfType { name -> - val reference = name.mainReference - val resolved = reference.resolve() as? PsiNamedElement ?: return@forEachDescendantOfType - name.internalUsageInfo = Source(name, reference, resolved, true) + internal var KtElement.updatableUsageInfo: K2MoveRenameUsageInfo? by CopyablePsiUserDataProperty(Key.create("UPDATABLE_INTERNAL_USAGE_INFO")) + + /** + * Internal usage info that can't be retargeted but might still be useful in, for example, conflict checking. + */ + internal var KtElement.nonUpdatableUsageInfo: K2MoveRenameUsageInfo? by CopyablePsiUserDataProperty(Key.create("NON_UPDATABLE_INTERNAL_USAGE_INFO")) + + /** + * Finds any usage inside [elem]. + * We need these usages because when moving [elem] to a different package references that where previously imported by default might + * now require an explicit import. + * @see com.intellij.codeInsight.ChangeContextUtil.encodeContextInfo for Java implementation + */ + fun markInternalUsages(elem: PsiElement, topLevelMoved: KtElement) { + when (elem) { + is KDocName -> { + val reference = elem.mainReference + val resolved = reference.resolve() as? PsiNamedElement + if (resolved != null && !PsiTreeUtil.isAncestor(topLevelMoved, resolved, false)) { + elem.updatableUsageInfo = Source(elem, reference, resolved, true) + } + } + is KtReferenceExpression -> { + elem.markInternalUsageInfo(topLevelMoved) + } } - containing.forEachDescendantOfType { refExpr -> - if (refExpr is KtEnumEntrySuperclassReferenceExpression) return@forEachDescendantOfType - if (refExpr.parent is KtSuperExpression || refExpr.parent is KtThisExpression) return@forEachDescendantOfType - if (refExpr.parentOfType() != null) return@forEachDescendantOfType - val mainReference = refExpr.mainReference - if (mainReference is KtConstructorDelegationReference) return@forEachDescendantOfType - val resolved = mainReference.resolve() as? PsiNamedElement ?: return@forEachDescendantOfType - refExpr.internalUsageInfo = Source(refExpr, mainReference, resolved, true) + for (child in elem.children) { + markInternalUsages(child, topLevelMoved) } } + private fun KtReferenceExpression.markInternalUsageInfo(topLevelMoved: KtElement) { + val expr = this + val mainReference = expr.mainReference + if (expr is KtCallExpression && mainReference !is KtInvokeFunctionReference) return // to avoid duplication when handling name + if (expr is KtEnumEntrySuperclassReferenceExpression) return + val parent = expr.parent + if (parent is KtSuperExpression || parent is KtThisExpression) return + if (expr.parentOfType() != null) return + if (expr.parentOfType(withSelf = false) != null) return + if (mainReference is KtConstructorDelegationReference) return + val resolved = mainReference.resolve() as? PsiNamedElement ?: return + val isExtensionReference = if (resolved is KtCallableDeclaration) { + analyze(resolved) { + val symbol = resolved.symbol + if (symbol is KaCallableSymbol) expr.isExtensionReference(symbol) else false + } + } else false + if (resolved is KtParameter) return + if (resolved is KtNamedDeclaration && resolved.isDeclaredInContainingContext(expr, topLevelMoved)) return + val usageInfo = Source(expr, mainReference, resolved, true) + if (expr.isFirstReferenceInQualifiedChain() || isExtensionReference) { + expr.updatableUsageInfo = usageInfo + } else { + expr.nonUpdatableUsageInfo = usageInfo + } + } + + private fun KtNamedDeclaration.isDeclaredInContainingContext(expr: KtReferenceExpression, topLevelMoved: KtElement): Boolean { + return generateSequence(expr.parentOfType()) { containing -> + if (containing == topLevelMoved) return@generateSequence null + containing.parentOfType() + }.firstOrNull { containing -> + if (containing is KtDeclarationContainer) { + containing.declarations.contains(this) + } else false + } != null + } + + context(KaSession) + private fun KtReferenceExpression.isExtensionReference(symbol: KaCallableSymbol): Boolean { + if (symbol.isExtension == true) return true + return if (symbol is KaPropertySymbol) { + val returnType = symbol.returnType + returnType is KaFunctionType && returnType.receiverType != null + } else false + } + + private fun KtReferenceExpression.isFirstReferenceInQualifiedChain(): Boolean { + if (this is KtOperationReferenceExpression) return false // don't consider unary expressions like !foo() + val qualifiedChain = getQualifiedChainElement().collectDescendantsOfType() + if (qualifiedChain.any { refExpr -> refExpr.updatableUsageInfo != null }) return false // chain is already covered + for (refExpr in qualifiedChain) { + val resolved = refExpr.mainReference.resolve() + if (resolved !is PsiPackage) return this == refExpr + } + return false + } + + private fun KtReferenceExpression.getQualifiedChainElement(): KtElement { + return generateSequence(this) { + it.parent as? KtQualifiedExpression ?: it.parent as? KtCallExpression ?: it.parent as? KtUserType + }.last() + } + fun unMarkAllUsages(containing: KtElement) = containing.forEachDescendantOfType { refExpr -> - refExpr.internalUsageInfo = null + refExpr.updatableUsageInfo = null + refExpr.nonUpdatableUsageInfo = null } - /** - * Removes any internal usage infos that don't need to be updated. - * In [markInternalUsages] we marked all internal usages, but some of these usages don't need to be updated. - * Like, for example, instance methods. - */ - fun unMarkNonUpdatableUsages(topLevelMovedElements: Iterable) { - val allMovedDeclarations = topLevelMovedElements.flatMap { it.withChildDeclarations() } - for (element in topLevelMovedElements) { - element.forEachDescendantOfType { refExpr -> - val usageInfo = refExpr.internalUsageInfo ?: return@forEachDescendantOfType - if (!usageInfo.isUpdatable(allMovedDeclarations)) refExpr.internalUsageInfo = null - } - } - } - - /** - * Filters out usages that are not updatable, such usages might be needed for conflict checking but don't need to be touched during the - * retargeting process. - */ - internal fun filterUpdatable(topLevelMovedElements: Iterable, usages: Array): List { - val allMovedDeclarations = topLevelMovedElements.flatMap { it.withChildDeclarations() } - return usages.filter { if (it is K2MoveRenameUsageInfo) it.isUpdatable(allMovedDeclarations) else true } - } - - /** * Finds usages to [declaration] excluding the usages inside [declaration]. */ @@ -305,15 +279,16 @@ sealed class K2MoveRenameUsageInfo( } internal fun retargetUsages(usages: List, oldToNewMap: Map) { - retargetInternalUsages(oldToNewMap) + // Retarget external usages before internal usages to make sure imports in moved files are properly updated retargetExternalUsages(usages, oldToNewMap) + retargetInternalUsages(oldToNewMap) } /** * * [org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo] * After moving, internal usages might have become invalid, this method restores these usage infos. - * @see internalUsageInfo + * @see updatableUsageInfo */ private fun restoreInternalUsages( containingElem: KtElement, @@ -322,7 +297,7 @@ sealed class K2MoveRenameUsageInfo( ): List { return (containingElem.collectDescendantsOfType() + containingElem.collectDescendantsOfType()) .mapNotNull { refExpr -> - val usageInfo = refExpr.internalUsageInfo + val usageInfo = refExpr.updatableUsageInfo if (!fromCopy && usageInfo?.element != null) return@mapNotNull usageInfo val referencedElement = (usageInfo as? Source)?.referencedElement ?: return@mapNotNull null val newReferencedElement = oldToNewMap[referencedElement] ?: referencedElement @@ -344,7 +319,7 @@ sealed class K2MoveRenameUsageInfo( val inCopy = fileCopy.collectDescendantsOfType().filter(skipPackageStmt) val original = originalFile.collectDescendantsOfType().filter(skipPackageStmt) val internalUsages = original.zip(inCopy).mapNotNull { (o, c) -> - val usageInfo = o.internalUsageInfo + val usageInfo = o.updatableUsageInfo val referencedElement = (usageInfo as? Source)?.referencedElement ?: return@mapNotNull null if (!referencedElement.isValid || referencedElement !is PsiNamedElement || diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveConflictUtil.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveConflictUtil.kt index e8801467b042..31d7f5fd1d06 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveConflictUtil.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveConflictUtil.kt @@ -17,6 +17,7 @@ import org.jetbrains.kotlin.idea.base.projectStructure.toKaModule import org.jetbrains.kotlin.idea.base.util.quoteIfNeeded import org.jetbrains.kotlin.idea.codeinsight.utils.toVisibility import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.internalUsageInfo +import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.K2MoveRenameUsageInfo.Companion.updatableUsageInfo import org.jetbrains.kotlin.idea.k2.refactoring.move.processor.conflict.* import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* @@ -112,17 +113,13 @@ fun createCopyTarget( val oldToNewMap = declarationsToMove.moveInto(fakeTargetFile) val usageInfos = fakeTargetFile.collectOldToNewUsageInfos(oldToNewMap) usageInfos.forEach { (originalUsageInfo, copyUsageInfo) -> - if (!originalUsageInfo.isUpdatable(oldToNewMap.values.toList())) { - (copyUsageInfo.reference?.element as? KtReferenceExpression)?.internalUsageInfo = originalUsageInfo - return@forEach - } - + if ((originalUsageInfo.element as KtElement).updatableUsageInfo == null) return@forEach // if not updatable, skip // Retarget all references to make sure all references are resolvable after moving val retargetResult = copyUsageInfo.retarget(copyUsageInfo.referencedElement as PsiNamedElement) as? KtElement ?: return@forEach val retargetReference = retargetResult.getQualifiedElementSelector() as? KtReferenceExpression ?: return@forEach // Attach physical usage info to the copied reference. // This will make it possible for the conflict checker to check whether a conflict exists before even calling the refactoring. - retargetReference.internalUsageInfo = originalUsageInfo + retargetReference.updatableUsageInfo = originalUsageInfo } fakeTargetFile.originalFile = declarationsToMove.firstOrNull()?.containingKtFile ?: error("Moved element is not in a Kotlin file") return fakeTargetFile to oldToNewMap diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveUsageUtil.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveUsageUtil.kt index 98770badf511..a1847b1a753d 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveUsageUtil.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/src/org/jetbrains/kotlin/idea/k2/refactoring/move/processor/moveUsageUtil.kt @@ -98,7 +98,7 @@ internal fun KtFile.findUsages( searchForText: Boolean, newPkgName: FqName ): List { - markInternalUsages(this) + markInternalUsages(this, this) return topLevelDeclarationsToUpdate.flatMap { decl -> K2MoveRenameUsageInfo.findExternalUsages(decl) + decl.findNonCodeUsages(searchInCommentsAndStrings, searchForText, newPkgName) } diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/AbstractK2MoveFileOrDirectoriesTest.kt b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/AbstractK2MoveFileOrDirectoriesTest.kt index 356aba9b4db8..ffa514d99903 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/AbstractK2MoveFileOrDirectoriesTest.kt +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/AbstractK2MoveFileOrDirectoriesTest.kt @@ -30,6 +30,16 @@ abstract class AbstractK2MoveFileOrDirectoriesTest : AbstractMultifileMoveRefact internal object K2MoveFileOrDirectoriesRefactoringAction : KotlinMoveRefactoringAction { override fun runRefactoring(rootDir: VirtualFile, mainFile: PsiFile, elementsAtCaret: List, config: JsonObject) { val project = mainFile.project + val fileNames = config.getAsJsonArray("filesToMove")?.map { it.asString } + ?: listOfNotNull(config.getString("mainFile")) + val files = if (fileNames.isEmpty()) { + listOf(mainFile) + } else { + fileNames.mapNotNull { path -> + val vFile = rootDir.findFileByRelativePath(path) + vFile?.toPsiFile(project) ?: vFile?.toPsiDirectory(project) + }.toSet() + } if (mainFile.name.endsWith(".java")) { val targetPackage = config.getNullableString("targetPackage") val targetDirPath = targetPackage?.replace('.', '/') ?: config.getNullableString("targetDirectory") ?: return @@ -40,7 +50,7 @@ internal object K2MoveFileOrDirectoriesRefactoringAction : KotlinMoveRefactoring } MoveFilesOrDirectoriesProcessor( project, - arrayOf(mainFile), + files.toTypedArray(), newParent, config.searchInComments(), /* searchInNonJavaFiles = */ true, @@ -48,13 +58,6 @@ internal object K2MoveFileOrDirectoriesRefactoringAction : KotlinMoveRefactoring /* prepareSuccessfulCallback = */ null ).run() } else { - val fileNames = config.getAsJsonArray("filesToMove")?.map { it.asString } - ?: listOfNotNull(config.getString("mainFile")) - if (fileNames.isEmpty()) fail("No file name specified") - val files = fileNames.mapNotNull { path -> - val vFile = rootDir.findFileByRelativePath(path) - vFile?.toPsiFile(project) ?: vFile?.toPsiDirectory(project) - }.toSet() val sourceDescriptor = K2MoveSourceDescriptor.FileSource(files) val targetPackage = config.getNullableString("targetPackage") val targetDir = config.getNullableString("targetDirectory") diff --git a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveFileOrDirectoriesTestGenerated.java b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveFileOrDirectoriesTestGenerated.java index 371744d7e3f9..e01c23ba2fc4 100644 --- a/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveFileOrDirectoriesTestGenerated.java +++ b/plugins/kotlin/refactorings/kotlin.refactorings.move.k2/test/org/jetbrains/kotlin/idea/k2/refactoring/move/K2MoveFileOrDirectoriesTestGenerated.java @@ -40,6 +40,11 @@ public class K2MoveFileOrDirectoriesTestGenerated extends AbstractK2MoveFileOrDi runTest("../../idea/tests/testData/refactoring/moveFile/java/moveFileToAnotherPackage/moveFileToAnotherPackage.test"); } + @TestMetadata("java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test") + public void testJava_movePackageWithDestructuringReference_MovePackageWithDestructuringReference() throws Exception { + runTest("../../idea/tests/testData/refactoring/moveFile/java/movePackageWithDestructuringReference/movePackageWithDestructuringReference.test"); + } + @TestMetadata("kotlin/addExtensionImport/addExtensionImport.test") public void testKotlin_addExtensionImport_AddExtensionImport() throws Exception { runTest("../../idea/tests/testData/refactoring/moveFile/kotlin/addExtensionImport/addExtensionImport.test");