Rename: Implement conflict analysis for parameters. Qualify property references to avoid shadowing by parameters

#KT-10687 Fixed
(cherry picked from commit 7d6466d)

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

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<out PsiElement, String>,
+            result: MutableList<UsageInfo>
+    ) {
+        if (newName == null) return
+        val declaration = element.namedUnwrappedElement as? KtNamedDeclaration ?: return
+        val descriptor = declaration.resolveToDescriptor() as VariableDescriptor
+
+        val collisions = SmartList<UsageInfo>()
+        checkRedeclarations(descriptor, newName, collisions)
+        checkOriginalUsagesRetargeting(declaration, newName, result, collisions)
+        checkNewNameUsagesRetargeting(declaration, newName, collisions)
+        result += collisions
+    }
+
     override fun prepareRenaming(element: PsiElement, newName: String?, allRenames: MutableMap<PsiElement, String>, 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<out UsageInfo>, listener: RefactoringElementListener?) {
+        super.renameElement(element, newName, usages, listener)
+
+        usages.forEach { (it as? KtResolvableCollisionUsageInfo)?.apply() }
+    }
 }

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<UsageInfo>
 ) {
-    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<PsiElement, MutableList<UsageInfo>>()
+
+        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<KtResolvableCollisionUsageInfo, MutableList<UsageInfo>>(newUsages)
+        }
+
+        return
+    }
+
     for (candidateDescriptor in declaration.getResolutionScope().getRelevantDescriptors(declaration, newName)) {
         val candidate = DescriptorToSourceUtilsIde.getAnyDeclaration(declaration.project, candidateDescriptor) as? PsiNamedElement ?: continue
         val usages = ReferencesSearch

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
+}

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
+}

idea/testData/refactoring/rename/clashParameterWithProperty/clashParameterWithProperty.test

@@ -0,0 +1,6 @@
+{
+  "type": "MARKED_ELEMENT",
+  "mainFile": "test/test.kt",
+  "newName": "foo",
+  "withRuntime": "true"
+}

idea/testData/refactoring/rename/parameterRedeclaration/before/test/test.kt

@@ -0,0 +1,5 @@
+package test
+
+fun foo(/*rename*/a: Int, b: String) {
+
+}

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'"
+}

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");