From 3f5489c1f9b1a1ebb0e613aa413f305b75bb2d9c Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Mon, 24 Oct 2022 12:30:36 +0900 Subject: [PATCH] CognitiveComplexity: count else/else-if as one complexity (#5458) * CognitiveComplexity: count else/else-if as one complexity * Don't increment nesting in else/else-if * Add +1 complexity and increase nesting level --- .../detekt/metrics/CognitiveComplexity.kt | 34 ++++++- .../detekt/metrics/CognitiveComplexitySpec.kt | 99 ++++++++++++++++++- .../CognitiveComplexityProcessorSpec.kt | 2 +- .../detekt/metrics/processors/package.kt | 8 +- detekt-psi-utils/api/detekt-psi-utils.api | 4 + .../arturbosch/detekt/rules/KtIfExpression.kt | 9 ++ .../detekt/rules/style/UseIfEmptyOrIfBlank.kt | 4 +- 7 files changed, 146 insertions(+), 14 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 969bc24728c..1be74733472 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,8 @@ 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 @@ -8,6 +10,7 @@ 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 @@ -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) { 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 6e808d1764e..b1b9f0fa125 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,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) + } + } } 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 b3d4ea669d3..d823010f012 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(50) } } 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 e9851a9e071..359346c7872 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 { // +1 println() } } @@ -98,7 +98,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4 while (true) { if (false) { println() - } else { + } else { // +1 println() } } @@ -131,7 +131,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4 while (true) { if (false) { println() - } else { + } else { // +1 println() } } @@ -143,7 +143,7 @@ class ComplexClass {// McCabe: 44, LLOC: 20 + 20 + 4x4 while (true) { if (false) { println() - } else { + } else { // +1 println() } } diff --git a/detekt-psi-utils/api/detekt-psi-utils.api b/detekt-psi-utils/api/detekt-psi-utils.api index 8469bb36e70..b1e2a840ba1 100644 --- a/detekt-psi-utils/api/detekt-psi-utils.api +++ b/detekt-psi-utils/api/detekt-psi-utils.api @@ -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 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 00000000000..9673bc36164 --- /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 7ea3826042f..37e8e053b12 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) {