Skip to content

Commit

Permalink
Improve error reporting for CascadingCallWrapping (#5439)
Browse files Browse the repository at this point in the history
Make the error report point of failure more precisely by highlighting
the exact part of expression which should be wrapped to a new line

Closes #5043
  • Loading branch information
dimsuz committed Oct 19, 2022
1 parent 5500354 commit d643963
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 5 deletions.
@@ -1,19 +1,24 @@
package io.gitlab.arturbosch.detekt.rules.style

import io.github.detekt.psi.toFilePath
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.Location
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.TextLocation
import io.gitlab.arturbosch.detekt.api.config
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtQualifiedExpression
import org.jetbrains.kotlin.psi.KtUnaryExpression
import org.jetbrains.kotlin.psi.psiUtil.endOffset
import org.jetbrains.kotlin.psi.psiUtil.startOffset

/**
* Requires that all chained calls are placed on a new line if a preceding one is.
Expand Down Expand Up @@ -62,7 +67,7 @@ class CascadingCallWrapping(config: Config = Config.empty) : Rule(config) {
report(
CodeSmell(
issue = issue,
entity = Entity.from(expression),
entity = expression.toErrorReportEntity(),
message = "Chained call$callTextOrEmpty should be wrapped to a new line since preceding calls were."
)
)
Expand Down Expand Up @@ -106,4 +111,21 @@ class CascadingCallWrapping(config: Config = Config.empty) : Rule(config) {
else -> false
}
}

private fun KtExpression.toErrorReportEntity(): Entity {
return when (this) {
is KtQualifiedExpression -> Entity.from(this.selectorExpression ?: this)
is KtBinaryExpression -> {
val rhs = this.right ?: return Entity.from(this)
val operationSourceLocation = Location.from(operationReference).source
val rhsSourceLocation = Location.from(rhs).endSource
val textLocation = TextLocation(operationReference.startOffset, rhs.endOffset)
Entity.from(
this,
Location(operationSourceLocation, rhsSourceLocation, textLocation, containingFile.toFilePath())
)
}
else -> Entity.from(this)
}
}
}
Expand Up @@ -19,7 +19,7 @@ class CascadingCallWrappingSpec {

assertThat(subject.compileAndLint(code))
.hasSize(1)
.hasTextLocations(8 to 30)
.hasTextLocations(23 to 30)
.first()
.hasMessage("Chained call `plus(0)` should be wrapped to a new line since preceding calls were.")
}
Expand Down Expand Up @@ -119,7 +119,9 @@ class CascadingCallWrappingSpec {
)
""".trimIndent()

assertThat(subject.compileAndLint(code)).hasSize(1)
assertThat(subject.compileAndLint(code))
.hasTextLocations(65 to 86)
.hasSize(1)
}

@Test
Expand Down Expand Up @@ -168,10 +170,27 @@ class CascadingCallWrappingSpec {
fun `reports missing wrapping`() {
val code = """
val a = 0
.plus(0) ?: 0
.plus(0) ?: 42
""".trimIndent()

assertThat(subjectIncludingElvis.compileAndLint(code)).hasSize(1)
assertThat(subjectIncludingElvis.compileAndLint(code))
.hasTextLocations(23 to 28)
.hasSize(1)
assertThat(subjectExcludingElvis.compileAndLint(code)).isEmpty()
}

@Test
fun `reports missing wrapping multiline call`() {
val code = """
val a = 0
.plus(0) ?: let {
42
}
""".trimIndent()

assertThat(subjectIncludingElvis.compileAndLint(code))
.hasTextLocations(23 to 38)
.hasSize(1)
assertThat(subjectExcludingElvis.compileAndLint(code)).isEmpty()
}
}
Expand Down

0 comments on commit d643963

Please sign in to comment.