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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CognitiveComplexity: count else/else-if as one complexity #5458

Merged
merged 3 commits into from Oct 24, 2022
Merged

CognitiveComplexity: count else/else-if as one complexity #5458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1f4912
CognitiveComplexity: count else/else-if as one complexity
t-kameyama Oct 21, 2022
693fa8a
Don't increment nesting in else/else-if
t-kameyama Oct 22, 2022
6b06079
Add +1 complexity and increase nesting level
t-kameyama Oct 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 31 additions & 3 deletions detekt-metrics/src/main/kotlin/io/github/detekt/metrics/CognitiveComplexity.kt
View file
Open in desktop
@@ -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++
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 since an else branch only increment the complexity without adding nesting

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
@@ -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
Open in desktop
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