Skip to content

Commit

Permalink
Separating ComplexMethod rule into CyclomaticComplexMethod and Cognit…
Browse files Browse the repository at this point in the history
…iveComplexMethod (#5442)

* feature: add rule CognitiveComplexMethod

* test: add test for CognitiveComplexMethod rule

* move: ComplexMethod -> CyclomaticComplexMethod

* fix: remove referencing ComplexMethod

* fix: detekt correct

* fix: set alias

* config: deactivate default cognitive complex method rule

* test: add positive and negative test cases

* Update detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/CyclomaticComplexMethod.kt

Co-authored-by: Brais Gabín <braisgabin@gmail.com>

* fix: remove conflicting activeByDefault and sync

Co-authored-by: Brais Gabín <braisgabin@gmail.com>
  • Loading branch information
sanggggg and BraisGabin committed Oct 23, 2022
1 parent 7697efd commit 7b632b2
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 19 deletions.
2 changes: 1 addition & 1 deletion config/detekt/detekt.yml
Expand Up @@ -23,7 +23,7 @@ complexity:
threshold: 10
includeStaticDeclarations: false
includePrivateDeclarations: false
ComplexMethod:
CyclomaticComplexMethod:
active: true
ignoreSingleWhenExpression: true
LargeClass:
Expand Down
5 changes: 4 additions & 1 deletion detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -91,6 +91,9 @@ comments:

complexity:
active: true
CognitiveComplexMethod:
active: false
threshold: 15
ComplexCondition:
active: true
threshold: 4
Expand All @@ -100,7 +103,7 @@ complexity:
includeStaticDeclarations: false
includePrivateDeclarations: false
ignoreOverloaded: false
ComplexMethod:
CyclomaticComplexMethod:
active: true
threshold: 15
ignoreSingleWhenExpression: false
Expand Down
1 change: 1 addition & 0 deletions detekt-core/src/main/resources/deprecation.properties
Expand Up @@ -17,3 +17,4 @@ formatting>TrailingComma=Rule is split between `TrailingCommaOnCallSite` and `Tr
style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin
style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin
style>LibraryEntitiesShouldNotBePublic=Rule migrated to `libraries` ruleset plugin
complexity>ComplexMethod=Rule is renamed to `CyclomaticComplexMethod` to distinguish between Cyclomatic Complexity and Cognitive Complexity
Expand Up @@ -41,5 +41,6 @@ internal fun writeMigratedRules(): String {
style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin
style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin
style>LibraryEntitiesShouldNotBePublic=Rule migrated to `libraries` ruleset plugin
complexity>ComplexMethod=Rule is renamed to `CyclomaticComplexMethod` to distinguish between Cyclomatic Complexity and Cognitive Complexity
""".trimIndent()
}
@@ -0,0 +1,60 @@
package io.gitlab.arturbosch.detekt.rules.complexity

import io.github.detekt.metrics.CognitiveComplexity
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.Metric
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.ThresholdedCodeSmell
import io.gitlab.arturbosch.detekt.api.config
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import org.jetbrains.kotlin.psi.KtNamedFunction

/**
* Complex methods are hard to understand and read. It might not be obvious what side-effects a complex method has.
* Prefer splitting up complex methods into smaller methods that are in turn easier to understand.
* Smaller methods can also be named much clearer which leads to improved readability of the code.
*
* This rule measures and restricts the complexity of the method through the [Cognitive Complexity metric of Sonasource](https://www.sonarsource.com/docs/CognitiveComplexity.pdf).
* Which improves McCabe's Cyclomatic Complexity ({@link CyclomaticComplexMethod}) considering the programmer's mental model.
*
* Similar to cyclomatic complexity, it is a mathematical model that increases +1 complexity for flow control statements,
* but increases additional complexity when the statements are deeply nested.
*
* The statements that increase the complexity or the nesting level are as follows.
* - __Complexity Increments__ - `if`, `when`, `for`, `while`, `do while`, `catch`, `labeled break`, `labeled continue`, `labeled return`, `recursion call`, `&&`, `||`
* - __Nesting Level Increments__ - `if`, `when`, `for`, `while`, `do while`, `catch`, `nested function`
* - __Additional Complexity Increments by Nesting Level__ - `if`, `when`, `for`, `while`, `do while`, `catch`
*/
class CognitiveComplexMethod(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
"CognitiveComplexMethod",
Severity.Maintainability,
"Prefer splitting up complex methods into smaller, easier to understand methods.",
Debt.TWENTY_MINS
)

@Configuration("Cognitive Complexity number for a method.")
private val threshold: Int by config(defaultValue = 15)

override fun visitNamedFunction(function: KtNamedFunction) {
val complexity = CognitiveComplexity.calculate(function)

if (complexity >= threshold) {
report(
ThresholdedCodeSmell(
issue,
Entity.atName(function),
Metric("CC", complexity, threshold),
"The function ${function.nameAsSafeName} appears to be too complex " +
"based on Cognitive Complexity (complexity: $complexity). " +
"Defined complexity threshold for methods is set to '$threshold'"
)
)
}
}
}
Expand Up @@ -20,7 +20,8 @@ class ComplexityProvider : DefaultRuleSetProvider {
LongMethod(config),
LargeClass(config),
ComplexInterface(config),
ComplexMethod(config),
CyclomaticComplexMethod(config),
CognitiveComplexMethod(config),
StringLiteralDuplication(config),
MethodOverloading(config),
NestedBlockDepth(config),
Expand Down
Expand Up @@ -38,15 +38,17 @@ import org.jetbrains.kotlin.psi.KtWhenExpression
* [Reference](https://kotlinlang.org/docs/scope-functions.html)
*/
@ActiveByDefault(since = "1.0.0")
class ComplexMethod(config: Config = Config.empty) : Rule(config) {
class CyclomaticComplexMethod(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
"ComplexMethod",
"CyclomaticComplexMethod",
Severity.Maintainability,
"Prefer splitting up complex methods into smaller, easier to understand methods.",
"Prefer splitting up complex methods into smaller, easier to test methods.",
Debt.TWENTY_MINS
)

override val defaultRuleIdAliases: Set<String> = setOf("ComplexMethod")

@Configuration("McCabe's Cyclomatic Complexity (MCC) number for a method.")
private val threshold: Int by config(defaultValue = 15)

Expand All @@ -68,9 +70,9 @@ class ComplexMethod(config: Config = Config.empty) : Rule(config) {
}

val complexity = CyclomaticComplexity.calculate(function) {
this.ignoreSimpleWhenEntries = this@ComplexMethod.ignoreSimpleWhenEntries
this.ignoreNestingFunctions = this@ComplexMethod.ignoreNestingFunctions
this.nestingFunctions = this@ComplexMethod.nestingFunctions
this.ignoreSimpleWhenEntries = this@CyclomaticComplexMethod.ignoreSimpleWhenEntries
this.ignoreNestingFunctions = this@CyclomaticComplexMethod.ignoreNestingFunctions
this.nestingFunctions = this@CyclomaticComplexMethod.nestingFunctions
}

if (complexity >= threshold) {
Expand All @@ -79,7 +81,8 @@ class ComplexMethod(config: Config = Config.empty) : Rule(config) {
issue,
Entity.atName(function),
Metric("MCC", complexity, threshold),
"The function ${function.nameAsSafeName} appears to be too complex ($complexity). " +
"The function ${function.nameAsSafeName} appears to be too complex " +
"based on Cyclomatic Complexity (complexity: $complexity). " +
"Defined complexity threshold for methods is set to '$threshold'"
)
)
Expand Down
@@ -0,0 +1,47 @@
package io.gitlab.arturbosch.detekt.rules.complexity

import io.gitlab.arturbosch.detekt.api.SourceLocation
import io.gitlab.arturbosch.detekt.test.TestConfig
import io.gitlab.arturbosch.detekt.test.assertThat
import io.gitlab.arturbosch.detekt.test.compileAndLint
import io.gitlab.arturbosch.detekt.test.isThresholded
import org.junit.jupiter.api.Test

class CognitiveComplexMethodSpec {

private val testConfig = TestConfig("threshold" to "1")

@Test
fun `should report complex function`() {
val code = """
fun sumOfPrimes(max: Int): Int { // total cognitive complexity is 7
var total = 0
next@ for (i in 1..max) { // +1
for (j in 2 until i) { // +1 +1
if (i % j == 0) { // +1 +2
continue@next // +1
}
}
println(i)
total++
}
return total
}
""".trimIndent()
val findings = CognitiveComplexMethod(testConfig).compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings).hasStartSourceLocations(SourceLocation(1, 5))
assertThat(findings.first()).isThresholded().withValue(7)
}

@Test
fun `should not report simple function`() {
val code = """
fun add(a: Int, b: Int): Int { // total cognitive complexity is 0
return a + b
}
""".trimIndent()
val findings = CognitiveComplexMethod(testConfig).compileAndLint(code)
assertThat(findings).isEmpty()
}
}
Expand Up @@ -12,7 +12,7 @@ import org.junit.jupiter.api.Test

private val defaultConfigMap: Map<String, Any> = mapOf("threshold" to "1")

class ComplexMethodSpec {
class CyclomaticComplexMethodSpec {

val defaultComplexity = 1

Expand All @@ -21,7 +21,7 @@ class ComplexMethodSpec {

@Test
fun `counts different loops`() {
val findings = ComplexMethod(TestConfig(defaultConfigMap)).compileAndLint(
val findings = CyclomaticComplexMethod(TestConfig(defaultConfigMap)).compileAndLint(
"""
fun test() {
for (i in 1..10) {}
Expand All @@ -37,7 +37,7 @@ class ComplexMethodSpec {

@Test
fun `counts catch blocks`() {
val findings = ComplexMethod(TestConfig(defaultConfigMap)).compileAndLint(
val findings = CyclomaticComplexMethod(TestConfig(defaultConfigMap)).compileAndLint(
"""
fun test() {
try {} catch(e: IllegalArgumentException) {} catch(e: Exception) {} finally {}
Expand All @@ -50,7 +50,7 @@ class ComplexMethodSpec {

@Test
fun `counts nested conditional statements`() {
val findings = ComplexMethod(TestConfig(defaultConfigMap)).compileAndLint(
val findings = CyclomaticComplexMethod(TestConfig(defaultConfigMap)).compileAndLint(
"""
fun test() {
try {
Expand Down Expand Up @@ -127,15 +127,15 @@ class ComplexMethodSpec {
"ignoreSingleWhenExpression" to "true"
)
)
val subject = ComplexMethod(config)
val subject = CyclomaticComplexMethod(config)

assertThat(subject.lint(path)).hasStartSourceLocations(SourceLocation(43, 5))
}

@Test
fun `reports all complex methods`() {
val config = TestConfig(mapOf("threshold" to "4"))
val subject = ComplexMethod(config)
val subject = CyclomaticComplexMethod(config)

assertThat(subject.lint(path)).hasStartSourceLocations(
SourceLocation(6, 5),
Expand All @@ -149,7 +149,7 @@ class ComplexMethodSpec {
@Test
fun `does not trip for a reasonable amount of simple when entries when ignoreSimpleWhenEntries is true`() {
val config = TestConfig(mapOf("ignoreSimpleWhenEntries" to "true"))
val subject = ComplexMethod(config)
val subject = CyclomaticComplexMethod(config)
val code = """
fun f() {
val map = HashMap<Any, String>()
Expand Down Expand Up @@ -227,13 +227,13 @@ class ComplexMethodSpec {

@Test
fun `should not count these overridden functions to base functions complexity`() {
assertThat(ComplexMethod().compileAndLint(code)).isEmpty()
assertThat(CyclomaticComplexMethod().compileAndLint(code)).isEmpty()
}
}
}

private fun assertExpectedComplexityValue(code: String, config: TestConfig, expectedValue: Int) {
val findings = ComplexMethod(config).lint(code)
val findings = CyclomaticComplexMethod(config).lint(code)

assertThat(findings).hasStartSourceLocations(SourceLocation(1, 5))

Expand Down

0 comments on commit 7b632b2

Please sign in to comment.