Introduce "cascade if" inspection suggesting replacement with when

So #KT-18615 Fixed
2017-06-27 18:21:08 +03:00
parent 1ddaee5b4a
commit 8273eff1a1
15 changed files with 256 additions and 0 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports if statements with three or more branches that can be replaced with when.
+</body>
+</html>
@@ -2369,6 +2369,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.CascadeIfInspection"
+                     displayName="Cascade if can be replaced with when"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2010-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.idea.inspections
+
+import com.intellij.codeInspection.IntentionWrapper
+import com.intellij.codeInspection.LocalInspectionToolSession
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.IfToWhenIntention
+import org.jetbrains.kotlin.idea.intentions.branchedTransformations.isIfBranch
+import org.jetbrains.kotlin.idea.intentions.branches
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
+import org.jetbrains.kotlin.psi.psiUtil.lastBlockStatementOrThis
+
+class CascadeIfInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) =
+            object : KtVisitorVoid() {
+                override fun visitIfExpression(expression: KtIfExpression) {
+                    super.visitIfExpression(expression)
+
+                    val branches = expression.branches
+                    if (branches.size <= 2) return
+                    if (branches.any {
+                        it == null ||
+                        it.lastBlockStatementOrThis() is KtIfExpression
+                    }) return
+
+                    if (expression.isIfBranch()) return
+
+                    if (expression.anyDescendantOfType<KtExpressionWithLabel> {
+                        it is KtBreakExpression || it is KtContinueExpression
+                    }) return
+
+                    holder.registerProblem(
+                            expression.ifKeyword,
+                            "Cascade if can be replaced with when",
+                            ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                            IntentionWrapper(IfToWhenIntention(), expression.containingKtFile)
+                    )
+                }
+            }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.CascadeIfInspection
@@ -0,0 +1,16 @@
+// PROBLEM: none
+
+fun foo(a: Boolean, b: Boolean, c: Boolean) {
+    if (a) {
+
+    }
+    else <caret>if (b) {
+
+    }
+    else if (c) {
+
+    }
+    else {
+
+    }
+}
@@ -0,0 +1,16 @@
+// PROBLEM: none
+
+fun println(s: String) {}
+
+fun foo(a: Boolean, b: Boolean) {
+    <caret>if (a) {
+        if (b) println("ab")
+        else println("a")
+    }
+    else if (b) {
+        println("b")
+    }
+    else {
+        println("none")
+    }
+}
@@ -0,0 +1,15 @@
+fun println(s: String) {}
+
+fun test(x: Double, a: Boolean, b: Boolean) {
+    if (x > 0.0) {
+        <caret>if (a) {
+            println("a")
+        }
+        else if (b) {
+            println("b")
+        }
+        else {
+            println("none")
+        }
+    }
+}
@@ -0,0 +1,11 @@
+fun println(s: String) {}
+
+fun test(x: Double, a: Boolean, b: Boolean) {
+    if (x > 0.0) {
+        when {
+            a -> println("a")
+            b -> println("b")
+            else -> println("none")
+        }
+    }
+}
@@ -0,0 +1,15 @@
+// PROBLEM: none
+
+fun println(s: String) {}
+
+fun test(a: Boolean, b: Boolean) {
+    if (a) {
+        println("a")
+    }
+    else <caret>if (b) {
+        println("b")
+    }
+    else {
+        println("none")
+    }
+}
@@ -0,0 +1,12 @@
+// PROBLEM: none
+
+fun println(s: String) {}
+
+fun test(a: Boolean, b: Boolean) {
+    <caret>if (a) {
+        println("a")
+    }
+    else if (b) {
+        println("b")
+    }
+}
@@ -0,0 +1,13 @@
+fun println(s: String) {}
+
+fun test(a: Boolean, b: Boolean) {
+    <caret>if (a) {
+        println("a")
+    }
+    else if (b) {
+        println("b")
+    }
+    else {
+        println("none")
+    }
+}
@@ -0,0 +1,9 @@
+fun println(s: String) {}
+
+fun test(a: Boolean, b: Boolean) {
+    when {
+        a -> println("a")
+        b -> println("b")
+        else -> println("none")
+    }
+}
@@ -0,0 +1,3 @@
+// PROBLEM: none
+
+val x = <caret>if (2 > 4) 1 else -1
@@ -0,0 +1,17 @@
+// PROBLEM: none
+
+fun println(s: String) {}
+
+fun foo(size: Int, a: Boolean, b: Boolean) {
+    for (i in 1..size) {
+        <caret>if (a) {
+            break
+        }
+        else if (b) {
+            println("$i")
+        }
+        else {
+            println("*")
+        }
+    }
+}
@@ -51,6 +51,63 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/cascadeIf")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class CascadeIf extends AbstractLocalInspectionTest {
+        public void testAllFilesPresentInCascadeIf() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/cascadeIf"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("four.kt")
+        public void testFour() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/four.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("ifTree.kt")
+        public void testIfTree() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/ifTree.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("insideOtherIf.kt")
+        public void testInsideOtherIf() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/insideOtherIf.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("middleIf.kt")
+        public void testMiddleIf() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/middleIf.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("noSecondElse.kt")
+        public void testNoSecondElse() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/noSecondElse.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("normal.kt")
+        public void testNormal() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/normal.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("shortIf.kt")
+        public void testShortIf() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/shortIf.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("withBreak.kt")
+        public void testWithBreak() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/cascadeIf/withBreak.kt");
+            doTest(fileName);
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/collections")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.CascadeIfInspection`