Correct handling of "Redundant if" for negation case #KT-14575 Fixed

idea/src/org/jetbrains/kotlin/idea/inspections/RedundantIfInspection.kt

@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.idea.inspections
 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.psi.*
 
 class RedundantIfInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
@@ -27,50 +28,67 @@ class RedundantIfInspection : AbstractKotlinInspection(), CleanupLocalInspection
         return object : KtVisitorVoid() {
             override fun visitIfExpression(expression: KtIfExpression) {
                 super.visitIfExpression(expression)
+                if (expression.condition == null) return
+                val type = RedundantType.of(expression)
+                if (type == RedundantType.NONE) return
+                holder.registerProblem(expression,
+                                       "Redundant 'if' statement",
+                                       ProblemHighlightType.WEAK_WARNING,
+                                       RemoveRedundantIf(type))
+            }
+        }
+    }
 
-                if (expression.condition != null && isRedundant(expression)) {
+    private enum class RedundantType {
-                    holder.registerProblem(expression,
+        NONE,
-                                           "Redundant 'if' statement",
+        THEN_TRUE,
-                                           ProblemHighlightType.LIKE_UNUSED_SYMBOL,
+        ELSE_TRUE;
-                                           RemoveRedundantIf)
+
+        companion object {
+            internal fun of(expression: KtIfExpression): RedundantType {
+                val thenReturn = getReturnedExpression(expression.then) ?: return NONE
+                val elseReturn = getReturnedExpression(expression.`else`) ?: return NONE
+
+                return if (KtPsiUtil.isTrueConstant(thenReturn) && KtPsiUtil.isFalseConstant(elseReturn)) {
+                    THEN_TRUE
+                }
+                else if (KtPsiUtil.isFalseConstant(thenReturn) && KtPsiUtil.isTrueConstant(elseReturn)) {
+                    ELSE_TRUE
+                }
+                else {
+                    NONE
+                }
+            }
+
+            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 -> {
+                        null
+                    }
                 }
             }
         }
     }
 
-    private fun isRedundant(expression: KtIfExpression): Boolean {
+    private class RemoveRedundantIf(private val redundantType: RedundantType) : LocalQuickFix {
-        val thenReturn = getReturnedExpression(expression.then) ?: return false
-        val elseReturn = getReturnedExpression(expression.`else`) ?: return false
-
-        if (KtPsiUtil.isTrueConstant(thenReturn) && KtPsiUtil.isFalseConstant(elseReturn)) {
-            return true
-        }
-
-        if (KtPsiUtil.isFalseConstant(thenReturn) && KtPsiUtil.isTrueConstant(elseReturn)) {
-            return true
-        }
-
-        return false
-    }
-
-    private fun getReturnedExpression(expression: KtExpression?) : KtExpression? {
-        when(expression) {
-            is KtReturnExpression -> return expression.returnedExpression
-            is KtBlockExpression -> {
-                val statement = expression.statements.singleOrNull() as? KtReturnExpression ?: return null
-                return statement.returnedExpression
-            }
-            else -> return null
-        }
-    }
-
-    private object RemoveRedundantIf : 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
-            element.replace(KtPsiFactory(element).createExpressionByPattern("return $0", element.condition!!.text));
+            val condition = when (redundantType) {
+                RedundantType.NONE -> return
+                RedundantType.THEN_TRUE -> element.condition!!
+                RedundantType.ELSE_TRUE -> element.condition!!.negate()
+            }
+            element.replace(KtPsiFactory(element).createExpressionByPattern("return $0", condition.text))
         }
     }
 

idea/testData/quickfix/redundantIf/negate.kt

@@ -0,0 +1,9 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(value: Int): Boolean {
+    <caret>if (value % 2 == 0) {
+        return false
+    }
+    else {
+        return true
+    }
+}

idea/testData/quickfix/redundantIf/negate.kt.after

@@ -0,0 +1,4 @@
+// "Remove redundant 'if' statement" "true"
+fun bar(value: Int): Boolean {
+    return value % 2 != 0
+}

idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java

@@ -7148,6 +7148,12 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
             KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/redundantIf"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true);
         }
 
+        @TestMetadata("negate.kt")
+        public void testNegate() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/negate.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("simple.kt")
         public void testSimple() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/redundantIf/simple.kt");