Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MaxChainedCallsOnSameLine style rule #4985

Merged
merged 1 commit into from Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -579,6 +579,9 @@ style:
active: false
MandatoryBracesLoops:
active: false
MaxChainedCallsOnSameLine:
active: false
maxChainedCalls: 5
MaxLineLength:
active: true
maxLineLength: 120
Expand Down
@@ -0,0 +1,79 @@
package io.gitlab.arturbosch.detekt.rules.style

import io.gitlab.arturbosch.detekt.api.CodeSmell
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.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.config
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtQualifiedExpression
import org.jetbrains.kotlin.psi.KtUnaryExpression

/**
* Limits the number of chained calls which can be placed on a single line.
*
* <noncompliant>
* a().b().c().d().e().f()
* </noncompliant>
*
* <compliant>
* a().b().c()
* .d().e().f()
* </compliant>
*/
class MaxChainedCallsOnSameLine(config: Config = Config.empty) : Rule(config) {
override val issue = Issue(
id = javaClass.simpleName,
severity = Severity.Style,
description = "Chained calls beyond the maximum should be wrapped to a new line.",
debt = Debt.FIVE_MINS,
)

@Configuration("maximum chained calls allowed on a single line")
private val maxChainedCalls: Int by config(defaultValue = 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Isn't 5 far too much? I would say 3 or 4. But this is really subjective.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was leaning toward making it more conservative, since in my experience it's not uncommon to have calls on e.g. network models that can be at least a few references deep. But I don't feel strongly; happy to change to any other value.


override fun visitQualifiedExpression(expression: KtQualifiedExpression) {
super.visitQualifiedExpression(expression)

// skip if the parent is also a call on the same line to avoid duplicated warnings
val parent = expression.parent
if (parent is KtQualifiedExpression && !parent.callOnNewLine()) return

val chainedCalls = expression.countChainedCalls() + 1
if (chainedCalls > maxChainedCalls) {
report(
CodeSmell(
issue = issue,
entity = Entity.from(expression),
message = "$chainedCalls chained calls on a single line; more than $maxChainedCalls calls should " +
"be wrapped to a new line."
)
)
}
}

private fun KtExpression.countChainedCalls(): Int {
return when (this) {
is KtQualifiedExpression ->
if (callOnNewLine()) 0 else receiverExpression.countChainedCalls() + 1
is KtUnaryExpression -> baseExpression?.countChainedCalls() ?: 0
else -> 0
}
}

private fun KtQualifiedExpression.callOnNewLine(): Boolean {
val receiver = receiverExpression
val selector = selectorExpression ?: return false

val receiverEnd = receiver.startOffsetInParent + receiver.textLength
val selectorStart = selector.startOffsetInParent

return text
.subSequence(startIndex = receiverEnd, endIndex = selectorStart)
.contains('\n')
}
}
Expand Up @@ -98,6 +98,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
UseOrEmpty(config),
UseAnyOrNoneInsteadOfFind(config),
UnnecessaryBackticks(config),
MaxChainedCallsOnSameLine(config),
)
)
}
@@ -0,0 +1,107 @@
package io.gitlab.arturbosch.detekt.rules.style

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 MaxChainedCallsOnSameLineSpec {
@Test
fun `does not report 2 calls on a single line with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = "val a = 0.plus(0)"

assertThat(rule.compileAndLint(code)).isEmpty()
}

@Test
fun `does not report 3 calls on a single line with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = "val a = 0.plus(0).plus(0)"

assertThat(rule.compileAndLint(code)).isEmpty()
}

@Test
fun `reports 4 calls on a single line with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = "val a = 0.plus(0).plus(0).plus(0)"

assertThat(rule.compileAndLint(code)).hasSize(1)
}

@Test
fun `reports 4 safe qualified calls on a single line with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = "val a = 0?.plus(0)?.plus(0)?.plus(0)"

assertThat(rule.compileAndLint(code)).hasSize(1)
}

@Test
fun `reports 4 non-null asserted calls on a single line with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = "val a = 0!!.plus(0)!!.plus(0)!!.plus(0)"

assertThat(rule.compileAndLint(code)).hasSize(1)
}

@Test
fun `reports once for 7 calls on a single line with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = "val a = 0.plus(0).plus(0).plus(0).plus(0).plus(0).plus(0)"

assertThat(rule.compileAndLint(code)).hasSize(1)
}

@Test
fun `does not report 5 calls on separate lines with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = """
val a = 0
.plus(0)
.plus(0)
.plus(0)
.plus(0)
"""

assertThat(rule.compileAndLint(code)).isEmpty()
}

@Test
fun `does not report 3 calls on same line with wrapped calls with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = """
val a = 0.plus(0).plus(0)
.plus(0).plus(0).plus(0)
.plus(0).plus(0).plus(0)
"""

assertThat(rule.compileAndLint(code)).isEmpty()
}

@Test
fun `reports 4 calls on same line with wrapped calls with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = """
val a = 0.plus(0).plus(0).plus(0)
.plus(0)
.plus(0)
"""

assertThat(rule.compileAndLint(code)).hasSize(1)
}

@Test
fun `reports 4 calls on wrapped line with with a max of 3`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = """
val a = 0
.plus(0)
.plus(0).plus(0).plus(0).plus(0)
.plus(0)
"""

assertThat(rule.compileAndLint(code)).hasSize(1)
}
}