From ac8ccf3c9d8ff36cc02d0ec8083db3945f912dcc Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 29 Mar 2011 18:50:20 +0200 Subject: [PATCH] groovy expression surrounders in the same order as in Java (IDEA-48059) --- .../descriptors/GroovySurroundDescriptor.java | 5 +-- .../GroovyManyStatementsSurrounder.java | 42 ++++++++++--------- .../blocks/open/GroovyWithIfSurrounder.java | 11 ----- .../open/GroovyWithWhileSurrounder.java | 11 ----- .../GroovyExpressionSurrounder.java | 2 +- .../GroovyWithWithExprSurrounder.java | 3 +- .../conditions/GroovyConditionSurrounder.java | 9 ++-- .../GroovyWithIfElseExprSurrounder.java | 2 +- .../GroovyWithIfExprSurrounder.java | 2 +- .../GroovyWithWhileExprSurrounder.java | 2 +- .../surroundWith/SurrounderOrderTest.groovy | 39 +++++++++++++---- 11 files changed, 64 insertions(+), 64 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/descriptors/GroovySurroundDescriptor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/descriptors/GroovySurroundDescriptor.java index 2e24431dbe6b..1843b5004a66 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/descriptors/GroovySurroundDescriptor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/descriptors/GroovySurroundDescriptor.java @@ -63,13 +63,12 @@ public class GroovySurroundDescriptor implements SurroundDescriptor { new GroovyWithTypeCastSurrounder(), //groovy-specific - new GroovyWithWithExprSurrounder(), new GroovyWithWithStatementsSurrounder(), new GroovyWithIfExprSurrounder(), new GroovyWithIfElseExprSurrounder(), - - new GroovyWithWhileExprSurrounder() + new GroovyWithWhileExprSurrounder(), + new GroovyWithWithExprSurrounder(), }; @NotNull diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/GroovyManyStatementsSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/GroovyManyStatementsSurrounder.java index 1cea4960723c..6ea405f999f8 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/GroovyManyStatementsSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/GroovyManyStatementsSurrounder.java @@ -30,34 +30,37 @@ import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrBlockStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrCodeBlock; +import org.jetbrains.plugins.groovy.lang.psi.api.util.GrStatementOwner; /** * User: Dmitry.Krasilschikov * Date: 22.05.2007 */ public abstract class GroovyManyStatementsSurrounder implements Surrounder { - public boolean isStatements(@NotNull PsiElement[] elements) { - for (PsiElement element : elements) { - if (!(element instanceof GrStatement)) { - return false; - } - } - return true; - } public boolean isApplicable(@NotNull PsiElement[] elements) { if (elements.length == 0) return false; - if (elements.length == 1) return elements[0] instanceof GrStatement && !(elements[0] instanceof GrBlockStatement); - return isStatements(elements); + + for (PsiElement element : elements) { + if (!isStatement(element)) return false; + } + + if (elements[0] instanceof GrBlockStatement) { + return false; + } + + return true; } - protected String getListElementsTemplateAsString(PsiElement... elements) { - StringBuffer result = new StringBuffer(); - for (PsiElement element : elements) { - result.append(element.getText()); - result.append("\n"); + public static boolean isStatement(PsiElement element) { + if (!(element instanceof GrStatement)) { + return false; } - return result.toString(); + PsiElement parent = element.getParent(); + if (!(parent instanceof GrStatementOwner)) { + return false; + } + return true; } @Nullable @@ -98,10 +101,9 @@ public abstract class GroovyManyStatementsSurrounder implements Surrounder { return getSurroundSelectionRange(newStmt); } - protected void addStatements(GrCodeBlock block, PsiElement[] elements) throws IncorrectOperationException { - for (int i = 0; i < elements.length; i++) { - PsiElement element = elements[i]; - final GrStatement statement = (GrStatement) element; + protected static void addStatements(GrCodeBlock block, PsiElement[] elements) throws IncorrectOperationException { + for (PsiElement element : elements) { + final GrStatement statement = (GrStatement)element; block.addStatementBefore(statement, null); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithIfSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithIfSurrounder.java index 0e2ff2cba9e5..adea05428297 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithIfSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithIfSurrounder.java @@ -59,15 +59,4 @@ public class GroovyWithIfSurrounder extends GroovyManyStatementsSurrounder { return "if"; } - public boolean isApplicable(@NotNull PsiElement[] elements) { - if (!super.isApplicable(elements)) return false; - - if (elements.length == 1 && elements[0] instanceof GrStatement) { - if (elements[0] instanceof GrExpression) { - PsiType type = ((GrExpression) elements[0]).getType(); - return type == null || !((PsiPrimitiveType) PsiType.BOOLEAN).getBoxedTypeName().equals(type.getCanonicalText()); - } - } - return true; - } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithWhileSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithWhileSurrounder.java index 520005fea88b..f8c20f2e1c76 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithWhileSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/blocks/open/GroovyWithWhileSurrounder.java @@ -53,17 +53,6 @@ public class GroovyWithWhileSurrounder extends GroovyManyStatementsSurrounder { return new TextRange(endOffset, endOffset); } - public boolean isApplicable(@NotNull PsiElement[] elements) { - if (!super.isApplicable(elements)) return false; - if (elements.length == 1 && elements[0] instanceof GrStatement) { - if (elements[0] instanceof GrExpression) { - PsiType type = ((GrExpression) elements[0]).getType(); - return type == null || !PsiType.BOOLEAN.getBoxedTypeName().equals(type.getCanonicalText()); - } - } - return true; - } - public String getTemplateDescription() { return "while"; } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyExpressionSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyExpressionSurrounder.java index 5793a3b0b1e4..2cb6b5717526 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyExpressionSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyExpressionSurrounder.java @@ -45,7 +45,7 @@ public abstract class GroovyExpressionSurrounder extends GroovySingleElementSurr protected abstract TextRange surroundExpression(GrExpression expression); - protected void replaceToOldExpression(GrExpression oldExpr, GrExpression replacement) { + protected static void replaceToOldExpression(GrExpression oldExpr, GrExpression replacement) { oldExpr.replaceWithExpression(replacement, false); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithWithExprSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithWithExprSurrounder.java index 97d1a9fdae8f..144408df7791 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithWithExprSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/GroovyWithWithExprSurrounder.java @@ -21,12 +21,13 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression; +import org.jetbrains.plugins.groovy.lang.surroundWith.surrounders.surroundersImpl.expressions.conditions.GroovyConditionSurrounder; /** * User: Dmitry.Krasilschikov * Date: 25.05.2007 */ -public class GroovyWithWithExprSurrounder extends GroovyExpressionSurrounder { +public class GroovyWithWithExprSurrounder extends GroovyConditionSurrounder { protected TextRange surroundExpression(GrExpression expression) { GrMethodCallExpression call = (GrMethodCallExpression) GroovyPsiElementFactory.getInstance(expression.getProject()).createTopElementFromText("with(a){4\n}"); replaceToOldExpression(call.getExpressionArguments()[0], expression); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyConditionSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyConditionSurrounder.java index 803a92423cea..047c560ebd11 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyConditionSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyConditionSurrounder.java @@ -15,6 +15,7 @@ */ package org.jetbrains.plugins.groovy.lang.surroundWith.surrounders.surroundersImpl.expressions.conditions; +import org.jetbrains.plugins.groovy.lang.surroundWith.surrounders.GroovyManyStatementsSurrounder; import org.jetbrains.plugins.groovy.lang.surroundWith.surrounders.surroundersImpl.expressions.GroovyExpressionSurrounder; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import com.intellij.psi.*; @@ -23,13 +24,11 @@ import com.intellij.psi.*; * User: Dmitry.Krasilschikov * Date: 30.07.2007 */ -abstract class GroovyConditionSurrounder extends GroovyExpressionSurrounder { +public abstract class GroovyConditionSurrounder extends GroovyExpressionSurrounder { protected boolean isApplicable(PsiElement element) { - if (! (element instanceof GrExpression)) return false; - - GrExpression expression = (GrExpression) element; - PsiType type = expression.getType(); + if (!GroovyManyStatementsSurrounder.isStatement(element) || !(element instanceof GrExpression)) return false; + PsiType type = ((GrExpression)element).getType(); return PsiType.BOOLEAN.equals(type) || PsiType.BOOLEAN.equals(PsiPrimitiveType.getUnboxedType(type)); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfElseExprSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfElseExprSurrounder.java index 117a56bdfc60..6e4d7bbaeb7f 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfElseExprSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfElseExprSurrounder.java @@ -46,6 +46,6 @@ public class GroovyWithIfElseExprSurrounder extends GroovyConditionSurrounder { } public String getTemplateDescription() { - return "if (...) / else"; + return "if (expr) / else"; } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfExprSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfExprSurrounder.java index ef559f2f644e..9488eb479063 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfExprSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithIfExprSurrounder.java @@ -45,6 +45,6 @@ public class GroovyWithIfExprSurrounder extends GroovyConditionSurrounder { } public String getTemplateDescription() { - return "if (...) {}"; + return "if (expr)"; } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithWhileExprSurrounder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithWhileExprSurrounder.java index a7234f2c50f3..e44cf961e02b 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithWhileExprSurrounder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/surroundWith/surrounders/surroundersImpl/expressions/conditions/GroovyWithWhileExprSurrounder.java @@ -45,6 +45,6 @@ public class GroovyWithWhileExprSurrounder extends GroovyConditionSurrounder { } public String getTemplateDescription() { - return "while (...) {}"; + return "while (expr)"; } } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/surroundWith/SurrounderOrderTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/surroundWith/SurrounderOrderTest.groovy index 35a4ba227a7b..7ae72e0a5154 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/surroundWith/SurrounderOrderTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/surroundWith/SurrounderOrderTest.groovy @@ -25,7 +25,35 @@ import com.intellij.openapi.actionSystem.Separator class SurrounderOrderTest extends LightCodeInsightFixtureTestCase { public void testStatementSurrounders() { - myFixture.configureByText("a.groovy", "println a") + def names = getSurrounders("println a") + assertOrderedEquals names, + "if", "if / else", "while", + "{ -> ... }.call()", + "for", "try / catch", "try / finally", "try / catch / finally", + "shouldFail () {...}", + "(expr)", "((Type) expr)", + "with () {...}" + } + + public void testInnerExpressionSurrounders() { + def names = getSurrounders("boolean a; println a") + assertOrderedEquals names, "(expr)", "((Type) expr)" + } + + public void testOuterExpressionSurrounders() { + def names = getSurrounders("boolean a; a") + assertOrderedEquals names, + "if", "if / else", "while", + "{ -> ... }.call()", + "for", "try / catch", "try / finally", "try / catch / finally", + "shouldFail () {...}", + "(expr)", "((Type) expr)", + "with () {...}", + "if (expr)", "if (expr) / else", "while (expr)", "with (expr)" + } + + private List getSurrounders(final String fileText) { + myFixture.configureByText("a.groovy", fileText) def actions = SurroundWithHandler.buildSurroundActions(project, myFixture.editor, myFixture.file, null) def names = [] @@ -37,13 +65,6 @@ class SurrounderOrderTest extends LightCodeInsightFixtureTestCase { def text = action.templatePresentation.text names << text.substring(text.indexOf('. ') + 2) } - assertOrderedEquals names, - "if", "if / else", "while", - "{ -> ... }.call()", - "for", "try / catch", "try / finally", "try / catch / finally", - "shouldFail () {...}", - "(expr)", "((Type) expr)", "with (expr)", - "with () {...}" + return names } - } \ No newline at end of file