SwapBinaryExpression: implementation cleaned up, tests renamed

#KT-4868 Fixed

idea/src/org/jetbrains/jet/plugin/intentions/SwapBinaryExpression.kt

@@ -20,60 +20,37 @@ import com.intellij.openapi.editor.Editor
 import org.jetbrains.jet.lang.psi.JetBinaryExpression
 import org.jetbrains.jet.lang.psi.JetPsiFactory
 import org.jetbrains.jet.lang.psi.JetExpression
-import org.jetbrains.jet.plugin.intentions.SwapBinaryExpression.Position.*
 import org.jetbrains.jet.plugin.util.JetPsiPrecedences
+import org.jetbrains.jet.lexer.JetTokens.*
+import org.jetbrains.jet.lang.types.expressions.OperatorConventions
 
 public class SwapBinaryExpression : JetSelfTargetingIntention<JetBinaryExpression>(
         "swap.binary.expression", javaClass()
 ) {
-    enum class Position {
+    class object {
-        FIRST_LEFT
+        val SUPPORTED_OPERATIONS = setOf(PLUS, MUL, OROR, ANDAND, EQEQ, EXCLEQ, EQEQEQ, EXCLEQEQEQ, GT, LT, GTEQ, LTEQ)
-        FIRST_RIGHT
-        NEXT_LEFT
-        NEXT_RIGHT
-    }
 
-    fun getInnermostOperand(element: JetBinaryExpression, position: Position): JetExpression? {
+        val SUPPORTED_OPERATION_NAMES = SUPPORTED_OPERATIONS.map { OperatorConventions.BINARY_OPERATION_NAMES[it]?.asString() }.toSet().filterNotNull() +
-        val left = element.getLeft()
+                                        setOf("xor", "or", "and", "equals", "identityEquals")
-        val right = element.getRight()
-        if (left == null || right == null) {
-            return null
-        }
-
-        val parentPrecedence = JetPsiPrecedences.getPrecedence(element)
-        val leftPrecedence = JetPsiPrecedences.getPrecedence(left)
-        val rightPrecedence = JetPsiPrecedences.getPrecedence(right)
-        return when (position) {
-            FIRST_LEFT -> if (leftPrecedence < parentPrecedence) left
-                else if (left is JetBinaryExpression) getInnermostOperand(left, NEXT_RIGHT)
-                else left as JetExpression
-            FIRST_RIGHT -> if (rightPrecedence < parentPrecedence) right
-                else if (right is JetBinaryExpression) getInnermostOperand(right, NEXT_LEFT)
-                else right as JetExpression
-            NEXT_LEFT -> if (leftPrecedence < parentPrecedence || left !is JetBinaryExpression) left
-                else getInnermostOperand(left, NEXT_LEFT)
-            NEXT_RIGHT -> if (rightPrecedence < parentPrecedence || right !is JetBinaryExpression) right
-                else getInnermostOperand(right, NEXT_RIGHT)
-        }
     }
 
     override fun isApplicableTo(element: JetBinaryExpression): Boolean {
-        if (getInnermostOperand(element, FIRST_LEFT) == null || getInnermostOperand(element, FIRST_RIGHT) == null) {
+        if (leftSubject(element) == null || rightSubject(element) == null) {
             return false
         }
 
-        val operatorTextLiteral = element.getOperationReference().getText()
+        val operationToken = element.getOperationToken()
-        val approvedOperators = setOf("+", "plus", "*", "times", "||", "or", "and",
+        val operationTokenText = element.getOperationReference().getText()
-                                      "&&", "==", "!=", "xor", ">", "<", ">=", "<=", "equals")
+        if (operationToken in SUPPORTED_OPERATIONS
-
+                || operationToken == IDENTIFIER && operationTokenText in SUPPORTED_OPERATION_NAMES) {
-        if (approvedOperators.contains(operatorTextLiteral)) {
+            setText("Flip '$operationTokenText'")
-            setText("Flip '$operatorTextLiteral'")
             return true
         }
         return false
     }
 
     override fun applyTo(element: JetBinaryExpression, editor: Editor) {
+        // Have to use text here to preserve names like "plus"
         val operator = element.getOperationReference().getText()!!
         val convertedOperator = when (operator) {
             ">" -> "<"
@@ -82,12 +59,30 @@ public class SwapBinaryExpression : JetSelfTargetingIntention<JetBinaryExpressio
             ">=" -> "<="
             else -> operator
         }
-        val left = getInnermostOperand(element, FIRST_LEFT)!!
+        val left = leftSubject(element)!!
-        val right = getInnermostOperand(element, FIRST_RIGHT)!!
+        val right = rightSubject(element)!!
         val newRight = JetPsiFactory.createExpression(element.getProject(), left.getText())
         val newLeft = JetPsiFactory.createExpression(element.getProject(), right.getText())
         left.replace(newLeft)
         right.replace(newRight)
         element.replace(JetPsiFactory.createBinaryExpression(element.getProject(), element.getLeft(), convertedOperator, element.getRight()))
     }
+
+    private fun leftSubject(element: JetBinaryExpression): JetExpression? {
+        return firstDescendantOfTighterPrecedence(element.getLeft(), JetPsiPrecedences.getPrecedence(element), JetBinaryExpression::getRight)
+    }
+
+    private fun rightSubject(element: JetBinaryExpression): JetExpression? {
+        return firstDescendantOfTighterPrecedence(element.getRight(), JetPsiPrecedences.getPrecedence(element), JetBinaryExpression::getLeft)
+    }
+
+    private fun firstDescendantOfTighterPrecedence(expression: JetExpression?, precedence: Int, getChild: JetBinaryExpression.() -> JetExpression?): JetExpression? {
+        if (expression is JetBinaryExpression) {
+            val expressionPrecedence = JetPsiPrecedences.getPrecedence(expression)
+            if (!JetPsiPrecedences.isTighter(expressionPrecedence, precedence)) {
+                return firstDescendantOfTighterPrecedence(expression.getChild(), precedence, getChild)
+            }
+        }
+        return expression
+    }
 }

idea/src/org/jetbrains/jet/plugin/util/JetPsiPrecedences.kt

@@ -61,4 +61,8 @@ public object JetPsiPrecedences {
             else -> PRECEDENCE_OF_ATOMIC_EXPRESSION
         }
     }
+
+    public fun isTighter(subject: Int, tighterThan: Int): Boolean {
+        return subject < tighterThan
+    }
 }

idea/testData/intentions/swapBinaryExpression/assignment.kt

@@ -0,0 +1,5 @@
+// IS_APPLICABLE: false
+fun main() {
+    var mutablevar = 20
+    mutablevar = <caret>10
+}

idea/testData/intentions/swapBinaryExpression/equals.kt

@@ -1,5 +1,3 @@
-// IS_APPLICABLE: false
+fun neq(a: Int, b: Int) {
-fun main() {
+    if (a ==<caret> b) {}
-    var mutablevar = 20
-    mutablevar = <caret>10
 }

idea/testData/intentions/swapBinaryExpression/identityEquals.kt

@@ -0,0 +1,3 @@
+fun neq(a: Int, b: Int) {
+    if (a ===<caret> b) {}
+}

idea/testData/intentions/swapBinaryExpression/identityEquals.kt.after

@@ -1,3 +1,3 @@
 fun neq(a: Int, b: Int) {
-    if (a ==<caret> b) {}
+    if (b === a) {}
 }

idea/testData/intentions/swapBinaryExpression/identityEqualsLiteral.kt

@@ -0,0 +1,3 @@
+fun neq(a: Int, b: Int) {
+    if (a identityEquals<caret> b) {}
+}

idea/testData/intentions/swapBinaryExpression/identityEqualsLiteral.kt.after

@@ -0,0 +1,3 @@
+fun neq(a: Int, b: Int) {
+    if (b identityEquals a) {}
+}

idea/testData/intentions/swapBinaryExpression/notIDentityEquals.kt.after

@@ -0,0 +1,3 @@
+fun neq(a: Int, b: Int) {
+    if (<caret>b !== a) {}
+}

idea/testData/intentions/swapBinaryExpression/notIdentityEquals.kt

@@ -1,4 +1,3 @@
-// IS_APPLICABLE: false
 fun neq(a: Int, b: Int) {
     if (<caret>a !== b) {}
 }

idea/testData/intentions/swapBinaryExpression/plusMinus.kt

@@ -0,0 +1,3 @@
+fun test(): Int {
+    return 1 <caret>+ 2 - 3
+}

idea/testData/intentions/swapBinaryExpression/plusMinus.kt.after

@@ -0,0 +1,3 @@
+fun test(): Int {
+    return 2 + 1 - 3
+}

idea/tests/org/jetbrains/jet/plugin/intentions/CodeTransformationTestGenerated.java

@@ -3255,6 +3255,11 @@ public class CodeTransformationTestGenerated extends AbstractCodeTransformationT
             JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/intentions/swapBinaryExpression"), Pattern.compile("^(.+)\\.kt$"), true);
         }
         
+        @TestMetadata("assignment.kt")
+        public void testAssignment() throws Exception {
+            doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/assignment.kt");
+        }
+        
         @TestMetadata("compareTo.kt")
         public void testCompareTo() throws Exception {
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/compareTo.kt");
@@ -3295,11 +3300,6 @@ public class CodeTransformationTestGenerated extends AbstractCodeTransformationT
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/divideLiteral.kt");
         }
         
-        @TestMetadata("doubleEquals.kt")
-        public void testDoubleEquals() throws Exception {
-            doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/doubleEquals.kt");
-        }
-        
         @TestMetadata("equals.kt")
         public void testEquals() throws Exception {
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/equals.kt");
@@ -3320,6 +3320,16 @@ public class CodeTransformationTestGenerated extends AbstractCodeTransformationT
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/greaterThanEquals.kt");
         }
         
+        @TestMetadata("identityEquals.kt")
+        public void testIdentityEquals() throws Exception {
+            doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/identityEquals.kt");
+        }
+        
+        @TestMetadata("identityEqualsLiteral.kt")
+        public void testIdentityEqualsLiteral() throws Exception {
+            doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/identityEqualsLiteral.kt");
+        }
+        
         @TestMetadata("in.kt")
         public void testIn() throws Exception {
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/in.kt");
@@ -3410,16 +3420,16 @@ public class CodeTransformationTestGenerated extends AbstractCodeTransformationT
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/nonBinaryExpr.kt");
         }
         
-        @TestMetadata("notDoubleEquals.kt")
-        public void testNotDoubleEquals() throws Exception {
-            doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/notDoubleEquals.kt");
-        }
-        
         @TestMetadata("notEquals.kt")
         public void testNotEquals() throws Exception {
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/notEquals.kt");
         }
         
+        @TestMetadata("notIdentityEquals.kt")
+        public void testNotIdentityEquals() throws Exception {
+            doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/notIdentityEquals.kt");
+        }
+        
         @TestMetadata("notIn.kt")
         public void testNotIn() throws Exception {
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/notIn.kt");
@@ -3455,6 +3465,11 @@ public class CodeTransformationTestGenerated extends AbstractCodeTransformationT
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/plusLiteral.kt");
         }
         
+        @TestMetadata("plusMinus.kt")
+        public void testPlusMinus() throws Exception {
+            doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/plusMinus.kt");
+        }
+        
         @TestMetadata("rangeTo.kt")
         public void testRangeTo() throws Exception {
             doTestSwapBinaryExpression("idea/testData/intentions/swapBinaryExpression/rangeTo.kt");