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

MaxChainedCallsOnSameLine: don't count package references as chained calls #5036

Merged
merged 2 commits into from Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Expand Up @@ -9,11 +9,15 @@ 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 io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import org.jetbrains.kotlin.descriptors.PackageViewDescriptor
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtImportDirective
import org.jetbrains.kotlin.psi.KtPackageDirective
import org.jetbrains.kotlin.psi.KtQualifiedExpression
import org.jetbrains.kotlin.psi.KtReferenceExpression
import org.jetbrains.kotlin.psi.KtUnaryExpression
import org.jetbrains.kotlin.resolve.BindingContext

/**
* Limits the number of chained calls which can be placed on a single line.
Expand All @@ -27,6 +31,7 @@ import org.jetbrains.kotlin.psi.KtUnaryExpression
* .d().e().f()
* </compliant>
*/
@RequiresTypeResolution
class MaxChainedCallsOnSameLine(config: Config = Config.empty) : Rule(config) {
override val issue = Issue(
id = javaClass.simpleName,
Expand All @@ -38,9 +43,12 @@ class MaxChainedCallsOnSameLine(config: Config = Config.empty) : Rule(config) {
@Configuration("maximum chained calls allowed on a single line")
private val maxChainedCalls: Int by config(defaultValue = 5)

@Suppress("ReturnCount")
override fun visitQualifiedExpression(expression: KtQualifiedExpression) {
super.visitQualifiedExpression(expression)

if (bindingContext == BindingContext.EMPTY) return

val parent = expression.parent

// skip if the parent is also a call on the same line to avoid duplicated warnings
Expand All @@ -63,13 +71,21 @@ class MaxChainedCallsOnSameLine(config: Config = Config.empty) : Rule(config) {

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

private fun KtExpression.isReferenceToPackage(): Boolean {
val selectorOrThis = (this as? KtQualifiedExpression)?.selectorExpression ?: this
if (selectorOrThis !is KtReferenceExpression) return false
return bindingContext[BindingContext.REFERENCE_TARGET, selectorOrThis] is PackageViewDescriptor
}

private fun KtQualifiedExpression.callOnNewLine(): Boolean {
val receiver = receiverExpression
val selector = selectorExpression ?: return false
Expand Down
@@ -1,58 +1,61 @@
package io.gitlab.arturbosch.detekt.rules.style

import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
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.lint
import io.gitlab.arturbosch.detekt.test.lintWithContext
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.junit.jupiter.api.Test

class MaxChainedCallsOnSameLineSpec {
@KotlinCoreEnvironmentTest
class MaxChainedCallsOnSameLineSpec(private val env: KotlinCoreEnvironment) {
@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()
assertThat(rule.lintWithContext(env, code)).isEmpty()
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use compileLintWithContext in all this cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

@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()
assertThat(rule.lintWithContext(env, 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)
assertThat(rule.lintWithContext(env, 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)
assertThat(rule.lintWithContext(env, 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)
assertThat(rule.lintWithContext(env, 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)
assertThat(rule.lintWithContext(env, code)).hasSize(1)
}

@Test
Expand All @@ -66,7 +69,7 @@ class MaxChainedCallsOnSameLineSpec {
.plus(0)
"""

assertThat(rule.compileAndLint(code)).isEmpty()
assertThat(rule.lintWithContext(env, code)).isEmpty()
}

@Test
Expand All @@ -78,7 +81,7 @@ class MaxChainedCallsOnSameLineSpec {
.plus(0).plus(0).plus(0)
"""

assertThat(rule.compileAndLint(code)).isEmpty()
assertThat(rule.lintWithContext(env, code)).isEmpty()
}

@Test
Expand All @@ -90,7 +93,7 @@ class MaxChainedCallsOnSameLineSpec {
.plus(0)
"""

assertThat(rule.compileAndLint(code)).hasSize(1)
assertThat(rule.lintWithContext(env, code)).hasSize(1)
}

@Test
Expand All @@ -103,7 +106,7 @@ class MaxChainedCallsOnSameLineSpec {
.plus(0)
"""

assertThat(rule.compileAndLint(code)).hasSize(1)
assertThat(rule.lintWithContext(env, code)).hasSize(1)
}

@Test
Expand All @@ -119,6 +122,32 @@ class MaxChainedCallsOnSameLineSpec {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = "package a.b.c.d.e"

assertThat(rule.lint(code)).isEmpty()
assertThat(rule.lintWithContext(env, code)).isEmpty()
}

@Test
fun `does not count package references as chained calls`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = """
package a.b.c
fun foo() = 1
fun test() {
a.b.c.foo().plus(0).plus(0)
}
"""
assertThat(rule.lintWithContext(env, code)).isEmpty()
}

@Test
fun `does not count a package reference as chained calls`() {
val rule = MaxChainedCallsOnSameLine(TestConfig(mapOf("maxChainedCalls" to 3)))
val code = """
package a
fun foo() = 1
fun test() {
a.foo().plus(0).plus(0)
}
"""
assertThat(rule.lintWithContext(env, code)).isEmpty()
}
}