From 5499bea181b5022bc2f0a4a2e8ebd51f65358bc0 Mon Sep 17 00:00:00 2001 From: Alexander Kirillin Date: Tue, 7 May 2013 20:07:01 +0400 Subject: [PATCH] fix for change signature when the order of named arguments is non-trivial --- .../changeSignature/JetChangeInfo.java | 28 ++++ .../JetChangeSignatureUsageProcessor.java | 3 +- .../JetFunctionPlatformDescriptorImpl.java | 4 + .../usages/JetFunctionCallUsage.java | 140 ++++++++++++------ .../FunctionsAddRemoveArgumentsAfter.kt | 5 +- .../FunctionsAddRemoveArgumentsBefore.kt | 3 + .../changeSignature/FunctionsAfter.kt | 2 + .../changeSignature/FunctionsBefore.kt | 2 + 8 files changed, 142 insertions(+), 45 deletions(-) diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeInfo.java b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeInfo.java index 0eccff3d7be..bb4a739b123 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeInfo.java +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeInfo.java @@ -23,6 +23,7 @@ import com.intellij.refactoring.changeSignature.ChangeInfo; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; +import org.jetbrains.jet.lang.descriptors.ValueParameterDescriptor; import org.jetbrains.jet.lang.descriptors.Visibilities; import org.jetbrains.jet.lang.descriptors.Visibility; import org.jetbrains.jet.lang.psi.JetFunction; @@ -31,7 +32,9 @@ import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.lexer.JetTokens; import org.jetbrains.jet.plugin.JetLanguage; +import java.util.HashMap; import java.util.List; +import java.util.Map; public class JetChangeInfo implements ChangeInfo { private final JetFunctionPlatformDescriptor oldDescriptor; @@ -43,6 +46,7 @@ public class JetChangeInfo implements ChangeInfo { private final PsiElement context; private final JetGeneratedInfo generatedInfo; private Boolean parameterNamesChanged; + private Map oldNameToParameterIndex; public JetChangeInfo( JetFunctionPlatformDescriptor oldDescriptor, @@ -119,6 +123,22 @@ public class JetChangeInfo implements ChangeInfo { return false; } + private Map initOldNameToParameterIndex() { + Map map = new HashMap(); + FunctionDescriptor descriptor = oldDescriptor.getDescriptor(); + + if (descriptor != null) { + List parameters = descriptor.getValueParameters(); + + for (int i = 0; i < parameters.size(); i++) { + ValueParameterDescriptor oldParameter = parameters.get(i); + map.put(oldParameter.getName().getName(), i); + } + } + + return map; + } + @NotNull @Override public JetParameterInfo[] getNewParameters() { @@ -141,6 +161,14 @@ public class JetChangeInfo implements ChangeInfo { return parameterNamesChanged; } + @Nullable + public Integer getOldParameterIndex(String oldParameterName) { + if (oldNameToParameterIndex == null) + oldNameToParameterIndex = initOldNameToParameterIndex(); + + return oldNameToParameterIndex.get(oldParameterName); + } + @Override public boolean isParameterTypesChanged() { return true; diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureUsageProcessor.java b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureUsageProcessor.java index 6bccdd75235..3da566624e7 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureUsageProcessor.java +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureUsageProcessor.java @@ -110,7 +110,8 @@ public class JetChangeSignatureUsageProcessor implements ChangeSignatureUsagePro for (PsiReference reference : ReferencesSearch.search(oldParam, oldParam.getUseScope())) { PsiElement element = reference.getElement(); - if (element.getParent() instanceof JetSimpleNameExpression) + if (element.getParent() instanceof JetSimpleNameExpression && + !(element.getParent().getParent() instanceof JetValueArgumentName)) // Usages in named arguments of the calls usage will be changed when the function call is changed result.add(new JetParameterUsage((JetSimpleNameExpression) element.getParent(), parameterInfo, functionPsi, isInherited)); } } diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetFunctionPlatformDescriptorImpl.java b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetFunctionPlatformDescriptorImpl.java index 50385dbe336..52293387f0c 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetFunctionPlatformDescriptorImpl.java +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/JetFunctionPlatformDescriptorImpl.java @@ -75,6 +75,10 @@ public class JetFunctionPlatformDescriptorImpl implements JetFunctionPlatformDes parameters.remove(index); } + public void clearParameters() { + parameters.clear(); + } + @Override public int getParametersCount() { return funDescriptor.getValueParameters().size(); diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionCallUsage.java b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionCallUsage.java index 474f8b74c77..f1441ed4530 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionCallUsage.java +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionCallUsage.java @@ -22,7 +22,9 @@ import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.plugin.refactoring.changeSignature.JetChangeInfo; import org.jetbrains.jet.plugin.refactoring.changeSignature.JetParameterInfo; +import java.util.HashMap; import java.util.List; +import java.util.Map; public class JetFunctionCallUsage extends JetUsageInfo { private final PsiElement function; @@ -48,51 +50,103 @@ public class JetFunctionCallUsage extends JetUsageInfo { if (arguments != null) { List oldArguments = arguments.getArguments(); - if (changeInfo.isParameterSetOrOrderChanged()) { - StringBuilder parametersBuilder = new StringBuilder("("); - boolean isFirst = true; - - for (JetParameterInfo parameterInfo : changeInfo.getNewParameters()) { - if (isFirst) - isFirst = false; - else - parametersBuilder.append(','); - - String defaultValueText = parameterInfo.getDefaultValueText(); - parametersBuilder.append(defaultValueText.isEmpty() ? '0' : defaultValueText); - } - - parametersBuilder.append(')'); - JetValueArgumentList newArguments = JetPsiFactory.createCallArguments(getProject(), parametersBuilder.toString()); - int argIndex = 0; - - for (JetValueArgument newArgument : newArguments.getArguments()) { - JetParameterInfo parameterInfo = changeInfo.getNewParameters()[argIndex++]; - int oldIndex = parameterInfo.getOldIndex(); - - if (oldIndex >= 0 && oldIndex < oldArguments.size()) - newArgument.replace(oldArguments.get(oldIndex)); - else if (parameterInfo.getDefaultValueText().isEmpty()) - newArgument.delete(); - } - - arguments.replace(newArguments); - } - else { - for (JetParameterInfo parameterInfo : changeInfo.getNewParameters()) { - JetValueArgument argument = parameterInfo.getOldIndex() < oldArguments.size() ? oldArguments.get(parameterInfo.getOldIndex()) : null; - JetValueArgumentName argumentName = argument != null ? argument.getArgumentName() : null; - JetSimpleNameExpression argumentNameExpression = argumentName != null ? argumentName.getReferenceExpression() : null; - PsiElement identifier = argumentNameExpression != null ? argumentNameExpression.getIdentifier() : null; - - if (identifier != null) { - String newName = parameterInfo.getInheritedName(isInherited, function, changeInfo.getFunctionDescriptor()); - identifier.replace(JetPsiFactory.createIdentifier(getProject(), newName)); - } - } - } + if (changeInfo.isParameterSetOrOrderChanged()) + arguments.replace(generateNewArgumentList(changeInfo, oldArguments)); + else + changeArgumentNames(changeInfo, oldArguments); } return true; } + + private JetValueArgumentList generateNewArgumentList(JetChangeInfo changeInfo, List oldArguments) { + boolean isNamedCall = oldArguments.size() > 1 && oldArguments.get(0).getArgumentName() != null; + StringBuilder parametersBuilder = new StringBuilder("("); + boolean isFirst = true; + + for (JetParameterInfo parameterInfo : changeInfo.getNewParameters()) { + if (isFirst) + isFirst = false; + else + parametersBuilder.append(','); + + String defaultValueText = parameterInfo.getDefaultValueText(); + + if (isNamedCall) { + String newName = parameterInfo.getInheritedName(isInherited, function, changeInfo.getFunctionDescriptor()); + parametersBuilder.append(newName).append('='); + } + + parametersBuilder.append(defaultValueText.isEmpty() ? '0' : defaultValueText); + } + + parametersBuilder.append(')'); + JetValueArgumentList newArguments = JetPsiFactory.createCallArguments(getProject(), parametersBuilder.toString()); + + Map argumentMap = getParamIndexToArgumentMap(changeInfo, oldArguments); + int argIndex = 0; + + for (JetValueArgument newArgument : newArguments.getArguments()) { + JetParameterInfo parameterInfo = changeInfo.getNewParameters()[argIndex++]; + JetValueArgument oldArgument = argumentMap.get(parameterInfo.getOldIndex()); + + if (oldArgument != null) { + JetValueArgumentName argumentName = oldArgument.getArgumentName(); + JetSimpleNameExpression argumentNameExpression = argumentName != null ? argumentName.getReferenceExpression() : null; + changeArgumentName(changeInfo, argumentNameExpression, parameterInfo); + newArgument.replace(oldArgument); + } + else if (parameterInfo.getDefaultValueText().isEmpty()) + newArgument.delete(); + } + + return newArguments; + } + + private static Map getParamIndexToArgumentMap(JetChangeInfo changeInfo, List oldArguments) { + Map argumentMap = new HashMap(); + + for (int i = 0; i < oldArguments.size(); i++) { + JetValueArgument argument = oldArguments.get(i); + JetValueArgumentName argumentName = argument.getArgumentName(); + JetSimpleNameExpression argumentNameExpression = argumentName != null ? argumentName.getReferenceExpression() : null; + String oldParameterName = argumentNameExpression != null ? argumentNameExpression.getReferencedName() : null; + + if (oldParameterName != null) { + Integer oldParameterIndex = changeInfo.getOldParameterIndex(oldParameterName); + + if (oldParameterIndex != null) + argumentMap.put(oldParameterIndex, argument); + } + else + argumentMap.put(i, argument); + } + + return argumentMap; + } + + private void changeArgumentNames(JetChangeInfo changeInfo, List oldArguments) { + for (JetValueArgument argument : oldArguments) { + JetValueArgumentName argumentName = argument.getArgumentName(); + JetSimpleNameExpression argumentNameExpression = argumentName != null ? argumentName.getReferenceExpression() : null; + + if (argumentNameExpression != null) { + Integer oldParameterIndex = changeInfo.getOldParameterIndex(argumentNameExpression.getReferencedName()); + + if (oldParameterIndex != null) { + JetParameterInfo parameterInfo = changeInfo.getNewParameters()[oldParameterIndex]; + changeArgumentName(changeInfo, argumentNameExpression, parameterInfo); + } + } + } + } + + private void changeArgumentName(JetChangeInfo changeInfo, JetSimpleNameExpression argumentNameExpression, JetParameterInfo parameterInfo) { + PsiElement identifier = argumentNameExpression != null ? argumentNameExpression.getIdentifier() : null; + + if (identifier != null) { + String newName = parameterInfo.getInheritedName(isInherited, function, changeInfo.getFunctionDescriptor()); + identifier.replace(JetPsiFactory.createIdentifier(getProject(), newName)); + } + } } diff --git a/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsAfter.kt b/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsAfter.kt index e80516bcaf8..bb5ea602529 100644 --- a/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsAfter.kt +++ b/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsAfter.kt @@ -5,8 +5,11 @@ fun foo(x0: Any?, val y1 = x1; val y2 = x2; val y3 = x3; + foo(x0 = null, x1 = 4, x2 = 5.5); } fun bar() { - foo(null, x1 = 2, x2 = 3.5); + foo(x0 = null, x1 = 2, x2 = 3.5); + foo(x0 = null, x1 = 3, x2 = 4.5); + foo(x0 = null, x1 = 4, x2 = 5.5); } diff --git a/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsBefore.kt b/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsBefore.kt index e730677d332..60814db3668 100644 --- a/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsBefore.kt +++ b/idea/testData/refactoring/changeSignature/FunctionsAddRemoveArgumentsBefore.kt @@ -3,8 +3,11 @@ protected fun foo(x1: Int = 1, x2: Float, x3: ((Int) -> Int)?) { val y1 = x1; val y2 = x2; val y3 = x3; + foo(x3 = null, x2 = 5.5, x1 = 4); } fun bar() { foo(x1 = 2, x2 = 3.5, x3 = null); + foo(x3 = null, x1 = 3, x2 = 4.5); + foo(x3 = null, x2 = 5.5, x1 = 4); } diff --git a/idea/testData/refactoring/changeSignature/FunctionsAfter.kt b/idea/testData/refactoring/changeSignature/FunctionsAfter.kt index 806feff5dea..f9de409e061 100644 --- a/idea/testData/refactoring/changeSignature/FunctionsAfter.kt +++ b/idea/testData/refactoring/changeSignature/FunctionsAfter.kt @@ -3,8 +3,10 @@ public fun foo(_x1: Int = 1, _x2: Float?, _x3: ((Int) -> Int)?) { val y1 = _x1; val y2 = _x2; val y3 = _x3; + foo(_x3 = null, _x1 = 2, _x2 = 3.5); } fun bar() { foo(_x1 = 2, _x2 = 3.5, _x3 = null); + foo(_x3 = null, _x1 = 2, _x2 = 3.5); } diff --git a/idea/testData/refactoring/changeSignature/FunctionsBefore.kt b/idea/testData/refactoring/changeSignature/FunctionsBefore.kt index ef266d93854..8e1075624dc 100644 --- a/idea/testData/refactoring/changeSignature/FunctionsBefore.kt +++ b/idea/testData/refactoring/changeSignature/FunctionsBefore.kt @@ -3,8 +3,10 @@ fun foo(x1: Int = 1, x2: Float, x3: ((Int) -> Int)?) { val y1 = x1; val y2 = x2; val y3 = x3; + foo(x3 = null, x1 = 2, x2 = 3.5); } fun bar() { foo(x1 = 2, x2 = 3.5, x3 = null); + foo(x3 = null, x1 = 2, x2 = 3.5); }