groovy expression surrounders in the same order as in Java (IDEA-48059)

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

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);
     }
   }

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;
-  }
 }

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";
   }

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);
   }
 }

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);

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));
   }
 }

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";
   }
 }

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)";
   }
 }

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)";
   }
 }

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", "<selection>println a</selection>")
+    def names = getSurrounders("<selection>println a</selection>")
+    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 <selection>a</selection>")
+    assertOrderedEquals names, "(expr)", "((Type) expr)"
+  }
+
+  public void testOuterExpressionSurrounders() {
+    def names = getSurrounders("boolean a; <selection>a</selection>")
+    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<String> 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
   }
-
 }