Change Signature: Implement improved call processing algorithm

#KT-8953 Fixed
 #KT-9525 Fixed

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

@@ -21,7 +21,7 @@ import com.intellij.psi.PsiWhiteSpace
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.psiUtil.siblings
 
-internal object EditCommaSeparatedListHelper {
+object EditCommaSeparatedListHelper {
     public fun <TItem: KtElement> addItem(list: KtElement, allItems: List<TItem>, item: TItem): TItem {
         return addItemBefore(list, allItems, item, null)
     }

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

@@ -345,7 +345,7 @@ public class KtPsiFactory(private val project: Project) {
             createExpressionByPattern("if ($0) $1", condition, thenExpr)) as KtIfExpression
     }
 
-    public fun createArgument(expression: KtExpression, name: Name? = null, isSpread: Boolean = false): KtValueArgument {
+    public fun createArgument(expression: KtExpression?, name: Name? = null, isSpread: Boolean = false): KtValueArgument {
         val argumentList = buildByPattern({ pattern, args -> createByPattern(pattern, *args) { createCallArguments(it) } }) {
             appendFixedText("(")
 

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

@@ -16,37 +16,35 @@
 
 package org.jetbrains.kotlin.psi
 
-import kotlin.properties.ReadWriteProperty
 import com.intellij.openapi.util.Key
 import com.intellij.openapi.util.UserDataHolder
 import com.intellij.psi.PsiElement
+import kotlin.reflect.KProperty
 
-public class UserDataProperty<in R: UserDataHolder, T : Any>(val key: Key<T>, val default: T? = null) {
+public class UserDataProperty<in R: UserDataHolder, T : Any>(val key: Key<T>) {
-    fun getValue(thisRef: R, desc: kotlin.PropertyMetadata): T? {
+    operator fun getValue(thisRef: R, desc: KProperty<*>) = thisRef.getUserData(key)
-        return thisRef.getUserData(key)
-    }
 
-    fun setValue(thisRef: R, desc: kotlin.PropertyMetadata, value: T?) {
+    operator fun setValue(thisRef: R, desc: KProperty<*>, value: T?) = thisRef.putUserData(key, value)
-        thisRef.putUserData(key, value)
-    }
 }
 
 public class NotNullableUserDataProperty<in R: UserDataHolder, T : Any>(val key: Key<T>, val defaultValue: T) {
-    fun getValue(thisRef: R, desc: kotlin.PropertyMetadata): T {
+    operator fun getValue(thisRef: R, desc: KProperty<*>) = thisRef.getUserData(key) ?: defaultValue
-        return thisRef.getUserData(key) ?: defaultValue
-    }
 
-    fun setValue(thisRef: R, desc: kotlin.PropertyMetadata, value: T) {
+    operator fun setValue(thisRef: R, desc: KProperty<*>, value: T) {
-        thisRef.putUserData(key, value)
+        thisRef.putUserData(key, if (value != defaultValue) value else null)
     }
 }
 
-public class CopyableUserDataProperty<in R: PsiElement, T : Any>(val key: Key<T>, val default: T? = null) {
+public class CopyableUserDataProperty<in R: PsiElement, T : Any>(val key: Key<T>) {
-    fun getValue(thisRef: R, property: PropertyMetadata): T? {
+    operator fun getValue(thisRef: R, property: KProperty<*>) = thisRef.getCopyableUserData(key)
-        return thisRef.getCopyableUserData(key)
-    }
 
-    fun setValue(thisRef: R, property: PropertyMetadata, value: T?) {
+    operator fun setValue(thisRef: R, property: KProperty<*>, value: T?) = thisRef.putCopyableUserData(key, value)
-        thisRef.putCopyableUserData(key, value)
+}
+
+public class NotNullableCopyableUserDataProperty<in R: PsiElement, T : Any>(val key: Key<T>, val defaultValue: T) {
+    operator fun getValue(thisRef: R, property: KProperty<*>) = thisRef.getCopyableUserData(key) ?: defaultValue
+
+    operator fun setValue(thisRef: R, property: KProperty<*>, value: T) {
+        thisRef.putCopyableUserData(key, if (value != defaultValue) value else null)
     }
 }

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

@@ -18,6 +18,8 @@ package org.jetbrains.kotlin.idea.refactoring.changeSignature
 
 import com.intellij.lang.Language
 import com.intellij.lang.java.JavaLanguage
+import com.intellij.openapi.util.UserDataHolder
+import com.intellij.openapi.util.UserDataHolderBase
 import com.intellij.psi.*
 import com.intellij.psi.search.searches.OverridingMethodsSearch
 import com.intellij.refactoring.changeSignature.*
@@ -62,7 +64,7 @@ public open class JetChangeInfo(
         receiver: JetParameterInfo? = methodDescriptor.receiver,
         val context: PsiElement,
         primaryPropagationTargets: Collection<PsiElement> = emptyList()
-) : ChangeInfo {
+) : ChangeInfo, UserDataHolder by UserDataHolderBase() {
     private class JvmOverloadSignature(
             val method: PsiMethod,
             val mandatoryParams: Set<KtParameter>,
@@ -120,6 +122,16 @@ public open class JetChangeInfo(
 
     override fun getNewParameters(): Array<JetParameterInfo> = newParameters.toTypedArray()
 
+    fun getToBeRemoved(): BooleanArray {
+        return BooleanArray(originalParameters.size).apply {
+            Arrays.fill(this, true)
+            for (newParameter in newParameters) {
+                if (newParameter.oldIndex < 0) continue
+                this[newParameter.oldIndex] = false
+            }
+        }
+    }
+
     fun getNonReceiverParametersCount(): Int = newParameters.size() - (if (receiverParameterInfo != null) 1 else 0)
 
     fun getNonReceiverParameters(): List<JetParameterInfo> {

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

@@ -113,7 +113,7 @@ class JetChangeSignatureUsageProcessor : ChangeSignatureUsageProcessor {
         }
         else {
             findSAMUsages(info, result)
-            findConstructorDelegationUsages(info, result)
+            //findConstructorDelegationUsages(info, result)
             findKotlinOverrides(info, result)
             if (info is JavaChangeInfo) {
                 findKotlinCallers(info, result)
@@ -418,21 +418,6 @@ class JetChangeSignatureUsageProcessor : ChangeSignatureUsageProcessor {
         }
     }
 
-    private fun findConstructorDelegationUsages(changeInfo: ChangeInfo, result: MutableSet<UsageInfo>) {
-        val method = changeInfo.method
-        if (!(method.isTrueJavaMethod())) return
-        method as PsiMethod
-
-        if (!method.isConstructor) return
-
-        method.processDelegationCallConstructorUsages(method.useScope) {
-            if (it is KtConstructorDelegationCall) {
-                result.add(JavaConstructorDeferredUsageInDelegationCall(it))
-            }
-            true
-        }
-    }
-
     private fun findKotlinOverrides(changeInfo: ChangeInfo, result: MutableSet<UsageInfo>) {
         val method = changeInfo.method
         if (!method.isTrueJavaMethod()) return

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

@@ -16,12 +16,11 @@
 
 package org.jetbrains.kotlin.idea.refactoring.changeSignature.usages
 
-import org.jetbrains.kotlin.psi.KtCallElement
 import com.intellij.psi.PsiElement
-import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetChangeInfo
 import com.intellij.usageView.UsageInfo
-import org.jetbrains.kotlin.psi.KtFunction
+import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetChangeInfo
-import org.jetbrains.kotlin.descriptors.FunctionDescriptor
+import org.jetbrains.kotlin.psi.KtCallElement
+import org.jetbrains.kotlin.psi.KtConstructorDelegationCall
 
 public abstract class JavaMethodKotlinUsageWithDelegate<T: PsiElement>(
         val psiElement: T,
@@ -34,10 +33,12 @@ public abstract class JavaMethodKotlinUsageWithDelegate<T: PsiElement>(
 public class JavaMethodKotlinCallUsage(
         callElement: KtCallElement,
         javaMethodChangeInfo: JetChangeInfo,
-        propagationCall: Boolean): JavaMethodKotlinUsageWithDelegate<KtCallElement>(callElement, javaMethodChangeInfo) {
+        propagationCall: Boolean
-    override val delegateUsage = if (propagationCall) {
+): JavaMethodKotlinUsageWithDelegate<KtCallElement>(callElement, javaMethodChangeInfo) {
-        KotlinCallerCallUsage(psiElement)
+    @Suppress("UNCHECKED_CAST")
-    } else {
+    override val delegateUsage = when {
-        JetFunctionCallUsage(psiElement, javaMethodChangeInfo.methodDescriptor.originalPrimaryCallable)
+        propagationCall -> KotlinCallerCallUsage(psiElement)
-    }
+        psiElement is KtConstructorDelegationCall -> JetConstructorDelegationCallUsage(psiElement, javaMethodChangeInfo)
+        else -> JetFunctionCallUsage(psiElement, javaMethodChangeInfo.methodDescriptor.originalPrimaryCallable)
+    } as JetUsageInfo<KtCallElement>
 }

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

@@ -16,11 +16,8 @@
 
 package org.jetbrains.kotlin.idea.refactoring.changeSignature.usages
 
-import com.intellij.psi.PsiComment
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiMethod
-import com.intellij.psi.PsiWhiteSpace
-import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.usageView.UsageInfo
 import org.jetbrains.kotlin.descriptors.CallableDescriptor
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
@@ -30,6 +27,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.getJavaMethodDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.codeInsight.shorten.addToShorteningWaitSet
 import org.jetbrains.kotlin.idea.core.refactoring.createPrimaryConstructorIfAbsent
+import org.jetbrains.kotlin.idea.core.refactoring.replaceListPsiAndKeepDelimiters
 import org.jetbrains.kotlin.idea.core.setVisibility
 import org.jetbrains.kotlin.idea.core.toKeywordToken
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetChangeInfo
@@ -203,7 +201,7 @@ class JetCallableDefinitionUsage<T : PsiElement>(
                 newParameterList = parameterList.replace(newParameterList) as KtParameterList
             }
             else {
-                newParameterList = replaceParameterListAndKeepDelimiters(parameterList, newParameterList)
+                newParameterList = replaceListPsiAndKeepDelimiters(parameterList, newParameterList) { parameters }
             }
         }
         else {
@@ -226,32 +224,6 @@ class JetCallableDefinitionUsage<T : PsiElement>(
         newParameterList.addToShorteningWaitSet(Options.DEFAULT)
     }
 
-    private fun replaceParameterListAndKeepDelimiters(parameterList: KtParameterList, newParameterList: KtParameterList): KtParameterList {
-        val oldParameters = parameterList.parameters
-        val newParameters = newParameterList.parameters
-        val oldCount = oldParameters.size
-        val newCount = newParameters.size
-
-        val commonCount = Math.min(oldCount, newCount)
-        for (i in 0..commonCount - 1) {
-            oldParameters[i] = oldParameters[i].replace(newParameters[i]) as KtParameter
-        }
-
-        if (commonCount == 0) return parameterList.replace(newParameterList) as KtParameterList
-
-        if (oldCount > commonCount) {
-            parameterList.deleteChildRange(oldParameters[commonCount - 1].nextSibling, oldParameters.last())
-        }
-        else if (newCount > commonCount) {
-            parameterList.addRangeAfter(newParameters[commonCount - 1].nextSibling,
-                                        newParameterList.lastChild.prevSibling,
-                                        PsiTreeUtil.skipSiblingsBackward(parameterList.lastChild,
-                                                                         PsiWhiteSpace::class.java, PsiComment::class.java))
-        }
-
-        return parameterList
-    }
-
     private fun changeVisibility(changeInfo: JetChangeInfo, element: PsiElement) {
         val newVisibilityToken = changeInfo.newVisibility.toKeywordToken()
         when (element) {

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

@@ -16,16 +16,17 @@
 
 package org.jetbrains.kotlin.idea.refactoring.changeSignature.usages
 
+import com.intellij.openapi.util.Key
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiReference
 import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.usageView.UsageInfo
 import com.intellij.util.containers.ContainerUtil
-import gnu.trove.TIntArrayList
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.codeInsight.shorten.addToShorteningWaitSet
 import org.jetbrains.kotlin.idea.core.moveFunctionLiteralOutsideParentheses
+import org.jetbrains.kotlin.idea.core.refactoring.replaceListPsiAndKeepDelimiters
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetChangeInfo
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetParameterInfo
 import org.jetbrains.kotlin.idea.refactoring.changeSignature.isInsideOfCallerBody
@@ -34,14 +35,14 @@ import org.jetbrains.kotlin.idea.refactoring.introduce.introduceVariable.KotlinI
 import org.jetbrains.kotlin.idea.util.ShortenReferences
 import org.jetbrains.kotlin.load.java.JvmAbi
 import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor
+import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypeAndBranch
+import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelector
 import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
 import org.jetbrains.kotlin.psi.psiUtil.startOffset
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
-import org.jetbrains.kotlin.resolve.calls.model.ArgumentMatch
+import org.jetbrains.kotlin.resolve.calls.model.*
-import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument
-import org.jetbrains.kotlin.resolve.calls.model.isReallySuccess
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver
 import org.jetbrains.kotlin.resolve.scopes.receivers.ExtensionReceiver
@@ -236,37 +237,68 @@ class JetFunctionCallUsage(
         return newExpression
     }
 
-    private fun updateArgumentsAndReceiver(changeInfo: JetChangeInfo, element: KtCallElement, allUsages: Array<out UsageInfo>) {
+    class ArgumentInfo(
-        var arguments = element.valueArgumentList.sure { "Argument list is expected: " + element.text }
+            val parameter: JetParameterInfo,
-        val oldArguments = element.valueArguments
+            val parameterIndex: Int,
+            val resolvedArgument: ResolvedValueArgument?,
+            val receiverValue: ReceiverValue?
+    ) {
+        val mainValueArgument: ValueArgument?
+            get() = resolvedArgument?.arguments?.firstOrNull()
 
+        val wasNamed: Boolean
+            get() = mainValueArgument?.isNamed() ?: false
+
+        var name: String? = null
+            private set
+
+        fun makeNamed(callee: JetCallableDefinitionUsage<*>) {
+            name = parameter.getInheritedName(callee)
+        }
+
+        fun shouldSkip() = parameter.defaultValueForParameter != null && mainValueArgument == null
+    }
+
+    private fun getResolvedValueArgument(oldIndex: Int): ResolvedValueArgument? {
+        if (oldIndex < 0) return null
+
+        val parameterDescriptor = resolvedCall!!.resultingDescriptor.valueParameters[oldIndex]
+        return resolvedCall.valueArguments[parameterDescriptor]
+    }
+
+    private var KtValueArgument.generatedArgumentValue: Boolean
+            by NotNullableCopyableUserDataProperty(Key.create("GENERATED_ARGUMENT_VALUE"), false)
+
+    private fun ArgumentInfo.getArgumentByDefaultValue(
+            element: KtCallElement,
+            allUsages: Array<out UsageInfo>,
+            psiFactory: KtPsiFactory
+    ): KtValueArgument {
+        val isInsideOfCallerBody = element.isInsideOfCallerBody(allUsages)
+        val defaultValueForCall = parameter.defaultValueForCall
+        val argValue = when {
+            isInsideOfCallerBody -> psiFactory.createExpression(parameter.name)
+            defaultValueForCall != null -> substituteReferences(defaultValueForCall, parameter.defaultValueParameterReferences, psiFactory)
+            else -> null
+        }
+        val argName = (if (isInsideOfCallerBody) null else name)?.let { Name.guess(it) }
+        return psiFactory.createArgument(argValue ?: psiFactory.createExpression("0"), argName).apply {
+            generatedArgumentValue = true
+            if (argValue == null) {
+                getArgumentExpression()!!.delete()
+            }
+        }
+    }
+
+    private fun updateArgumentsAndReceiver(changeInfo: JetChangeInfo, element: KtCallElement, allUsages: Array<out UsageInfo>) {
         if (isPropertyJavaUsage) return updateJavaPropertyCall(changeInfo, element)
 
-        val isNamedCall = oldArguments.size > 1 && oldArguments[0].isNamed()
+        val fullCallElement = element.getQualifiedExpressionForSelector() ?: element
 
-        val indicesOfArgumentsWithDefaultValues = TIntArrayList()
+        val oldArguments = element.valueArguments
+        val newParameters = changeInfo.getNonReceiverParameters()
 
-        val psiFactory = KtPsiFactory(element.project)
+        val purelyNamedCall = element is KtCallExpression && oldArguments.isNotEmpty() && oldArguments.all { it.isNamed() }
-
-        val newSignatureParameters = changeInfo.getNonReceiverParameters()
-        val newArgumentListText = newSignatureParameters
-                .map { parameterInfo ->
-                    val defaultValueForCall = parameterInfo.defaultValueForCall
-                    val defaultValueText = when {
-                        element.isInsideOfCallerBody(allUsages) ->
-                            parameterInfo.name
-                        defaultValueForCall != null ->
-                            substituteReferences(defaultValueForCall, parameterInfo.defaultValueParameterReferences, psiFactory).text
-                        else ->
-                            ""
-                    }
-                    val argumentValue = if (defaultValueText.isEmpty()) "0" else defaultValueText
-                    if (isNamedCall) "${parameterInfo.getInheritedName(callee)}=$argumentValue" else argumentValue
-                }
-                .joinToString(prefix = "(", postfix = ")")
-        val newArgumentList = KtPsiFactory(project).createCallArguments(newArgumentListText)
-
-        val argumentMap = getParamIndexToArgumentMap(changeInfo, oldArguments)
 
         val newReceiverInfo = changeInfo.receiverParameterInfo
         val originalReceiverInfo = changeInfo.methodDescriptor.receiver
@@ -274,90 +306,118 @@ class JetFunctionCallUsage(
         val extensionReceiver = if (resolvedCall != null) resolvedCall.extensionReceiver else ReceiverValue.NO_RECEIVER
         val dispatchReceiver = if (resolvedCall != null) resolvedCall.dispatchReceiver else ReceiverValue.NO_RECEIVER
 
-        var elementToReplace: PsiElement = element
-        val parent = element.parent
-        if (parent is KtQualifiedExpression && parent.selectorExpression == element) {
-            elementToReplace = parent
-        }
-
         // Do not add extension receiver to calls with explicit dispatch receiver
-        if (newReceiverInfo != null && elementToReplace is KtQualifiedExpression && dispatchReceiver is ExpressionReceiver) return
+        if (newReceiverInfo != null && fullCallElement is KtQualifiedExpression && dispatchReceiver is ExpressionReceiver) return
 
-        val newArguments = newArgumentList.arguments
+        val newArgumentInfos = newParameters.withIndex().map {
-        var actualIndex = 0
+            val (index, param) = it
-        for (i in newArguments.indices) {
+            val oldIndex = param.oldIndex
-            val newArgument = newArguments[i]
+            val resolvedArgument = if (oldIndex >= 0) getResolvedValueArgument(oldIndex) else null
-            val parameterInfo = newSignatureParameters[i]
+            val receiverValue = if (param == originalReceiverInfo) extensionReceiver else null
-            if (parameterInfo == originalReceiverInfo) {
+            ArgumentInfo(param, index, resolvedArgument, receiverValue)
-                val receiverExpression = getReceiverExpression(extensionReceiver, psiFactory)
-                if (receiverExpression != null) {
-                    newArgument.replace(receiverExpression)
-                }
-                actualIndex++
-                continue
-            }
-
-            val oldArgument = argumentMap[parameterInfo.oldIndex]
-
-            if (oldArgument != null) {
-                val argumentName = oldArgument.getArgumentName()
-                val argumentNameExpression = argumentName?.referenceExpression
-                changeArgumentName(argumentNameExpression, parameterInfo)
-                //noinspection ConstantConditions
-                val argumentReplacement = newArgument.replace(
-                        if (oldArgument is KtFunctionLiteralArgument)
-                            psiFactory.createArgument(oldArgument.getArgumentExpression(), null, false)
-                        else
-                            oldArgument.asElement()) as ValueArgument
-                argumentMap.put(parameterInfo.oldIndex, argumentReplacement)
-            }
-            else if (parameterInfo.defaultValueForCall == null) {
-                if (parameterInfo.defaultValueForParameter != null) {
-                    newArgumentList.removeArgument(newArgument)
-                }
-                else {
-                    newArgument.delete() // keep space between commas
-                }
-            }
-            else {
-                indicesOfArgumentsWithDefaultValues.add(actualIndex++)
-            }// TODO: process default arguments in the middle
         }
 
-        val lambdaArguments = element.functionLiteralArguments
+        val lastParameterIndex = newParameters.lastIndex
-        val hasLambdaArgumentsBefore = !lambdaArguments.isEmpty()
+        var firstNamedIndex = newArgumentInfos.firstOrNull {
-        if (hasLambdaArgumentsBefore) {
+            it.wasNamed
-            element.deleteChildRange(lambdaArguments.first(), lambdaArguments.last())
+            || (it.parameter.isNewParameter && purelyNamedCall)
+            || (it.resolvedArgument is VarargValueArgument && it.parameterIndex < lastParameterIndex)
+        }?.parameterIndex
+        if (firstNamedIndex == null) {
+            val lastNonDefaultArgIndex = (lastParameterIndex downTo 0).firstOrNull { !newArgumentInfos[it].shouldSkip() }
+                                         ?: -1
+            firstNamedIndex = (0..lastNonDefaultArgIndex).firstOrNull { newArgumentInfos[it].shouldSkip() }
         }
 
-        val lastArgument = newArgumentList.arguments.lastOrNull()
+        val lastPositionalIndex = if (firstNamedIndex != null) firstNamedIndex - 1 else lastParameterIndex
-        val lastLambdaExpr = if (lastArgument != null) lastArgument.getArgumentExpression()?.unpackFunctionLiteral() else null
+        (lastPositionalIndex + 1 .. lastParameterIndex).forEach { newArgumentInfos[it].makeNamed(callee) }
-        val lastNewParam = changeInfo.newParameters.lastOrNull()
-        val hasTrailingLambdaInArgumentListAfter =
-                lastLambdaExpr != null && lastNewParam != null && argumentMap[lastNewParam.oldIndex] == lastArgument
-        val newLambdaWithDefaultValueWasAdded =
-                lastNewParam != null
-                && lastNewParam.isNewParameter
-                && lastNewParam.defaultValueForCall is KtFunctionLiteralExpression
-                && lastArgument != null
-                && !lastArgument.isNamed()
-        val shouldMoveLambdaOut = hasTrailingLambdaInArgumentListAfter && hasLambdaArgumentsBefore || newLambdaWithDefaultValueWasAdded
 
-        arguments = arguments.replace(newArgumentList) as KtValueArgumentList
+        val psiFactory = KtPsiFactory(element.project)
 
-        val argumentsToShorten = ArrayList<KtElement>(indicesOfArgumentsWithDefaultValues.size())
+        val newArgumentList = psiFactory.createCallArguments("()").apply {
-        val argumentList = arguments.arguments
+            for (argInfo in newArgumentInfos) {
-        indicesOfArgumentsWithDefaultValues.forEach {
+                if (argInfo.shouldSkip()) continue
-            argumentsToShorten.add(argumentList[it])
+
-            true
+                val name = argInfo.name?.let { Name.guess(it) }
+
+                if (argInfo.receiverValue != null) {
+                    val receiverExpression = getReceiverExpression(argInfo.receiverValue, psiFactory) ?: continue
+                    addArgument(psiFactory.createArgument(receiverExpression, name))
+                    continue
+                }
+
+                val resolvedArgument = argInfo.resolvedArgument
+                when (resolvedArgument) {
+                    null, is DefaultValueArgument -> addArgument(argInfo.getArgumentByDefaultValue(element, allUsages, psiFactory))
+
+                    is ExpressionValueArgument -> {
+                        val valueArgument = resolvedArgument.valueArgument
+                        val newValueArgument: KtValueArgument = when {
+                            valueArgument == null -> argInfo.getArgumentByDefaultValue(element, allUsages, psiFactory)
+                            valueArgument is KtFunctionLiteralArgument -> psiFactory.createArgument(valueArgument.getArgumentExpression(), name)
+                            valueArgument is KtValueArgument && valueArgument.getArgumentName()?.asName == name -> valueArgument
+                            else -> psiFactory.createArgument(valueArgument.getArgumentExpression(), name)
+                        }
+                        addArgument(newValueArgument)
+                    }
+
+                    // TODO: Support Kotlin varargs
+                    is VarargValueArgument -> resolvedArgument.arguments.forEach {
+                        if (it is KtValueArgument) addArgument(it)
+                    }
+
+                    else -> return
+                }
+            }
         }
-        argumentsToShorten.forEach { it.addToShorteningWaitSet(SHORTEN_ARGUMENTS_OPTIONS) }
+
+        newArgumentList.arguments.singleOrNull()?.let {
+            if (it.getArgumentExpression() == null) {
+                newArgumentList.removeArgument(it)
+            }
+        }
+
+        val lastOldArgument = oldArguments.lastOrNull()
+        val lastNewParameter = newParameters.lastOrNull()
+        val lastNewArgument = newArgumentList.arguments.lastOrNull()
+        val oldLastResolvedArgument = getResolvedValueArgument(lastNewParameter?.oldIndex ?: -1) as? ExpressionValueArgument
+        val lambdaArgumentNotTouched =
+                lastOldArgument is KtFunctionLiteralArgument && oldLastResolvedArgument?.valueArgument == lastOldArgument
+        val newLambdaArgumentAddedLast = lastNewParameter != null
+                                         && lastNewParameter.isNewParameter
+                                         && lastNewParameter.defaultValueForCall is KtFunctionLiteralExpression
+                                         && lastNewArgument != null
+                                         && !lastNewArgument.isNamed()
+
+        if (lambdaArgumentNotTouched) {
+            newArgumentList.removeArgument(newArgumentList.arguments.last())
+        }
+        else {
+            val lambdaArguments = element.functionLiteralArguments
+            if (lambdaArguments.isNotEmpty()) {
+                element.deleteChildRange(lambdaArguments.first(), lambdaArguments.last())
+            }
+        }
+
+        var oldArgumentList = element.valueArgumentList.sure { "Argument list is expected: " + element.text }
+        replaceListPsiAndKeepDelimiters(oldArgumentList, newArgumentList) { arguments }
+
+        element.accept(
+                object: KtTreeVisitorVoid() {
+                    override fun visitArgument(argument: KtValueArgument) {
+                        if (argument.generatedArgumentValue) {
+                            argument.generatedArgumentValue = false
+                            argument.addToShorteningWaitSet(SHORTEN_ARGUMENTS_OPTIONS)
+                        }
+                    }
+                }
+        )
 
         var newElement: KtElement = element
         if (newReceiverInfo != originalReceiverInfo) {
             val replacingElement: PsiElement
             if (newReceiverInfo != null) {
-                val receiverArgument = argumentMap[newReceiverInfo.oldIndex]
+                val receiverArgument = getResolvedValueArgument(newReceiverInfo.oldIndex)?.arguments?.singleOrNull()
                 val extensionReceiverExpression = receiverArgument?.getArgumentExpression()
                 val defaultValueForCall = newReceiverInfo.defaultValueForCall
                 val receiver = extensionReceiverExpression?.let { psiFactory.createExpression(it.text) }
@@ -370,10 +430,10 @@ class JetFunctionCallUsage(
                 replacingElement = psiFactory.createExpression(element.text)
             }
 
-            newElement = elementToReplace.replace(replacingElement) as KtElement
+            newElement = fullCallElement.replace(replacingElement) as KtElement
         }
 
-        if (shouldMoveLambdaOut) {
+        if (!lambdaArgumentNotTouched && newLambdaArgumentAddedLast) {
             val newCallExpression = ((newElement as? KtQualifiedExpression)?.selectorExpression ?: newElement) as KtCallExpression
             newCallExpression.moveFunctionLiteralOutsideParentheses()
         }
@@ -451,26 +511,5 @@ class JetFunctionCallUsage(
                 else -> null
             }
         }
-
-        private fun getParamIndexToArgumentMap(changeInfo: JetChangeInfo, oldArguments: List<ValueArgument>): MutableMap<Int, ValueArgument> {
-            val argumentMap = HashMap<Int, ValueArgument>()
-
-            for (i in oldArguments.indices) {
-                val argument = oldArguments[i]
-                val argumentName = argument.getArgumentName()
-                val oldParameterName = if (argumentName != null) argumentName.asName.asString() else null
-
-                if (oldParameterName != null) {
-                    val oldParameterIndex = changeInfo.getOldParameterIndex(oldParameterName)
-
-                    if (oldParameterIndex != null)
-                        argumentMap.put(oldParameterIndex, argument)
-                }
-                else
-                    argumentMap.put(i, argument)
-            }
-
-            return argumentMap
-        }
     }
 }

idea/src/org/jetbrains/kotlin/idea/refactoring/jetRefactoringUtil.kt

@@ -711,3 +711,36 @@ public fun KtClass.createPrimaryConstructorParameterListIfAbsent(): KtParameterL
 }
 
 fun PsiNamedElement.isInterfaceClass(): Boolean = this is KtClass && isInterface() || this is PsiClass && isInterface
+
+fun <ListType : KtElement> replaceListPsiAndKeepDelimiters(
+        originalList: ListType,
+        newList: ListType,
+        itemsFun: ListType.() -> List<KtElement>
+): ListType {
+    originalList.children.takeWhile { it is PsiErrorElement }.forEach { it.delete() }
+
+    val oldParameters = originalList.itemsFun().toArrayList()
+    val newParameters = newList.itemsFun()
+    val oldCount = oldParameters.size
+    val newCount = newParameters.size
+
+    val commonCount = Math.min(oldCount, newCount)
+    for (i in 0..commonCount - 1) {
+        oldParameters[i] = oldParameters[i].replace(newParameters[i]) as KtElement
+    }
+
+    @Suppress("UNCHECKED_CAST")
+    if (commonCount == 0) return originalList.replace(newList) as ListType
+
+    if (oldCount > commonCount) {
+        originalList.deleteChildRange(oldParameters[commonCount - 1].nextSibling, oldParameters.last())
+    }
+    else if (newCount > commonCount) {
+        originalList.addRangeAfter(newParameters[commonCount - 1].nextSibling,
+                                   newList.lastChild.prevSibling,
+                                   PsiTreeUtil.skipSiblingsBackward(originalList.lastChild,
+                                                                    PsiWhiteSpace::class.java, PsiComment::class.java))
+    }
+
+    return originalList
+}

idea/testData/quickfix/changeSignature/addFunctionParameterLongNameRuntime.kt.after

@@ -4,7 +4,7 @@ import java.util.LinkedHashSet
 // DISABLE-ERRORS
 
 fun foo(x: Int, linkedHashSet: LinkedHashSet<Int>) {
-    foo(, java.util.LinkedHashSet<Int>());
+    foo(, LinkedHashSet<Int>());
-    foo(1, java.util.LinkedHashSet<Int>());
+    foo(1, LinkedHashSet<Int>());
     foo(2, java.util.LinkedHashSet<Int>());
 }

idea/testData/refactoring/changeSignature/AddParameterKeepFormatAfter.kt

@@ -0,0 +1,16 @@
+fun <caret>foo(a: Int, b: Int, d: Int, c: Int, e: Int) {
+
+}
+
+fun test() {
+    foo(1,
+            2,
+            4, 3, 5)
+    foo(1, 2,
+            4, 3, 5)
+    foo(
+            1,
+            2,
+            4, 3, 5
+    )
+}

idea/testData/refactoring/changeSignature/AddParameterKeepFormatBefore.kt

@@ -0,0 +1,16 @@
+fun <caret>foo(a: Int, b: Int, c: Int) {
+
+}
+
+fun test() {
+    foo(1,
+        2,
+        3)
+    foo(1, 2,
+        3)
+    foo(
+            1,
+            2,
+            3
+    )
+}

idea/testData/refactoring/changeSignature/RemoveDefaultParameterBeforeLambdaAfter.kt

@@ -0,0 +1,29 @@
+fun foo(a: String, c: Boolean = false, block: (String) -> Unit) {
+    block(a)
+}
+
+fun test() {
+    foo("Hello", c = true) {
+        println(it)
+    }
+
+    foo("Hello", true) {
+        println(it)
+    }
+
+    foo("Hello", c = true) {
+        println(it)
+    }
+
+    foo("Hello") {
+        println(it)
+    }
+
+    foo("Hello") {
+        println(it)
+    }
+
+    foo("Hello") {
+        println("Don't let $it go!")
+    }
+}

idea/testData/refactoring/changeSignature/RemoveDefaultParameterBeforeLambdaBefore.kt

@@ -0,0 +1,29 @@
+fun <caret>foo(a: String, b: Boolean = false, c: Boolean = false, block: (String) -> Unit) {
+    block(a)
+}
+
+fun test() {
+    foo("Hello", b = false, c = true) {
+        println(it)
+    }
+
+    foo("Hello", false, true) {
+        println(it)
+    }
+
+    foo("Hello", c = true) {
+        println(it)
+    }
+
+    foo("Hello", b = false) {
+        println(it)
+    }
+
+    foo("Hello", false) {
+        println(it)
+    }
+
+    foo("Hello") {
+        println("Don't let $it go!")
+    }
+}

idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1After.kt

@@ -0,0 +1,13 @@
+fun <caret>foo(b: Int, c: Int) {
+
+}
+
+fun test() {
+    foo(2,
+            3)
+    foo(2, 3)
+    foo(
+            2,
+            3
+    )
+}

idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1Before.kt

@@ -0,0 +1,16 @@
+fun <caret>foo(a: Int, b: Int, c: Int) {
+
+}
+
+fun test() {
+    foo(1,
+        2,
+        3)
+    foo(1, 2,
+        3)
+    foo(
+            1,
+            2,
+            3
+    )
+}

idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2After.kt

@@ -0,0 +1,13 @@
+fun <caret>foo(a: Int, c: Int) {
+
+}
+
+fun test() {
+    foo(1,
+            3)
+    foo(1, 3)
+    foo(
+            1,
+            3
+    )
+}

idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2Before.kt

@@ -0,0 +1,16 @@
+fun <caret>foo(a: Int, b: Int, c: Int) {
+
+}
+
+fun test() {
+    foo(1,
+        2,
+        3)
+    foo(1, 2,
+        3)
+    foo(
+            1,
+            2,
+            3
+    )
+}

idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3After.kt

@@ -0,0 +1,13 @@
+fun <caret>foo(a: Int, b: Int) {
+
+}
+
+fun test() {
+    foo(1,
+            2)
+    foo(1, 2)
+    foo(
+            1,
+            2
+    )
+}

idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3Before.kt

@@ -0,0 +1,16 @@
+fun <caret>foo(a: Int, b: Int, c: Int) {
+
+}
+
+fun test() {
+    foo(1,
+        2,
+        3)
+    foo(1, 2,
+        3)
+    foo(
+            1,
+            2,
+            3
+    )
+}

idea/testData/refactoring/changeSignature/SwapParametersKeepFormatAfter.kt

@@ -0,0 +1,16 @@
+fun <caret>foo(c: Int, b: Int, a: Int) {
+
+}
+
+fun test() {
+    foo(3,
+            2,
+            1)
+    foo(3, 2,
+            1)
+    foo(
+            3,
+            2,
+            1
+    )
+}

idea/testData/refactoring/changeSignature/SwapParametersKeepFormatBefore.kt

@@ -0,0 +1,16 @@
+fun <caret>foo(a: Int, b: Int, c: Int) {
+
+}
+
+fun test() {
+    foo(1,
+        2,
+        3)
+    foo(1, 2,
+        3)
+    foo(
+            1,
+            2,
+            3
+    )
+}

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

@@ -835,4 +835,22 @@ class JetChangeSignatureTest : KotlinCodeInsightTestCase() {
     fun testJvmOverloadedConstructorSwapParams() = doTest { swapParameters(1, 2) }
 
     fun testDefaultAfterLambda() = doTest { swapParameters(0, 1) }
-}
+
+    fun testRemoveDefaultParameterBeforeLambda() = doTest { removeParameter(1) }
+
+    fun testAddParameterKeepFormat() = doTest {
+        val psiFactory = KtPsiFactory(project)
+        val defaultValue1 = psiFactory.createExpression("4")
+        val defaultValue2 = psiFactory.createExpression("5")
+        addParameter(JetParameterInfo(originalBaseFunctionDescriptor, -1, "d", BUILT_INS.intType, null, defaultValue1), 2)
+        addParameter(JetParameterInfo(originalBaseFunctionDescriptor, -1, "e", BUILT_INS.intType, null, defaultValue2))
+    }
+
+    fun testRemoveParameterKeepFormat1() = doTest { removeParameter(0) }
+
+    fun testRemoveParameterKeepFormat2() = doTest { removeParameter(1) }
+
+    fun testRemoveParameterKeepFormat3() = doTest { removeParameter(2) }
+
+    fun testSwapParametersKeepFormat() = doTest { swapParameters(0, 2) }
+}