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.

Sign up for GitHub

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

Multiline expression wrapping #2290

Merged
merged 3 commits into from
Oct 3, 2023
Merged

Multiline expression wrapping #2290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
234d73c
Prevent wrapping of nested multiline binary expression before operati…
paul-dingemans Oct 2, 2023
afbef10
Fix lint violation
paul-dingemans Oct 2, 2023
afe650b
Merge branch 'master' into 2286-mulitline-expression-wrapping
paul-dingemans Oct 2, 2023
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 36 additions & 5 deletions ...ain/kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultilineExpressionWrappingRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
}
}
}
}
Expand Down
91 changes: 89 additions & 2 deletions ...kotlin/com/pinterest/ktlint/ruleset/standard/rules/MultilineExpressionWrappingRuleTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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)
}
}