Change Signature: Avoid creating parameters from scratch

#KT-15519 Fixed

compiler/frontend/src/org/jetbrains/kotlin/psi/addRemoveModifier.kt

@@ -24,13 +24,26 @@ import org.jetbrains.kotlin.lexer.KtTokens.*
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.siblings
 
-private fun createModifierList(text: String, owner: KtModifierListOwner): KtModifierList {
-    val newModifierList = KtPsiFactory(owner).createModifierList(text)
-    val anchor = owner.firstChild!!
+private fun KtModifierListOwner.addModifierList(newModifierList: KtModifierList): KtModifierList {
+    val anchor = firstChild!!
             .siblings(forward = true)
             .dropWhile { it is PsiComment || it is PsiWhiteSpace }
             .first()
-    return owner.addBefore(newModifierList, anchor) as KtModifierList
+    return addBefore(newModifierList, anchor) as KtModifierList
+}
+
+private fun createModifierList(text: String, owner: KtModifierListOwner): KtModifierList {
+    return owner.addModifierList(KtPsiFactory(owner).createModifierList(text))
+}
+
+fun KtModifierListOwner.setModifierList(newModifierList: KtModifierList) {
+    val currentModifierList = modifierList
+    if (currentModifierList != null) {
+        currentModifierList.replace(newModifierList)
+    }
+    else {
+        addModifierList(newModifierList)
+    }
 }
 
 fun addModifier(owner: KtModifierListOwner, modifier: KtModifierKeywordToken) {

idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt

@@ -313,3 +313,10 @@ fun KtCallableDeclaration.setReceiverType(type: KotlinType) {
     ShortenReferences.DEFAULT.process(receiverTypeReference!!)
 }
 
+fun KtParameter.setDefaultValue(newDefaultValue: KtExpression): PsiElement? {
+    defaultValue?.let { return it.replaced(newDefaultValue) }
+
+    val psiFactory = KtPsiFactory(this)
+    val eq = equalsToken ?: add(psiFactory.createEQ())
+    return addAfter(newDefaultValue, eq) as KtExpression
+}

idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/KotlinChangeInfo.kt

@@ -284,11 +284,11 @@ open class KotlinChangeInfo(
 
         val isLambda = inheritedCallable.declaration is KtFunctionLiteral
         if (isLambda && signatureParameters.size == 1 && !signatureParameters[0].requiresExplicitType(inheritedCallable)) {
-            return signatureParameters[0].getDeclarationSignature(0, inheritedCallable)
+            return signatureParameters[0].getDeclarationSignature(0, inheritedCallable).text
         }
 
         return signatureParameters.indices
-                .map { i -> signatureParameters[i].getDeclarationSignature(i, inheritedCallable) }
+                .map { i -> signatureParameters[i].getDeclarationSignature(i, inheritedCallable).text }
                 .joinToString(separator = ", ")
     }
 

idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/KotlinChangeSignatureData.kt

@@ -65,8 +65,7 @@ class KotlinChangeSignatureData(
                             name = parameterDescriptor.name.asString(),
                             originalTypeInfo = KotlinTypeInfo(false, parameterType, parameterTypeText),
                             defaultValueForParameter = jetParameter?.defaultValue,
-                            valOrVar = jetParameter?.valOrVarKeyword.toValVar(),
-                            modifierList = jetParameter?.modifierList
+                            valOrVar = jetParameter?.valOrVarKeyword.toValVar()
                     )
                 }
     }

idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/KotlinParameterInfo.kt

@@ -22,11 +22,14 @@ import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.descriptors.impl.AnonymousFunctionDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.core.compareDescriptors
+import org.jetbrains.kotlin.idea.core.copied
 import org.jetbrains.kotlin.idea.core.quoteIfNeeded
+import org.jetbrains.kotlin.idea.core.setDefaultValue
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.usages.KotlinCallableDefinitionUsage
 import org.jetbrains.kotlin.idea.references.KtReference
 import org.jetbrains.kotlin.idea.references.mainReference
 import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.addRemoveModifier.setModifierList
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
@@ -186,7 +189,15 @@ class KotlinParameterInfo @JvmOverloads constructor (
         return parameter.typeReference != null
     }
 
-    fun getDeclarationSignature(parameterIndex: Int, inheritedCallable: KotlinCallableDefinitionUsage<*>): String {
+    private fun getOriginalParameter(inheritedCallable: KotlinCallableDefinitionUsage<*>): KtParameter? {
+        val currentFunction = inheritedCallable.declaration as? KtFunction ?: return null
+        val originalParameterIndex = if (currentFunction.receiverTypeReference == null) originalIndex else originalIndex - 1
+        return currentFunction.valueParameters.getOrNull(originalParameterIndex)
+    }
+
+    private fun buildNewParameter(inheritedCallable: KotlinCallableDefinitionUsage<*>, parameterIndex: Int): KtParameter {
+        val psiFactory = KtPsiFactory(inheritedCallable.project)
+
         val buffer = StringBuilder()
 
         if (modifierList != null) {
@@ -207,6 +218,35 @@ class KotlinParameterInfo @JvmOverloads constructor (
             defaultValueForParameter?.let { buffer.append(" = ").append(it.text) }
         }
 
-        return buffer.toString()
+        return psiFactory.createParameter(buffer.toString())
+    }
+
+    fun getDeclarationSignature(parameterIndex: Int, inheritedCallable: KotlinCallableDefinitionUsage<*>): KtParameter {
+        val originalParameter = getOriginalParameter(inheritedCallable)
+                                ?: return buildNewParameter(inheritedCallable, parameterIndex)
+
+        val psiFactory = KtPsiFactory(originalParameter)
+        val newParameter = originalParameter.copied()
+
+        modifierList?.let { newParameter.setModifierList(it) }
+
+        if (valOrVar != newParameter.valOrVarKeyword.toValVar()) {
+            newParameter.setValOrVar(valOrVar)
+        }
+
+        val newName = getInheritedName(inheritedCallable)
+        if (newParameter.name != newName) {
+            newParameter.setName(newName.quoteIfNeeded())
+        }
+
+        if (newParameter.typeReference != null || requiresExplicitType(inheritedCallable)) {
+            newParameter.typeReference = psiFactory.createType(renderType(parameterIndex, inheritedCallable))
+        }
+
+        if (!inheritedCallable.isInherited) {
+            defaultValueForParameter?.let { newParameter.setDefaultValue(it) }
+        }
+
+        return newParameter
     }
 }

idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/KotlinValVar.kt

@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.refactoring.changeSignature
 
 import com.intellij.psi.PsiElement
 import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.KtParameter
 import org.jetbrains.kotlin.psi.KtPsiFactory
 
 enum class KotlinValVar(val keywordName: String) {
@@ -44,3 +45,24 @@ fun PsiElement?.toValVar(): KotlinValVar {
         else -> throw IllegalArgumentException("Unknown val/var token: " + text)
     }
 }
+
+fun KtParameter.setValOrVar(valOrVar: KotlinValVar): PsiElement? {
+    val newKeyword = valOrVar.createKeyword(KtPsiFactory(this))
+    val currentKeyword = valOrVarKeyword
+
+    if (currentKeyword != null) {
+        return if (newKeyword == null) {
+            currentKeyword.delete()
+            null
+        }
+        else {
+            currentKeyword.replace(newKeyword)
+        }
+    }
+
+    if (newKeyword == null) return null
+
+    nameIdentifier?.let { return addBefore(newKeyword, it) }
+    modifierList?.let { return addAfter(newKeyword, it) }
+    return addAfter(newKeyword, null)
+}

idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/KotlinCallableDefinitionUsage.kt

@@ -28,14 +28,11 @@ import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.codeInsight.shorten.addToShorteningWaitSet
 import org.jetbrains.kotlin.idea.core.setVisibility
 import org.jetbrains.kotlin.idea.core.toKeywordToken
-import org.jetbrains.kotlin.idea.refactoring.changeSignature.KotlinChangeInfo
-import org.jetbrains.kotlin.idea.refactoring.changeSignature.KotlinParameterInfo
-import org.jetbrains.kotlin.idea.refactoring.changeSignature.KotlinValVar
-import org.jetbrains.kotlin.idea.refactoring.changeSignature.getCallableSubstitutor
 import org.jetbrains.kotlin.idea.refactoring.dropOverrideKeywordIfNecessary
 import org.jetbrains.kotlin.idea.refactoring.replaceListPsiAndKeepDelimiters
 import org.jetbrains.kotlin.idea.core.ShortenReferences
 import org.jetbrains.kotlin.idea.core.ShortenReferences.Options
+import org.jetbrains.kotlin.idea.refactoring.changeSignature.*
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.getElementTextWithContext
@@ -237,31 +234,15 @@ class KotlinCallableDefinitionUsage<T : PsiElement>(
     }
 
     private fun changeParameter(parameterIndex: Int, parameter: KtParameter, parameterInfo: KotlinParameterInfo) {
-        val valOrVarKeyword = parameter.valOrVarKeyword
-        val valOrVar = parameterInfo.valOrVar
+        parameter.setValOrVar(parameterInfo.valOrVar)
 
         val psiFactory = KtPsiFactory(project)
-        val newKeyword = valOrVar.createKeyword(psiFactory)
-        if (valOrVarKeyword != null) {
-            if (newKeyword != null) {
-                valOrVarKeyword.replace(newKeyword)
-            }
-            else {
-                valOrVarKeyword.delete()
-            }
-        }
-        else if (valOrVar != KotlinValVar.None && newKeyword != null) {
-            val firstChild = parameter.firstChild
-            parameter.addBefore(newKeyword, firstChild)
-            parameter.addBefore(psiFactory.createWhiteSpace(), firstChild)
-        }
 
         if (parameterInfo.isTypeChanged && parameter.typeReference != null) {
             val renderedType = parameterInfo.renderType(parameterIndex, this)
             parameter.typeReference = psiFactory.createType(renderedType)
         }
 
-
         val inheritedName = parameterInfo.getInheritedName(this)
         if (Name.isValidIdentifier(inheritedName)) {
             val newIdentifier = psiFactory.createIdentifier(inheritedName)

idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/KotlinCallerUsages.kt

@@ -34,15 +34,12 @@ class KotlinCallerUsage(element: KtNamedDeclaration): KotlinUsageInfo<KtNamedDec
             is KtClass -> element.createPrimaryConstructorParameterListIfAbsent()
             else -> null
         } ?: return true
-        val psiFactory = KtPsiFactory(project)
         changeInfo.getNonReceiverParameters()
                 .withIndex()
                 .filter { it.value.isNewParameter }
                 .forEach {
-                    val parameterText = it.value.getDeclarationSignature(it.index, changeInfo.methodDescriptor.originalPrimaryCallable)
-                    parameterList
-                            .addParameter(psiFactory.createParameter(parameterText))
-                            .addToShorteningWaitSet()
+                    val newParameter = it.value.getDeclarationSignature(it.index, changeInfo.methodDescriptor.originalPrimaryCallable)
+                    parameterList.addParameter(newParameter).addToShorteningWaitSet()
                 }
 
         return true

idea/testData/refactoring/changeSignature/RemoveParameterKeepOtherCommentsAfter.kt

@@ -0,0 +1,7 @@
+data class <caret>VertexAttribute(
+
+        /**
+         * The number of components this attribute has.
+         **/
+        val numComponents: Int
+) {}

idea/testData/refactoring/changeSignature/RemoveParameterKeepOtherCommentsBefore.kt

@@ -0,0 +1,13 @@
+data class <caret>VertexAttribute(
+
+        /**
+         * The number of components this attribute has.
+         **/
+        val numComponents: Int,
+
+        /**
+         * If true and [type] is not [Gl20.FLOAT], the data will be mapped to the range -1 to 1 for signed types and
+         * the range 0 to 1 for unsigned types.
+         */
+        val normalized: Boolean
+) {}

idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/KotlinChangeSignatureTest.kt

@@ -942,4 +942,6 @@ class KotlinChangeSignatureTest : KotlinLightCodeInsightFixtureTestCase() {
     }
 
     fun testReceiverInSafeCall() = doTestConflict { receiverParameterInfo = null }
+
+    fun testRemoveParameterKeepOtherComments() = doTest { removeParameter(1) }
 }