Keep braces in redundant cascade if #KT-19704 Fixed

2017-08-16 20:19:33 +03:00
parent 97e4dbe330
commit ba19931aef
14 changed files with 201 additions and 17 deletions
@@ -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<LocalQuickFix>()

                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<KtIfExpression>(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
        }
    }
@@ -39,4 +39,20 @@
        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
        <description>Condition is always 'true'</description>
    </problem>
+    <problem>
+        <file>test.kt</file>
+        <line>31</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
+        <description>Condition is always 'true'</description>
+    </problem>
+    <problem>
+        <file>test.kt</file>
+        <line>37</line>
+        <module>light_idea_test_case</module>
+        <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+        <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Condition of 'if' expression is constant</problem_class>
+        <description>Condition is always 'true'</description>
+    </problem>
 </problems>
@@ -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
 }
@@ -0,0 +1,12 @@
+// WITH_RUNTIME
+
+fun foo(x: Int) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else if (<caret>true) {
+        2
+    }
+}
@@ -0,0 +1,12 @@
+// WITH_RUNTIME
+
+fun foo(x: Int) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else {
+        2
+    }
+}
@@ -0,0 +1,10 @@
+// WITH_RUNTIME
+
+fun foo(x: Int) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else if (<caret>true) 2
+}
@@ -0,0 +1,10 @@
+// WITH_RUNTIME
+
+fun foo(x: Int) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else 2
+}
@@ -0,0 +1,16 @@
+// WITH_RUNTIME
+
+fun foo(x: Int) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else if (<caret>true) {
+        foo(1)
+        2
+    }
+    else {
+        3
+    }
+}
@@ -0,0 +1,13 @@
+// WITH_RUNTIME
+
+fun foo(x: Int) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else {
+        foo(1)
+        2
+    }
+}
@@ -0,0 +1,14 @@
+fun foo(s: String) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        foo("a")
+        foo("b")
+    }
+    else if (<caret>true) {
+    }
+    else {
+        foo("c")
+        foo("d")
+    }
+}
@@ -0,0 +1,10 @@
+fun foo(s: String) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        foo("a")
+        foo("b")
+    }
+    else {
+    }
+}
@@ -0,0 +1,15 @@
+fun foo(s: String) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else if (<caret>true) {
+        foo("a")
+        foo("b")
+        2
+    }
+    else {
+        3
+    }
+}
@@ -0,0 +1,12 @@
+fun foo(s: String) {}
+
+fun bar(s: String?) {
+    if (s == null) {
+        1
+    }
+    else {
+        foo("a")
+        foo("b")
+        2
+    }
+}
@@ -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")