From a6690e4e3530d6679b62ef88512fb21ddcc408fd Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Mon, 22 Jul 2019 23:01:28 +0900 Subject: [PATCH] ReplaceGuardClause inspection: do not remove 'else' branch #KT-32797 Fixed --- ...ceGuardClauseWithFunctionCallInspection.kt | 19 ++++++++++++++++-- .../require/hasElse.kt | 4 ++++ .../require/hasElse.kt.after | 5 +++++ .../require/hasElse2.kt | 8 ++++++++ .../require/hasElse2.kt.after | 6 ++++++ .../require/hasElse3.kt | 11 ++++++++++ .../require/hasElse3.kt.after | 10 ++++++++++ .../requireNotNull/hasElse.kt | 4 ++++ .../requireNotNull/hasElse.kt.after | 5 +++++ .../usedAsExpression.kt | 6 ++++++ .../LocalInspectionTestGenerated.java | 20 +++++++++++++++++++ 11 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt.after create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt.after create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt.after create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt.after create mode 100644 idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/usedAsExpression.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceGuardClauseWithFunctionCallInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceGuardClauseWithFunctionCallInspection.kt index bbeef6ee54f..6fb89fc2f96 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceGuardClauseWithFunctionCallInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceGuardClauseWithFunctionCallInspection.kt @@ -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 { diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt new file mode 100644 index 00000000000..095fd761e7c --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(flag: Boolean) { + if (!flag) throw IllegalArgumentException() else println(1) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt.after b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt.after new file mode 100644 index 00000000000..511179d2399 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse.kt.after @@ -0,0 +1,5 @@ +// WITH_RUNTIME +fun test(flag: Boolean) { + require(flag) + println(1) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt new file mode 100644 index 00000000000..9274f190bb3 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt @@ -0,0 +1,8 @@ +// WITH_RUNTIME +fun test(flag: Boolean) { + if (!flag) throw IllegalArgumentException() + else { + println(1) + println(2) + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt.after b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt.after new file mode 100644 index 00000000000..f55b6760ccb --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse2.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun test(flag: Boolean) { + require(flag) + println(1) + println(2) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt new file mode 100644 index 00000000000..dfb5b95573b --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +fun test(flag: Boolean, i: Int) { + if (!flag) { + throw IllegalArgumentException() + } else if (i == 0) { + println(0) + } else { + println(1) + println(2) + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt.after b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt.after new file mode 100644 index 00000000000..4896d76724c --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/hasElse3.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME +fun test(flag: Boolean, i: Int) { + require(flag) + if (i == 0) { + println(0) + } else { + println(1) + println(2) + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt new file mode 100644 index 00000000000..4f2b2d6357e --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(foo: Int?) { + if (foo == null) throw IllegalArgumentException("test") else println(1) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt.after b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt.after new file mode 100644 index 00000000000..0695c044ab5 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/hasElse.kt.after @@ -0,0 +1,5 @@ +// WITH_RUNTIME +fun test(foo: Int?) { + requireNotNull(foo) { "test" } + println(1) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/usedAsExpression.kt b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/usedAsExpression.kt new file mode 100644 index 00000000000..17f346ec0f3 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/usedAsExpression.kt @@ -0,0 +1,6 @@ +// PROBLEM: none +// WITH_RUNTIME +fun test(flag: Boolean): Int { + return if (!flag) throw IllegalArgumentException() else 1 +} + diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 37d06e46fff..ead7dd4a94c 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -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"); + } } }