Redundant if inspection supports now assignments and just if (...) true else false #KT-13259 Fixed

2016-11-21 16:12:59 +03:00
parent 14787e3583
commit c6997a1213
12 changed files with 126 additions and 33 deletions
@@ -1,6 +1,6 @@
 <html>
 <body>
-Reports <b>if</b> statements which can be simplified to single <b>return</b> statements.
+Reports <b>if</b> statements which can be simplified to single statements.
 <p>
  For example:
  <code><pre>
@@ -20,6 +20,7 @@ import com.intellij.codeInspection.*
 import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiElementVisitor
 import org.jetbrains.kotlin.idea.intentions.negate
+import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*

 class RedundantIfInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
@@ -29,66 +30,81 @@ class RedundantIfInspection : AbstractKotlinInspection(), CleanupLocalInspection
            override fun visitIfExpression(expression: KtIfExpression) {
                super.visitIfExpression(expression)
                if (expression.condition == null) return
-                val type = RedundantType.of(expression)
-                if (type == RedundantType.NONE) return
+                val (redundancyType, branchType) = RedundancyType.of(expression)
+                if (redundancyType == RedundancyType.NONE) return
+
                holder.registerProblem(expression,
                                       "Redundant 'if' statement",
                                       ProblemHighlightType.WEAK_WARNING,
-                                       RemoveRedundantIf(type))
+                                       RemoveRedundantIf(redundancyType, branchType))
            }
        }
    }

-    private enum class RedundantType {
+    private sealed class BranchType {
+        object Simple : BranchType()
+
+        object Return : BranchType()
+
+        class Assign(val lvalue: KtExpression) : BranchType() {
+            override fun equals(other: Any?) = other is Assign && lvalue.text == other.lvalue.text
+
+            override fun hashCode() = lvalue.text.hashCode()
+        }
+    }
+
+    private enum class RedundancyType {
        NONE,
        THEN_TRUE,
        ELSE_TRUE;

        companion object {
-            internal fun of(expression: KtIfExpression): RedundantType {
-                val thenReturn = getReturnedExpression(expression.then) ?: return NONE
-                val elseReturn = getReturnedExpression(expression.`else`) ?: return NONE
+            internal fun of(expression: KtIfExpression): Pair<RedundancyType, BranchType> {
+                val (thenReturn, thenType) = expression.then.getBranchExpression() ?: return NONE to BranchType.Simple
+                val (elseReturn, elseType) = expression.`else`.getBranchExpression() ?: return NONE to BranchType.Simple

-                return if (KtPsiUtil.isTrueConstant(thenReturn) && KtPsiUtil.isFalseConstant(elseReturn)) {
-                    THEN_TRUE
-                }
-                else if (KtPsiUtil.isFalseConstant(thenReturn) && KtPsiUtil.isTrueConstant(elseReturn)) {
-                    ELSE_TRUE
-                }
-                else {
-                    NONE
+                return when {
+                    thenType != elseType -> NONE to BranchType.Simple
+                    KtPsiUtil.isTrueConstant(thenReturn) && KtPsiUtil.isFalseConstant(elseReturn) -> THEN_TRUE to thenType
+                    KtPsiUtil.isFalseConstant(thenReturn) && KtPsiUtil.isTrueConstant(elseReturn) -> ELSE_TRUE to thenType
+                    else -> NONE to BranchType.Simple
                }
            }

-            private fun getReturnedExpression(expression: KtExpression?) : KtExpression? {
-                return when (expression) {
-                    is KtReturnExpression -> {
-                        expression.returnedExpression
-                    }
-                    is KtBlockExpression -> {
-                        val statement = expression.statements.singleOrNull() as? KtReturnExpression ?: return null
-                        statement.returnedExpression
-                    }
-                    else -> {
+            private fun KtExpression?.getBranchExpression(): Pair<KtExpression?, BranchType>? {
+                return when (this) {
+                    is KtReturnExpression -> returnedExpression to BranchType.Return
+                    is KtBlockExpression -> statements.singleOrNull()?.getBranchExpression()
+                    is KtBinaryExpression -> if (operationToken == KtTokens.EQ && left != null)
+                        right to BranchType.Assign(left!!)
+                    else
                        null
-                    }
+                    is KtExpression -> this to BranchType.Simple
+                    else -> null
                }
            }
        }
    }

-    private class RemoveRedundantIf(private val redundantType: RedundantType) : LocalQuickFix {
+    private class RemoveRedundantIf(private val redundancyType: RedundancyType, private val branchType: BranchType) : LocalQuickFix {
        override fun getName() = "Remove redundant 'if' statement"
        override fun getFamilyName() = name

        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
            val element = descriptor.psiElement as KtIfExpression
-            val condition = when (redundantType) {
-                RedundantType.NONE -> return
-                RedundantType.THEN_TRUE -> element.condition!!
-                RedundantType.ELSE_TRUE -> element.condition!!.negate()
+            val condition = when (redundancyType) {
+                RedundancyType.NONE -> return
+                RedundancyType.THEN_TRUE -> element.condition!!
+                RedundancyType.ELSE_TRUE -> element.condition!!.negate()
            }
-            element.replace(KtPsiFactory(element).createExpressionByPattern("return $0", condition.text))
+            val factory = KtPsiFactory(element)
+            element.replace(
+                    when (branchType) {
+                        is BranchType.Return -> factory.createExpressionByPattern("return $0", condition)
+                        is BranchType.Assign -> factory.createExpressionByPattern("$0 = $1", branchType.lvalue, condition)
+                        else -> condition
+                    }
+            )
        }
    }

@@ -0,0 +1,4 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(p: Int) {
+    val v1 = <caret>if (p > 0) true else false
+}
@@ -0,0 +1,4 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(p: Int) {
+    val v1 = p > 0
+}
@@ -0,0 +1,10 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(p: Int) {
+    var v2 = false
+    <caret>if (p > 0) {
+        v2 = false
+    }
+    else {
+        v2 = true
+    }
+}
@@ -0,0 +1,5 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(p: Int) {
+    var v2 = false
+    v2 = p <= 0
+}
@@ -0,0 +1,10 @@
+// "Remove redundant 'if' statement" "false"
+// ACTION: Add braces to 'if' statement
+// ACTION: Invert 'if' condition
+// ACTION: Replace 'if' with 'when'
+
+fun bar(p: Int) {
+    var v1 = false
+    var v2 = false
+    <caret>if (p > 0) v2 = true else v1 = false
+}
@@ -0,0 +1,5 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(value: Int): Boolean {
+    val x = <caret>if (value % 2 == 0) false else true
+    return x
+}
@@ -0,0 +1,5 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(value: Int): Boolean {
+    val x = value % 2 != 0
+    return x
+}
@@ -0,0 +1,2 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(value: Int) = <caret>if (value % 2 == 0) true else false
@@ -0,0 +1,2 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(value: Int) = value % 2 == 0
@@ -7173,6 +7173,36 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/redundantIf"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
        }

+        @TestMetadata("assignExpression.kt")
+        public void testAssignExpression() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/assignExpression.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("assignment.kt")
+        public void testAssignment() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/assignment.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("assignmentFake.kt")
+        public void testAssignmentFake() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/assignmentFake.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("expression.kt")
+        public void testExpression() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/expression.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("expressionBody.kt")
+        public void testExpressionBody() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/expressionBody.kt");
+            doTest(fileName);
+        }
+
        @TestMetadata("negate.kt")
        public void testNegate() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/negate.kt");