From ec608c5db250b2a3a5bd95640d34caac057d8f23 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Thu, 11 Dec 2014 14:01:36 +0300 Subject: [PATCH] Change Signature: Retain formatting of existing parameters when parameter count/order changes #KT-6412 Fixed --- .../usages/JetFunctionDefinitionUsage.java | 164 ++++++++++++------ .../ExpressionFunctionAfter.kt | 2 +- .../NoDefaultValuesInOverridesAfter.kt | 9 +- .../ParameterListAddParamAfter.kt | 6 + .../ParameterListAddParamBefore.kt | 6 + .../ParameterListRemoveAllParamsAfter.kt | 3 + .../ParameterListRemoveAllParamsBefore.kt | 6 + .../ParameterListRemoveParamAfter.kt | 5 + .../ParameterListRemoveParamBefore.kt | 6 + .../JetChangeSignatureTest.java | 23 ++- 10 files changed, 167 insertions(+), 63 deletions(-) create mode 100644 idea/testData/refactoring/changeSignature/ParameterListAddParamAfter.kt create mode 100644 idea/testData/refactoring/changeSignature/ParameterListAddParamBefore.kt create mode 100644 idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsAfter.kt create mode 100644 idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsBefore.kt create mode 100644 idea/testData/refactoring/changeSignature/ParameterListRemoveParamAfter.kt create mode 100644 idea/testData/refactoring/changeSignature/ParameterListRemoveParamBefore.kt diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionDefinitionUsage.java b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionDefinitionUsage.java index 4f848e100d3..f6e6abdd14b 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionDefinitionUsage.java +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/changeSignature/usages/JetFunctionDefinitionUsage.java @@ -17,8 +17,12 @@ package org.jetbrains.jet.plugin.refactoring.changeSignature.usages; import com.intellij.lang.ASTNode; +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 kotlin.KotlinPackage; import kotlin.Pair; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -41,6 +45,8 @@ import org.jetbrains.jet.plugin.refactoring.changeSignature.JetChangeInfo; import org.jetbrains.jet.plugin.refactoring.changeSignature.JetParameterInfo; import org.jetbrains.jet.plugin.refactoring.changeSignature.JetValVar; +import java.util.List; + import static org.jetbrains.jet.lang.psi.PsiPackage.JetPsiFactory; public class JetFunctionDefinitionUsage extends JetUsageInfo { @@ -188,63 +194,7 @@ public class JetFunctionDefinitionUsage extends JetUsageIn } if (changeInfo.isParameterSetOrOrderChanged()) { - int parametersCount = changeInfo.getNewParametersCount(); - boolean isLambda = element instanceof JetFunctionLiteral; - - JetParameterList newParameterList = null; - if (isLambda) { - if (parametersCount == 0 && ((JetFunctionLiteral) element).getTypeReference() == null) { - if (parameterList != null) { - parameterList.delete(); - ASTNode arrowNode = ((JetFunctionLiteral)element).getArrowNode(); - if (arrowNode != null) { - arrowNode.getPsi().delete(); - } - } - } - else { - newParameterList = psiFactory.createFunctionLiteralParameterList(changeInfo.getNewParametersSignature( - (JetFunctionDefinitionUsage) this, 0) - ); - } - } - else { - newParameterList = psiFactory.createParameterList(changeInfo.getNewParametersSignature( - (JetFunctionDefinitionUsage) this, 0) - ); - } - - if (newParameterList != null) { - if (parameterList != null) { - newParameterList = (JetParameterList) parameterList.replace(newParameterList); - } - else { - if (element instanceof JetClass) { - PsiElement anchor = ((JetClass) element).getTypeParameterList(); - - if (anchor == null) { - anchor = ((JetClass) element).getNameIdentifier(); - } - if (anchor != null) { - newParameterList = (JetParameterList) element.addAfter(newParameterList, anchor); - } - } - else if (isLambda) { - //noinspection ConstantConditions - JetFunctionLiteral functionLiteral = (JetFunctionLiteral) element; - PsiElement anchor = functionLiteral.getLBrace(); - newParameterList = (JetParameterList) element.addAfter(newParameterList, anchor); - if (functionLiteral.getArrowNode() == null) { - Pair whitespaceAndArrow = psiFactory.createWhitespaceAndArrow(); - element.addRangeAfter(whitespaceAndArrow.getFirst(), whitespaceAndArrow.getSecond(), newParameterList); - } - } - } - } - - if (newParameterList != null) { - ShortenPackage.addToShorteningWaitSet(newParameterList); - } + processParameterListWithStructuralChanges(changeInfo, element, parameterList, psiFactory); } else if (parameterList != null) { int paramIndex = 0; @@ -265,6 +215,106 @@ public class JetFunctionDefinitionUsage extends JetUsageIn return true; } + private void processParameterListWithStructuralChanges( + JetChangeInfo changeInfo, + PsiElement element, + JetParameterList parameterList, + JetPsiFactory psiFactory + ) { + int parametersCount = changeInfo.getNewParametersCount(); + boolean isLambda = element instanceof JetFunctionLiteral; + boolean canReplaceEntireList = false; + + JetParameterList newParameterList = null; + if (isLambda) { + if (parametersCount == 0 && ((JetFunctionLiteral) element).getTypeReference() == null) { + if (parameterList != null) { + parameterList.delete(); + ASTNode arrowNode = ((JetFunctionLiteral)element).getArrowNode(); + if (arrowNode != null) { + arrowNode.getPsi().delete(); + } + parameterList = null; + } + } + else { + newParameterList = psiFactory.createFunctionLiteralParameterList(changeInfo.getNewParametersSignature( + (JetFunctionDefinitionUsage) this, 0) + ); + canReplaceEntireList = true; + } + } + else { + newParameterList = psiFactory.createParameterList(changeInfo.getNewParametersSignature( + (JetFunctionDefinitionUsage) this, 0) + ); + } + + if (newParameterList == null) return; + + if (parameterList != null) { + if (canReplaceEntireList) { + newParameterList = (JetParameterList) parameterList.replace(newParameterList); + } + else { + newParameterList = replaceParameterListAndKeepDelimiters(parameterList, newParameterList); + } + } + else { + if (element instanceof JetClass) { + PsiElement anchor = ((JetClass) element).getTypeParameterList(); + + if (anchor == null) { + anchor = ((JetClass) element).getNameIdentifier(); + } + if (anchor != null) { + newParameterList = (JetParameterList) element.addAfter(newParameterList, anchor); + } + } + else if (isLambda) { + //noinspection ConstantConditions + JetFunctionLiteral functionLiteral = (JetFunctionLiteral) element; + PsiElement anchor = functionLiteral.getLBrace(); + newParameterList = (JetParameterList) element.addAfter(newParameterList, anchor); + if (functionLiteral.getArrowNode() == null) { + Pair whitespaceAndArrow = psiFactory.createWhitespaceAndArrow(); + element.addRangeAfter(whitespaceAndArrow.getFirst(), whitespaceAndArrow.getSecond(), newParameterList); + } + } + } + + if (newParameterList != null) { + ShortenPackage.addToShorteningWaitSet(newParameterList); + } + } + + private static JetParameterList replaceParameterListAndKeepDelimiters(JetParameterList parameterList, JetParameterList newParameterList) { + List oldParameters = parameterList.getParameters(); + List newParameters = newParameterList.getParameters(); + int oldCount = oldParameters.size(); + int newCount = newParameters.size(); + + int commonCount = Math.min(oldCount, newCount); + for (int i = 0; i < commonCount; i++) { + oldParameters.set(i, (JetParameter) oldParameters.get(i).replace(newParameters.get(i))); + } + + if (commonCount == 0) return (JetParameterList) parameterList.replace(newParameterList); + + if (oldCount > commonCount) { + parameterList.deleteChildRange(oldParameters.get(commonCount - 1).getNextSibling(), + KotlinPackage.last(oldParameters)); + } + else if (newCount > commonCount) { + parameterList.addRangeAfter(newParameters.get(commonCount - 1).getNextSibling(), + newParameterList.getLastChild().getPrevSibling(), + PsiTreeUtil.skipSiblingsBackward(parameterList.getLastChild(), + PsiWhiteSpace.class, PsiComment.class)); + } + + return parameterList; + } + private static void changeVisibility(JetChangeInfo changeInfo, PsiElement element) { JetModifierKeywordToken newVisibilityToken = JetRefactoringUtil.getVisibilityToken(changeInfo.getNewVisibility()); diff --git a/idea/testData/refactoring/changeSignature/ExpressionFunctionAfter.kt b/idea/testData/refactoring/changeSignature/ExpressionFunctionAfter.kt index eca46c606b0..bfac65fd6d8 100644 --- a/idea/testData/refactoring/changeSignature/ExpressionFunctionAfter.kt +++ b/idea/testData/refactoring/changeSignature/ExpressionFunctionAfter.kt @@ -1 +1 @@ -fun fun1(x1: Int, y1: Int): Int = x1 * 2 + fun1(x1, ) +fun fun1(x1: Int, y1: Int) : Int = x1 * 2 + fun1(x1, ) diff --git a/idea/testData/refactoring/changeSignature/NoDefaultValuesInOverridesAfter.kt b/idea/testData/refactoring/changeSignature/NoDefaultValuesInOverridesAfter.kt index 45c60318503..8cd2f78a6a6 100644 --- a/idea/testData/refactoring/changeSignature/NoDefaultValuesInOverridesAfter.kt +++ b/idea/testData/refactoring/changeSignature/NoDefaultValuesInOverridesAfter.kt @@ -1,15 +1,18 @@ trait T { - fun foo(a: Int = 1, b: String = "2") + fun foo(a: Int = 1, + b: String = "2") } open class A: T { - override fun foo(a: Int, b: String) { + override fun foo(a: Int, + b: String) { throw UnsupportedOperationException() } } class B: A() { - override fun foo(a: Int, b: String) { + override fun foo(a: Int, + b: String) { throw UnsupportedOperationException() } } \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/ParameterListAddParamAfter.kt b/idea/testData/refactoring/changeSignature/ParameterListAddParamAfter.kt new file mode 100644 index 00000000000..56ab3fc042e --- /dev/null +++ b/idea/testData/refactoring/changeSignature/ParameterListAddParamAfter.kt @@ -0,0 +1,6 @@ +fun foo( + n: Int, s: String, + a: Any, l: Long +) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/ParameterListAddParamBefore.kt b/idea/testData/refactoring/changeSignature/ParameterListAddParamBefore.kt new file mode 100644 index 00000000000..04bfefcc643 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/ParameterListAddParamBefore.kt @@ -0,0 +1,6 @@ +fun foo( + n: Int, s: String, + a: Any +) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsAfter.kt b/idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsAfter.kt new file mode 100644 index 00000000000..a2c4e958bb5 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsAfter.kt @@ -0,0 +1,3 @@ +fun foo() { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsBefore.kt b/idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsBefore.kt new file mode 100644 index 00000000000..04bfefcc643 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/ParameterListRemoveAllParamsBefore.kt @@ -0,0 +1,6 @@ +fun foo( + n: Int, s: String, + a: Any +) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/ParameterListRemoveParamAfter.kt b/idea/testData/refactoring/changeSignature/ParameterListRemoveParamAfter.kt new file mode 100644 index 00000000000..2735b331aac --- /dev/null +++ b/idea/testData/refactoring/changeSignature/ParameterListRemoveParamAfter.kt @@ -0,0 +1,5 @@ +fun foo( + n: Int, s: String +) { + +} \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/ParameterListRemoveParamBefore.kt b/idea/testData/refactoring/changeSignature/ParameterListRemoveParamBefore.kt new file mode 100644 index 00000000000..04bfefcc643 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/ParameterListRemoveParamBefore.kt @@ -0,0 +1,6 @@ +fun foo( + n: Int, s: String, + a: Any +) { + +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureTest.java b/idea/tests/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureTest.java index e241b73f61d..921ad813002 100644 --- a/idea/tests/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureTest.java +++ b/idea/tests/org/jetbrains/jet/plugin/refactoring/changeSignature/JetChangeSignatureTest.java @@ -318,8 +318,7 @@ public class JetChangeSignatureTest extends KotlinCodeInsightTestCase { public void testFunctionJavaUsagesAndOverridesAddParam() throws Exception { JetChangeInfo changeInfo = getChangeInfo(); changeInfo.addParameter(new JetParameterInfo(-1, "s", KotlinBuiltIns.getInstance().getStringType(), null, "\"abc\"", null, null)); - changeInfo.addParameter(new JetParameterInfo(-1, "o", KotlinBuiltIns.getInstance().getNullableAnyType(), null, "\"def\"", null, - null)); + changeInfo.addParameter(new JetParameterInfo(-1, "o", KotlinBuiltIns.getInstance().getNullableAnyType(), null, "\"def\"", null, null)); doTest(changeInfo); } @@ -563,6 +562,26 @@ public class JetChangeSignatureTest extends KotlinCodeInsightTestCase { doTest(changeInfo); } + public void testParameterListAddParam() throws Exception { + JetChangeInfo changeInfo = getChangeInfo(); + changeInfo.addParameter(new JetParameterInfo(-1, "l", KotlinBuiltIns.getInstance().getLongType(), null, "", null, null)); + doTest(changeInfo); + } + + public void testParameterListRemoveParam() throws Exception { + JetChangeInfo changeInfo = getChangeInfo(); + changeInfo.removeParameter(changeInfo.getNewParametersCount() - 1); + doTest(changeInfo); + } + + public void testParameterListRemoveAllParams() throws Exception { + JetChangeInfo changeInfo = getChangeInfo(); + for (int i = changeInfo.getNewParametersCount() - 1; i >= 0; i--) { + changeInfo.removeParameter(i); + } + doTest(changeInfo); + } + @NotNull @Override protected String getTestDataPath() {