From 1e696fd9fca384060f3e03a7711318f00523b93f Mon Sep 17 00:00:00 2001 From: Dominic Zirbel Date: Tue, 28 Jun 2022 06:28:01 -0700 Subject: [PATCH] Add MaxChainedCallsOnSameLine style rule (#4985) Add a new rule MaxChainedCallsOnSameLine to limit the number of chained calls on placed on a single line. This works well alongside CascadingCallWrapping in #4979 to make long call chains more readable by wrapping them on new lines. --- .../main/resources/default-detekt-config.yml | 3 + .../rules/style/MaxChainedCallsOnSameLine.kt | 79 +++++++++++++ .../detekt/rules/style/StyleGuideProvider.kt | 1 + .../style/MaxChainedCallsOnSameLineSpec.kt | 107 ++++++++++++++++++ 4 files changed, 190 insertions(+) create mode 100644 detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLine.kt create mode 100644 detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLineSpec.kt diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 2218451948a..462058ca590 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -582,6 +582,9 @@ style: active: false MandatoryBracesLoops: active: false + MaxChainedCallsOnSameLine: + active: false + maxChainedCalls: 5 MaxLineLength: active: true maxLineLength: 120 diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLine.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLine.kt new file mode 100644 index 00000000000..f9680a0fc0a --- /dev/null +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLine.kt @@ -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. + * + * + * a().b().c().d().e().f() + * + * + * + * a().b().c() + * .d().e().f() + * + */ +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) + + 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') + } +} diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt index 981e73134c6..4591807eb81 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt @@ -99,6 +99,7 @@ class StyleGuideProvider : DefaultRuleSetProvider { UseOrEmpty(config), UseAnyOrNoneInsteadOfFind(config), UnnecessaryBackticks(config), + MaxChainedCallsOnSameLine(config), ) ) } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLineSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLineSpec.kt new file mode 100644 index 00000000000..9d135a648d2 --- /dev/null +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/MaxChainedCallsOnSameLineSpec.kt @@ -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) + } +}