Skip to content

Commit

Permalink
CognitiveComplexity: count else/else-if as one complexity (#5458)
Browse files Browse the repository at this point in the history 
* CognitiveComplexity: count else/else-if as one complexity

* Don't increment nesting in else/else-if

* Add +1 complexity and increase nesting level
  • Loading branch information
@t-kameyama
t-kameyama committed Oct 24, 2022
1 parent f20ffec commit 3f5489c
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 14 deletions.
34 changes: 31 additions & 3 deletions detekt-metrics/src/main/kotlin/io/github/detekt/metrics/CognitiveComplexity.kt
View file
@@ -1,13 +1,16 @@
package io.github.detekt.metrics

import io.gitlab.arturbosch.detekt.api.DetektVisitor
import io.gitlab.arturbosch.detekt.rules.isElseIf
import org.jetbrains.kotlin.KtNodeTypes
import org.jetbrains.kotlin.com.intellij.openapi.util.Key
import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtBreakExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCatchClause
import org.jetbrains.kotlin.psi.KtContainerNodeForControlStructureBody
import org.jetbrains.kotlin.psi.KtContinueExpression
import org.jetbrains.kotlin.psi.KtDoWhileExpression
import org.jetbrains.kotlin.psi.KtElement
Expand Down Expand Up @@ -88,9 +91,34 @@ class CognitiveComplexity private constructor() : DetektVisitor() {
nestAround { super.visitForExpression(expression) }
}

override fun visitIfExpression(expression: KtIfExpression) {
addComplexity()
nestAround { super.visitIfExpression(expression) }
override fun visitKtElement(element: KtElement) {
val parent = element.parent
if (element is KtContainerNodeForControlStructureBody && parent is KtIfExpression) {
when (element.node.elementType) {
KtNodeTypes.THEN -> {
if (parent.isElseIf()) {
complexity++
} else {
addComplexity()
}
nestAround { super.visitKtElement(element) }
}

KtNodeTypes.ELSE -> {
if (element.expression is KtIfExpression) {
super.visitKtElement(element)
} else {
complexity++
nestAround { super.visitKtElement(element) }
}
}

else ->
super.visitKtElement(element)
}
} else {
super.visitKtElement(element)
}
}

override fun visitBreakExpression(expression: KtBreakExpression) {
Expand Down
99 changes: 96 additions & 3 deletions detekt-metrics/src/test/kotlin/io/github/detekt/metrics/CognitiveComplexitySpec.kt
View file
Expand Up @@ -60,7 +60,7 @@ class CognitiveComplexitySpec {
""".trimIndent()
)

assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2)
assertThat(CognitiveComplexity.calculate(code)).isEqualTo(3)
}

@Test
Expand All @@ -72,7 +72,7 @@ class CognitiveComplexitySpec {
""".trimIndent()
)

assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2)
assertThat(CognitiveComplexity.calculate(code)).isEqualTo(3)
}

@Test
Expand All @@ -85,7 +85,7 @@ class CognitiveComplexitySpec {
""".trimIndent()
)

assertThat(CognitiveComplexity.calculate(code)).isEqualTo(1)
assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2)
}
}

Expand Down Expand Up @@ -282,4 +282,97 @@ class CognitiveComplexitySpec {
}
}
}

@Nested
inner class `if-else expressions` {
@Test
fun `should count else as complexity`() {
val code = compileContentForTest(
"""
fun test(condition: Boolean) {
if (condition) { // +1
} else { // +1
}
}
""".trimIndent()
)
assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2)
}

@Test
fun `should count else-if as 1 complexity`() {
val code = compileContentForTest(
"""
fun test(condition: Boolean) {
if (condition) { // +1
} else if (condition) { // +1
}
}
""".trimIndent()
)
assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2)
}

@Test
fun `should count else-if and else correctly`() {
val code = compileContentForTest(
"""
fun test(condition: Boolean) {
if (condition) { // +1
} else if (condition) { // +1
} else if (condition) { // +1
} else { // + 1
}
}
""".trimIndent()
)
assertThat(CognitiveComplexity.calculate(code)).isEqualTo(4)
}

@Test
fun `should count nested else-if correctly`() {
val code = compileContentForTest(
"""
fun test(condition: Boolean) {
// 18
if (condition) { // +1
if (condition) { // +2
while(true) { // +3
}
} else if (condition) { // +1
while(true) { // +3
}
} else if (condition) { // +1
while(true) { // +3
}
} else { // +1
while(true) { // +3
}
}
// 10
} else if (condition) { // +1
if (condition) { // +2
while(true) { // +3
}
} else if (condition) // +1
while(true) { // +3
}
// 10
} else { // +1
if (condition) { // +2
while(true) { // +3
}
} else // +1
while(true) { // +3
}
}
// 1
if (condition) { // +1
}
}
""".trimIndent()
)
assertThat(CognitiveComplexity.calculate(code)).isEqualTo(39)
}
}
}
2 changes: 1 addition & 1 deletion ...s/src/test/kotlin/io/github/detekt/metrics/processors/CognitiveComplexityProcessorSpec.kt
View file
Expand Up @@ -14,6 +14,6 @@ class CognitiveComplexityProcessorSpec {
val value = MetricProcessorTester(file)
.test(ProjectCognitiveComplexityProcessor(), CognitiveComplexity.KEY)

assertThat(value).isEqualTo(46)
assertThat(value).isEqualTo(50)
}
}
8 changes: 4 additions & 4 deletions detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/package.kt
View file
Expand Up @@ -86,7 +86,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4
while (true) {
if (false) {
println()
} else {
} else { // +1
println()
}
}
Expand All @@ -98,7 +98,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4
while (true) {
if (false) {
println()
} else {
} else { // +1
println()
}
}
Expand Down Expand Up @@ -131,7 +131,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4
while (true) {
if (false) {
println()
} else {
} else { // +1
println()
}
}
Expand All @@ -143,7 +143,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4
while (true) {
if (false) {
println()
} else {
} else { // +1
println()
}
}
Expand Down
4 changes: 4 additions & 0 deletions detekt-psi-utils/api/detekt-psi-utils.api
View file
Expand Up @@ -87,6 +87,10 @@ public final class io/gitlab/arturbosch/detekt/rules/KtCallExpressionKt {
public static final fun isCallingWithNonNullCheckArgument (Lorg/jetbrains/kotlin/psi/KtCallExpression;Lorg/jetbrains/kotlin/name/FqName;Lorg/jetbrains/kotlin/resolve/BindingContext;)Z
}

public final class io/gitlab/arturbosch/detekt/rules/KtIfExpressionKt {
public static final fun isElseIf (Lorg/jetbrains/kotlin/psi/KtIfExpression;)Z
}

public final class io/gitlab/arturbosch/detekt/rules/KtLambdaExpressionKt {
public static final fun firstParameter (Lorg/jetbrains/kotlin/psi/KtLambdaExpression;Lorg/jetbrains/kotlin/resolve/BindingContext;)Lorg/jetbrains/kotlin/descriptors/ValueParameterDescriptor;
public static final fun hasImplicitParameterReference (Lorg/jetbrains/kotlin/psi/KtLambdaExpression;Lorg/jetbrains/kotlin/descriptors/ValueParameterDescriptor;Lorg/jetbrains/kotlin/resolve/BindingContext;)Z
Expand Down
9 changes: 9 additions & 0 deletions detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtIfExpression.kt
View file
@@ -0,0 +1,9 @@
package io.gitlab.arturbosch.detekt.rules

import org.jetbrains.kotlin.KtNodeTypes
import org.jetbrains.kotlin.psi.KtContainerNodeForControlStructureBody
import org.jetbrains.kotlin.psi.KtIfExpression

fun KtIfExpression.isElseIf(): Boolean =
parent.node.elementType == KtNodeTypes.ELSE &&
parent.safeAs<KtContainerNodeForControlStructureBody>()?.expression == this
4 changes: 1 addition & 3 deletions ...ules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseIfEmptyOrIfBlank.kt
View file
Expand Up @@ -8,7 +8,7 @@ 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.KtNodeTypes
import io.gitlab.arturbosch.detekt.rules.isElseIf
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.FqName
Expand Down Expand Up @@ -84,8 +84,6 @@ class UseIfEmptyOrIfBlank(config: Config = Config.empty) : Rule(config) {
report(CodeSmell(issue, Entity.from(conditionCalleeExpression), message))
}

private fun KtExpression.isElseIf(): Boolean = parent.node.elementType == KtNodeTypes.ELSE

private fun KtIfExpression.condition(): Pair<KtExpression, Boolean>? {
val condition = this.condition ?: return null
return if (condition is KtPrefixExpression) {
Expand Down

0 comments on commit 3f5489c

Please sign in to comment.