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

ArgumentListWrappingRule: fix wrong indentation in if condition #929

Merged
merged 2 commits into from
Sep 29, 2020
Merged

ArgumentListWrappingRule: fix wrong indentation in if condition #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
601de2e
ArgumentListWrappingRule: fix wrong indentation in `if` condition
t-kameyama Sep 28, 2020
74656c3
Fix wrong indentation in `when/while/do-while` condition
t-kameyama Sep 28, 2020
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
18 changes: 17 additions & 1 deletion ...tal/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/ArgumentListWrappingRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.pinterest.ktlint.core.ast.isRoot
import com.pinterest.ktlint.core.ast.isWhiteSpace
import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline
import com.pinterest.ktlint.core.ast.lineIndent
import com.pinterest.ktlint.core.ast.lineNumber
import com.pinterest.ktlint.core.ast.prevLeaf
import com.pinterest.ktlint.core.ast.visit
import kotlin.math.max
Expand All @@ -18,6 +19,11 @@ import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.psi.KtContainerNode
import org.jetbrains.kotlin.psi.KtDoWhileExpression
import org.jetbrains.kotlin.psi.KtIfExpression
import org.jetbrains.kotlin.psi.KtWhileExpression
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType

/**
* https://kotlinlang.org/docs/reference/coding-conventions.html#method-call-formatting
Expand Down Expand Up @@ -86,7 +92,8 @@ class ArgumentListWrappingRule : Rule("argument-list-wrapping") {
// <line indent + indentSize> VALUE_ARGUMENT...
// <line indent> RPAR
val lineIndent = node.lineIndent()
val indent = "\n" + lineIndent.substring(0, (lineIndent.length - adjustedIndent).coerceAtLeast(0))
val indent = ("\n" + lineIndent.substring(0, (lineIndent.length - adjustedIndent).coerceAtLeast(0)))
.let { if (node.isOnSameLineAsIfKeyword()) it + " ".repeat(indentSize) else it }
val paramIndent = indent + " ".repeat(indentSize)
nextChild@ for (child in node.children()) {
when (child.elementType) {
Expand Down Expand Up @@ -206,4 +213,13 @@ class ArgumentListWrappingRule : Rule("argument-list-wrapping") {
}
return null
}

private fun ASTNode.isOnSameLineAsIfKeyword(): Boolean {
val containerNode = psi.getStrictParentOfType<KtContainerNode>() ?: return false
return when (val parent = containerNode.parent) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what about KtWhenExpression?

Copy link
Contributor Author

@t-kameyama t-kameyama Sep 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a test for when.
https://github.com/t-kameyama/ktlint/blob/74656c35796b2541335419ba55cc9d5ba2ec54fd/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/ArgumentListWrappingRuleTest.kt#L366-L384

In when condition, indent size is 4.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But shouldn't it be

when(foo(
        i
    )
) { ... }

or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IntelliJ formats it as follows:

fun foo(i: Int) = true

fun test(i: Int) {
    when (foo(
            i
    )) {
        true -> println(1)
        false -> println(2)
    }
}

to

fun foo(i: Int) = true

fun test(i: Int) {
    when (foo(
        i
    )) {
        true -> println(1)
        false -> println(2)
    }
}

Copy link
Collaborator

@romtsn romtsn Sep 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, thanks for clarification

is KtIfExpression, is KtWhileExpression -> parent.node
is KtDoWhileExpression -> parent.whileKeyword?.node
else -> null
}?.lineNumber() == lineNumber()
}
}
108 changes: 108 additions & 0 deletions ...src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/ArgumentListWrappingRuleTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,4 +314,112 @@ class ArgumentListWrappingRuleTest {
)
).isEmpty()
}

@Test
fun testLintInIfCondition() {
assertThat(
ArgumentListWrappingRule().lint(
"""
fun test(param1: Int, param2: Int) {
if (listOfNotNull(
param1
).isEmpty()
) {
println(1)
} else if (listOfNotNull(
param2
).isEmpty()
) {
println(2)
}
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun testLintInIfCondition2() {
assertThat(
ArgumentListWrappingRule().lint(
"""
fun test(param1: Int, param2: Int) {
if (
listOfNotNull(
param1
).isEmpty()
) {
println(1)
} else if (
listOfNotNull(
param2
).isEmpty()
) {
println(2)
}
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun testLintInWhenCondition() {
assertThat(
ArgumentListWrappingRule().lint(
"""
fun foo(i: Int) = true

fun test(i: Int) {
when (foo(
i
)) {
true -> println(1)
false -> println(2)
}
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun testLintInWhileCondition() {
assertThat(
ArgumentListWrappingRule().lint(
"""
fun foo(i: Int) = true

fun test(i: Int) {
while (foo(
i
)
) {
println()
}
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun testLintInDoWhileCondition() {
assertThat(
ArgumentListWrappingRule().lint(
"""
fun foo(i: Int) = true

fun test(i: Int) {
do {
println()
} while (foo(
i
)
)
}
""".trimIndent()
)
).isEmpty()
}
}