Add CascadingCallWrapping style rule (#4979)

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.

config/detekt/detekt.yml

@@ -155,6 +155,8 @@ potential-bugs:
 style:
   CanBeNonNullable:
     active: true
+  CascadingCallWrapping:
+    active: true
   ClassOrdering:
     active: true
   CollapsibleIfStatements:

detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/SplitPattern.kt

@@ -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,

detekt-core/src/main/resources/default-detekt-config.yml

@@ -490,6 +490,9 @@ style:
   active: true
   CanBeNonNullable:
     active: false
+  CascadingCallWrapping:
+    active: false
+    includeElvis: true
   ClassOrdering:
     active: false
   CollapsibleIfStatements:

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt

@@ -171,7 +171,9 @@ class ConfigurationCollector {
 
         private fun KtValueArgument.getReferenceIdentifierOrNull(): String? =
             (getArgumentExpression() as? KtCallableReferenceExpression)
-                ?.callableReference?.getIdentifier()?.text
+                ?.callableReference
+                ?.getIdentifier()
+                ?.text
     }
 
     private object ConfigWithAndroidVariantsSupport {

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/MultiRuleCollector.kt

@@ -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

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleVisitor.kt

@@ -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
@@ -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")

detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/extensions/DetektExtension.kt

@@ -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
 

detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/TooManyFunctions.kt

@@ -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

detekt-rules-coroutines/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunWithCoroutineScopeReceiver.kt

@@ -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(

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/CanBeNonNullable.kt

@@ -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
@@ -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)
                 }
@@ -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,

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/CascadingCallWrapping.kt

@@ -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
+        }
+    }
+}

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenVoid.kt

@@ -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

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ObjectLiteralToLambda.kt

@@ -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))

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/SerialVersionUIDInSerializableClass.kt

@@ -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
     }

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt

@@ -24,6 +24,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
         ruleSetId,
         listOf(
             CanBeNonNullable(config),
+            CascadingCallWrapping(config),
             ClassOrdering(config),
             CollapsibleIfStatements(config),
             DestructuringDeclarationWithTooManyEntries(config),

detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/CascadingCallWrappingSpec.kt

@@ -0,0 +1,178 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+import io.gitlab.arturbosch.detekt.api.Config
+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.Nested
+import org.junit.jupiter.api.Test
+
+class CascadingCallWrappingSpec {
+    private val subject = CascadingCallWrapping(Config.empty)
+
+    @Test
+    fun `reports missing wrapping`() {
+        val code = """
+            val a = 0
+                .plus(0).plus(0).plus(0)
+        """
+
+        assertThat(subject.compileAndLint(code))
+            .hasSize(1)
+            .hasTextLocations(8 to 30)
+            .first()
+            .hasMessage("Chained call `plus(0)` should be wrapped to a new line since preceding calls were.")
+    }
+
+    @Test
+    fun `does not report when chained calls are on a single line`() {
+        val code = """
+            val a = 0.plus(0).plus(0)
+        """
+
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does not report wrapped calls`() {
+        val code = """
+            val a = 0
+                .plus(0)
+                .plus(0)
+        """
+
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `does not report unwrapped initial calls`() {
+        val code = """
+            val a = 0.plus(0).plus(0)
+                .plus(0)
+                .plus(0)
+        """
+
+        assertThat(subject.compileAndLint(code)).isEmpty()
+    }
+
+    @Test
+    fun `reports missing wrapping for safe qualified calls`() {
+        val code = """
+            val a = 0
+                ?.plus(0)?.plus(0)
+        """
+
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
+    @Test
+    fun `reports missing wrapping for calls with non-null assertions`() {
+        val code = """
+            val a = 0!!
+                .plus(0)!!.plus(0)
+        """
+
+        assertThat(subject.compileAndLint(code)).hasSize(1)
+    }
+
+    @Test
+    fun `reports missing wrapping for properties`() {
+        val code = """
+            val a = ""
+                .plus("").length
+
+            val b = ""
+                .length.plus(0)
+        """
+
+        assertThat(subject.compileAndLint(code)).hasSize(2)
+    }
+
+    @Nested
+    inner class `with multiline calls` {
+        @Test
+        fun `does not report with wrapping`() {
+            val code = """
+                val a = 0
+                    .plus(
+                        0
+                    )
+                    .let { 
+                        0
+                    }
+            """
+
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+
+        @Test
+        fun `reports missing wrapping`() {
+            val code = """
+                val a = 0
+                    .plus(
+                        0
+                    )
+                    .let { 
+                        0
+                    }.plus(
+                        0
+                    )
+            """
+
+            assertThat(subject.compileAndLint(code)).hasSize(1)
+        }
+
+        @Test
+        fun `does not report when calls are multiline but never wrapped`() {
+            val code = """
+                val a = 0.plus(
+                    0
+                ).let {
+                    0
+                }
+            """
+
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+
+        @Test
+        fun `does not report for single multiline call`() {
+            val code = """
+                val a = 0.plus(
+                    0
+                )
+            """
+
+            assertThat(subject.compileAndLint(code)).isEmpty()
+        }
+    }
+
+    @Nested
+    inner class `with elvis operators` {
+        private val subjectIncludingElvis = CascadingCallWrapping(TestConfig(mapOf("includeElvis" to true)))
+        private val subjectExcludingElvis = CascadingCallWrapping(TestConfig(mapOf("includeElvis" to false)))
+
+        @Test
+        fun `does not report with wrapping`() {
+            val code = """
+                val a = 0
+                    .plus(0)
+                    ?: 0
+            """
+
+            assertThat(subjectIncludingElvis.compileAndLint(code)).isEmpty()
+            assertThat(subjectExcludingElvis.compileAndLint(code)).isEmpty()
+        }
+
+        @Test
+        fun `reports missing wrapping`() {
+            val code = """
+                val a = 0
+                    .plus(0) ?: 0
+            """
+
+            assertThat(subjectIncludingElvis.compileAndLint(code)).hasSize(1)
+            assertThat(subjectExcludingElvis.compileAndLint(code)).isEmpty()
+        }
+    }
+}