Skip to content

Commit

Permalink
Add CascadingCallWrapping style rule (#4979)
Browse files Browse the repository at this point in the history
Add a new rule CascadingCallWrapping which requires that if a chained call is placed on a newline then all subsequent calls must be as well, improving readability of long chains.
  • Loading branch information
dzirbel committed Jun 22, 2022
1 parent 0e67d14 commit 89f6ec1
Show file tree
Hide file tree
Showing 16 changed files with 324 additions and 15 deletions.
2 changes: 2 additions & 0 deletions config/detekt/detekt.yml
Expand Up @@ -155,6 +155,8 @@ potential-bugs:
style:
CanBeNonNullable:
active: true
CascadingCallWrapping:
active: true
ClassOrdering:
active: true
CollapsibleIfStatements:
Expand Down
Expand Up @@ -17,7 +17,8 @@ open class SplitPattern(
.mapIf(removeTrailingAsterisks) { seq ->
seq.map { it.removePrefix("*") }
.map { it.removeSuffix("*") }
}.toList()
}
.toList()

private fun <T> Sequence<T>.mapIf(
condition: Boolean,
Expand Down
3 changes: 3 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -490,6 +490,9 @@ style:
active: true
CanBeNonNullable:
active: false
CascadingCallWrapping:
active: false
includeElvis: true
ClassOrdering:
active: false
CollapsibleIfStatements:
Expand Down
Expand Up @@ -171,7 +171,9 @@ class ConfigurationCollector {

private fun KtValueArgument.getReferenceIdentifierOrNull(): String? =
(getArgumentExpression() as? KtCallableReferenceExpression)
?.callableReference?.getIdentifier()?.text
?.callableReference
?.getIdentifier()
?.text
}

private object ConfigWithAndroidVariantsSupport {
Expand Down
Expand Up @@ -64,7 +64,8 @@ class MultiRuleVisitor : DetektVisitor() {
override fun visitSuperTypeList(list: KtSuperTypeList) {
val isMultiRule = list.entries
?.mapNotNull { it.typeAsUserType?.referencedName }
?.any { it == multiRule } ?: false
?.any { it == multiRule }
?: false

val containingClass = list.containingClass()
val className = containingClass?.name
Expand Down
Expand Up @@ -63,7 +63,8 @@ internal class RuleVisitor : DetektVisitor() {
val isRule = list.entries
?.asSequence()
?.map { it.typeAsUserType?.referencedName }
?.any { ruleClasses.contains(it) } ?: false
?.any { ruleClasses.contains(it) }
?: false

val containingClass = list.containingClass()
val className = containingClass?.name
Expand Down Expand Up @@ -138,7 +139,8 @@ internal class RuleVisitor : DetektVisitor() {
.singleOrNull { it.name == "issue" }
?.initializer as? KtCallExpression
)
?.valueArguments.orEmpty()
?.valueArguments
.orEmpty()

if (arguments.size >= ISSUE_ARGUMENT_SIZE) {
severity = getArgument(arguments[1], "Severity")
Expand Down
Expand Up @@ -47,7 +47,8 @@ open class DetektExtension @Inject constructor(objects: ObjectFactory) : CodeQua

var baseline: File? = objects.fileProperty()
.fileValue(File("detekt-baseline.xml"))
.get().asFile
.get()
.asFile

var basePath: String? = null

Expand Down
Expand Up @@ -156,7 +156,8 @@ class TooManyFunctions(config: Config = Config.empty) : Rule(config) {
declarations
.filterIsInstance<KtNamedFunction>()
.count { !isIgnoredFunction(it) }
} ?: 0
}
?: 0

private fun isIgnoredFunction(function: KtNamedFunction): Boolean = when {
ignoreDeprecated && function.hasAnnotation(DEPRECATED) -> true
Expand Down
Expand Up @@ -67,7 +67,10 @@ class SuspendFunWithCoroutineScopeReceiver(config: Config) : Rule(config) {
private fun checkReceiver(function: KtNamedFunction) {
val suspendModifier = function.modifierList?.getModifier(KtTokens.SUSPEND_KEYWORD) ?: return
val receiver = bindingContext[BindingContext.FUNCTION, function]
?.extensionReceiverParameter?.value?.type ?: return
?.extensionReceiverParameter
?.value
?.type
?: return
if (receiver.isCoroutineScope()) {
report(
CodeSmell(
Expand Down
Expand Up @@ -14,7 +14,6 @@ import io.gitlab.arturbosch.detekt.rules.isNonNullCheck
import io.gitlab.arturbosch.detekt.rules.isNullCheck
import io.gitlab.arturbosch.detekt.rules.isOpen
import io.gitlab.arturbosch.detekt.rules.isOverride
import org.jetbrains.kotlin.com.intellij.codeInsight.NullableNotNullManager.isNullable
import org.jetbrains.kotlin.descriptors.CallableDescriptor
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
Expand Down Expand Up @@ -137,11 +136,13 @@ class CanBeNonNullable(config: Config = Config.empty) : Rule(config) {
function.valueParameters.asSequence()
.filter {
it.typeReference?.typeElement is KtNullableType
}.mapNotNull { parameter ->
}
.mapNotNull { parameter ->
bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, parameter]?.let {
it to parameter
}
}.forEach { (descriptor, param) ->
}
.forEach { (descriptor, param) ->
candidateDescriptors.add(descriptor)
nullableParams[descriptor] = NullableParam(param)
}
Expand Down Expand Up @@ -175,7 +176,8 @@ class CanBeNonNullable(config: Config = Config.empty) : Rule(config) {
.filter {
val onlyNonNullCheck = validSingleChildExpression && it.isNonNullChecked && !it.isNullChecked
it.isNonNullForced || onlyNonNullCheck
}.forEach { nullableParam ->
}
.forEach { nullableParam ->
report(
CodeSmell(
issue,
Expand Down
@@ -0,0 +1,109 @@
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.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

/**
* Requires that all chained calls are placed on a new line if a preceding one is.
*
* <noncompliant>
* foo()
* .bar().baz()
* </noncompliant>
*
* <compliant>
* foo().bar().baz()
*
* foo()
* .bar()
* .baz()
* </compliant>
*/
class CascadingCallWrapping(config: Config = Config.empty) : Rule(config) {
override val issue = Issue(
id = javaClass.simpleName,
severity = Severity.Style,
description = "If a chained call is wrapped to a new line, subsequent chained calls should be as well.",
debt = Debt.FIVE_MINS,
)

@Configuration("require trailing elvis expressions to be wrapped on a new line")
private val includeElvis: Boolean by config(true)

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

checkExpression(expression, callExpression = expression.selectorExpression)
}

override fun visitBinaryExpression(expression: KtBinaryExpression) {
super.visitBinaryExpression(expression)

if (includeElvis && expression.operationToken == KtTokens.ELVIS) {
checkExpression(expression, callExpression = expression.right)
}
}

private fun checkExpression(expression: KtExpression, callExpression: KtExpression?) {
if (!expression.containsNewline() && expression.receiverContainsNewline()) {
val callTextOrEmpty = callExpression?.text?.let { " `$it`" }.orEmpty()
report(
CodeSmell(
issue = issue,
entity = Entity.from(expression),
message = "Chained call$callTextOrEmpty should be wrapped to a new line since preceding calls were."
)
)
}
}

@Suppress("ReturnCount")
private fun KtExpression.containsNewline(): Boolean {
val lhs: KtExpression
val rhs: KtExpression

when (this) {
is KtQualifiedExpression -> {
lhs = receiverExpression
rhs = selectorExpression ?: return false
}
is KtBinaryExpression -> {
if (operationToken != KtTokens.ELVIS) return false
lhs = left ?: return false
rhs = right ?: return false
}
else -> return false
}

val receiverEnd = lhs.startOffsetInParent + lhs.textLength
val selectorStart = rhs.startOffsetInParent

return (receiverEnd until selectorStart).any { text[it] == '\n' }
}

private fun KtExpression.receiverContainsNewline(): Boolean {
val lhs = when (this) {
is KtQualifiedExpression -> receiverExpression
is KtBinaryExpression -> left ?: return false
else -> return false
}

return when (lhs) {
is KtQualifiedExpression -> lhs.containsNewline()
is KtUnaryExpression -> (lhs.baseExpression as? KtQualifiedExpression)?.containsNewline() == true
else -> false
}
}
}
Expand Up @@ -80,7 +80,8 @@ class ForbiddenVoid(config: Config = Config.empty) : Rule(config) {
private fun KtTypeReference.isPartOfReturnTypeOfFunction() =
getStrictParentOfType<KtNamedFunction>()
?.typeReference
?.anyDescendantOfType<KtTypeReference> { it == this } ?: false
?.anyDescendantOfType<KtTypeReference> { it == this }
?: false

private fun KtTypeReference.isParameterTypeOfFunction() =
getStrictParentOfType<KtParameter>() != null
Expand Down
Expand Up @@ -102,7 +102,8 @@ class ObjectLiteralToLambda(config: Config = Config.empty) : Rule(config) {
if (
declaration.name == null &&
bindingContext.getType(expression)
?.singleSuperTypeOrNull()?.couldBeSamInterface == true &&
?.singleSuperTypeOrNull()
?.couldBeSamInterface == true &&
declaration.hasConvertibleMethod()
) {
report(CodeSmell(issue, Entity.from(expression), issue.description))
Expand Down
Expand Up @@ -100,7 +100,8 @@ class SerialVersionUIDInSerializableClass(config: Config = Config.empty) : Rule(

private fun hasLongAssignment(property: KtProperty): Boolean {
val assignmentText = property.children
.singleOrNull { it is KtConstantExpression || it is KtPrefixExpression }?.text
.singleOrNull { it is KtConstantExpression || it is KtPrefixExpression }
?.text
return assignmentText != null && assignmentText.last() == 'L' &&
assignmentText.substring(0, assignmentText.length - 1).toLongOrNull() != null
}
Expand Down
Expand Up @@ -24,6 +24,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
ruleSetId,
listOf(
CanBeNonNullable(config),
CascadingCallWrapping(config),
ClassOrdering(config),
CollapsibleIfStatements(config),
DestructuringDeclarationWithTooManyEntries(config),
Expand Down

0 comments on commit 89f6ec1

Please sign in to comment.