Fix indentation of multiline parameter list in function literal (#1997)

Closes #1976
pinterest · May 7, 2023 · 7ba9ed0 · 7ba9ed0
1 parent fc16aa0
commit 7ba9ed0
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 * Prevent nullpointer exception (NPE) if class without body is followed by multiple blank lines until end of file `no-consecutive-blank-lines` ([#1987](https://github.com/pinterest/ktlint/issues/1987))
 * Allow to 'unset' the `.editorconfig` property `ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than` when using `ktlint_official` code style `function-signature` ([#1977](https://github.com/pinterest/ktlint/issues/1977))
 * Prevent nullpointer exception (NPE) if or operator at start of line is followed by dot qualified expression `indent` ([#1993](https://github.com/pinterest/ktlint/issues/1993))
+* Fix indentation of multiline parameter list in function literal `indent` ([#1976](https://github.com/pinterest/ktlint/issues/1976))
 * Restrict indentation of closing quotes to `ktlint_official` code style to keep formatting of other code styles consistent with `0.48.x` and before `indent` ([#1971](https://github.com/pinterest/ktlint/issues/1971))
 
 ### Changed

diff --git a/...t-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IndentationRule.kt b/...t-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IndentationRule.kt
@@ -53,7 +53,6 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROPERTY
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.PROPERTY_ACCESSOR
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.RBRACE
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.RBRACKET
-import com.pinterest.ktlint.rule.engine.core.api.ElementType.REFERENCE_EXPRESSION
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.REGULAR_STRING_PART
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.RETURN_KEYWORD
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.RPAR
@@ -96,6 +95,7 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_SIZE_PROPER
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPERTY
 import com.pinterest.ktlint.rule.engine.core.api.firstChildLeafOrSelf
 import com.pinterest.ktlint.rule.engine.core.api.indent
+import com.pinterest.ktlint.rule.engine.core.api.isPartOf
 import com.pinterest.ktlint.rule.engine.core.api.isPartOfComment
 import com.pinterest.ktlint.rule.engine.core.api.isRoot
 import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace
@@ -452,7 +452,7 @@ public class IndentationRule :
     }
 
     private fun ASTNode.calculateIndentOfFunctionLiteralParameters() =
-        if (codeStyle == ktlint_official || isFirstParameterOfFunctionLiteralPrecededByNewLine()) {
+        if (isFirstParameterOfFunctionLiteralPrecededByNewLine()) {
             // val fieldExample =
             //      LongNameClass {
             //              paramA,
@@ -469,13 +469,23 @@ public class IndentationRule :
             //                     paramC ->
             //         ClassB(paramA, paramB, paramC)
             //     }
-            parent(CALL_EXPRESSION)
-                ?.let { callExpression ->
-                    val textBeforeFirstParameter =
-                        callExpression.findChildByType(REFERENCE_EXPRESSION)?.text +
-                            " { "
-                    " ".repeat(textBeforeFirstParameter.length)
-                }
+            // val fieldExample =
+            //     someFunction(
+            //         1,
+            //         2,
+            //     ) { paramA,
+            //         paramB,
+            //         paramC ->
+            //         ClassB(paramA, paramB, paramC)
+            //     }
+            this
+                .takeIf { it.isPartOf(CALL_EXPRESSION) }
+                ?.treeParent
+                ?.leaves(false)
+                ?.takeWhile { !it.isWhiteSpaceWithNewline() }
+                ?.sumOf { it.textLength }
+                ?.plus(2) // need to add spaces to compensate for "{ "
+                ?.let { length -> " ".repeat(length) }
                 ?: indentConfig.indent.repeat(2)
         }
 

diff --git a/.../src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt b/.../src/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRule.kt
@@ -19,8 +19,11 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_SIZE_PROPERTY
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPERTY
 import com.pinterest.ktlint.rule.engine.core.api.editorconfig.MAX_LINE_LENGTH_PROPERTY
+import com.pinterest.ktlint.rule.engine.core.api.firstChildLeafOrSelf
 import com.pinterest.ktlint.rule.engine.core.api.indent
 import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline
+import com.pinterest.ktlint.rule.engine.core.api.parent
+import com.pinterest.ktlint.rule.engine.core.api.prevCodeLeaf
 import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
 import com.pinterest.ktlint.rule.engine.core.api.prevSibling
 import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe
@@ -33,6 +36,7 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
 import org.jetbrains.kotlin.psi.KtTypeArgumentList
 import org.jetbrains.kotlin.psi.psiUtil.children
 import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
+import org.jetbrains.kotlin.psi.psiUtil.leaves
 
 public class ParameterListWrappingRule :
     StandardRule(
@@ -120,7 +124,9 @@ public class ParameterListWrappingRule :
     private fun ASTNode.needToWrapParameterList() =
         when {
             hasNoParameters() -> false
-            isPartOfFunctionLiteralInNonKtlintOfficialCodeStyle() -> false
+            codeStyle != ktlint_official && isPartOfFunctionLiteralInNonKtlintOfficialCodeStyle() -> false
+            codeStyle == ktlint_official && isPartOfFunctionLiteralStartingOnSameLineAsClosingParenthesisOfPrecedingReferenceExpression() ->
+                false
             isFunctionTypeWrappedInNullableType() -> false
             textContains('\n') -> true
             codeStyle == ktlint_official && containsAnnotatedParameter() -> true
@@ -135,7 +141,23 @@ public class ParameterListWrappingRule :
 
     private fun ASTNode.isPartOfFunctionLiteralInNonKtlintOfficialCodeStyle(): Boolean {
         require(elementType == VALUE_PARAMETER_LIST)
-        return codeStyle != ktlint_official && treeParent?.elementType == FUNCTION_LITERAL
+        return treeParent?.elementType == FUNCTION_LITERAL
+    }
+
+    private fun ASTNode.isPartOfFunctionLiteralStartingOnSameLineAsClosingParenthesisOfPrecedingReferenceExpression(): Boolean {
+        require(elementType == VALUE_PARAMETER_LIST)
+        return firstChildLeafOrSelf()
+            .let { startOfFunctionLiteral ->
+                treeParent
+                    ?.takeIf { it.elementType == FUNCTION_LITERAL }
+                    ?.prevCodeLeaf()
+                    ?.takeIf { it.treeParent.elementType == ElementType.VALUE_ARGUMENT_LIST }
+                    ?.takeIf { it.treeParent.treeParent.elementType == ElementType.CALL_EXPRESSION }
+                    ?.leaves()
+                    ?.takeWhile { it != startOfFunctionLiteral }
+                    ?.none { it.isWhiteSpaceWithNewline() }
+                    ?: false
+            }
     }
 
     private fun ASTNode.isFunctionTypeWrappedInNullableType(): Boolean {

diff --git a/...andard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IndentationRuleTest.kt b/...andard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IndentationRuleTest.kt
@@ -4819,10 +4819,11 @@ internal class IndentationRuleTest {
                 .addAdditionalRuleProvider { ParameterListWrappingRule() }
                 .withEditorConfigOverride(CODE_STYLE_PROPERTY to ktlint_official)
                 .hasLintViolationForAdditionalRule(2, 20, "Parameter should start on a newline")
-                .hasLintViolations(
-                    LintViolation(3, 1, "Unexpected indentation (19) (should be 12)"),
-                    LintViolation(4, 1, "Unexpected indentation (19) (should be 12)"),
-                ).isFormattedAs(formattedCode)
+//                .hasLintViolations(
+//                    LintViolation(3, 1, "Unexpected indentation (19) (should be 12)"),
+//                    LintViolation(4, 1, "Unexpected indentation (19) (should be 12)"),
+//                )
+                .isFormattedAs(formattedCode)
         }
 
         @Test

diff --git a/.../test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRuleTest.kt b/.../test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/ParameterListWrappingRuleTest.kt
@@ -179,7 +179,7 @@ class ParameterListWrappingRuleTest {
     }
 
     @Nested
-    inner class `Given a function literal having a multiline parameter list` {
+    inner class `Given a function literal having a multiline parameter list and the first parameter starts on same line as LBRACE` {
         private val code =
             """
             val fieldExample =
@@ -204,10 +204,9 @@ class ParameterListWrappingRuleTest {
                 """.trimIndent()
             parameterListWrappingRuleAssertThat(code)
                 .withEditorConfigOverride(CODE_STYLE_PROPERTY to ktlint_official)
-                .hasLintViolationsForAdditionalRule(
-                    LintViolation(3, 1, "Unexpected indentation (20) (should be 12)"),
-                    LintViolation(4, 1, "Unexpected indentation (20) (should be 12)"),
-                ).isFormattedAs(formattedCode)
+                // Indent violations will not be reported until after the wrapping of the first parameter is completed and as of that will
+                // not be found during linting
+                .isFormattedAs(formattedCode)
         }
 
         @ParameterizedTest(name = "Code style = {0}")
@@ -216,17 +215,35 @@ class ParameterListWrappingRuleTest {
             mode = EnumSource.Mode.EXCLUDE,
             names = ["ktlint_official"],
         )
-        fun `Given another than ktlint_official code style then do not reformat`(codeStyleValue: CodeStyleValue) {
+        fun `Given another code style than ktlint_official then do not reformat`(codeStyleValue: CodeStyleValue) {
             parameterListWrappingRuleAssertThat(code)
                 .withEditorConfigOverride(CODE_STYLE_PROPERTY to codeStyleValue)
                 .hasNoLintViolations()
-//                .hasLintViolationsForAdditionalRule(
-//                    LintViolation(3, 1, "Unexpected indentation (20) (should be 12)"),
-//                    LintViolation(4, 1, "Unexpected indentation (20) (should be 12)"),
-//                )
         }
     }
 
+    @ParameterizedTest(name = "Code style = {0}")
+    @EnumSource(value = CodeStyleValue::class)
+    fun `Given a multiline reference expression with trailing lambda having a multiline parameter list and the first parameter starts on same line as LBRACE`(
+        codeStyleValue: CodeStyleValue,
+    ) {
+        val code =
+            """
+            val foo =
+                bar(
+                    Any(),
+                    Any()
+                ) { a,
+                    b
+                    ->
+                    foobar()
+                }
+            """.trimIndent()
+        parameterListWrappingRuleAssertThat(code)
+            .withEditorConfigOverride(CODE_STYLE_PROPERTY to codeStyleValue)
+            .hasNoLintViolations()
+    }
+
     @Test
     fun `Given a function with annotated parameters then start each parameter on a separate line but preserve spacing between annotation and parameter name`() {
         val code =