diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt index f38428e5c16..d5a84246883 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ConstantConditionIfInspection.kt @@ -23,6 +23,7 @@ import com.intellij.openapi.project.Project import com.intellij.psi.PsiElementVisitor import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.core.replaced +import org.jetbrains.kotlin.idea.intentions.branchedTransformations.isElseIf import org.jetbrains.kotlin.idea.intentions.branchedTransformations.unwrapBlockOrParenthesis import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.getParentOfType @@ -48,7 +49,8 @@ class ConstantConditionIfInspection : AbstractKotlinInspection() { val fixes = mutableListOf() if (expression.branch(constantValue) != null) { - fixes += SimplifyFix(constantValue, expression.isUsedAsExpression(context)) + val keepBraces = expression.isElseIf() && expression.branch(constantValue) is KtBlockExpression + fixes += SimplifyFix(constantValue, expression.isUsedAsExpression(context), keepBraces) } if (!constantValue && expression.`else` == null) { @@ -64,7 +66,8 @@ class ConstantConditionIfInspection : AbstractKotlinInspection() { private class SimplifyFix( private val conditionValue: Boolean, - private val isUsedAsExpression: Boolean + private val isUsedAsExpression: Boolean, + private val keepBraces: Boolean ) : LocalQuickFix { override fun getFamilyName() = name @@ -73,9 +76,11 @@ class ConstantConditionIfInspection : AbstractKotlinInspection() { override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val ifExpression = descriptor.psiElement.getParentOfType(strict = true) ?: return - val branch = ifExpression.branch(conditionValue)?.unwrapBlockOrParenthesis() ?: return + val branch = ifExpression.branch(conditionValue)?.let { + if (keepBraces) it else it.unwrapBlockOrParenthesis() + } ?: return - ifExpression.replaceWithBranch(branch, isUsedAsExpression) + ifExpression.replaceWithBranch(branch, isUsedAsExpression, keepBraces) } } @@ -100,7 +105,7 @@ private fun KtExpression.constantBooleanValue(context: BindingContext): Boolean? return constantValue?.value as? Boolean } -fun KtExpression.replaceWithBranch(branch: KtExpression, isUsedAsExpression: Boolean) { +fun KtExpression.replaceWithBranch(branch: KtExpression, isUsedAsExpression: Boolean, keepBraces: Boolean = false) { val lastExpression = when { branch !is KtBlockExpression -> replaced(branch) isUsedAsExpression -> { @@ -108,18 +113,17 @@ fun KtExpression.replaceWithBranch(branch: KtExpression, isUsedAsExpression: Boo replaced(factory.createExpressionByPattern("run $0", branch.text)) } else -> { - val firstChild = branch.firstChild.nextSibling - - if (firstChild == branch.lastChild) { - delete() + val firstChildSibling = branch.firstChild.nextSibling + val lastChild = branch.lastChild + if (firstChildSibling != lastChild) { + if (keepBraces) { + parent.addAfter(branch, this) + } + else { + parent.addRangeAfter(firstChildSibling, lastChild.prevSibling, this) + } } - else { - val lastChild = branch.lastChild.prevSibling - val parent = parent - parent.addRangeAfter(firstChild, lastChild, this) - delete() - } - + delete() null } } diff --git a/idea/testData/inspections/constantConditionIf/inspectionData/expected.xml b/idea/testData/inspections/constantConditionIf/inspectionData/expected.xml index d639626e8d3..4714d2035ec 100644 --- a/idea/testData/inspections/constantConditionIf/inspectionData/expected.xml +++ b/idea/testData/inspections/constantConditionIf/inspectionData/expected.xml @@ -39,4 +39,20 @@ Condition of 'if' expression is constant Condition is always 'true' + + test.kt + 31 + light_idea_test_case + + Condition of 'if' expression is constant + Condition is always 'true' + + + test.kt + 37 + light_idea_test_case + + Condition of 'if' expression is constant + Condition is always 'true' + \ No newline at end of file diff --git a/idea/testData/inspections/constantConditionIf/test.kt b/idea/testData/inspections/constantConditionIf/test.kt index e6b763eeffd..0a322d90db3 100644 --- a/idea/testData/inspections/constantConditionIf/test.kt +++ b/idea/testData/inspections/constantConditionIf/test.kt @@ -3,7 +3,7 @@ fun baz(s: String) {} const val TRUE = true -fun bar() { +fun bar(s: String?) { foo(if (TRUE) 1 else 2) foo(if (false && TRUE) { @@ -25,4 +25,14 @@ fun bar() { baz("a") 1 } else 2) + + if (s ==null) { + 1 + } else if (true) { + 2 + } + + if (s ==null) { + 1 + } else if (true) 2 } \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIf.kt b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIf.kt new file mode 100644 index 00000000000..26c56b775bb --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIf.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +fun foo(x: Int) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else if (true) { + 2 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIf.kt.after b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIf.kt.after new file mode 100644 index 00000000000..b35d2202941 --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIf.kt.after @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +fun foo(x: Int) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else { + 2 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIfNoBraces.kt b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIfNoBraces.kt new file mode 100644 index 00000000000..ac38e309877 --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIfNoBraces.kt @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +fun foo(x: Int) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else if (true) 2 +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIfNoBraces.kt.after b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIfNoBraces.kt.after new file mode 100644 index 00000000000..74fb361c40b --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/endWithElseIfNoBraces.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +fun foo(x: Int) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else 2 +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/expressionElseIfBlock.kt b/idea/testData/inspectionsLocal/constantConditionIf/expressionElseIfBlock.kt new file mode 100644 index 00000000000..40bf5b39c4f --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/expressionElseIfBlock.kt @@ -0,0 +1,16 @@ +// WITH_RUNTIME + +fun foo(x: Int) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else if (true) { + foo(1) + 2 + } + else { + 3 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/expressionElseIfBlock.kt.after b/idea/testData/inspectionsLocal/constantConditionIf/expressionElseIfBlock.kt.after new file mode 100644 index 00000000000..313411c67e1 --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/expressionElseIfBlock.kt.after @@ -0,0 +1,13 @@ +// WITH_RUNTIME + +fun foo(x: Int) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else { + foo(1) + 2 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/noStatementsElseIf.kt b/idea/testData/inspectionsLocal/constantConditionIf/noStatementsElseIf.kt new file mode 100644 index 00000000000..dc0128fde53 --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/noStatementsElseIf.kt @@ -0,0 +1,14 @@ +fun foo(s: String) {} + +fun bar(s: String?) { + if (s == null) { + foo("a") + foo("b") + } + else if (true) { + } + else { + foo("c") + foo("d") + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/noStatementsElseIf.kt.after b/idea/testData/inspectionsLocal/constantConditionIf/noStatementsElseIf.kt.after new file mode 100644 index 00000000000..cffc49075d3 --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/noStatementsElseIf.kt.after @@ -0,0 +1,10 @@ +fun foo(s: String) {} + +fun bar(s: String?) { + if (s == null) { + foo("a") + foo("b") + } + else { + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/statementElseIf.kt b/idea/testData/inspectionsLocal/constantConditionIf/statementElseIf.kt new file mode 100644 index 00000000000..f65dfaa2634 --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/statementElseIf.kt @@ -0,0 +1,15 @@ +fun foo(s: String) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else if (true) { + foo("a") + foo("b") + 2 + } + else { + 3 + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/constantConditionIf/statementElseIf.kt.after b/idea/testData/inspectionsLocal/constantConditionIf/statementElseIf.kt.after new file mode 100644 index 00000000000..8bb02f21bc5 --- /dev/null +++ b/idea/testData/inspectionsLocal/constantConditionIf/statementElseIf.kt.after @@ -0,0 +1,12 @@ +fun foo(s: String) {} + +fun bar(s: String?) { + if (s == null) { + 1 + } + else { + foo("a") + foo("b") + 2 + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 63983327a8a..8831197735d 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -428,18 +428,42 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { doTest(fileName); } + @TestMetadata("endWithElseIf.kt") + public void testEndWithElseIf() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/endWithElseIf.kt"); + doTest(fileName); + } + + @TestMetadata("endWithElseIfNoBraces.kt") + public void testEndWithElseIfNoBraces() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/endWithElseIfNoBraces.kt"); + doTest(fileName); + } + @TestMetadata("expression.kt") public void testExpression() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/expression.kt"); doTest(fileName); } + @TestMetadata("expressionElseIfBlock.kt") + public void testExpressionElseIfBlock() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/expressionElseIfBlock.kt"); + doTest(fileName); + } + @TestMetadata("noStatements.kt") public void testNoStatements() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/noStatements.kt"); doTest(fileName); } + @TestMetadata("noStatementsElseIf.kt") + public void testNoStatementsElseIf() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/noStatementsElseIf.kt"); + doTest(fileName); + } + @TestMetadata("simpleBlock.kt") public void testSimpleBlock() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/simpleBlock.kt"); @@ -463,6 +487,12 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/statement.kt"); doTest(fileName); } + + @TestMetadata("statementElseIf.kt") + public void testStatementElseIf() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/constantConditionIf/statementElseIf.kt"); + doTest(fileName); + } } @TestMetadata("idea/testData/inspectionsLocal/copyWithoutNamedArguments")