Merge pull request #2493 from pinterest/2492-binary-expression-wrapping

Do not wrap binary expression value argument if it is already preceded by a newline
pinterest · Jan 7, 2024 · 9981e62 · 9981e62
2 parents 6a6cc17 + d9d3e52
commit 9981e62
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/RELEASE_TESTING.MD b/RELEASE_TESTING.MD
@@ -40,7 +40,7 @@ Before releasing a new version of KtLint, the release candidate is tested on a s
    ```
 4. Create an alias or script to run the latest released version of ktlint (note that this script will print the version as reference to which version is used):
    ```shell
-   alias ktlint-prev="ktlint-0.50.0 $@" # Replace with the latest release version
+   alias ktlint-prev="ktlint-1.1.0 $@" # Replace with the latest release version
    ```
    Note that `~/git/ktlint` is the directory in which the ktlint project is checked out and that `~/git/ktlint/ktlint` refers to the `ktlint` CLI-module.
 5. Create an alias or script to run the latest development-version of ktlint (note that this script will print the version and the datetime of compilation as reference to which version is used):
@@ -112,8 +112,8 @@ Formatting projects in which ktlint is not used may result in a huge amount of f
    ```shell
    ./exec-in-each-project.sh "git add --all && git commit -m \"Format with previous ktlint version -F\""
    ```
-   Repeat step 3 and 4 until no files are changed anymore. Starting from 0.50, all changes should be resolved in one run as format internally reruns 3 times in case new violations are introduced which can be autocorrected as well.
-5. Check that besides the `baseline.xml` no files are changed (in step 1 and 2 all violations which could be autocorrected have already been committed). Remaining violations which could not be autocorrected are saved in the `baseline.xml` which is stored outside the project directories.
+   Repeat step 3 and 4 until no files are changed anymore. Although ktlint reruns up to 3 times in case new violations are introduced, it can still happen that not all violations have been fixed with a single invocation.
+5. Check that besides the `baseline.xml` no files are changed (in step 3 and 4 all violations which could be autocorrected have already been committed). Remaining violations which could not be autocorrected are saved in the `baseline.xml` which is stored outside the project directories.
    ```shell
    ./exec-in-each-project.sh "git status"
    ```
@@ -127,8 +127,8 @@ Formatting projects in which ktlint is not used may result in a huge amount of f
    ktlint-dev --baseline=baseline.xml --relative --reporter=plain-summary # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.
    ```
    Inspect the output roughly (detailed inspection is done when formatting):
-  * Is the amount of logging messages comparable to before? If not, are the changes intended?
-  * Are violations related to rules that have actually been added or changed?
+   * Is the amount of logging messages comparable to before? If not, are the changes intended?
+   * Are violations related to rules that have actually been added or changed?
 7. Format with *latest development* version:
    ```shell
    ktlint-dev -F --baseline=baseline.xml --relative # Do not call this command via the "./exec-in-each-project.sh" script as we want to use the one combined baseline.xml file for all projects.

diff --git a/...c/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRule.kt b/...c/main/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRule.kt
@@ -108,9 +108,24 @@ public class BinaryExpressionWrappingRule :
                 }
             }
 
+        // Prefer to wrap the entire binary expression to a newline instead of wrapping the binary expression at the operation reference.
+        // E.g. prefer:
+        //     fooBar(
+        //         "foooooo" + "bar",
+        //     )
+        // instead of
+        //     fooBar("foooooo" +
+        //         "bar")
         node
             .takeIf { it.treeParent.elementType == VALUE_ARGUMENT }
-            ?.takeIf { it.causesMaxLineLengthToBeExceeded() }
+            ?.takeUnless {
+                // Allow
+                //     fooBar(
+                //         "tooLongToFitOnSingleLine" +
+                //             "bar",
+                //     )
+                node.prevLeaf().isWhiteSpaceWithNewline()
+            }?.takeIf { it.causesMaxLineLengthToBeExceeded() }
             ?.let { expression ->
                 emit(
                     expression.startOffset,

diff --git a/...st/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRuleTest.kt b/...st/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRuleTest.kt
@@ -441,4 +441,20 @@ class BinaryExpressionWrappingRuleTest {
                 LintViolation(3, 59, "Line is exceeding max line length. Break line after 'returns' in binary expression"),
             ).isFormattedAs(formattedCode)
     }
+
+    @Test
+    fun `Issue 2492 - Given a binary expression as value argument on a separate line`() {
+        val code =
+            """
+            // $MAX_LINE_LENGTH_MARKER  $EOL_CHAR
+            val foo =
+                foo(
+                    "longgggggggggggggg" +
+                        "foo",
+                )
+            """.trimIndent()
+        binaryExpressionWrappingRuleAssertThat(code)
+            .setMaxLineLength()
+            .hasNoLintViolations()
+    }
 }