From fe01035e3c5d0368812a58598502fe27042627a6 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Wed, 1 Jun 2016 12:19:28 +0300 Subject: [PATCH] Rename: Do not rename ambiguous references in import directives #KT-6363 Fixed --- ChangeLog.md | 1 + .../rename/AutomaticOverloadsRenamer.kt | 11 ++++ .../rename/RenameKotlinFunctionProcessor.kt | 52 +++++++++++++++++-- .../after/lib/lib.kt | 9 ++++ .../after/usage/JUsage.java | 12 +++++ .../after/usage/ktUsage.kt | 9 ++++ ...omaticRenamerOverloadsAmbiguousImport.test | 7 +++ .../before/lib/lib.kt | 9 ++++ .../before/usage/JUsage.java | 10 ++++ .../before/usage/ktUsage.kt | 8 +++ .../refactoring/rename/AbstractRenameTest.kt | 20 ++++--- .../rename/RenameTestGenerated.java | 6 +++ 12 files changed, 141 insertions(+), 13 deletions(-) create mode 100644 idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/lib/lib.kt create mode 100644 idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/JUsage.java create mode 100644 idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/ktUsage.kt create mode 100644 idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/automaticRenamerOverloadsAmbiguousImport.test create mode 100644 idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/lib/lib.kt create mode 100644 idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/JUsage.java create mode 100644 idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/ktUsage.kt diff --git a/ChangeLog.md b/ChangeLog.md index dd9de1aa71c..491614e0f35 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -185,6 +185,7 @@ - [`KT-8512`](https://youtrack.jetbrains.com/issue/KT-8512) Support "Rename tests" options in Rename dialog ###### Issues fixed +- [`KT-6363`](https://youtrack.jetbrains.com/issue/KT-6363) Do not rename ambiguous references in import directives - [`KT-8541`](https://youtrack.jetbrains.com/issue/KT-8541), [`KT-8786`](https://youtrack.jetbrains.com/issue/KT-8786) Do now show 'Rename overloads' options if target function has no overloads - [`KT-8544`](https://youtrack.jetbrains.com/issue/KT-8544) Show more detailed description in Rename dialog - [`KT-8860`](https://youtrack.jetbrains.com/issue/KT-8860) Allow renaming class by constructor delegation call referencing primary constructor diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/AutomaticOverloadsRenamer.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/AutomaticOverloadsRenamer.kt index 44a835fa593..0bc933436bb 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/AutomaticOverloadsRenamer.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/AutomaticOverloadsRenamer.kt @@ -16,12 +16,14 @@ package org.jetbrains.kotlin.idea.refactoring.rename +import com.intellij.openapi.util.Key import com.intellij.psi.PsiElement import com.intellij.refactoring.JavaRefactoringSettings import com.intellij.refactoring.RefactoringBundle import com.intellij.refactoring.rename.naming.AutomaticRenamer import com.intellij.refactoring.rename.naming.AutomaticRenamerFactory import com.intellij.usageView.UsageInfo +import org.jetbrains.annotations.TestOnly import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor @@ -30,13 +32,22 @@ import org.jetbrains.kotlin.idea.util.getAllAccessibleFunctions import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.UserDataProperty import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.resolve.source.getPsi class AutomaticOverloadsRenamer(function: KtNamedFunction, newName: String) : AutomaticRenamer() { + companion object { + @get:TestOnly + @set:TestOnly + internal var PsiElement.elementFilter: ((PsiElement) -> Boolean)? by UserDataProperty(Key.create("ELEMENT_FILTER")) + } + init { + val filter = function.elementFilter function.getOverloads().mapNotNullTo(myElements) { val candidate = it.source.getPsi() as? KtNamedFunction ?: return@mapNotNullTo null + if (filter != null && !filter(candidate)) return@mapNotNullTo null if (candidate != function) candidate else null } suggestAllNames(function.name, newName) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinFunctionProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinFunctionProcessor.kt index b06d0e7bec0..3b6fc835ea8 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinFunctionProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinFunctionProcessor.kt @@ -17,13 +17,14 @@ package org.jetbrains.kotlin.idea.refactoring.rename import com.intellij.openapi.editor.Editor -import com.intellij.psi.PsiElement -import com.intellij.psi.PsiMethod -import com.intellij.psi.PsiReference +import com.intellij.openapi.util.Key +import com.intellij.psi.* import com.intellij.psi.search.SearchScope import com.intellij.refactoring.listeners.RefactoringElementListener import com.intellij.refactoring.rename.RenameJavaMethodProcessor +import com.intellij.refactoring.rename.RenameUtil import com.intellij.usageView.UsageInfo +import com.intellij.util.SmartList import org.jetbrains.kotlin.asJava.KtLightElement import org.jetbrains.kotlin.asJava.KtLightMethod import org.jetbrains.kotlin.asJava.LightClassUtil @@ -33,8 +34,12 @@ import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor import org.jetbrains.kotlin.idea.refactoring.dropOverrideKeywordIfNecessary import org.jetbrains.kotlin.idea.references.KtReference import org.jetbrains.kotlin.idea.util.application.runReadAction +import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType +import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.ImportPath import java.util.* class RenameKotlinFunctionProcessor : RenameKotlinPsiProcessor() { @@ -89,18 +94,55 @@ class RenameKotlinFunctionProcessor : RenameKotlinPsiProcessor() { } } + private var PsiElement.ambiguousImportUsages: List? by UserDataProperty(Key.create("AMBIGUOUS_IMPORT_USAGES")) + + override fun getPostRenameCallback(element: PsiElement, newName: String?, elementListener: RefactoringElementListener?): Runnable? { + if (newName == null) return null + + return Runnable { + element.ambiguousImportUsages?.forEach { + val ref = it.reference as? PsiPolyVariantReference ?: return@forEach + if (ref.multiResolve(false).isEmpty()) { + ref.handleElementRename(newName) + } + else { + ref.element?.getStrictParentOfType()?.let { importDirective -> + val fqName = importDirective.importedFqName!! + val newFqName = fqName.parent().child(Name.identifier(newName)) + val importList = importDirective.parent as KtImportList + if (importList.imports.none { it.importedFqName == newFqName }) { + val newImportDirective = KtPsiFactory(element).createImportDirective(ImportPath(newFqName, false)) + importDirective.parent.addAfter(newImportDirective, importDirective) + } + } + } + } + element.ambiguousImportUsages = null + } + } + override fun renameElement(element: PsiElement, newName: String?, usages: Array, listener: RefactoringElementListener?) { val simpleUsages = ArrayList(usages.size) + val ambiguousImportUsages = SmartList() for (usage in usages) { if (usage is LostDefaultValuesInOverridingFunctionUsageInfo) { usage.apply() continue } - simpleUsages += usage + val ref = usage.reference as? PsiPolyVariantReference ?: continue + val refElement = ref.element + if (refElement.parents.any { (it is KtImportDirective && !it.isAllUnder) || (it is PsiImportStaticStatement && !it.isOnDemand) } + && ref.multiResolve(false).size > 1) { + ambiguousImportUsages += usage + } + else { + simpleUsages += usage + } } + element.ambiguousImportUsages = ambiguousImportUsages - super.renameElement(element, newName, usages, listener) + RenameUtil.doRenameGenericNamedElement(element, newName, simpleUsages.toTypedArray(), listener) (element.unwrapped as? KtNamedDeclaration)?.let { dropOverrideKeywordIfNecessary(it) } } diff --git a/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/lib/lib.kt b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/lib/lib.kt new file mode 100644 index 00000000000..4ae4efc9970 --- /dev/null +++ b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/lib/lib.kt @@ -0,0 +1,9 @@ +package lib + +fun bar(n: Int) { + +} + +fun foo(n: Int, s: String) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/JUsage.java b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/JUsage.java new file mode 100644 index 00000000000..7e2067df9de --- /dev/null +++ b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/JUsage.java @@ -0,0 +1,12 @@ +package usage; + +import lib.LibKt; + +import static lib.LibKt.foo; + +public class JUsage { + void test() { + LibKt.bar(1); + foo(1, "2"); + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/ktUsage.kt b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/ktUsage.kt new file mode 100644 index 00000000000..4843d744dc2 --- /dev/null +++ b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/after/usage/ktUsage.kt @@ -0,0 +1,9 @@ +package usage + +import lib.foo +import lib.bar + +fun test() { + bar(1) + foo(1, "2") +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/automaticRenamerOverloadsAmbiguousImport.test b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/automaticRenamerOverloadsAmbiguousImport.test new file mode 100644 index 00000000000..aef815f1808 --- /dev/null +++ b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/automaticRenamerOverloadsAmbiguousImport.test @@ -0,0 +1,7 @@ +{ + "type": "MARKED_ELEMENT", + "mainFile": "lib/lib.kt", + "newName": "bar", + "withRuntime": "true", + "overloadRenamer.onlyPrimaryElement": "true" +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/lib/lib.kt b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/lib/lib.kt new file mode 100644 index 00000000000..35f7647db02 --- /dev/null +++ b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/lib/lib.kt @@ -0,0 +1,9 @@ +package lib + +fun /*rename*/foo(n: Int) { + +} + +fun foo(n: Int, s: String) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/JUsage.java b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/JUsage.java new file mode 100644 index 00000000000..002b10f819f --- /dev/null +++ b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/JUsage.java @@ -0,0 +1,10 @@ +package usage; + +import static lib.LibKt.foo; + +public class JUsage { + void test() { + foo(1); + foo(1, "2"); + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/ktUsage.kt b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/ktUsage.kt new file mode 100644 index 00000000000..b6413c6006b --- /dev/null +++ b/idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/before/usage/ktUsage.kt @@ -0,0 +1,8 @@ +package usage + +import lib.foo + +fun test() { + foo(1) + foo(1, "2") +} \ 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 333b65266d3..963b8482918 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/AbstractRenameTest.kt +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/AbstractRenameTest.kt @@ -199,7 +199,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { val searchInComments = renameParamsObject["searchInComments"]?.asBoolean ?: true val searchInTextOccurrences = renameParamsObject["searchInTextOccurrences"]?.asBoolean ?: true - runRenameProcessor(context, newName, substitution, searchInComments, searchInTextOccurrences) + runRenameProcessor(context, newName, substitution, renameParamsObject, searchInComments, searchInTextOccurrences) } } @@ -211,7 +211,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { val aClass = context.javaFacade.findClass(classFQN, context.project.allScope())!! val substitution = RenamePsiElementProcessor.forElement(aClass).substituteElementToRename(aClass, null) - runRenameProcessor(context, newName, substitution, true, true) + runRenameProcessor(context, newName, substitution, renameParamsObject, true, true) } } @@ -229,7 +229,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { if (method == null) throw IllegalStateException("Method with signature '$methodSignature' wasn't found in class $classFQN") val substitution = RenamePsiElementProcessor.forElement(method).substituteElementToRename(method, null) - runRenameProcessor(context, newName, substitution, false, false) + runRenameProcessor(context, newName, substitution, renameParamsObject, false, false) } } @@ -272,7 +272,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { val psiElement = segmentReference.resolve()!! val substitution = RenamePsiElementProcessor.forElement(psiElement).substituteElementToRename(psiElement, null) - runRenameProcessor(context, newName, substitution, true, true) + runRenameProcessor(context, newName, substitution, renameParamsObject, true, true) } } @@ -284,7 +284,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { val mainFile = rootDir.findChild(file)!! val psiFile = PsiManager.getInstance(context.project).findFile(mainFile) - runRenameProcessor(context, newName, psiFile, true, true) + runRenameProcessor(context, newName, psiFile, renameParamsObject, true, true) } } @@ -298,7 +298,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { val psiFile = PsiManager.getInstance(context.project).findFile(mainFile) as PropertiesFile val property = psiFile.findPropertyByKey(oldName) as Property - runRenameProcessor(context, newName, property, true, true) + runRenameProcessor(context, newName, property, renameParamsObject, true, true) } } @@ -334,7 +334,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { val substitution = RenamePsiElementProcessor.forElement(psiElement).substituteElementToRename(psiElement, null) - runRenameProcessor(context, newName, substitution, true, true) + runRenameProcessor(context, newName, substitution, renameParamsObject, true, true) } } @@ -358,7 +358,7 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { val substitution = RenamePsiElementProcessor.forElement(propertyWrapper).substituteElementToRename(propertyWrapper, null) - runRenameProcessor(context, newName, substitution, true, true) + runRenameProcessor(context, newName, substitution, renameParamsObject, true, true) } } @@ -366,10 +366,14 @@ abstract class AbstractRenameTest : KotlinMultiFileTestCase() { context: TestContext, newName: String, substitution: PsiElement?, + renameParamsObject: JsonObject, isSearchInComments: Boolean, isSearchTextOccurrences: Boolean ) { val renameProcessor = RenameProcessor(context.project, substitution, newName, isSearchInComments, isSearchTextOccurrences) + if (renameParamsObject["overloadRenamer.onlyPrimaryElement"]?.asBoolean ?: false) { + with(AutomaticOverloadsRenamer) { substitution?.elementFilter = { false } } + } Extensions.getExtensions(AutomaticRenamerFactory.EP_NAME).forEach { renameProcessor.addRenamerFactory(it) } renameProcessor.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 b7840634ed3..4f81370039d 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java @@ -65,6 +65,12 @@ public class RenameTestGenerated extends AbstractRenameTest { doTest(fileName); } + @TestMetadata("automaticRenamerOverloadsAmbiguousImport/automaticRenamerOverloadsAmbiguousImport.test") + public void testAutomaticRenamerOverloadsAmbiguousImport_AutomaticRenamerOverloadsAmbiguousImport() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/automaticRenamerOverloadsAmbiguousImport/automaticRenamerOverloadsAmbiguousImport.test"); + doTest(fileName); + } + @TestMetadata("automaticRenamerOverloadsClass/class.test") public void testAutomaticRenamerOverloadsClass_Class() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/automaticRenamerOverloadsClass/class.test");