Add ElseCaseInsteadOfExhaustiveWhen rule (#4632)

* Add ElseCaseInEnumOrSealedWhen rule

* Adapt to new rule by replacing else cases

* Change rule name, support booleans, improve docs

* Change variable name of boolean condition

* Rename rule to ElseCaseInsteadOfExhaustiveWhen

* Improve rule descriptions

* Suppress rule for outdated documentation method

* Not report sealed class subjects that have an expect modifier

* Fix sample code syntax and not compile mulitplatform code

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

@@ -397,6 +397,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

@@ -152,6 +152,7 @@ class OutdatedDocumentation(config: Config = Config.empty) : Rule(config) {
             .fold(emptyList()) { acc, declarations -> acc + declarations }
     }
 
+    @Suppress("ElseCaseInsteadOfExhaustiveWhen")
     private fun processDocTag(docTag: KDocTag): List<Declaration> {
         val knownTag = docTag.knownTag
         val subjectName = docTag.getSubjectName() ?: return emptyList()

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

@@ -0,0 +1,99 @@
+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.KotlinType
+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 = isNonExpectedSealedClass(subjectType)
+        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))
+        }
+    }
+
+    /**
+     * `when` expressions on `expect` sealed classes in the common code of multiplatform projects still require an
+     * `else` branch. This happens because subclasses of `actual` platform implementations aren't known in the common
+     *  code.
+     */
+    private fun isNonExpectedSealedClass(type: KotlinType?): Boolean {
+        val sealedClassDescriptor = WhenChecker.getClassDescriptorOfTypeIfSealed(type)
+        return sealedClassDescriptor != null && !sealedClassDescriptor.isExpect
+    }
+}

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),