Do not wrap a binary expression after an elvis operator in case the m…

…ax line length is exceeded (#2134) Use sets internally in KtlintAssertThat rather than lists for storage and comparing of LintViolations. Like the KtlintRuleEngine duplicate violations are to be ignored. Closes #2128
pinterest · Jul 16, 2023 · dd45e41 · dd45e41
1 parent 5c17a62
commit dd45e41
Show file tree

Hide file tree

Showing 4 changed files with 120 additions and 96 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 * Log message `Format was not able to resolve all violations which (theoretically) can be autocorrected in file ... in 3 consecutive runs of format` is now only displayed in case a new ktlint rule is actually needed. [#2129](https://github.com/pinterest/ktlint/issues/2129)
 * Fix wrapping of function signature in case the opening brace of the function body block exceeds the maximum line length. Fix upsert of whitespace into composite nodes. `function-signature` [#2130](https://github.com/pinterest/ktlint/issues/2130)
 * Fix spacing around colon in annotations `spacing-around-colon` ([#2093](https://github.com/pinterest/ktlint/issues/2093))
+* Do not wrap a binary expression after an elvis operator in case the max line length is exceeded ([#2128](https://github.com/pinterest/ktlint/issues/2128))
 * Fix indent of IS_EXPRESSION, PREFIX_EXPRESSION and POSTFIX_EXPRESSION in case it contains a linebreak `indent` [#2094](https://github.com/pinterest/ktlint/issues/2094)
 
 ### Changed

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
@@ -2,6 +2,7 @@ package com.pinterest.ktlint.ruleset.standard.rules
 
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.BINARY_EXPRESSION
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.CALL_EXPRESSION
+import com.pinterest.ktlint.rule.engine.core.api.ElementType.ELVIS
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.EQ
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.FUN
 import com.pinterest.ktlint.rule.engine.core.api.ElementType.LAMBDA_ARGUMENT
@@ -20,11 +21,13 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPE
 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.leavesOnLine
 import com.pinterest.ktlint.rule.engine.core.api.nextCodeSibling
 import com.pinterest.ktlint.rule.engine.core.api.nextSibling
 import com.pinterest.ktlint.rule.engine.core.api.noNewLineInClosedRange
 import com.pinterest.ktlint.rule.engine.core.api.parent
+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.upsertWhitespaceBeforeMe
 import com.pinterest.ktlint.ruleset.standard.StandardRule
@@ -121,7 +124,16 @@ public class BinaryExpressionWrappingRule :
                 if (node.isCallExpressionFollowedByLambdaArgument() || cannotBeWrappedAtOperationReference(operationReference)) {
                     // Wrapping after operation reference might not be the best place in case of a call expression or just won't work as
                     // the left hand side still does not fit on a single line
-                    emit(operationReference.startOffset, "Line is exceeding max line length", false)
+                    val offset =
+                        operationReference
+                            .prevLeaf { it.isWhiteSpaceWithNewline() }
+                            ?.let { previousNewlineNode ->
+                                previousNewlineNode.startOffset +
+                                    previousNewlineNode.text.indexOfLast { it == '\n' } +
+                                    1
+                            }
+                            ?: operationReference.startOffset
+                    emit(offset, "Line is exceeding max line length", false)
                 } else {
                     operationReference
                         .nextSibling()
@@ -146,17 +158,21 @@ public class BinaryExpressionWrappingRule :
             .let { it?.elementType == LAMBDA_ARGUMENT }
 
     private fun cannotBeWrappedAtOperationReference(operationReference: ASTNode) =
-        operationReference
-            .takeUnless { it.nextCodeSibling()?.elementType == BINARY_EXPRESSION }
-            ?.let {
-                val stopAtOperationReferenceLeaf = operationReference.firstChildLeafOrSelf()
-                maxLineLength <=
-                    it
-                        .leavesOnLine()
-                        .takeWhile { leaf -> leaf != stopAtOperationReferenceLeaf }
-                        .lengthWithoutNewlinePrefix()
-            }
-            ?: false
+        if (operationReference.firstChildNode.elementType == ELVIS) {
+            true
+        } else {
+            operationReference
+                .takeUnless { it.nextCodeSibling()?.elementType == BINARY_EXPRESSION }
+                ?.let {
+                    val stopAtOperationReferenceLeaf = operationReference.firstChildLeafOrSelf()
+                    maxLineLength <=
+                        it
+                            .leavesOnLine()
+                            .takeWhile { leaf -> leaf != stopAtOperationReferenceLeaf }
+                            .lengthWithoutNewlinePrefix()
+                }
+                ?: false
+        }
 
     private fun ASTNode.isOnLineExceedingMaxLineLength() = leavesOnLine().lengthWithoutNewlinePrefix() > maxLineLength
 

diff --git a/...st/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRuleTest.kt b/...st/kotlin/com/pinterest/ktlint/ruleset/standard/rules/BinaryExpressionWrappingRuleTest.kt
@@ -176,9 +176,9 @@ class BinaryExpressionWrappingRuleTest {
                 // When linting, a violation is reported for each operation reference. While when formatting, the nested binary expression
                 // is evaluated (working from outside to inside). After wrapping an outer binary expression, the inner binary expressions
                 // are evaluated and only wrapped again at the operation reference when needed.
+                LintViolation(3, 1, "Line is exceeding max line length", canBeAutoCorrected = false),
                 LintViolation(3, 35, "Line is exceeding max line length. Break line after operator in binary expression"),
                 LintViolation(3, 63, "Line is exceeding max line length. Break line after operator in binary expression"),
-                LintViolation(3, 92, "Line is exceeding max line length", canBeAutoCorrected = false),
             ).isFormattedAs(formattedCode)
     }
 
@@ -194,7 +194,7 @@ class BinaryExpressionWrappingRuleTest {
             """.trimIndent()
         binaryExpressionWrappingRuleAssertThat(code)
             .setMaxLineLength()
-            .hasLintViolationWithoutAutoCorrect(3, 36, "Line is exceeding max line length")
+            .hasLintViolationWithoutAutoCorrect(3, 1, "Line is exceeding max line length")
     }
 
     @Test
@@ -211,10 +211,9 @@ class BinaryExpressionWrappingRuleTest {
             """.trimIndent()
         binaryExpressionWrappingRuleAssertThat(code)
             .setMaxLineLength()
-            .hasLintViolationsWithoutAutoCorrect(
+            .hasLintViolations(
+                LintViolation(2, 1, "Line is exceeding max line length", canBeAutoCorrected = false),
                 LintViolation(2, 12, "Line is exceeding max line length. Break line between assignment and expression"),
-                LintViolation(2, 20, "Line is exceeding max line length. Break line after operator in binary expression"),
-                LintViolation(2, 36, "Line is exceeding max line length"),
             )
     }
 
@@ -307,6 +306,23 @@ class BinaryExpressionWrappingRuleTest {
             .setMaxLineLength()
             .addAdditionalRuleProvider { ArgumentListWrappingRule() }
             .addAdditionalRuleProvider { WrappingRule() }
-            .hasLintViolationWithoutAutoCorrect(3, 17, "Line is exceeding max line length")
+            .hasLintViolationWithoutAutoCorrect(3, 1, "Line is exceeding max line length")
+    }
+
+    @Test
+    fun `Issue 2128 - Given an elvis expression exceeding the line length`() {
+        val code =
+            """
+            // $MAX_LINE_LENGTH_MARKER                            $EOL_CHAR
+            val foo =
+                bar
+                    ?: throw UnsupportedOperationException("foobar")
+            """.trimIndent()
+        binaryExpressionWrappingRuleAssertThat(code)
+            .setMaxLineLength()
+            .addAdditionalRuleProvider { ArgumentListWrappingRule() }
+            .addAdditionalRuleProvider { WrappingRule() }
+            .addAdditionalRuleProvider { MaxLineLengthRule() }
+            .hasLintViolationWithoutAutoCorrect(4, 1, "Line is exceeding max line length")
     }
 }