diff --git a/ChangeLog.md b/ChangeLog.md index e144e1df234..af54e085bee 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -237,6 +237,7 @@ - [`KT-9444`](https://youtrack.jetbrains.com/issue/KT-9444) Rename dialog: Allow typing any identifier without backquotes - [`KT-9446`](https://youtrack.jetbrains.com/issue/KT-9446) Warn about calls with default arguments if function to be renamed inherits default values from some base function which is excluded from rename - [`KT-10713`](https://youtrack.jetbrains.com/issue/KT-10713) Skip read-only declarations when renaming parameters +- [`KT-10687`](https://youtrack.jetbrains.com/issue/KT-10687) Qualify property references to avoid shadowing by parameters - [`KT-12543`](https://youtrack.jetbrains.com/issue/KT-12543) Qualify property references with 'this' to avoid renaming conflicts #### Java to Kotlin converter diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinParameterProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinParameterProcessor.kt index da48e278655..0a5f4d30c84 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinParameterProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/RenameKotlinParameterProcessor.kt @@ -20,18 +20,41 @@ import com.intellij.psi.PsiElement import com.intellij.psi.PsiMethod import com.intellij.psi.PsiNamedElement import com.intellij.psi.search.SearchScope +import com.intellij.refactoring.listeners.RefactoringElementListener +import com.intellij.usageView.UsageInfo +import org.jetbrains.kotlin.asJava.namedUnwrappedElement import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.VariableDescriptor import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor import org.jetbrains.kotlin.idea.refactoring.canRefactor import org.jetbrains.kotlin.idea.refactoring.getAffectedCallables import org.jetbrains.kotlin.psi.KtCallableDeclaration +import org.jetbrains.kotlin.psi.KtNamedDeclaration import org.jetbrains.kotlin.psi.KtNamedFunction import org.jetbrains.kotlin.psi.KtParameter import org.jetbrains.kotlin.resolve.OverrideResolver +import org.jetbrains.kotlin.utils.SmartList class RenameKotlinParameterProcessor : RenameKotlinPsiProcessor() { override fun canProcessElement(element: PsiElement) = element is KtParameter && element.ownerFunction is KtNamedFunction + override fun findCollisions( + element: PsiElement, + newName: String?, + allRenames: MutableMap, + result: MutableList + ) { + if (newName == null) return + val declaration = element.namedUnwrappedElement as? KtNamedDeclaration ?: return + val descriptor = declaration.resolveToDescriptor() as VariableDescriptor + + val collisions = SmartList() + checkRedeclarations(descriptor, newName, collisions) + checkOriginalUsagesRetargeting(declaration, newName, result, collisions) + checkNewNameUsagesRetargeting(declaration, newName, collisions) + result += collisions + } + override fun prepareRenaming(element: PsiElement, newName: String?, allRenames: MutableMap, scope: SearchScope) { super.prepareRenaming(element, newName, allRenames, scope) @@ -51,4 +74,10 @@ class RenameKotlinParameterProcessor : RenameKotlinPsiProcessor() { allRenames[parameter] = newName } } + + override fun renameElement(element: PsiElement, newName: String?, usages: Array, listener: RefactoringElementListener?) { + super.renameElement(element, newName, usages, listener) + + usages.forEach { (it as? KtResolvableCollisionUsageInfo)?.apply() } + } } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/renameConflictUtils.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/renameConflictUtils.kt index 566fb9f6f62..0dfae8c7c18 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/renameConflictUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/rename/renameConflictUtils.kt @@ -30,6 +30,7 @@ import org.jetbrains.kotlin.idea.core.NewDeclarationNameValidator import org.jetbrains.kotlin.idea.core.copied import org.jetbrains.kotlin.idea.imports.importableFqName import org.jetbrains.kotlin.idea.refactoring.explicateAsText +import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.refactoring.getThisLabelName import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.search.and @@ -59,6 +60,7 @@ import org.jetbrains.kotlin.resolve.source.getPsi import org.jetbrains.kotlin.types.ErrorUtils import org.jetbrains.kotlin.utils.SmartList import org.jetbrains.kotlin.utils.addToStdlib.singletonOrEmptyList +import java.util.* internal fun ResolvedCall<*>.noReceivers() = dispatchReceiver == null && extensionReceiver == null @@ -80,24 +82,31 @@ internal fun checkRedeclarations( newName: String, result: MutableList ) { - val containingDescriptor = descriptor.containingDeclaration - val containingScope = when (containingDescriptor) { - is ClassDescriptor -> containingDescriptor.unsubstitutedMemberScope - is PackageFragmentDescriptor -> containingDescriptor.getMemberScope() - else -> return - } - val descriptorKindFilter = when (descriptor) { - is ClassDescriptor -> DescriptorKindFilter.CLASSIFIERS - is PropertyDescriptor -> DescriptorKindFilter.VARIABLES - else -> return - } - containingScope.getDescriptorsFiltered(descriptorKindFilter) { it.asString() == newName }.firstOrNull()?.let { candidateDescriptor -> - val candidate = (candidateDescriptor as? DeclarationDescriptorWithSource)?.source?.getPsi() as? KtNamedDeclaration ?: return - val what = candidate.renderDescription().capitalize() - val where = candidate.representativeContainer()?.renderDescription() ?: return - val message = "$what is already declared in $where" - result += BasicUnresolvableCollisionUsageInfo(candidate, candidate, message) + fun getSiblingWithNewName(): DeclarationDescriptor? { + if (descriptor is ValueParameterDescriptor) { + return descriptor.containingDeclaration.valueParameters.firstOrNull { it.name.asString() == newName } + } + + val containingDescriptor = descriptor.containingDeclaration + val containingScope = when (containingDescriptor) { + is ClassDescriptor -> containingDescriptor.unsubstitutedMemberScope + is PackageFragmentDescriptor -> containingDescriptor.getMemberScope() + else -> return null + } + val descriptorKindFilter = when (descriptor) { + is ClassDescriptor -> DescriptorKindFilter.CLASSIFIERS + is PropertyDescriptor -> DescriptorKindFilter.VARIABLES + else -> return null + } + return containingScope.getDescriptorsFiltered(descriptorKindFilter) { it.asString() == newName }.firstOrNull() } + + val candidateDescriptor = getSiblingWithNewName() ?: return + val candidate = (candidateDescriptor as? DeclarationDescriptorWithSource)?.source?.getPsi() as? KtNamedDeclaration ?: return + val what = candidate.renderDescription().capitalize() + val where = candidate.representativeContainer()?.renderDescription() ?: return + val message = "$what is already declared in $where" + result += BasicUnresolvableCollisionUsageInfo(candidate, candidate, message) } private fun LexicalScope.getRelevantDescriptors( @@ -261,6 +270,32 @@ internal fun checkNewNameUsagesRetargeting( ) { val currentName = declaration.name ?: return val descriptor = declaration.resolveToDescriptor() + + if (declaration is KtParameter && !declaration.hasValOrVar()) { + val ownerFunction = declaration.ownerFunction + val searchScope = (if (ownerFunction is KtPrimaryConstructor) ownerFunction.containingClassOrObject else ownerFunction) ?: return + + val usagesByCandidate = LinkedHashMap>() + + searchScope.accept( + object: KtTreeVisitorVoid() { + override fun visitSimpleNameExpression(expression: KtSimpleNameExpression) { + if (expression.getReferencedName() != newName) return + val ref = expression.mainReference + val candidate = ref.resolve() as? PsiNamedElement ?: return + usagesByCandidate.getOrPut(candidate) { SmartList() }.add(MoveRenameUsageInfo(ref, candidate)) + } + } + ) + + for ((candidate, usages) in usagesByCandidate) { + checkUsagesRetargeting(candidate, declaration, currentName, false, listOf(descriptor), usages, newUsages) + usages.filterIsInstanceTo>(newUsages) + } + + return + } + for (candidateDescriptor in declaration.getResolutionScope().getRelevantDescriptors(declaration, newName)) { val candidate = DescriptorToSourceUtilsIde.getAnyDeclaration(declaration.project, candidateDescriptor) as? PsiNamedElement ?: continue val usages = ReferencesSearch diff --git a/idea/testData/refactoring/rename/clashParameterWithProperty/after/test/test.kt b/idea/testData/refactoring/rename/clashParameterWithProperty/after/test/test.kt new file mode 100644 index 00000000000..98c111eaee1 --- /dev/null +++ b/idea/testData/refactoring/rename/clashParameterWithProperty/after/test/test.kt @@ -0,0 +1,11 @@ +package test + +class Foo { + var foo: Int = 0 + var bar: Int = 0 +} + +fun makeFoo(foo: Int, _bar: Int) = Foo().apply { + this.foo = foo + bar = _bar +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/clashParameterWithProperty/before/test/test.kt b/idea/testData/refactoring/rename/clashParameterWithProperty/before/test/test.kt new file mode 100644 index 00000000000..cf74e142a36 --- /dev/null +++ b/idea/testData/refactoring/rename/clashParameterWithProperty/before/test/test.kt @@ -0,0 +1,11 @@ +package test + +class Foo { + var foo: Int = 0 + var bar: Int = 0 +} + +fun makeFoo(/*rename*/_foo: Int, _bar: Int) = Foo().apply { + foo = _foo + bar = _bar +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/clashParameterWithProperty/clashParameterWithProperty.test b/idea/testData/refactoring/rename/clashParameterWithProperty/clashParameterWithProperty.test new file mode 100644 index 00000000000..97c4fd11919 --- /dev/null +++ b/idea/testData/refactoring/rename/clashParameterWithProperty/clashParameterWithProperty.test @@ -0,0 +1,6 @@ +{ + "type": "MARKED_ELEMENT", + "mainFile": "test/test.kt", + "newName": "foo", + "withRuntime": "true" +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/parameterRedeclaration/before/test/test.kt b/idea/testData/refactoring/rename/parameterRedeclaration/before/test/test.kt new file mode 100644 index 00000000000..676c79297f8 --- /dev/null +++ b/idea/testData/refactoring/rename/parameterRedeclaration/before/test/test.kt @@ -0,0 +1,5 @@ +package test + +fun foo(/*rename*/a: Int, b: String) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/rename/parameterRedeclaration/parameterRedeclaration.test b/idea/testData/refactoring/rename/parameterRedeclaration/parameterRedeclaration.test new file mode 100644 index 00000000000..723a26f0428 --- /dev/null +++ b/idea/testData/refactoring/rename/parameterRedeclaration/parameterRedeclaration.test @@ -0,0 +1,7 @@ +{ + "type": "MARKED_ELEMENT", + "mainFile": "test/test.kt", + "newName": "b", + "withRuntime": "true", + "hint": "Parameter 'b' is already declared in function 'foo'" +} \ No newline at end of file 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 a1c4e1f6ade..979d644b6d5 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/rename/RenameTestGenerated.java @@ -119,6 +119,12 @@ public class RenameTestGenerated extends AbstractRenameTest { doTest(fileName); } + @TestMetadata("clashParameterWithProperty/clashParameterWithProperty.test") + public void testClashParameterWithProperty_ClashParameterWithProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/clashParameterWithProperty/clashParameterWithProperty.test"); + doTest(fileName); + } + @TestMetadata("clashWithInnerClass/clashWithInnerClass.test") public void testClashWithInnerClass_ClashWithInnerClass() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/clashWithInnerClass/clashWithInnerClass.test"); @@ -185,6 +191,12 @@ public class RenameTestGenerated extends AbstractRenameTest { doTest(fileName); } + @TestMetadata("parameterRedeclaration/parameterRedeclaration.test") + public void testParameterRedeclaration_ParameterRedeclaration() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/parameterRedeclaration/parameterRedeclaration.test"); + doTest(fileName); + } + @TestMetadata("propertyAccidentalOverrideSubclass/propertyAccidentalOverrideSubclass.test") public void testPropertyAccidentalOverrideSubclass_PropertyAccidentalOverrideSubclass() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/rename/propertyAccidentalOverrideSubclass/propertyAccidentalOverrideSubclass.test");