Do not apply ExpressionSurrounders for expressions with Unit type and for call expressions inside qualified expression

idea/src/org/jetbrains/jet/plugin/codeInsight/surroundWith/expression/KotlinExpressionSurrounder.java

@@ -5,10 +5,15 @@ import com.intellij.openapi.editor.Editor;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.TextRange;
 import com.intellij.psi.PsiElement;
-import com.intellij.util.IncorrectOperationException;
+import com.intellij.psi.util.PsiTreeUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.jet.lang.psi.JetCallExpression;
 import org.jetbrains.jet.lang.psi.JetExpression;
+import org.jetbrains.jet.lang.psi.JetQualifiedExpression;
+import org.jetbrains.jet.lang.types.JetType;
+import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns;
+import org.jetbrains.jet.plugin.codeInsight.surroundWith.KotlinSurrounderUtils;
 
 public abstract class KotlinExpressionSurrounder implements Surrounder {
 
@@ -17,7 +22,16 @@ public abstract class KotlinExpressionSurrounder implements Surrounder {
         if (elements.length != 1 || !(elements[0] instanceof JetExpression)) {
             return false;
         }
-        return isApplicable((JetExpression) elements[0]);
+
+        JetExpression expression = (JetExpression) elements[0];
+        if (expression instanceof JetCallExpression && expression.getParent() instanceof JetQualifiedExpression) {
+            return false;
+        }
+        JetType type = KotlinSurrounderUtils.getExpressionType(expression);
+        if (type == null || type.equals(KotlinBuiltIns.getInstance().getUnitType())) {
+            return false;
+        }
+        return isApplicable(expression);
     }
 
     protected abstract boolean isApplicable(@NotNull JetExpression expression);

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/if.kt

@@ -0,0 +1,5 @@
+fun foo() {
+    <selection>if (1 == 2) {}</selection>
+}
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/import.kt

@@ -0,0 +1,3 @@
+import <selection>kotlin</selection>
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/importQualifiedFirst.kt

@@ -0,0 +1,3 @@
+import <selection>kotlin</selection>.io
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/importQualifiedSecond.kt

@@ -0,0 +1,3 @@
+import kotlin.<selection>io</selection>
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/packageName.kt

@@ -0,0 +1,3 @@
+package <selection>foo</selection>
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/parameterName.kt

@@ -0,0 +1,3 @@
+fun foo(<selection>a</selection>: Int) {}
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/parameterWithType.kt

@@ -0,0 +1,3 @@
+fun foo(<selection>a: Int</selection>) {}
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/qualifiedExpressionSecond.kt

@@ -0,0 +1,6 @@
+fun foo() {
+    val a = ""
+    a.<selection>charAt</selection>(1)
+}
+
+// IS_APPLICABLE: false

idea/testData/codeInsight/surroundWith/parentheses/notApplicable/qualifiedExpressionSecondWithBracket.kt

@@ -0,0 +1,6 @@
+fun foo() {
+    val a = ""
+    a.<selection>charAt(1)</selection>
+}
+
+// IS_APPLICABLE: false

idea/tests/org/jetbrains/jet/plugin/codeInsight/surroundWith/AbstractSurroundWithTest.java

@@ -17,15 +17,22 @@
 package org.jetbrains.jet.plugin.codeInsight.surroundWith;
 
 import com.intellij.codeInsight.generation.surroundWith.SurroundWithHandler;
+import com.intellij.lang.LanguageSurrounders;
+import com.intellij.lang.surroundWith.SurroundDescriptor;
 import com.intellij.lang.surroundWith.Surrounder;
+import com.intellij.openapi.editor.SelectionModel;
+import com.intellij.openapi.util.io.FileUtil;
+import com.intellij.psi.PsiElement;
 import com.intellij.testFramework.LightCodeInsightTestCase;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.jet.plugin.codeInsight.surroundWith.expression.KotlinNotSurrounder;
+import org.jetbrains.annotations.Nullable;
-import org.jetbrains.jet.plugin.codeInsight.surroundWith.expression.KotlinParenthesesSurrounder;
+import org.jetbrains.jet.InTextDirectivesUtils;
-import org.jetbrains.jet.plugin.codeInsight.surroundWith.expression.KotlinStringTemplateSurrounder;
+import org.jetbrains.jet.plugin.codeInsight.surroundWith.expression.*;
-import org.jetbrains.jet.plugin.codeInsight.surroundWith.expression.KotlinWhenSurrounder;
 import org.jetbrains.jet.plugin.codeInsight.surroundWith.statement.*;
 
+import java.io.File;
+import java.util.List;
+
 public abstract class AbstractSurroundWithTest extends LightCodeInsightTestCase {
 
     public void doTestWithIfSurrounder(String path) throws Exception {
@@ -47,7 +54,7 @@ public abstract class AbstractSurroundWithTest extends LightCodeInsightTestCase
     public void doTestWithStringTemplateSurrounder(String path) throws Exception {
         doTest(path, new KotlinStringTemplateSurrounder());
     }
-    
+
     public void doTestWithWhenSurrounder(String path) throws Exception {
         doTest(path, new KotlinWhenSurrounder());
     }
@@ -70,10 +77,51 @@ public abstract class AbstractSurroundWithTest extends LightCodeInsightTestCase
 
     private void doTest(String path, Surrounder surrounder) throws Exception {
         configureByFile(path);
+
+        String fileText = FileUtil.loadFile(new File(path));
+        String isApplicableString = InTextDirectivesUtils.findStringWithPrefix("// IS_APPLICABLE: ", fileText);
+
+        if (isApplicableString != null) {
+            boolean isApplicableExpected = toString().equals("true");
+            PsiElement[] elementsToSurround = getElementsToSurround(surrounder);
+            assert elementsToSurround != null : "Couldn't find elements to surround";
+            assert isApplicableExpected == surrounder.isApplicable(elementsToSurround)
+                    : "isApplicable() for " + surrounder.getClass() + " should return " + isApplicableExpected;
+            if (isApplicableExpected) {
+                invokeSurroundAndCheck(path, surrounder);
+            }
+        }
+        else {
+            invokeSurroundAndCheck(path, surrounder);
+        }
+    }
+
+    private void invokeSurroundAndCheck(@NotNull String path, @NotNull Surrounder surrounder) {
         SurroundWithHandler.invoke(getProject(), getEditor(), getFile(), surrounder);
         checkResultByFile(path + ".after");
     }
 
+    @Nullable
+    private PsiElement[] getElementsToSurround(@NotNull Surrounder surrounder) {
+        List<SurroundDescriptor> surroundDescriptors =
+                LanguageSurrounders.INSTANCE.allForLanguage(getFile().getViewProvider().getBaseLanguage());
+
+        String surrounderDescription = surrounder.getTemplateDescription();
+        for (SurroundDescriptor descriptor : surroundDescriptors) {
+            Surrounder[] surrounders = descriptor.getSurrounders();
+            for (Surrounder surrounderInDescriptor : surrounders) {
+                if (surrounderInDescriptor.getTemplateDescription().equals(surrounderDescription)) {
+                    SelectionModel selection = getEditor().getSelectionModel();
+                    PsiElement[] elements = descriptor.getElementsToSurround(
+                            getFile(), selection.getSelectionStart(), selection.getSelectionEnd());
+                    return elements;
+                }
+            }
+        }
+
+        return null;
+    }
+
     @NotNull
     @Override
     protected String getTestDataPath() {

idea/tests/org/jetbrains/jet/plugin/codeInsight/surroundWith/SurroundWithTestGenerated.java

@@ -142,6 +142,7 @@ public class SurroundWithTestGenerated extends AbstractSurroundWithTest {
     }
     
     @TestMetadata("idea/testData/codeInsight/surroundWith/parentheses")
+    @InnerTestClasses({Parentheses.NotApplicable.class})
     public static class Parentheses extends AbstractSurroundWithTest {
         public void testAllFilesPresentInParentheses() throws Exception {
             JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("idea/testData/codeInsight/surroundWith/parentheses"), Pattern.compile("^(.+)\\.kt$"), true);
@@ -162,6 +163,65 @@ public class SurroundWithTestGenerated extends AbstractSurroundWithTest {
             doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/partOfExpr.kt");
         }
         
+        @TestMetadata("idea/testData/codeInsight/surroundWith/parentheses/notApplicable")
+        public static class NotApplicable extends AbstractSurroundWithTest {
+            public void testAllFilesPresentInNotApplicable() throws Exception {
+                JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("idea/testData/codeInsight/surroundWith/parentheses/notApplicable"), Pattern.compile("^(.+)\\.kt$"), true);
+            }
+            
+            @TestMetadata("if.kt")
+            public void testIf() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/if.kt");
+            }
+            
+            @TestMetadata("import.kt")
+            public void testImport() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/import.kt");
+            }
+            
+            @TestMetadata("importQualifiedFirst.kt")
+            public void testImportQualifiedFirst() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/importQualifiedFirst.kt");
+            }
+            
+            @TestMetadata("importQualifiedSecond.kt")
+            public void testImportQualifiedSecond() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/importQualifiedSecond.kt");
+            }
+            
+            @TestMetadata("packageName.kt")
+            public void testPackageName() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/packageName.kt");
+            }
+            
+            @TestMetadata("parameterName.kt")
+            public void testParameterName() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/parameterName.kt");
+            }
+            
+            @TestMetadata("parameterWithType.kt")
+            public void testParameterWithType() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/parameterWithType.kt");
+            }
+            
+            @TestMetadata("qualifiedExpressionSecond.kt")
+            public void testQualifiedExpressionSecond() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/qualifiedExpressionSecond.kt");
+            }
+            
+            @TestMetadata("qualifiedExpressionSecondWithBracket.kt")
+            public void testQualifiedExpressionSecondWithBracket() throws Exception {
+                doTestWithParenthesesSurrounder("idea/testData/codeInsight/surroundWith/parentheses/notApplicable/qualifiedExpressionSecondWithBracket.kt");
+            }
+            
+        }
+        
+        public static Test innerSuite() {
+            TestSuite suite = new TestSuite("Parentheses");
+            suite.addTestSuite(Parentheses.class);
+            suite.addTestSuite(NotApplicable.class);
+            return suite;
+        }
     }
     
     @TestMetadata("idea/testData/codeInsight/surroundWith/stringTemplate")
@@ -292,7 +352,7 @@ public class SurroundWithTestGenerated extends AbstractSurroundWithTest {
         suite.addTestSuite(If.class);
         suite.addTestSuite(IfElse.class);
         suite.addTestSuite(Not.class);
-        suite.addTestSuite(Parentheses.class);
+        suite.addTest(Parentheses.innerSuite());
         suite.addTestSuite(StringTemplate.class);
         suite.addTestSuite(When.class);
         suite.addTestSuite(TryCatch.class);