Add inspection for when with only else (#1206)

Fixes #KT-12523
2017-08-03 15:30:28 +02:00
parent 61b10ac330
commit 9ac1a0140c
13 changed files with 199 additions and 31 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports 'when' expressions with only an 'else' branch that can be simplified to the 'else' branch's expression.
+</body>
+</html>
@@ -2437,6 +2437,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.WhenWithOnlyElseInspection"
+                     displayName="'when' has only 'else' branch and can be simplified"
+                     groupPath="Kotlin"
+                     groupName="Redundant constructs"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -72,34 +72,10 @@ class ConstantConditionIfInspection : AbstractKotlinInspection() {

        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
            val ifExpression = descriptor.psiElement.getParentOfType<KtIfExpression>(strict = true) ?: return
-            val caretModel = ifExpression.findExistingEditor()?.caretModel

            val branch = ifExpression.branch(conditionValue)?.unwrapBlockOrParenthesis() ?: return

-            val lastExpression = when {
-                branch !is KtBlockExpression -> ifExpression.replaced(branch)
-                isUsedAsExpression -> {
-                    val factory = KtPsiFactory(ifExpression)
-                    ifExpression.replaced(factory.createExpressionByPattern("run $0", branch.text))
-                }
-                else -> {
-                    val firstChild = branch.firstChild.nextSibling
-
-                    if (firstChild == branch.lastChild) {
-                        ifExpression.delete()
-                    }
-                    else {
-                        val lastChild = branch.lastChild.prevSibling
-                        val parent = ifExpression.parent
-                        parent.addRangeAfter(firstChild, lastChild, ifExpression)
-                        ifExpression.delete()
-                    }
-
-                    null
-                }
-            }
-
-            caretModel?.moveToOffset(lastExpression?.startOffset ?: return)
+            ifExpression.replaceWithBranch(branch, isUsedAsExpression)
        }
    }

@@ -113,15 +89,41 @@ class ConstantConditionIfInspection : AbstractKotlinInspection() {
            ifExpression.delete()
        }
    }
+}

-    private companion object {
-        private fun KtIfExpression.branch(thenBranch: Boolean) = if (thenBranch) then else `else`
+private fun KtIfExpression.branch(thenBranch: Boolean) = if (thenBranch) then else `else`

-        private fun KtExpression.constantBooleanValue(context: BindingContext): Boolean? {
-            val type = getType(context) ?: return null
+private fun KtExpression.constantBooleanValue(context: BindingContext): Boolean? {
+    val type = getType(context) ?: return null

-            val constantValue = ConstantExpressionEvaluator.getConstant(this, context)?.toConstantValue(type)
-            return constantValue?.value as? Boolean
+    val constantValue = ConstantExpressionEvaluator.getConstant(this, context)?.toConstantValue(type)
+    return constantValue?.value as? Boolean
+}
+
+fun KtExpression.replaceWithBranch(branch: KtExpression, isUsedAsExpression: Boolean) {
+    val lastExpression = when {
+        branch !is KtBlockExpression -> replaced(branch)
+        isUsedAsExpression -> {
+            val factory = KtPsiFactory(this)
+            replaced(factory.createExpressionByPattern("run $0", branch.text))
+        }
+        else -> {
+            val firstChild = branch.firstChild.nextSibling
+
+            if (firstChild == branch.lastChild) {
+                delete()
+            }
+            else {
+                val lastChild = branch.lastChild.prevSibling
+                val parent = parent
+                parent.addRangeAfter(firstChild, lastChild, this)
+                delete()
+            }
+
+            null
        }
    }
+
+    val caretModel = branch.findExistingEditor()?.caretModel
+    caretModel?.moveToOffset(lastExpression?.startOffset ?: return)
 }
@@ -0,0 +1,61 @@
+/*
+ * 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.codeInsight.FileModificationService
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.psi.KtVisitorVoid
+import org.jetbrains.kotlin.psi.KtWhenExpression
+import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
+
+class WhenWithOnlyElseInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
+        return object : KtVisitorVoid() {
+            override fun visitWhenExpression(expression: KtWhenExpression) {
+                val singleEntry = expression.entries.singleOrNull()
+                if (singleEntry?.isElse != true) return
+
+                val usedAsExpression = expression.isUsedAsExpression(expression.analyze())
+
+                holder.registerProblem(expression,
+                                       "'when' has only 'else' branch and can be simplified",
+                                       SimplifyFix(usedAsExpression)
+                )
+            }
+        }
+    }
+
+    private class SimplifyFix(
+            private val isUsedAsExpression: Boolean
+    ) : LocalQuickFix {
+        override fun getFamilyName() = name
+
+        override fun getName() = "Simplify expression"
+
+        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+            val whenExpression = descriptor.psiElement as? KtWhenExpression ?: return
+            FileModificationService.getInstance().preparePsiElementForWrite(whenExpression)
+
+            whenExpression.replaceWithBranch(whenExpression.elseExpression ?: return, isUsedAsExpression)
+        }
+    }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.WhenWithOnlyElseInspection
@@ -0,0 +1,12 @@
+// WITH_RUNTIME
+
+fun println(s: String) {}
+
+fun foo() {
+    val a = <caret>when ("") {
+        else -> {
+            println("")
+            1
+        }
+    }
+}
@@ -0,0 +1,10 @@
+// WITH_RUNTIME
+
+fun println(s: String) {}
+
+fun foo() {
+    val a = <caret>run {
+        println("")
+        1
+    }
+}
@@ -0,0 +1,11 @@
+// PROBLEM: none
+
+fun println(s: String) {}
+
+fun foo(a: Boolean, b: Boolean) {
+    <caret>when ("") {
+        "a" -> println("a")
+        else -> println("else")
+
+    }
+}
@@ -0,0 +1,10 @@
+fun println(s: String) {}
+
+fun foo() {
+    <caret>when ("") {
+        else -> {
+            println("")
+            1
+        }
+    }
+}
@@ -0,0 +1,6 @@
+fun println(s: String) {}
+
+fun foo() {
+    <caret>println("")
+    1
+}
@@ -0,0 +1,5 @@
+fun foo() {
+    val a = <caret>when ("") {
+        else -> 1
+    }
+}
@@ -0,0 +1,3 @@
+fun foo() {
+    val a = <caret>1
+}
@@ -1929,6 +1929,39 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class WhenWithOnlyElse extends AbstractLocalInspectionTest {
+        public void testAllFilesPresentInWhenWithOnlyElse() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/whenWithOnlyElse"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("complexExpression.kt")
+        public void testComplexExpression() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/complexExpression.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("hasOtherBranches.kt")
+        public void testHasOtherBranches() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/hasOtherBranches.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("notExpression.kt")
+        public void testNotExpression() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/notExpression.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("simpleExpression.kt")
+        public void testSimpleExpression() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/whenWithOnlyElse/simpleExpression.kt");
+            doTest(fileName);
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/wrapUnaryOperator")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.WhenWithOnlyElseInspection`