From 7805ee01eba74ba3782a650ad2fcdb75b9efcce9 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Tue, 4 Jun 2013 15:35:43 +0400 Subject: [PATCH] Fix moving of value parameters and arguments --- .../jetbrains/jet/lang/psi/JetPsiFactory.java | 7 ++ .../upDownMover/AbstractJetUpDownMover.java | 5 + .../upDownMover/JetExpressionMover.java | 95 ++++++++++++++----- .../functionAnchors/valueArgs1.kt | 7 ++ .../functionAnchors/valueArgs1.kt.after | 7 ++ .../functionAnchors/valueArgs2.kt | 7 ++ .../functionAnchors/valueArgs2.kt.after | 7 ++ .../functionAnchors/valueArgs3.kt | 8 ++ .../functionAnchors/valueArgs4.kt | 7 ++ .../functionAnchors/valueArgs4.kt.after | 7 ++ .../functionAnchors/valueArgs5.kt | 7 ++ .../functionAnchors/valueArgs5.kt.after | 7 ++ .../functionAnchors/valueArgs6.kt | 8 ++ .../functionAnchors/valueParams2.kt.after | 6 +- .../moveUpDown/AbstractCodeMoverTest.java | 13 ++- .../moveUpDown/CodeMoverTestGenerated.java | 30 ++++++ 16 files changed, 200 insertions(+), 28 deletions(-) create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt.after create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt.after create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs3.kt create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt.after create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt.after create mode 100644 idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs6.kt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiFactory.java b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiFactory.java index c9f83314700..22a15e9a39f 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiFactory.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiFactory.java @@ -73,6 +73,13 @@ public class JetPsiFactory { return star; } + @NotNull + public static PsiElement createComma(Project project) { + PsiElement comma = createType(project, "T").findElementAt(3); + assert comma != null; + return comma; + } + //the pair contains the first and the last elements of a range public static Pair createColonAndWhiteSpaces(Project project) { JetProperty property = createProperty(project, "val x : Int"); diff --git a/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/AbstractJetUpDownMover.java b/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/AbstractJetUpDownMover.java index a97f06c754a..1e81b14fa56 100644 --- a/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/AbstractJetUpDownMover.java +++ b/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/AbstractJetUpDownMover.java @@ -64,6 +64,11 @@ public abstract class AbstractJetUpDownMover extends LineMover { return firstNonWhiteElement(down ? lineRange.lastElement.getNextSibling() : lineRange.firstElement.getPrevSibling(), down); } + @Nullable + protected static PsiElement firstNonWhiteSibling(@NotNull PsiElement element, boolean down) { + return firstNonWhiteElement(down ? element.getNextSibling() : element.getPrevSibling(), down); + } + @Override public boolean checkAvailable(@NotNull Editor editor, @NotNull PsiFile file, @NotNull MoveInfo info, boolean down) { return (file instanceof JetFile) && super.checkAvailable(editor, file, info, down); diff --git a/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/JetExpressionMover.java b/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/JetExpressionMover.java index 81d4d544464..6dea19786ba 100644 --- a/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/JetExpressionMover.java +++ b/idea/src/org/jetbrains/jet/plugin/codeInsight/upDownMover/JetExpressionMover.java @@ -5,10 +5,7 @@ import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.TextRange; -import com.intellij.psi.PsiComment; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiFile; -import com.intellij.psi.PsiWhiteSpace; +import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -20,7 +17,7 @@ public class JetExpressionMover extends AbstractJetUpDownMover { } @SuppressWarnings("FieldMayBeFinal") - private static Class[] MOVABLE_ELEMENT_CLASSES = {JetExpression.class, JetWhenEntry.class, PsiComment.class}; + private static Class[] MOVABLE_ELEMENT_CLASSES = {JetExpression.class, JetWhenEntry.class, JetValueArgument.class, PsiComment.class}; @SuppressWarnings("FieldMayBeFinal") private static Class[] BLOCKLIKE_ELEMENT_CLASSES = @@ -231,12 +228,35 @@ public class JetExpressionMover extends AbstractJetUpDownMover { } @Nullable - private static LineRange getTargetRange( + private LineRange getValueParamOrArgTargetRange(@NotNull Editor editor, @NotNull PsiElement elementToCheck, @NotNull PsiElement sibling, boolean down) { + PsiElement next = sibling; + + if (next.getNode().getElementType() == JetTokens.COMMA) { + next = firstNonWhiteSibling(next, down); + } + + LineRange range = (next instanceof JetParameter || next instanceof JetValueArgument) + ? new LineRange(next, next, editor.getDocument()) + : null; + + if (range != null) { + parametersOrArgsToMove = new Pair(elementToCheck, next); + } + + return range; + } + + @Nullable + private LineRange getTargetRange( @NotNull Editor editor, @Nullable PsiElement elementToCheck, @NotNull PsiElement sibling, boolean down ) { + if (elementToCheck instanceof JetParameter || elementToCheck instanceof JetValueArgument) { + return getValueParamOrArgTargetRange(editor, elementToCheck, sibling, down); + } + if (elementToCheck instanceof JetExpression || elementToCheck instanceof PsiComment) { return getExpressionTargetRange(editor, sibling, down); } @@ -276,19 +296,16 @@ public class JetExpressionMover extends AbstractJetUpDownMover { return movableElement; } - private static enum MoveStatus { - DEFAULT, - FORBIDDEN, - PERMITTED + private static boolean isLastOfItsKind(@NotNull PsiElement element, boolean down) { + return getSiblingOfType(element, down, element.getClass()) == null; } - private static MoveStatus getMoveStatus(@NotNull PsiElement element, boolean down) { - if (element instanceof JetParameter) { - PsiElement sibling = getSiblingOfType(element, down, element.getClass()); - return (sibling != null) ? MoveStatus.DEFAULT : MoveStatus.FORBIDDEN; + private static boolean isForbiddenMove(@NotNull PsiElement element, boolean down) { + if (element instanceof JetParameter || element instanceof JetValueArgument) { + return isLastOfItsKind(element, down); } - return MoveStatus.PERMITTED; + return false; } private static boolean isBracelessBlock(@NotNull PsiElement element) { @@ -348,6 +365,8 @@ public class JetExpressionMover extends AbstractJetUpDownMover { @Override public boolean checkAvailable(@NotNull Editor editor, @NotNull PsiFile file, @NotNull MoveInfo info, boolean down) { + parametersOrArgsToMove = null; + if (!super.checkAvailable(editor, file, info, down)) return false; switch (checkForMovableClosingBrace(editor, file, info, down)) { @@ -370,16 +389,13 @@ public class JetExpressionMover extends AbstractJetUpDownMover { if (firstElement == null || lastElement == null) return false; - MoveStatus firstMoveStatus = getMoveStatus(firstElement, down); - MoveStatus lastMoveStatus = getMoveStatus(lastElement, down); - - if (firstMoveStatus == MoveStatus.DEFAULT || lastMoveStatus == MoveStatus.DEFAULT) { + if (isForbiddenMove(firstElement, down) || isForbiddenMove(lastElement, down)) { + info.toMove2 = null; return true; } - if (firstMoveStatus == MoveStatus.FORBIDDEN || lastMoveStatus == MoveStatus.FORBIDDEN) { - info.toMove2 = null; - return true; + if ((firstElement instanceof JetParameter || firstElement instanceof JetValueArgument) && PsiTreeUtil.isAncestor(lastElement, firstElement, false)) { + lastElement = firstElement; } LineRange sourceRange = getSourceRange(firstElement, lastElement, editor, oldRange); @@ -394,4 +410,39 @@ public class JetExpressionMover extends AbstractJetUpDownMover { info.toMove2 = getTargetRange(editor, sourceRange.firstElement, sibling, down); return true; } + + @Nullable + private Pair parametersOrArgsToMove; + + private static PsiElement getComma(@NotNull PsiElement element) { + PsiElement sibling = firstNonWhiteSibling(element, true); + return sibling != null && (sibling.getNode().getElementType() == JetTokens.COMMA) ? sibling : null; + } + + private static void fixCommaIfNeeded(@NotNull PsiElement element, boolean willBeLast) { + PsiElement comma = getComma(element); + if (willBeLast && comma != null) { + comma.delete(); + } + else if (!willBeLast && comma == null) { + PsiElement parent = element.getParent(); + assert parent != null; + + parent.addAfter(JetPsiFactory.createComma(parent.getProject()), element); + } + } + + @Override + public void beforeMove(@NotNull Editor editor, @NotNull MoveInfo info, boolean down) { + if (parametersOrArgsToMove != null) { + PsiElement element1 = parametersOrArgsToMove.first; + PsiElement element2 = parametersOrArgsToMove.second; + + fixCommaIfNeeded(element1, down && isLastOfItsKind(element2, true)); + fixCommaIfNeeded(element2, !down && isLastOfItsKind(element1, true)); + + //noinspection ConstantConditions + PsiDocumentManager.getInstance(editor.getProject()).doPostponedOperationsAndUnblockDocument(editor.getDocument()); + } + } } diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt new file mode 100644 index 00000000000..7b11f2e4d49 --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt @@ -0,0 +1,7 @@ +// MOVE: down +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + a, + b, + c +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt.after b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt.after new file mode 100644 index 00000000000..f9c60de097e --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt.after @@ -0,0 +1,7 @@ +// MOVE: down +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + b, + a, + c +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt new file mode 100644 index 00000000000..f9c60de097e --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt @@ -0,0 +1,7 @@ +// MOVE: down +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + b, + a, + c +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt.after b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt.after new file mode 100644 index 00000000000..3db2d20f7ac --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt.after @@ -0,0 +1,7 @@ +// MOVE: down +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + b, + c, + a +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs3.kt b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs3.kt new file mode 100644 index 00000000000..6bd9df3b1dc --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs3.kt @@ -0,0 +1,8 @@ +// MOVE: down +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +// IS_APPLICABLE: false +val x = foo( + b, + c, + a +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt new file mode 100644 index 00000000000..b08d329c8b9 --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt @@ -0,0 +1,7 @@ +// MOVE: up +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + b, + c, + a +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt.after b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt.after new file mode 100644 index 00000000000..ec326d77a59 --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt.after @@ -0,0 +1,7 @@ +// MOVE: up +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + b, + a, + c +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt new file mode 100644 index 00000000000..ec326d77a59 --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt @@ -0,0 +1,7 @@ +// MOVE: up +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + b, + a, + c +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt.after b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt.after new file mode 100644 index 00000000000..5a760ce3e34 --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt.after @@ -0,0 +1,7 @@ +// MOVE: up +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +val x = foo( + a, + b, + c +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs6.kt b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs6.kt new file mode 100644 index 00000000000..abfadee7eeb --- /dev/null +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs6.kt @@ -0,0 +1,8 @@ +// MOVE: up +// MOVER_CLASS: org.jetbrains.jet.plugin.codeInsight.upDownMover.JetExpressionMover +// IS_APPLICABLE: false +val x = foo( + a, + b, + c +) \ No newline at end of file diff --git a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueParams2.kt.after b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueParams2.kt.after index ed71321452d..216202bc19c 100644 --- a/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueParams2.kt.after +++ b/idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueParams2.kt.after @@ -5,9 +5,9 @@ class A { U, W>( b: Int, - c: Int - a: Int, - ) { + c: Int, + a: Int + ) { } class B { diff --git a/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/AbstractCodeMoverTest.java b/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/AbstractCodeMoverTest.java index 53ade244493..c4dd225c1cb 100644 --- a/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/AbstractCodeMoverTest.java +++ b/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/AbstractCodeMoverTest.java @@ -19,9 +19,11 @@ package org.jetbrains.jet.plugin.codeInsight.moveUpDown; import com.intellij.codeInsight.editorActions.moveUpDown.MoveStatementDownAction; import com.intellij.codeInsight.editorActions.moveUpDown.MoveStatementUpAction; import com.intellij.codeInsight.editorActions.moveUpDown.StatementUpDownMover; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.editor.actionSystem.EditorAction; import com.intellij.openapi.extensions.Extensions; import com.intellij.openapi.util.io.FileUtil; +import com.intellij.psi.PsiDocumentManager; import com.intellij.testFramework.LightCodeInsightTestCase; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.InTextDirectivesUtils; @@ -83,9 +85,14 @@ public abstract class AbstractCodeMoverTest extends LightCodeInsightTestCase { } } - private void invokeAndCheck(@NotNull String path, boolean down) { - EditorAction action = down ? new MoveStatementDownAction() : new MoveStatementUpAction(); - action.actionPerformed(getEditor(), getCurrentEditorDataContext()); + private void invokeAndCheck(@NotNull String path, final boolean down) { + ApplicationManager.getApplication().runWriteAction(new Runnable() { + @Override + public void run() { + EditorAction action = down ? new MoveStatementDownAction() : new MoveStatementUpAction(); + action.actionPerformed(getEditor(), getCurrentEditorDataContext()); + } + }); checkResultByFile(path + ".after"); } diff --git a/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/CodeMoverTestGenerated.java b/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/CodeMoverTestGenerated.java index 8172b77ac32..c7ca70b13a4 100644 --- a/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/CodeMoverTestGenerated.java +++ b/idea/tests/org/jetbrains/jet/plugin/codeInsight/moveUpDown/CodeMoverTestGenerated.java @@ -385,6 +385,36 @@ public class CodeMoverTestGenerated extends AbstractCodeMoverTest { doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/typeParams3.kt"); } + @TestMetadata("valueArgs1.kt") + public void testValueArgs1() throws Exception { + doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs1.kt"); + } + + @TestMetadata("valueArgs2.kt") + public void testValueArgs2() throws Exception { + doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs2.kt"); + } + + @TestMetadata("valueArgs3.kt") + public void testValueArgs3() throws Exception { + doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs3.kt"); + } + + @TestMetadata("valueArgs4.kt") + public void testValueArgs4() throws Exception { + doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs4.kt"); + } + + @TestMetadata("valueArgs5.kt") + public void testValueArgs5() throws Exception { + doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs5.kt"); + } + + @TestMetadata("valueArgs6.kt") + public void testValueArgs6() throws Exception { + doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueArgs6.kt"); + } + @TestMetadata("valueParams1.kt") public void testValueParams1() throws Exception { doTestClassBodyDeclaration("idea/testData/codeInsight/moveUpDown/classBodyDeclarations/functionAnchors/valueParams1.kt");