diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/EditCommaSeparatedListHelper.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/EditCommaSeparatedListHelper.kt index b2c129b26e3..0bd0b748150 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/EditCommaSeparatedListHelper.kt +++ b/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 addItem(list: KtElement, allItems: List, item: TItem): TItem { return addItemBefore(list, allItems, item, null) } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt index 64cd682ec18..c82b9613fa8 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt +++ b/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("(") diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/userDataUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/userDataUtil.kt index 6adfca6c519..e414dd3c30f 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/userDataUtil.kt +++ b/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(val key: Key, val default: T? = null) { - fun getValue(thisRef: R, desc: kotlin.PropertyMetadata): T? { - return thisRef.getUserData(key) - } +public class UserDataProperty(val key: Key) { + operator fun getValue(thisRef: R, desc: KProperty<*>) = thisRef.getUserData(key) - fun setValue(thisRef: R, desc: kotlin.PropertyMetadata, value: T?) { - thisRef.putUserData(key, value) - } + operator fun setValue(thisRef: R, desc: KProperty<*>, value: T?) = thisRef.putUserData(key, value) } public class NotNullableUserDataProperty(val key: Key, val defaultValue: T) { - fun getValue(thisRef: R, desc: kotlin.PropertyMetadata): T { - return thisRef.getUserData(key) ?: defaultValue - } + operator fun getValue(thisRef: R, desc: KProperty<*>) = thisRef.getUserData(key) ?: defaultValue - fun setValue(thisRef: R, desc: kotlin.PropertyMetadata, value: T) { - thisRef.putUserData(key, value) + operator fun setValue(thisRef: R, desc: KProperty<*>, value: T) { + thisRef.putUserData(key, if (value != defaultValue) value else null) } } -public class CopyableUserDataProperty(val key: Key, val default: T? = null) { - fun getValue(thisRef: R, property: PropertyMetadata): T? { - return thisRef.getCopyableUserData(key) - } +public class CopyableUserDataProperty(val key: Key) { + operator fun getValue(thisRef: R, property: KProperty<*>) = thisRef.getCopyableUserData(key) - fun setValue(thisRef: R, property: PropertyMetadata, value: T?) { - thisRef.putCopyableUserData(key, value) + operator fun setValue(thisRef: R, property: KProperty<*>, value: T?) = thisRef.putCopyableUserData(key, value) +} + +public class NotNullableCopyableUserDataProperty(val key: Key, 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) } } \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeInfo.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeInfo.kt index 4bbe99421ff..2841513ded0 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeInfo.kt +++ b/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 = emptyList() -) : ChangeInfo { +) : ChangeInfo, UserDataHolder by UserDataHolderBase() { private class JvmOverloadSignature( val method: PsiMethod, val mandatoryParams: Set, @@ -120,6 +122,16 @@ public open class JetChangeInfo( override fun getNewParameters(): Array = 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 { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureUsageProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureUsageProcessor.kt index a0bcebe288f..f77a85549fa 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureUsageProcessor.kt +++ b/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) { - 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) { val method = changeInfo.method if (!method.isTrueJavaMethod()) return diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JavaMethodKotlinUsageWithDelegate.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JavaMethodKotlinUsageWithDelegate.kt index 1c1209924bf..496f64dd2ca 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JavaMethodKotlinUsageWithDelegate.kt +++ b/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.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.idea.refactoring.changeSignature.JetChangeInfo +import org.jetbrains.kotlin.psi.KtCallElement +import org.jetbrains.kotlin.psi.KtConstructorDelegationCall public abstract class JavaMethodKotlinUsageWithDelegate( val psiElement: T, @@ -34,10 +33,12 @@ public abstract class JavaMethodKotlinUsageWithDelegate( public class JavaMethodKotlinCallUsage( callElement: KtCallElement, javaMethodChangeInfo: JetChangeInfo, - propagationCall: Boolean): JavaMethodKotlinUsageWithDelegate(callElement, javaMethodChangeInfo) { - override val delegateUsage = if (propagationCall) { - KotlinCallerCallUsage(psiElement) - } else { - JetFunctionCallUsage(psiElement, javaMethodChangeInfo.methodDescriptor.originalPrimaryCallable) - } + propagationCall: Boolean +): JavaMethodKotlinUsageWithDelegate(callElement, javaMethodChangeInfo) { + @Suppress("UNCHECKED_CAST") + override val delegateUsage = when { + propagationCall -> KotlinCallerCallUsage(psiElement) + psiElement is KtConstructorDelegationCall -> JetConstructorDelegationCallUsage(psiElement, javaMethodChangeInfo) + else -> JetFunctionCallUsage(psiElement, javaMethodChangeInfo.methodDescriptor.originalPrimaryCallable) + } as JetUsageInfo } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetCallableDefinitionUsage.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetCallableDefinitionUsage.kt index d81bd8bae85..df73a182ea9 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetCallableDefinitionUsage.kt +++ b/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( newParameterList = parameterList.replace(newParameterList) as KtParameterList } else { - newParameterList = replaceParameterListAndKeepDelimiters(parameterList, newParameterList) + newParameterList = replaceListPsiAndKeepDelimiters(parameterList, newParameterList) { parameters } } } else { @@ -226,32 +224,6 @@ class JetCallableDefinitionUsage( 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) { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetFunctionCallUsage.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetFunctionCallUsage.kt index 9f14f969a7f..43b1aa94a19 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetFunctionCallUsage.kt +++ b/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.ExpressionValueArgument -import org.jetbrains.kotlin.resolve.calls.model.isReallySuccess +import org.jetbrains.kotlin.resolve.calls.model.* 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) { - var arguments = element.valueArgumentList.sure { "Argument list is expected: " + element.text } - val oldArguments = element.valueArguments + class ArgumentInfo( + val parameter: JetParameterInfo, + 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, + 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) { 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 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 purelyNamedCall = element is KtCallExpression && oldArguments.isNotEmpty() && oldArguments.all { it.isNamed() } 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 - var actualIndex = 0 - for (i in newArguments.indices) { - val newArgument = newArguments[i] - val parameterInfo = newSignatureParameters[i] - if (parameterInfo == originalReceiverInfo) { - 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 newArgumentInfos = newParameters.withIndex().map { + val (index, param) = it + val oldIndex = param.oldIndex + val resolvedArgument = if (oldIndex >= 0) getResolvedValueArgument(oldIndex) else null + val receiverValue = if (param == originalReceiverInfo) extensionReceiver else null + ArgumentInfo(param, index, resolvedArgument, receiverValue) } - val lambdaArguments = element.functionLiteralArguments - val hasLambdaArgumentsBefore = !lambdaArguments.isEmpty() - if (hasLambdaArgumentsBefore) { - element.deleteChildRange(lambdaArguments.first(), lambdaArguments.last()) + val lastParameterIndex = newParameters.lastIndex + var firstNamedIndex = newArgumentInfos.firstOrNull { + it.wasNamed + || (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 lastLambdaExpr = if (lastArgument != null) lastArgument.getArgumentExpression()?.unpackFunctionLiteral() else null - 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 + val lastPositionalIndex = if (firstNamedIndex != null) firstNamedIndex - 1 else lastParameterIndex + (lastPositionalIndex + 1 .. lastParameterIndex).forEach { newArgumentInfos[it].makeNamed(callee) } - arguments = arguments.replace(newArgumentList) as KtValueArgumentList + val psiFactory = KtPsiFactory(element.project) - val argumentsToShorten = ArrayList(indicesOfArgumentsWithDefaultValues.size()) - val argumentList = arguments.arguments - indicesOfArgumentsWithDefaultValues.forEach { - argumentsToShorten.add(argumentList[it]) - true + val newArgumentList = psiFactory.createCallArguments("()").apply { + for (argInfo in newArgumentInfos) { + if (argInfo.shouldSkip()) continue + + 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): MutableMap { - val argumentMap = HashMap() - - 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 - } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/jetRefactoringUtil.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/jetRefactoringUtil.kt index b7777ee8205..b8220226d87 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/jetRefactoringUtil.kt +++ b/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 replaceListPsiAndKeepDelimiters( + originalList: ListType, + newList: ListType, + itemsFun: ListType.() -> List +): 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 +} diff --git a/idea/testData/quickfix/changeSignature/addFunctionParameterLongNameRuntime.kt.after b/idea/testData/quickfix/changeSignature/addFunctionParameterLongNameRuntime.kt.after index e59e5562a80..d7b0580aabc 100644 --- a/idea/testData/quickfix/changeSignature/addFunctionParameterLongNameRuntime.kt.after +++ b/idea/testData/quickfix/changeSignature/addFunctionParameterLongNameRuntime.kt.after @@ -4,7 +4,7 @@ import java.util.LinkedHashSet // DISABLE-ERRORS fun foo(x: Int, linkedHashSet: LinkedHashSet) { - foo(, java.util.LinkedHashSet()); - foo(1, java.util.LinkedHashSet()); + foo(, LinkedHashSet()); + foo(1, LinkedHashSet()); foo(2, java.util.LinkedHashSet()); } \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/AddParameterKeepFormatAfter.kt b/idea/testData/refactoring/changeSignature/AddParameterKeepFormatAfter.kt new file mode 100644 index 00000000000..a8b1501f735 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/AddParameterKeepFormatAfter.kt @@ -0,0 +1,16 @@ +fun 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 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/AddParameterKeepFormatBefore.kt b/idea/testData/refactoring/changeSignature/AddParameterKeepFormatBefore.kt new file mode 100644 index 00000000000..191e470cbc3 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/AddParameterKeepFormatBefore.kt @@ -0,0 +1,16 @@ +fun foo(a: Int, b: Int, c: Int) { + +} + +fun test() { + foo(1, + 2, + 3) + foo(1, 2, + 3) + foo( + 1, + 2, + 3 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveDefaultParameterBeforeLambdaAfter.kt b/idea/testData/refactoring/changeSignature/RemoveDefaultParameterBeforeLambdaAfter.kt new file mode 100644 index 00000000000..9e287a2dfb1 --- /dev/null +++ b/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!") + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveDefaultParameterBeforeLambdaBefore.kt b/idea/testData/refactoring/changeSignature/RemoveDefaultParameterBeforeLambdaBefore.kt new file mode 100644 index 00000000000..6695fa11357 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/RemoveDefaultParameterBeforeLambdaBefore.kt @@ -0,0 +1,29 @@ +fun 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!") + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1After.kt b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1After.kt new file mode 100644 index 00000000000..7fb9579c69b --- /dev/null +++ b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1After.kt @@ -0,0 +1,13 @@ +fun foo(b: Int, c: Int) { + +} + +fun test() { + foo(2, + 3) + foo(2, 3) + foo( + 2, + 3 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1Before.kt b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1Before.kt new file mode 100644 index 00000000000..191e470cbc3 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat1Before.kt @@ -0,0 +1,16 @@ +fun foo(a: Int, b: Int, c: Int) { + +} + +fun test() { + foo(1, + 2, + 3) + foo(1, 2, + 3) + foo( + 1, + 2, + 3 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2After.kt b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2After.kt new file mode 100644 index 00000000000..1079d7d2fbe --- /dev/null +++ b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2After.kt @@ -0,0 +1,13 @@ +fun foo(a: Int, c: Int) { + +} + +fun test() { + foo(1, + 3) + foo(1, 3) + foo( + 1, + 3 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2Before.kt b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2Before.kt new file mode 100644 index 00000000000..191e470cbc3 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat2Before.kt @@ -0,0 +1,16 @@ +fun foo(a: Int, b: Int, c: Int) { + +} + +fun test() { + foo(1, + 2, + 3) + foo(1, 2, + 3) + foo( + 1, + 2, + 3 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3After.kt b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3After.kt new file mode 100644 index 00000000000..138eb4d8a75 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3After.kt @@ -0,0 +1,13 @@ +fun foo(a: Int, b: Int) { + +} + +fun test() { + foo(1, + 2) + foo(1, 2) + foo( + 1, + 2 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3Before.kt b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3Before.kt new file mode 100644 index 00000000000..191e470cbc3 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/RemoveParameterKeepFormat3Before.kt @@ -0,0 +1,16 @@ +fun foo(a: Int, b: Int, c: Int) { + +} + +fun test() { + foo(1, + 2, + 3) + foo(1, 2, + 3) + foo( + 1, + 2, + 3 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/SwapParametersKeepFormatAfter.kt b/idea/testData/refactoring/changeSignature/SwapParametersKeepFormatAfter.kt new file mode 100644 index 00000000000..80046ba50d4 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/SwapParametersKeepFormatAfter.kt @@ -0,0 +1,16 @@ +fun foo(c: Int, b: Int, a: Int) { + +} + +fun test() { + foo(3, + 2, + 1) + foo(3, 2, + 1) + foo( + 3, + 2, + 1 + ) +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/SwapParametersKeepFormatBefore.kt b/idea/testData/refactoring/changeSignature/SwapParametersKeepFormatBefore.kt new file mode 100644 index 00000000000..191e470cbc3 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/SwapParametersKeepFormatBefore.kt @@ -0,0 +1,16 @@ +fun foo(a: Int, b: Int, c: Int) { + +} + +fun test() { + foo(1, + 2, + 3) + foo(1, 2, + 3) + foo( + 1, + 2, + 3 + ) +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureTest.kt b/idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureTest.kt index 2b72ede6199..9c799dab7ba 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureTest.kt +++ b/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) } +} \ No newline at end of file