diff --git a/detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethod.kt b/detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethod.kt index 59c586be0..e8148b29e 100644 --- a/detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethod.kt +++ b/detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethod.kt @@ -37,15 +37,23 @@ class LongMethod( Debt.TWENTY_MINS) private val functionToLinesCache = HashMap() + private val functionToBodyLinesCache = HashMap() private val nestedFunctionTracking = IdentityHashMap>() override fun preVisit(root: KtFile) { functionToLinesCache.clear() + functionToBodyLinesCache.clear() nestedFunctionTracking.clear() } override fun postVisit(root: KtFile) { - for ((function, lines) in functionToLinesCache) { + val functionToLines = HashMap() + functionToLinesCache.map { (function, lines) -> + val isNested = function.getStrictParentOfType() != null + if (isNested) functionToLines[function] = functionToBodyLinesCache[function] ?: 0 + else functionToLines[function] = lines + } + for ((function, lines) in functionToLines) { if (lines >= threshold) { report( ThresholdedCodeSmell( @@ -61,10 +69,12 @@ class LongMethod( } override fun visitNamedFunction(function: KtNamedFunction) { - val lines = function.linesOfCode() + val parentMethods = function.getStrictParentOfType() + val bodyEntity = function.bodyBlockExpression ?: function.bodyExpression + val lines = (if (parentMethods != null) function else bodyEntity)?.linesOfCode() ?: 0 functionToLinesCache[function] = lines - function.getStrictParentOfType() - ?.let { nestedFunctionTracking.getOrPut(it) { HashSet() }.add(function) } + functionToBodyLinesCache[function] = bodyEntity?.linesOfCode() ?: 0 + parentMethods?.let { nestedFunctionTracking.getOrPut(it) { HashSet() }.add(function) } super.visitNamedFunction(function) findAllNestedFunctions(function) .fold(0) { acc, next -> acc + (functionToLinesCache[next] ?: 0) } diff --git a/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethodSpec.kt b/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethodSpec.kt index f52b09b5a..8ecfc7423 100644 --- a/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethodSpec.kt +++ b/detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongMethodSpec.kt @@ -1,5 +1,6 @@ package io.gitlab.arturbosch.detekt.rules.complexity +import io.gitlab.arturbosch.detekt.api.ThresholdedCodeSmell import io.gitlab.arturbosch.detekt.test.assertThat import io.gitlab.arturbosch.detekt.test.compileAndLint import org.spekframework.spek2.Spek @@ -44,5 +45,108 @@ class LongMethodSpec : Spek({ assertThat(subject.compileAndLint(code)).isEmpty() } + + it("should not find too long method with params on newlines") { + val code = """ + fun methodWithParams( + param1: String + ) { // 4 lines + println() + println() + } + """ + + assertThat(subject.compileAndLint(code)).isEmpty() + } + + it("should find too long method with params on newlines") { + val code = """ + fun longMethodWithParams( + param1: String + ) { // 5 lines + println() + println() + println() + } + """ + + val findings = subject.compileAndLint(code) + + assertThat(findings).hasSize(1) + assertThat(findings[0] as ThresholdedCodeSmell).hasValue(5) + } + + it("should find long method with method call with params on separate lines") { + val code = """ + fun longMethod( + x1: Int, + x2: Int, + y1: Int, + y2: Int + ) { // 8 lines + listOf( + x1, + y1, + x2, + y2 + ) + } + """ + + val findings = subject.compileAndLint(code) + + assertThat(findings).hasSize(1) + assertThat(findings[0] as ThresholdedCodeSmell).hasValue(8) + } + + it("should find two long methods with params on separate lines") { + val code = """ + fun longMethod( + param1: String + ) { // 5 lines + println() + println() + println() + + fun nestedLongMethod( + param1: String + ) { // 5 lines + println() + println() + println() + } + } + """ + + val findings = subject.compileAndLint(code) + + assertThat(findings).hasSize(2) + assertThat(findings).hasTextLocations("longMethod", "nestedLongMethod") + } + + it("should find nested long methods with params on separate lines") { + val code = """ + fun longMethod( + param1: String + ) { // 4 lines + println() + println() + + fun nestedLongMethod( + param1: String + ) { // 5 lines + println() + println() + println() + } + } + """ + + val findings = subject.compileAndLint(code) + + assertThat(findings).hasSize(1) + assertThat(findings).hasTextLocations("nestedLongMethod") + assertThat(findings[0] as ThresholdedCodeSmell).hasValue(5) + } } })