Add ElseCaseInsteadOfExhaustiveWhen rule

config/detekt/detekt.yml

@@ -136,6 +136,8 @@ potential-bugs:
     active: true
   DoubleMutabilityForCollection:
     active: false
+  ElseCaseInsteadOfExhaustiveWhen:
+    active: true
   ExitOutsideMain:
     active: false
   HasPlatformType:

detekt-core/src/main/resources/default-detekt-config.yml

@@ -395,6 +395,8 @@ potential-bugs:
       - 'java.util.HashMap'
   DuplicateCaseInWhenExpression:
     active: true
+  ElseCaseInsteadOfExhaustiveWhen:
+    active: false
   EqualsAlwaysReturnsTrueOrFalse:
     active: true
   EqualsWithHashCodeExist:

detekt-rules-documentation/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/documentation/OutdatedDocumentation.kt

@@ -153,12 +153,14 @@ class OutdatedDocumentation(config: Config = Config.empty) : Rule(config) {
     }
 
     private fun processDocTag(docTag: KDocTag): List<Declaration> {
-        val knownTag = docTag.knownTag
+        val knownTag = docTag.knownTag ?: return emptyList()
         val subjectName = docTag.getSubjectName() ?: return emptyList()
         return when (knownTag) {
             KDocKnownTag.PARAM -> listOf(Declaration(subjectName, DeclarationType.PARAM))
             KDocKnownTag.PROPERTY -> listOf(Declaration(subjectName, DeclarationType.PROPERTY))
-            else -> emptyList()
+            KDocKnownTag.AUTHOR, KDocKnownTag.THROWS, KDocKnownTag.EXCEPTION, KDocKnownTag.RECEIVER,
+            KDocKnownTag.RETURN, KDocKnownTag.SEE, KDocKnownTag.SINCE, KDocKnownTag.CONSTRUCTOR, KDocKnownTag.SAMPLE,
+            KDocKnownTag.SUPPRESS -> emptyList()
         }
     }
 

detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/ElseCaseInsteadOfExhaustiveWhen.kt

@@ -0,0 +1,88 @@
+package io.gitlab.arturbosch.detekt.rules.bugs
+
+import io.gitlab.arturbosch.detekt.api.CodeSmell
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.Debt
+import io.gitlab.arturbosch.detekt.api.Entity
+import io.gitlab.arturbosch.detekt.api.Issue
+import io.gitlab.arturbosch.detekt.api.Rule
+import io.gitlab.arturbosch.detekt.api.Severity
+import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
+import org.jetbrains.kotlin.cfg.WhenChecker
+import org.jetbrains.kotlin.psi.KtWhenExpression
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.calls.callUtil.getType
+import org.jetbrains.kotlin.types.typeUtil.isBooleanOrNullableBoolean
+
+/**
+ * This rule reports `when` expressions that contain an `else` case even though they have an exhaustive set of cases.
+ *
+ * This occurs when the subject of the `when` expression is either an enum class, sealed class or of type boolean.
+ * Using `else` cases for these expressions can lead to unintended behavior when adding new enum types, sealed subtypes
+ * or changing the nullability of a boolean, since this will be implicitly handled by the `else` case.
+ *
+ * <noncompliant>
+ * enum class Color {
+ *     RED,
+ *     GREEN,
+ *     BLUE
+ * }
+ *
+ * when(c) {
+ *     Color.RED -> {}
+ *     Color.GREEN -> {}
+ *     else -> {}
+ * }
+ * </noncompliant>
+ *
+ * <compliant>
+ * enum class Color {
+ *     RED,
+ *     GREEN,
+ *     BLUE
+ * }
+ *
+ * when(c) {
+ *     Color.RED -> {}
+ *     Color.GREEN -> {}
+ *     Color.BLUE -> {}
+ * }
+ * </compliant>
+ */
+@RequiresTypeResolution
+class ElseCaseInsteadOfExhaustiveWhen(config: Config = Config.empty) : Rule(config) {
+
+    override val issue: Issue = Issue(
+        "ElseCaseInsteadOfExhaustiveWhen",
+        Severity.Warning,
+        "A `when` expression that has an exhaustive set of cases should not contain an `else` case.",
+        Debt.FIVE_MINS
+    )
+
+    override fun visitWhenExpression(whenExpression: KtWhenExpression) {
+        super.visitWhenExpression(whenExpression)
+
+        if (bindingContext == BindingContext.EMPTY) return
+        checkForElseCaseInsteadOfExhaustiveWhenExpression(whenExpression)
+    }
+
+    private fun checkForElseCaseInsteadOfExhaustiveWhenExpression(whenExpression: KtWhenExpression) {
+        val subjectExpression = whenExpression.subjectExpression ?: return
+        if (whenExpression.elseExpression == null) return
+
+        val subjectType = subjectExpression.getType(bindingContext)
+        val isEnumSubject = WhenChecker.getClassDescriptorOfTypeIfEnum(subjectType) != null
+        val isSealedSubject = WhenChecker.getClassDescriptorOfTypeIfSealed(subjectType) != null
+        val isBooleanSubject = subjectType?.isBooleanOrNullableBoolean() == true
+
+        if (isEnumSubject || isSealedSubject || isBooleanSubject) {
+            val subjectTypeName = when {
+                isEnumSubject -> "enum class"
+                isSealedSubject -> "sealed class"
+                else -> "boolean"
+            }
+            val message = "When expression with $subjectTypeName subject should not contain an `else` case."
+            report(CodeSmell(issue, Entity.from(whenExpression), message))
+        }
+    }
+}

detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt

@@ -21,6 +21,7 @@ class PotentialBugProvider : DefaultRuleSetProvider {
             DontDowncastCollectionTypes(config),
             DoubleMutabilityForCollection(config),
             DuplicateCaseInWhenExpression(config),
+            ElseCaseInsteadOfExhaustiveWhen(config),
             EqualsAlwaysReturnsTrueOrFalse(config),
             EqualsWithHashCodeExist(config),
             ExitOutsideMain(config),

detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/ElseCaseInsteadOfExhaustiveWhenSpec.kt

@@ -0,0 +1,267 @@
+package io.gitlab.arturbosch.detekt.rules.bugs
+
+import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
+import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
+import org.assertj.core.api.Assertions.assertThat
+import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
+import org.junit.jupiter.api.Nested
+import org.junit.jupiter.api.Test
+
+@KotlinCoreEnvironmentTest
+class ElseCaseInsteadOfExhaustiveWhenSpec(private val env: KotlinCoreEnvironment) {
+    private val subject = ElseCaseInsteadOfExhaustiveWhen()
+
+    @Nested
+    inner class `ElseCaseInsteadOfExhaustiveWhen rule` {
+        @Nested
+        inner class Enum {
+            @Test
+            fun `reports when enum _when_ expression used as statement contains _else_ case`() {
+                val code = """
+                enum class Color {
+                    RED,
+                    GREEN,
+                    BLUE
+                }
+
+                fun whenOnEnumFail(c: Color) {
+                    when (c) {
+                        Color.BLUE -> {}
+                        Color.GREEN -> {}
+                        else -> {}
+                    }
+                }
+                """
+                val actual = subject.compileAndLintWithContext(env, code)
+                assertThat(actual).hasSize(1)
+            }
+
+            @Test
+            fun `reports when enum _when_ expression contains _else_ case`() {
+                val code = """
+                enum class Color {
+                    RED,
+                    GREEN,
+                    BLUE
+                }
+
+                fun whenOnEnumFail(c: Color) {
+                    val x = when (c) {
+                        Color.BLUE -> 1
+                        Color.GREEN -> 2
+                        else -> 100
+                    }
+                }
+                """
+                val actual = subject.compileAndLintWithContext(env, code)
+                assertThat(actual).hasSize(1)
+            }
+
+            @Test
+            fun `does not report when enum _when_ expression does not contain _else_ case`() {
+                val code = """
+                enum class Color {
+                    RED,
+                    GREEN,
+                    BLUE
+                }
+
+                fun whenOnEnumPassA(c: Color) {
+                    when (c) {
+                        Color.BLUE -> {}
+                        Color.GREEN -> {}
+                        Color.RED -> {}
+                    }
+
+                    val x = when (c) {
+                        Color.BLUE -> 1
+                        Color.GREEN -> 2
+                        Color.RED -> 3
+                    }
+                }
+
+                fun whenOnEnumPassB(c: Color) {
+                    when (c) {
+                        Color.BLUE -> {}
+                        Color.GREEN -> {}
+                    }
+                }
+                """
+                assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+            }
+        }
+
+        @Nested
+        inner class `Sealed class` {
+            @Test
+            fun `reports when sealed _when_ expression used as statement contains _else_ case`() {
+                val code = """
+                    sealed class Variant {
+                        object VariantA : Variant()
+                        class VariantB : Variant()
+                        object VariantC : Variant()
+                    }
+
+                    fun whenOnSealedFail(v: Variant) {
+                        when (v) {
+                            is Variant.VariantA -> {}
+                            is Variant.VariantB -> {}
+                            else -> {}
+                        }
+                    }
+                """
+                assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            }
+
+            @Test
+            fun `reports when sealed _when_ expression contains _else_ case`() {
+                val code = """
+                    sealed class Variant {
+                        object VariantA : Variant()
+                        class VariantB : Variant()
+                        object VariantC : Variant()
+                    }
+
+                    fun whenOnSealedFail(v: Variant) {
+                        val x = when (v) {
+                            is Variant.VariantA -> "a"
+                            is Variant.VariantB -> "b"
+                            else -> "other"
+                        }
+                    }
+                """
+                assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            }
+
+            @Test
+            fun `does not report when sealed _when_ expression does not contain _else_ case`() {
+                val code = """
+                    sealed class Variant {
+                        object VariantA : Variant()
+                        class VariantB : Variant()
+                        object VariantC : Variant()
+                    }
+
+                    fun whenOnSealedPass(v: Variant) {
+                        when (v) {
+                            is Variant.VariantA -> {}
+                            is Variant.VariantB -> {}
+                        }
+                    }
+                """
+                assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+            }
+        }
+
+        @Nested
+        inner class Boolean {
+            @Test
+            fun `reports when boolean _when_ expression used as statement contains _else_ case`() {
+                val code = """
+                fun whenOnBooleanFail(b: Boolean) {
+                    when (b) {
+                        true -> {}
+                        else -> {}
+                    }
+                }
+                """
+                val actual = subject.compileAndLintWithContext(env, code)
+                assertThat(actual).hasSize(1)
+            }
+
+            @Test
+            fun `reports when nullable boolean _when_ expression contains _else_ case`() {
+                val code = """
+                fun whenOnNullableBooleanFail(b: Boolean?) {
+                    val x = when (b) {
+                        true -> 1
+                        false -> 2
+                        else -> 100
+                    }
+                }
+                """
+                val actual = subject.compileAndLintWithContext(env, code)
+                assertThat(actual).hasSize(1)
+            }
+
+            @Test
+            fun `does not report when boolean _when_ expression does not contain _else_ case`() {
+                val code = """
+                fun whenOnBooleanPassA(b: Boolean) {
+                    when (b) {
+                        true -> {}
+                        false -> {}
+                    }
+
+                    val x = when (b) {
+                        true -> 1
+                        false -> 2
+                    }
+                }
+                """
+                assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+            }
+
+            @Test
+            fun `does not report when nullable boolean _when_ expression does not contain _else_ case`() {
+                val code = """
+                fun whenOnNullableBooleanPassA(b: Boolean?) {
+                    when (b) {
+                        true -> {}
+                        false -> {}
+                        null -> {}
+                    }
+
+                    val x = when (b) {
+                        true -> 1
+                        false -> 2
+                        null -> 100
+                    }
+                }
+                """
+                assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+            }
+        }
+
+        @Nested
+        inner class `Standard when` {
+            @Test
+            fun `does not report when _else_ case is used for non enum or sealed _when_ expression`() {
+                val code = """
+                    fun whenChecks() {
+                        val x = 3
+                        val s = "3"
+
+                        when (x) {
+                            0, 1 -> print("x == 0 or x == 1")
+                            else -> print("otherwise")
+                        }
+
+                        when (x) {
+                            Integer.parseInt(s) -> print("s encodes x")
+                            else -> print("s does not encode x")
+                        }
+
+                        when (x) {
+                            in 1..10 -> print("x is in the range")
+                            !in 10..20 -> print("x is outside the range")
+                            else -> print("none of the above")
+                        }
+
+                        val y = when(s) {
+                            is String -> s.startsWith("prefix")
+                            else -> false
+                        }
+
+                        when {
+                            x.equals(s) -> print("x equals s")
+                            x.plus(3) == 4 -> print("x is 1")
+                            else -> print("x is funny")
+                        }
+                    }
+                """
+                assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
+            }
+        }
+    }
+}