Inline adds parentheses when it is necessary.

Test data for Remove Unnecessary Parentheses was updated, since parentheses in foo && (bar && baz) is not "unnecessary": removing it changes program semantics. #KT-2637 in progress
2013-07-08 19:19:41 +04:00
parent a394afcce1
commit 55b221776b
57 changed files with 413 additions and 21 deletions
@@ -17,6 +17,8 @@
 package org.jetbrains.jet.lang.psi;

 import com.intellij.lang.ASTNode;
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.impl.source.tree.TreeElement;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.jet.JetNodeType;

@@ -41,4 +43,13 @@ abstract class JetExpressionImpl extends JetElementImpl implements JetExpression

        return containerNode.findChildByClass(JetExpression.class);
    }
+
+    @Override
+    public void replaceChildInternal(PsiElement child, TreeElement newElement) {
+        PsiElement newPsi = newElement.getPsi();
+        if (newPsi instanceof JetExpression && JetPsiUtil.areParenthesesNecessary((JetExpression) newPsi, ((JetExpression) child), this)) {
+            newElement = (TreeElement) JetPsiFactory.createExpression(getProject(), "(" + newElement.getText() + ")").getNode();
+        }
+        super.replaceChildInternal(child, newElement);
+    }
 }
@@ -20,10 +20,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Lists;
 import com.intellij.lang.ASTNode;
-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.impl.CheckUtil;
 import com.intellij.psi.impl.source.codeStyle.CodeEditUtil;
 import com.intellij.psi.tree.IElementType;
@@ -604,52 +601,89 @@ public class JetPsiUtil {
        return null;
    }

-    private static int getPrecedenceOfOperation(@NotNull JetExpression expression, @NotNull IElementType operation) {
+    private static int getPrecedenceOfOperation(@NotNull JetExpression expression, @Nullable IElementType operation) {
        if (expression instanceof JetPostfixExpression) return 0;
        if (expression instanceof JetQualifiedExpression) return 0;
+        if (expression instanceof JetCallExpression) return 0;
+        if (expression instanceof JetArrayAccessExpression) return 0;
+
        if (expression instanceof JetPrefixExpression) return 1;

+        if (expression instanceof JetDeclaration) return 100;
+        if (expression instanceof JetStatementExpression) return 100;
+        if (expression instanceof JetIfExpression) return 100;
+
        for (JetExpressionParsing.Precedence precedence : JetExpressionParsing.Precedence.values()) {
            if (precedence != JetExpressionParsing.Precedence.PREFIX && precedence != JetExpressionParsing.Precedence.POSTFIX &&
                precedence.getOperations().contains(operation)) {
                return precedence.ordinal();
            }
        }
-        throw new IllegalStateException("Unknown operation");
+
+        return -1;
    }

    public static boolean areParenthesesUseless(@NotNull JetParenthesizedExpression expression) {
        JetExpression innerExpression = expression.getExpression();
-        JetExpression parentExpression = PsiTreeUtil.getParentOfType(expression, JetExpression.class, true);
-        if (innerExpression == null || parentExpression == null) return true;
+        if (innerExpression == null) return true;
+
+        PsiElement parent = expression.getParent();
+        if (!(parent instanceof JetExpression)) return true;
+
+        return !areParenthesesNecessary(innerExpression, expression, (JetExpression) parent);
+    }
+
+    public static boolean areParenthesesNecessary(@NotNull JetExpression innerExpression, @NotNull JetExpression currentInner, @NotNull JetExpression parentExpression) {
+        if (parentExpression instanceof JetParenthesizedExpression || innerExpression instanceof JetParenthesizedExpression) {
+            return false;
+        }
+
+        if (parentExpression instanceof JetWhenExpression || innerExpression instanceof JetWhenExpression) {
+            return false;
+        }
+
+        if (innerExpression instanceof JetIfExpression) {
+            PsiElement current = parentExpression;
+
+            while (!(current instanceof JetBlockExpression || current instanceof JetDeclaration || current instanceof JetStatementExpression)) {
+                if (current.getTextRange().getEndOffset() != currentInner.getTextRange().getEndOffset()) {
+                    return current.getText().charAt(current.getTextLength() - 1) != ')'; // if current expression is "guarded" by parenthesis, no extra parenthesis is necessary
+                }
+
+                current = current.getParent();
+            }
+        }

        IElementType innerOperation = getOperation(innerExpression);
        IElementType parentOperation = getOperation(parentExpression);

        // 'return (@label{...})' case
        if (parentExpression instanceof JetReturnExpression && innerOperation == JetTokens.LABEL_IDENTIFIER) {
-            return false;
+            return true;
        }

        // '(x: Int) < y' case
        if (innerExpression instanceof JetBinaryExpressionWithTypeRHS && parentOperation == JetTokens.LT) {
-            return false;
-        }
-
-        // associative operations
-        if (innerOperation == parentOperation && (innerOperation == JetTokens.OROR || innerOperation == JetTokens.ANDAND)) {
            return true;
        }

-        if (innerOperation == null) return true;
-        if (parentExpression instanceof JetArrayAccessExpression) {
-            return ((JetArrayAccessExpression) parentExpression).getArrayExpression() != expression;
-        }
-        if (parentOperation == null) return true;
-
        int innerPrecedence = getPrecedenceOfOperation(innerExpression, innerOperation);
        int parentPrecedence = getPrecedenceOfOperation(parentExpression, parentOperation);
-        return innerPrecedence < parentPrecedence;
+
+        if (innerPrecedence == parentPrecedence) {
+            if (parentExpression instanceof JetBinaryExpression) {
+                if (innerOperation == JetTokens.ANDAND || innerOperation == JetTokens.OROR) {
+                    return false;
+                }
+                return ((JetBinaryExpression) parentExpression).getRight() == currentInner;
+            }
+            if (parentExpression instanceof JetPrefixExpression || innerExpression instanceof JetPrefixExpression) {
+                return innerOperation == parentOperation && (innerOperation == JetTokens.PLUS || innerOperation == JetTokens.MINUS);
+            }
+            return false;
+        }
+
+        return innerPrecedence >= parentPrecedence;
    }

    public static boolean isAssignment(@NotNull PsiElement element) {
@@ -0,0 +1,4 @@
+fun f() {
+    val v = s1 + s2
+    println(<caret>v[1])
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((s1 + s2)[1])
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = foo()
+    println(<caret>v[1])
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(foo()[1])
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = 150 + 150
+    println(<caret>v * 1)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((150 + 150) * 1)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = 150 * 2
+    println(<caret>v + 0)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(150 * 2 + 0)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = -predicate
+    println(<caret>v())
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((-predicate)())
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = foo[1]
+    println(<caret>v())
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(foo[1]())
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = 2..3
+    println(<caret>v: IntRange)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((2..3): IntRange)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = -1
+    println(<caret>v: Int)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(-1: Int)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = 2 + 3
+    println(if (true) 1 else <caret>v)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(if (true) 1 else 2 + 3)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = if (true) a else b
+    println(<caret>v[0])
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((if (true) a else b)[0])
+}
@@ -0,0 +1,7 @@
+import java.util.ArrayList
+
+fun f() {
+    val a = ArrayList<Int>()
+    val v = if (true) 1 else 2
+    println(a[<caret>v])
+}
@@ -0,0 +1,6 @@
+import java.util.ArrayList
+
+fun f() {
+    val a = ArrayList<Int>()
+    println(a[if (true) 1 else 2])
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = if (true) 1 else 2
+    println(<caret>v * 1)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((if (true) 1 else 2) * 1)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = if (true) 1 else 2
+    println(1 * <caret>v)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(1 * if (true) 1 else 2)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = if (true) 1 else 2
+    println(1 * <caret>v + 2)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(1 * (if (true) 1 else 2) + 2)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = if (true) 1 else 2
+    println((1 * <caret>v) + 2)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((1 * if (true) 1 else 2) + 2)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = if (true) a else b
+    println(<caret>v++)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((if (true) a else b)++)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = if (true) a else b
+    println(+<caret>v)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(+if (true) a else b)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = foo()
+    println(<caret>v is String)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(foo() is String)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = "" is String
+    println(<caret>v.toString())
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(("" is String).toString())
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = a - b
+    println(c - <caret>v)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(c - (a - b))
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = a && b
+    println(c && <caret>v)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(c && a && b)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = 150 - 150
+    println(<caret>v + 1)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(150 - 150 + 1)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = y++
+    println(-<caret>v)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(-y++)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = -y
+    println(<caret>v!!)
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((-y)!!)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = 150 + 150
+    println(<caret>v.toString())
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println((150 + 150).toString())
+}
@@ -0,0 +1,4 @@
+fun f() {
+    val v = foo()
+    println(<caret>v.toString())
+}
@@ -0,0 +1,3 @@
+fun f() {
+    println(foo().toString())
+}
@@ -0,0 +1,5 @@
+fun f() {
+    // this case is not processed specifically in implementation, but it works
+    val x = -y
+    println(5-<caret>x)
+}
@@ -0,0 +1,4 @@
+fun f() {
+    // this case is not processed specifically in implementation, but it works
+    println(5- -y)
+}
@@ -31,6 +31,7 @@ import org.jetbrains.jet.plugin.refactoring.inline.AbstractInlineTest;
 /** This class is generated by {@link org.jetbrains.jet.generators.tests.GenerateTests}. DO NOT MODIFY MANUALLY */
@SuppressWarnings("all")
@TestMetadata("idea/testData/refactoring/inline")
+@InnerTestClasses({InlineTestGenerated.AddParenthesis.class})
 public class InlineTestGenerated extends AbstractInlineTest {
    public void testAllFilesPresentInInline() throws Exception {
        JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("idea/testData/refactoring/inline"), Pattern.compile("^(.+)\\.kt$"), true);
@@ -76,4 +77,153 @@ public class InlineTestGenerated extends AbstractInlineTest {
        doTest("idea/testData/refactoring/inline/Var.kt");
    }
    
+    @TestMetadata("idea/testData/refactoring/inline/addParenthesis")
+    public static class AddParenthesis extends AbstractInlineTest {
+        public void testAllFilesPresentInAddParenthesis() throws Exception {
+            JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("idea/testData/refactoring/inline/addParenthesis"), Pattern.compile("^(.+)\\.kt$"), true);
+        }
+        
+        @TestMetadata("ArrayAccess.kt")
+        public void testArrayAccess() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/ArrayAccess.kt");
+        }
+        
+        @TestMetadata("ArrayAccessDontAdd.kt")
+        public void testArrayAccessDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/ArrayAccessDontAdd.kt");
+        }
+        
+        @TestMetadata("Binary.kt")
+        public void testBinary() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/Binary.kt");
+        }
+        
+        @TestMetadata("BinaryDontAdd.kt")
+        public void testBinaryDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/BinaryDontAdd.kt");
+        }
+        
+        @TestMetadata("Call.kt")
+        public void testCall() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/Call.kt");
+        }
+        
+        @TestMetadata("CallDontAdd.kt")
+        public void testCallDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/CallDontAdd.kt");
+        }
+        
+        @TestMetadata("Colon.kt")
+        public void testColon() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/Colon.kt");
+        }
+        
+        @TestMetadata("ColonDontAdd.kt")
+        public void testColonDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/ColonDontAdd.kt");
+        }
+        
+        @TestMetadata("If.kt")
+        public void testIf() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/If.kt");
+        }
+        
+        @TestMetadata("IfIntoArrayAccess.kt")
+        public void testIfIntoArrayAccess() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoArrayAccess.kt");
+        }
+        
+        @TestMetadata("IfIntoArrayAccessBrackets.kt")
+        public void testIfIntoArrayAccessBrackets() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoArrayAccessBrackets.kt");
+        }
+        
+        @TestMetadata("IfIntoBinaryLeft.kt")
+        public void testIfIntoBinaryLeft() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoBinaryLeft.kt");
+        }
+        
+        @TestMetadata("IfIntoBinaryRight.kt")
+        public void testIfIntoBinaryRight() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoBinaryRight.kt");
+        }
+        
+        @TestMetadata("IfIntoBinaryRightLeft.kt")
+        public void testIfIntoBinaryRightLeft() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoBinaryRightLeft.kt");
+        }
+        
+        @TestMetadata("IfIntoBinaryRightLeftDontAdd.kt")
+        public void testIfIntoBinaryRightLeftDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoBinaryRightLeftDontAdd.kt");
+        }
+        
+        @TestMetadata("IfIntoPostfix.kt")
+        public void testIfIntoPostfix() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoPostfix.kt");
+        }
+        
+        @TestMetadata("IfIntoPrefix.kt")
+        public void testIfIntoPrefix() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IfIntoPrefix.kt");
+        }
+        
+        @TestMetadata("IsDontAdd.kt")
+        public void testIsDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IsDontAdd.kt");
+        }
+        
+        @TestMetadata("IsIntoCall.kt")
+        public void testIsIntoCall() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/IsIntoCall.kt");
+        }
+        
+        @TestMetadata("LeftAssociative.kt")
+        public void testLeftAssociative() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/LeftAssociative.kt");
+        }
+        
+        @TestMetadata("LeftAssociativeBoolean.kt")
+        public void testLeftAssociativeBoolean() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/LeftAssociativeBoolean.kt");
+        }
+        
+        @TestMetadata("LeftAssociativeDontAdd.kt")
+        public void testLeftAssociativeDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/LeftAssociativeDontAdd.kt");
+        }
+        
+        @TestMetadata("PostfixIntoPrefix.kt")
+        public void testPostfixIntoPrefix() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/PostfixIntoPrefix.kt");
+        }
+        
+        @TestMetadata("PrefixIntoPostfix.kt")
+        public void testPrefixIntoPostfix() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/PrefixIntoPostfix.kt");
+        }
+        
+        @TestMetadata("Qualified.kt")
+        public void testQualified() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/Qualified.kt");
+        }
+        
+        @TestMetadata("QualifiedDontAdd.kt")
+        public void testQualifiedDontAdd() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/QualifiedDontAdd.kt");
+        }
+                
+        @TestMetadata("UnaryIntoBinary.kt")
+        public void testUnaryIntoBinary() throws Exception {
+            doTest("idea/testData/refactoring/inline/addParenthesis/UnaryIntoBinary.kt");
+        }
+        
+    }
+    
+    public static Test suite() {
+        TestSuite suite = new TestSuite("InlineTestGenerated");
+        suite.addTestSuite(InlineTestGenerated.class);
+        suite.addTestSuite(AddParenthesis.class);
+        return suite;
+    }
 }