Invert if: more correct handling of empty returns #KT-13444 Fixed

(cherry picked from commit be2adaf)

idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt

@@ -113,14 +113,21 @@ class InvertIfConditionIntention : SelfTargetingIntention<KtIfExpression>(KtIfEx
                         val last = afterIfInBlock.last()
                         // build new then branch from statements after if (we will add exit statement if necessary later)
                         //TODO: no block if single?
-                        val newThenRange = PsiChildRange(first, last).trimWhiteSpaces()
+                        val newThenRange = if (isEmptyReturn(lastThenStatement) && isEmptyReturn(lastStatementInBlock)) {
+                            PsiChildRange(first, lastStatementInBlock.prevSibling).trimWhiteSpaces()
+                        }
+                        else {
+                            PsiChildRange(first, last).trimWhiteSpaces()
+                        }
                         val newIf = factory.createExpressionByPattern("if ($0) { $1 }", newCondition, newThenRange) as KtIfExpression
 
                         // remove statements after if as they are moving under if
                         block.deleteChildRange(first, last)
 
-                        if (lastThenStatement is KtReturnExpression && lastThenStatement.returnedExpression == null) {
-                            lastThenStatement.delete()
+                        if (isEmptyReturn(lastThenStatement)) {
+                            if (block.parent is KtDeclarationWithBody && block.parent !is KtFunctionLiteral) {
+                                lastThenStatement.delete()
+                            }
                         }
                         val updatedIf = copyThenBranchAfter(ifExpression)
 
@@ -148,6 +155,9 @@ class InvertIfConditionIntention : SelfTargetingIntention<KtIfExpression>(KtIfEx
         return updatedIf.replace(newIf) as KtIfExpression
     }
 
+    private fun isEmptyReturn(statement: KtExpression) =
+            statement is KtReturnExpression && statement.returnedExpression == null && statement.labeledExpression == null
+
     private fun copyThenBranchAfter(ifExpression: KtIfExpression): KtIfExpression {
         val factory = KtPsiFactory(ifExpression)
         val thenBranch = ifExpression.then ?: return ifExpression

idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt

@@ -0,0 +1,7 @@
+fun println(s: String) {}
+
+fun foo(y: Boolean) {
+    <caret>if (!y) return
+    println("no1")
+    return
+}

idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt.after

@@ -0,0 +1,7 @@
+fun println(s: String) {}
+
+fun foo(y: Boolean) {
+    if (y) {
+        println("no1")
+    }
+}

idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt

@@ -0,0 +1,6 @@
+var foo: Boolean = false
+    set(arg) {
+        <caret>if (field == arg) return
+        field = arg
+        return
+    }

idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt.after

@@ -0,0 +1,6 @@
+var foo: Boolean = false
+    set(arg) {
+        if (field != arg) {
+            field = arg
+        }
+    }

idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt

@@ -0,0 +1,12 @@
+inline fun call(f: () -> Unit) = f()
+
+fun bar() {}
+
+fun foo(arg: Boolean) {
+    call {
+        <caret>if (!arg) return@call
+        bar()
+        return
+    }
+    bar()
+}

idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt.after

@@ -0,0 +1,14 @@
+inline fun call(f: () -> Unit) = f()
+
+fun bar() {}
+
+fun foo(arg: Boolean) {
+    call {
+        if (arg) {
+            bar()
+            return
+        }
+        return@call
+    }
+    bar()
+}

idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt

@@ -0,0 +1,12 @@
+inline fun call(f: () -> Unit) = f()
+
+fun bar() {}
+
+fun foo(arg: Boolean) {
+    call {
+        <caret>if (!arg) return
+        bar()
+        return
+    }
+    bar()
+}

idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt.after

@@ -0,0 +1,13 @@
+inline fun call(f: () -> Unit) = f()
+
+fun bar() {}
+
+fun foo(arg: Boolean) {
+    call {
+        if (arg) {
+            bar()
+        }
+        return
+    }
+    bar()
+}

idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt

@@ -0,0 +1,10 @@
+fun println(s: String) {}
+
+fun foo(x: Boolean, y: Boolean) {
+    if (x) {
+        <caret>if (!y) return
+        println("no1")
+        return
+    }
+    println("no2")
+}

idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt.after

@@ -0,0 +1,11 @@
+fun println(s: String) {}
+
+fun foo(x: Boolean, y: Boolean) {
+    if (x) {
+        if (y) {
+            println("no1")
+        }
+        return
+    }
+    println("no2")
+}

idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java

@@ -6816,6 +6816,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             doTest(fileName);
         }
 
+        @TestMetadata("ifWithBothBranchesReturn.kt")
+        public void testIfWithBothBranchesReturn() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("ifWithBothBranchesSetter.kt")
+        public void testIfWithBothBranchesSetter() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("in.kt")
         public void testIn() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/in.kt");
@@ -6834,6 +6846,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             doTest(fileName);
         }
 
+        @TestMetadata("lambdaNonLocalAndLocalReturn.kt")
+        public void testLambdaNonLocalAndLocalReturn() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("lambdaNonLocalReturn.kt")
+        public void testLambdaNonLocalReturn() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("lastStatement1.kt")
         public void testLastStatement1() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/lastStatement1.kt");
@@ -6900,6 +6924,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             doTest(fileName);
         }
 
+        @TestMetadata("nestedIfWithReturn.kt")
+        public void testNestedIfWithReturn() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("notIn.kt")
         public void testNotIn() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/invertIfCondition/notIn.kt");