From c7d47fd58f6145984874b3242c4f8037a7c435dc Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Fri, 21 Oct 2022 12:34:50 +0900 Subject: [PATCH] CognitiveComplexity: count else/else-if as one complexity --- .../detekt/metrics/CognitiveComplexity.kt | 11 ++ .../detekt/metrics/CognitiveComplexitySpec.kt | 100 +++++++++++++++++- .../CognitiveComplexityProcessorSpec.kt | 2 +- .../detekt/metrics/processors/package.kt | 8 +- .../arturbosch/detekt/rules/KtIfExpression.kt | 9 ++ .../detekt/rules/style/UseIfEmptyOrIfBlank.kt | 4 +- 6 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtIfExpression.kt diff --git a/detekt-metrics/src/main/kotlin/io/github/detekt/metrics/CognitiveComplexity.kt b/detekt-metrics/src/main/kotlin/io/github/detekt/metrics/CognitiveComplexity.kt index 969bc24728ca..f3d2f5676d13 100644 --- a/detekt-metrics/src/main/kotlin/io/github/detekt/metrics/CognitiveComplexity.kt +++ b/detekt-metrics/src/main/kotlin/io/github/detekt/metrics/CognitiveComplexity.kt @@ -1,6 +1,7 @@ package io.github.detekt.metrics import io.gitlab.arturbosch.detekt.api.DetektVisitor +import io.gitlab.arturbosch.detekt.rules.isElseIf import org.jetbrains.kotlin.com.intellij.openapi.util.Key import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType import org.jetbrains.kotlin.lexer.KtTokens @@ -89,8 +90,18 @@ class CognitiveComplexity private constructor() : DetektVisitor() { } override fun visitIfExpression(expression: KtIfExpression) { + val isElseIf = expression.isElseIf() + + if (isElseIf) nesting-- + addComplexity() + val elseBranch = expression.`else` + if (elseBranch != null && elseBranch !is KtIfExpression) { + addComplexity() + } nestAround { super.visitIfExpression(expression) } + + if (isElseIf) nesting++ } override fun visitBreakExpression(expression: KtBreakExpression) { diff --git a/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/CognitiveComplexitySpec.kt b/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/CognitiveComplexitySpec.kt index 6e808d1764ea..376df5c4f323 100644 --- a/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/CognitiveComplexitySpec.kt +++ b/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/CognitiveComplexitySpec.kt @@ -60,7 +60,7 @@ class CognitiveComplexitySpec { """.trimIndent() ) - assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2) + assertThat(CognitiveComplexity.calculate(code)).isEqualTo(3) } @Test @@ -72,7 +72,7 @@ class CognitiveComplexitySpec { """.trimIndent() ) - assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2) + assertThat(CognitiveComplexity.calculate(code)).isEqualTo(3) } @Test @@ -85,7 +85,7 @@ class CognitiveComplexitySpec { """.trimIndent() ) - assertThat(CognitiveComplexity.calculate(code)).isEqualTo(1) + assertThat(CognitiveComplexity.calculate(code)).isEqualTo(2) } } @@ -282,4 +282,98 @@ 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) { + if (condition) { // +1 + if (condition) { // +2 + while(true) { // +3 + } + } else if (condition) { // +2 + while(true) { // +3 + } + } else { // +2 + while(true) { // +3 + } + } + } else if (condition) { // +1 + if (condition) { // +2 + while(true) { // +3 + } + } else if (condition) { // +2 + while(true) { // +3 + } + } else { // +2 + while(true) { // +3 + } + } + } else { // + 1 + if (condition) { // +2 + while(true) { // +3 + } + } else if (condition) { // +2 + while(true) { // +3 + } + } else { // +2 + while(true) { // +3 + } + } + } + if (condition) { // +1 + } + } + """.trimIndent() + ) + assertThat(CognitiveComplexity.calculate(code)).isEqualTo(49) + } + } } diff --git a/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/CognitiveComplexityProcessorSpec.kt b/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/CognitiveComplexityProcessorSpec.kt index b3d4ea669d31..c1f193ff125a 100644 --- a/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/CognitiveComplexityProcessorSpec.kt +++ b/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/CognitiveComplexityProcessorSpec.kt @@ -14,6 +14,6 @@ class CognitiveComplexityProcessorSpec { val value = MetricProcessorTester(file) .test(ProjectCognitiveComplexityProcessor(), CognitiveComplexity.KEY) - assertThat(value).isEqualTo(46) + assertThat(value).isEqualTo(60) } } diff --git a/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/package.kt b/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/package.kt index e9851a9e071f..36ec371578a0 100644 --- a/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/package.kt +++ b/detekt-metrics/src/test/kotlin/io/github/detekt/metrics/processors/package.kt @@ -86,7 +86,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4 while (true) { if (false) { println() - } else { + } else { // 4 println() } } @@ -98,7 +98,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4 while (true) { if (false) { println() - } else { + } else { // 3 println() } } @@ -131,7 +131,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4 while (true) { if (false) { println() - } else { + } else { // 4 println() } } @@ -143,7 +143,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4 while (true) { if (false) { println() - } else { + } else { // 3 println() } } diff --git a/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtIfExpression.kt b/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtIfExpression.kt new file mode 100644 index 000000000000..9673bc36164b --- /dev/null +++ b/detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtIfExpression.kt @@ -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()?.expression == this diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseIfEmptyOrIfBlank.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseIfEmptyOrIfBlank.kt index 7ea3826042fa..37e8e053b123 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseIfEmptyOrIfBlank.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseIfEmptyOrIfBlank.kt @@ -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 @@ -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? { val condition = this.condition ?: return null return if (condition is KtPrefixExpression) {