Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not wrap a binary expression after an elvis operator #2134

Merged
merged 2 commits into from
Jul 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

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

Expand Down Expand Up @@ -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")
}
}
Loading