2286 mulitline expression wrapping (#2290)

Prevent wrapping of nested multiline binary expression before operation reference as it results in a compilation error Closes #2286
pinterest · Oct 3, 2023 · 49fc7d6 · 49fc7d6
1 parent 95d95ee
commit 49fc7d6
Show file tree

Hide file tree

Showing 3 changed files with 126 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 * Do not force blank line before function in right hand side of assignment `blank-line-before-declaration` [#2260](https://github.com/pinterest/ktlint/issue/2260)
 * Ignore override of function in rule `function-naming` [#2271](https://github.com/pinterest/ktlint/issue/2271)
 * Do not replace function body having a return statement only in case the return statement contains an intermediate exit point 'function-expression-body' [#2269](https://github.com/pinterest/ktlint/issue/2269)
+* Prevent wrapping of nested multiline binary expression before operation reference as it results in a compilation error `multiline-expression-wrapping` [#2286](https://github.com/pinterest/ktlint/issue/2286)
 * Force blank line before object declaration if preceded by another declaration `blank-line-before-declaration` [#2284](https://github.com/pinterest/ktlint/issues/2284)
 
 ### Changed

diff --git a/...ain/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultilineExpressionWrappingRule.kt b/...ain/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultilineExpressionWrappingRule.kt
@@ -23,6 +23,8 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.RPAR
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.SAFE_ACCESS_EXPRESSION
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.TRY
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_ARGUMENT
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_ARGUMENT_LIST
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_PARAMETER_LIST
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.WHEN
 import com.pinterest.ktlint.rule.engine.core.api.IndentConfig
 import com.pinterest.ktlint.rule.engine.core.api.IndentConfig.Companion.DEFAULT_INDENT_CONFIG
@@ -108,11 +110,40 @@ public class MultilineExpressionWrappingRule :
                         emit(node.startOffset, "A multiline expression should start on a new line", true)
                         if (autoCorrect) {
                             node.upsertWhitespaceBeforeMe(indentConfig.siblingIndentOf(node))
-                            node
-                                .lastChildLeafOrSelf()
-                                .nextLeaf { !it.isWhiteSpaceWithoutNewline() && !it.isPartOfComment() && it.elementType != COMMA }
-                                ?.takeIf { !it.isWhiteSpaceWithNewline() }
-                                ?.upsertWhitespaceBeforeMe(indentConfig.siblingIndentOf(node))
+                            val leafOnSameLineAfterMultilineExpression =
+                                node
+                                    .lastChildLeafOrSelf()
+                                    .nextLeaf { !it.isWhiteSpaceWithoutNewline() && !it.isPartOfComment() }
+                                    ?.takeIf { !it.isWhiteSpaceWithNewline() }
+                            when {
+                                leafOnSameLineAfterMultilineExpression == null -> Unit
+
+                                leafOnSameLineAfterMultilineExpression.treeParent.elementType == OPERATION_REFERENCE -> {
+                                    // When binary expressions are wrapped, each binary expression for itself is checked whether it is a
+                                    // multiline expression. So there is no need to check whether wrapping after the operation reference is
+                                    // needed
+                                    Unit
+                                }
+
+                                leafOnSameLineAfterMultilineExpression.elementType == COMMA &&
+                                    (
+                                        leafOnSameLineAfterMultilineExpression.treeParent.elementType == VALUE_ARGUMENT_LIST ||
+                                            leafOnSameLineAfterMultilineExpression.treeParent.elementType == VALUE_PARAMETER_LIST
+                                    ) -> {
+                                    // Keep comma on same line as multiline expression:
+                                    //   foo(
+                                    //      fooBar
+                                    //          .filter { it.bar },
+                                    //   )
+                                    leafOnSameLineAfterMultilineExpression
+                                        .nextLeaf()
+                                        ?.upsertWhitespaceBeforeMe(indentConfig.siblingIndentOf(node))
+                                }
+
+                                else -> {
+                                    leafOnSameLineAfterMultilineExpression.upsertWhitespaceBeforeMe(indentConfig.siblingIndentOf(node))
+                                }
+                            }
                         }
                     }
                 }

diff --git a/...kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultilineExpressionWrappingRuleTest.kt b/...kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultilineExpressionWrappingRuleTest.kt
@@ -5,14 +5,15 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CodeStyleValue
 import com.pinterest.ktlint.test.KtLintAssertThat
 import com.pinterest.ktlint.test.LintViolation
 import com.pinterest.ktlint.test.MULTILINE_STRING_QUOTE
+import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 
 class MultilineExpressionWrappingRuleTest {
     private val multilineExpressionWrappingRuleAssertThat = KtLintAssertThat.assertThatRule { MultilineExpressionWrappingRule() }
 
     @Nested
-    inner class `Given a function call using a named parameter` {
+    inner class `Given a function call using a named argument` {
         @Test
         fun `Given value argument for a named parameter in a function with a multiline dot qualified expression on the same line as the assignment`() {
             val code =
@@ -160,7 +161,7 @@ class MultilineExpressionWrappingRuleTest {
     }
 
     @Nested
-    inner class `Given a function call using an unnamed parameter` {
+    inner class `Given a function call using an unnamed argument` {
         @Test
         fun `Given value argument in a function with a multiline binary expression on the same line as the assignment`() {
             val code =
@@ -297,6 +298,31 @@ class MultilineExpressionWrappingRuleTest {
         }
     }
 
+    @Test
+    fun `Given a declaration with parameter having a default value which is a multiline expression then keep trailing comma after the parameter`() {
+        val code =
+            """
+            fun foo(
+                val string: String = barFoo
+                    .count { it == "bar" },
+                val int: Int
+            )
+            """.trimIndent()
+        val formattedCode =
+            """
+            fun foo(
+                val string: String =
+                    barFoo
+                        .count { it == "bar" },
+                val int: Int
+            )
+            """.trimIndent()
+        multilineExpressionWrappingRuleAssertThat(code)
+            .addAdditionalRuleProvider { IndentationRule() }
+            .hasLintViolation(2, 26, "A multiline expression should start on a new line")
+            .isFormattedAs(formattedCode)
+    }
+
     @Test
     fun `Given a return statement with a multiline expression then do not reformat as it would result in a compilation error`() {
         val code =
@@ -830,4 +856,65 @@ class MultilineExpressionWrappingRuleTest {
             .hasLintViolation(1, 11, "A multiline expression should start on a new line")
             .isFormattedAs(formattedCode)
     }
+
+    @Test
+    fun `Issue 2286 - `() {
+        val code =
+            """
+            val foo = foo() + bar1 {
+                "bar1"
+            } +
+            bar2 {
+                "bar2"
+            }
+            """.trimIndent()
+        val formattedCode =
+            """
+            val foo =
+                foo() +
+                    bar1 {
+                        "bar1"
+                    } +
+                    bar2 {
+                        "bar2"
+                    }
+            """.trimIndent()
+        multilineExpressionWrappingRuleAssertThat(code)
+            .addAdditionalRuleProvider { IndentationRule() }
+            .hasLintViolations(
+                LintViolation(1, 11, "A multiline expression should start on a new line"),
+                LintViolation(1, 19, "A multiline expression should start on a new line"),
+            ).isFormattedAs(formattedCode)
+    }
+
+    @Disabled
+    @Test
+    fun `Issue 2286 - xx `() {
+        val code =
+            """
+            val foo = foo() + bar1 {
+                "bar1"
+            } + "bar3" +
+            bar2 {
+                "bar2"
+            }
+            """.trimIndent()
+        val formattedCode =
+            """
+            val foo =
+                foo() +
+                    bar1 {
+                        "bar1"
+                    } +
+                    bar2 {
+                        "bar2"
+                    }
+            """.trimIndent()
+        multilineExpressionWrappingRuleAssertThat(code)
+            .addAdditionalRuleProvider { IndentationRule() }
+            .hasLintViolations(
+                LintViolation(1, 11, "A multiline expression should start on a new line"),
+                LintViolation(1, 19, "A multiline expression should start on a new line"),
+            ).isFormattedAs(formattedCode)
+    }
 }