Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ElseCaseInsteadOfExhaustiveWhen rule #4632

Merged
merged 9 commits into from Mar 23, 2022
2 changes: 2 additions & 0 deletions config/detekt/detekt.yml
Expand Up @@ -136,6 +136,8 @@ potential-bugs:
active: true
DoubleMutabilityForCollection:
active: false
ElseCaseInsteadOfExhaustiveWhen:
active: true
ExitOutsideMain:
active: false
HasPlatformType:
Expand Down
2 changes: 2 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -395,6 +395,8 @@ potential-bugs:
- 'java.util.HashMap'
DuplicateCaseInWhenExpression:
active: true
ElseCaseInsteadOfExhaustiveWhen:
active: false
EqualsAlwaysReturnsTrueOrFalse:
active: true
EqualsWithHashCodeExist:
Expand Down
Expand Up @@ -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()
Expand Down
@@ -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
}
}
Expand Up @@ -21,6 +21,7 @@ class PotentialBugProvider : DefaultRuleSetProvider {
DontDowncastCollectionTypes(config),
DoubleMutabilityForCollection(config),
DuplicateCaseInWhenExpression(config),
ElseCaseInsteadOfExhaustiveWhen(config),
EqualsAlwaysReturnsTrueOrFalse(config),
EqualsWithHashCodeExist(config),
ExitOutsideMain(config),
Expand Down