ReplaceGuardClause inspection: do not remove 'else' branch

#KT-32797 Fixed
2019-07-22 23:01:28 +09:00
parent a3a3c52c24
commit a6690e4e35
11 changed files with 96 additions and 2 deletions
@@ -19,6 +19,7 @@ import org.jetbrains.kotlin.idea.util.textRangeIn
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.startOffset
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
 import org.jetbrains.kotlin.resolve.calls.callUtil.getType
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
@@ -82,7 +83,7 @@ class ReplaceGuardClauseWithFunctionCallInspection : AbstractApplicabilityBasedI
                } else {
                    psiFactory.createExpressionByPattern("${kotlinFunction.fqName}($excl$0) { $1 }", newCondition, argument)
                }
-                val replaced = element.replaced(newExpression)
+                val replaced = element.replaceWith(newExpression, psiFactory)
                val newCall = (replaced as? KtDotQualifiedExpression)?.callExpression
                val negatedExpression = newCall?.valueArguments?.firstOrNull()?.getArgumentExpression() as? KtPrefixExpression
                if (negatedExpression != null) {
@@ -97,13 +98,27 @@ class ReplaceGuardClauseWithFunctionCallInspection : AbstractApplicabilityBasedI
                } else {
                    psiFactory.createExpressionByPattern("${kotlinFunction.fqName}($0) { $1 }", nullCheckedExpression, argument)
                }
-                element.replaced(newExpression)
+                element.replaceWith(newExpression, psiFactory)
            }
            else -> return
        }
        commentSaver.restore(replaced)
+        editor?.caretModel?.moveToOffset(replaced.startOffset) 
        ShortenReferences.DEFAULT.process(replaced)
    }
+    
+    private fun KtIfExpression.replaceWith(newExpression: KtExpression, psiFactory: KtPsiFactory): KtExpression {
+        val parent = parent
+        val elseBranch = `else`
+        return if (elseBranch != null) {
+            val added = parent.addBefore(newExpression, this) as KtExpression
+            parent.addBefore(psiFactory.createNewLine(), this)
+            replaceWithBranch(elseBranch, isUsedAsExpression = false, keepBraces = false)
+            added
+        } else {
+            replaced(newExpression)
+        }
+    }

    private fun KtIfExpression.getCallExpression(): KtCallExpression? {
        val throwExpression = this.then?.let {
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(flag: Boolean) {
+    <caret>if (!flag) throw IllegalArgumentException() else println(1)
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+fun test(flag: Boolean) {
+    <caret>require(flag)
+    println(1)
+}
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+fun test(flag: Boolean) {
+    <caret>if (!flag) throw IllegalArgumentException()
+    else {
+        println(1)
+        println(2)
+    }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(flag: Boolean) {
+    <caret>require(flag)
+    println(1)
+    println(2)
+}
@@ -0,0 +1,11 @@
+// WITH_RUNTIME
+fun test(flag: Boolean, i: Int) {
+    <caret>if (!flag) {
+        throw IllegalArgumentException()
+    } else if (i == 0) {
+        println(0)
+    } else {
+        println(1)
+        println(2)
+    }
+}
@@ -0,0 +1,10 @@
+// WITH_RUNTIME
+fun test(flag: Boolean, i: Int) {
+    <caret>require(flag)
+    if (i == 0) {
+        println(0)
+    } else {
+        println(1)
+        println(2)
+    }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(foo: Int?) {
+    <caret>if (foo == null) throw IllegalArgumentException("test") else println(1)
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+fun test(foo: Int?) {
+    <caret>requireNotNull(foo) { "test" }
+    println(1)
+}
@@ -0,0 +1,6 @@
+// PROBLEM: none
+// WITH_RUNTIME
+fun test(flag: Boolean): Int {
+    return <caret>if (!flag) throw IllegalArgumentException() else 1
+}
+
@@ -9007,6 +9007,21 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
                runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/comment2.kt");
            }

+            @TestMetadata("hasElse.kt")
+            public void testHasElse() throws Exception {
+                runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt");
+            }
+
+            @TestMetadata("hasElse2.kt")
+            public void testHasElse2() throws Exception {
+                runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt");
+            }
+
+            @TestMetadata("hasElse3.kt")
+            public void testHasElse3() throws Exception {
+                runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt");
+            }
+
            @TestMetadata("not.kt")
            public void testNot() throws Exception {
                runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/not.kt");
@@ -9034,6 +9049,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
            public void testBasic() throws Exception {
                runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/basic.kt");
            }
+
+            @TestMetadata("hasElse.kt")
+            public void testHasElse() throws Exception {
+                runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt");
+            }
        }
    }