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 CascadingCallWrapping style rule #4979

Merged
merged 1 commit into from Jun 22, 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
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