From 605ac0b25e3e42428582c58222bd17466e707110 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 22 Aug 2016 17:11:25 +0300 Subject: [PATCH] Invert if: more correct handling of empty returns #KT-13444 Fixed (cherry picked from commit be2adaf) --- .../intentions/InvertIfConditionIntention.kt | 16 ++++++++-- .../ifWithBothBranchesReturn.kt | 7 +++++ .../ifWithBothBranchesReturn.kt.after | 7 +++++ .../ifWithBothBranchesSetter.kt | 6 ++++ .../ifWithBothBranchesSetter.kt.after | 6 ++++ .../lambdaNonLocalAndLocalReturn.kt | 12 ++++++++ .../lambdaNonLocalAndLocalReturn.kt.after | 14 +++++++++ .../invertIfCondition/lambdaNonLocalReturn.kt | 12 ++++++++ .../lambdaNonLocalReturn.kt.after | 13 ++++++++ .../invertIfCondition/nestedIfWithReturn.kt | 10 +++++++ .../nestedIfWithReturn.kt.after | 11 +++++++ .../intentions/IntentionTestGenerated.java | 30 +++++++++++++++++++ 12 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt create mode 100644 idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt.after create mode 100644 idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt create mode 100644 idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt.after create mode 100644 idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt create mode 100644 idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt.after create mode 100644 idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt create mode 100644 idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt.after create mode 100644 idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt create mode 100644 idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt index 1fa2be62c08..ff7a3c4f497 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/InvertIfConditionIntention.kt @@ -113,14 +113,21 @@ class InvertIfConditionIntention : SelfTargetingIntention(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(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 diff --git a/idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt new file mode 100644 index 00000000000..f88297e9e87 --- /dev/null +++ b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt @@ -0,0 +1,7 @@ +fun println(s: String) {} + +fun foo(y: Boolean) { + if (!y) return + println("no1") + return +} \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt.after b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt.after new file mode 100644 index 00000000000..2fe6e9c2a81 --- /dev/null +++ b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesReturn.kt.after @@ -0,0 +1,7 @@ +fun println(s: String) {} + +fun foo(y: Boolean) { + if (y) { + println("no1") + } +} \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt new file mode 100644 index 00000000000..5f224b6ef3a --- /dev/null +++ b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt @@ -0,0 +1,6 @@ +var foo: Boolean = false + set(arg) { + if (field == arg) return + field = arg + return + } \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt.after b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt.after new file mode 100644 index 00000000000..12a46e3ff6f --- /dev/null +++ b/idea/testData/intentions/invertIfCondition/ifWithBothBranchesSetter.kt.after @@ -0,0 +1,6 @@ +var foo: Boolean = false + set(arg) { + if (field != arg) { + field = arg + } + } \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt b/idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt new file mode 100644 index 00000000000..2cc6d29d896 --- /dev/null +++ b/idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt @@ -0,0 +1,12 @@ +inline fun call(f: () -> Unit) = f() + +fun bar() {} + +fun foo(arg: Boolean) { + call { + if (!arg) return@call + bar() + return + } + bar() +} \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt.after b/idea/testData/intentions/invertIfCondition/lambdaNonLocalAndLocalReturn.kt.after new file mode 100644 index 00000000000..f46f15fa7ab --- /dev/null +++ b/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() +} \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt b/idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt new file mode 100644 index 00000000000..69d157df395 --- /dev/null +++ b/idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt @@ -0,0 +1,12 @@ +inline fun call(f: () -> Unit) = f() + +fun bar() {} + +fun foo(arg: Boolean) { + call { + if (!arg) return + bar() + return + } + bar() +} \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt.after b/idea/testData/intentions/invertIfCondition/lambdaNonLocalReturn.kt.after new file mode 100644 index 00000000000..6121b5f697a --- /dev/null +++ b/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() +} \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt b/idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt new file mode 100644 index 00000000000..3fee1ba9226 --- /dev/null +++ b/idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt @@ -0,0 +1,10 @@ +fun println(s: String) {} + +fun foo(x: Boolean, y: Boolean) { + if (x) { + if (!y) return + println("no1") + return + } + println("no2") +} \ No newline at end of file diff --git a/idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt.after b/idea/testData/intentions/invertIfCondition/nestedIfWithReturn.kt.after new file mode 100644 index 00000000000..424bc31e6bd --- /dev/null +++ b/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") +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 37c19b34522..e97af3face1 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/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");