Skip to content

Commit

Permalink
Add NestedScopeFunctions Rule
Browse files Browse the repository at this point in the history
  • Loading branch information
WildOrangutan committed Apr 28, 2022
1 parent f53a497 commit f5cdb11
Show file tree
Hide file tree
Showing 4 changed files with 208 additions and 0 deletions.
7 changes: 7 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -133,6 +133,13 @@ complexity:
NestedBlockDepth:
active: true
threshold: 4
NestedScopeFunctions:
active: false
threshold: 1
functions:
- 'apply'
- 'run'
- 'with'
ReplaceSafeCallChainWithRun:
active: false
StringLiteralDuplication:
Expand Down
Expand Up @@ -24,6 +24,7 @@ class ComplexityProvider : DefaultRuleSetProvider {
StringLiteralDuplication(config),
MethodOverloading(config),
NestedBlockDepth(config),
NestedScopeFunctions(config),
TooManyFunctions(config),
ComplexCondition(config),
LabeledExpression(config),
Expand Down
@@ -0,0 +1,95 @@
package io.gitlab.arturbosch.detekt.rules.complexity

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.DetektVisitor
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.KtCallExpression
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.psiUtil.getCallNameExpression

/**
* Although the scope functions are a way of making the code more concise, avoid overusing them: it can decrease
* your code readability and lead to errors. Avoid nesting scope functions and be careful when chaining them:
* it's easy to get confused about the current context object and the value of this or it.
*
* [Reference](https://kotlinlang.org/docs/scope-functions.html)
*/
class NestedScopeFunctions(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
"ComplexMethod",
Severity.Maintainability,
"Over-using scope functions makes code confusing, hard to read and bug prone.",
Debt.FIVE_MINS
)

@Configuration("Number of nested scope functions allowed.")
private val threshold: Int by config(defaultValue = 1)

@Configuration("Set of scope function names which add complexity.")
private val functions: Set<String> by config(DEFAULT_FUNCTIONS) { it.toSet() }

override fun visitNamedFunction(function: KtNamedFunction) {
function.accept(FunctionDepthVisitor())
}

private fun report(element: KtCallExpression, depth: Int) {
val finding = ThresholdedCodeSmell(
issue,
Entity.from(element),
Metric("SIZE", depth, threshold),
"The scope function '${element.calleeExpression?.text}' is nested too deeply ('$depth'). " +
"Complexity threshold is set to '$threshold'."
)
report(finding)
}

private companion object {
val DEFAULT_FUNCTIONS = listOf(
"apply",
"run",
"with",
)
}

private inner class FunctionDepthVisitor : DetektVisitor() {
private var depth = 0

override fun visitCallExpression(expression: KtCallExpression) {
fun callSuper() = super.visitCallExpression(expression)

if (expression.isScopeFunction()) {
doWithIncrementedDepth {
reportIfOverThreshold(expression)
callSuper()
}
} else {
callSuper()
}
}

private fun doWithIncrementedDepth(block: () -> Unit) {
depth++
block()
depth--
}

private fun reportIfOverThreshold(expression: KtCallExpression) {
if (depth > threshold) {
report(expression, depth)
}
}

private fun KtCallExpression.isScopeFunction(): Boolean =
getCallNameExpression()?.text?.let { functions.contains(it) }
?: false
}
}
@@ -0,0 +1,105 @@
package io.gitlab.arturbosch.detekt.rules.complexity

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

class NestedScopeFunctionsSpec {

private var defaultConfig = TestConfig(mapOf("threshold" to 1, "functions" to listOf("one", "two")))
private val subject = NestedScopeFunctions(defaultConfig)

private lateinit var givenCode: String
private lateinit var actual: List<Finding>

@Test
fun `should report nesting`() {
givenCode = """
fun f() {
1.one {
1.one { }
}
}
"""
whenLintRuns()
expectSourceLocation(3 to 11)
expectFunctionInMsg("one")
}

@Test
fun `should report mixed nesting`() {
givenCode = """
fun f() {
1.one {
two(1) { }
}
}
"""
whenLintRuns()
expectSourceLocation(3 to 9)
expectFunctionInMsg("two")
}

@Test
fun `should report when valid scope in between`() {
givenCode = """
fun f() {
1.one {
1.valid {
two(1) { }
}
}
}
"""
whenLintRuns()
expectSourceLocation(4 to 13)
}

@Test
fun `should not report in nested function`() {
givenCode = """
fun f() {
1.one { }
fun f2() {
1.two { }
}
}
"""
whenLintRuns()
expectNoFindings()
}

@Test
fun `should not report in neighboring scope functions`() {
givenCode = """
fun f() {
1.one { }
1.one { }
two(1) {}
two(1) {}
}
"""
whenLintRuns()
expectNoFindings()
}

private fun whenLintRuns() {
actual = subject.compileAndLint(givenCode)
}

private fun expectSourceLocation(location: Pair<Int, Int>) {
assertThat(actual).hasSourceLocation(location.first, location.second)
}

private fun expectFunctionInMsg(scopeFunction: String) {
val expected = "The scope function '$scopeFunction' is nested too deeply ('2'). " +
"Complexity threshold is set to '1'."
assertThat(actual[0]).hasMessage(expected)
}

private fun expectNoFindings() {
assertThat(actual).describedAs("findings size").isEmpty()
}
}

0 comments on commit f5cdb11

Please sign in to comment.