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

Fix false positive with value argument list has lambda (indent) #1176

Merged
merged 1 commit into from
Jun 25, 2021
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 @@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- Fix false positive with eol comment (`annotation-spacing`) ([#1124](https://github.com/pinterest/ktlint/issues/1124))
- Fix KtLint dependency variant selection ([#1114](https://github.com/pinterest/ktlint/issues/1114))
- Fix false positive with 'by lazy {}' (`indent`) ([#1162](https://github.com/pinterest/ktlint/issues/1162))
- Fix false positive with value argument list has lambda (`indent`) ([#764](https://github.com/pinterest/ktlint/issues/764))
### Changed
- Updated to dokka 1.4.32 ([#1148](https://github.com/pinterest/ktlint/pull/1148))
- Updated Kotlin to 1.5.20 version
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import java.util.LinkedList
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
import org.jetbrains.kotlin.psi.KtStringTemplateExpression
import org.jetbrains.kotlin.psi.KtSuperTypeList
Expand Down Expand Up @@ -189,9 +190,11 @@ class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast {
var firstArg: ASTNode? = null
// matching ), ] or }
val r = node.nextSibling {
newlineInBetween = newlineInBetween || it.textContains('\n')
val isValueArgument = it.elementType == VALUE_ARGUMENT
val hasLineBreak = if (isValueArgument) it.hasLineBreak(LAMBDA_EXPRESSION, FUN) else it.hasLineBreak()
newlineInBetween = newlineInBetween || hasLineBreak
parameterListInBetween = parameterListInBetween || it.elementType == VALUE_PARAMETER_LIST
if (it.elementType == VALUE_ARGUMENT) {
if (isValueArgument) {
numberOfArgs++
firstArg = it
}
Expand Down Expand Up @@ -291,10 +294,12 @@ class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast {
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
for (c in node.children()) {
if (
(c.elementType == VALUE_PARAMETER || c.elementType == VALUE_ARGUMENT || c.elementType == ANNOTATION) &&
c.textContains('\n')
) {
val hasLineBreak = when (c.elementType) {
VALUE_ARGUMENT -> c.hasLineBreak(LAMBDA_EXPRESSION, FUN)
VALUE_PARAMETER, ANNOTATION -> c.hasLineBreak()
else -> false
}
if (hasLineBreak) {
// rearrange
//
// a, b, value(
Expand Down Expand Up @@ -460,7 +465,8 @@ class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast {
LPAR, LBRACE, LBRACKET -> {
// ({[ should increase expectedIndent by 1
val prevBlockLine = ctx.blockOpeningLineStack.peek() ?: -1
if (prevBlockLine != line) {
val leftBrace = n.takeIf { it.elementType == LBRACE }
if (prevBlockLine != line && !leftBrace.isAfterLambdaArgumentOnSameLine()) {
expectedIndent++
debug { "++${n.text} -> $expectedIndent" }
}
Expand All @@ -470,12 +476,10 @@ class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast {
// ]}) should decrease expectedIndent by 1
val blockLine = ctx.blockOpeningLineStack.pop()
val prevBlockLine = ctx.blockOpeningLineStack.peek() ?: -1
if (prevBlockLine != blockLine) {
val pairedLeftBrace = n.pairedLeftBrace()
if (prevBlockLine != blockLine && !pairedLeftBrace.isAfterLambdaArgumentOnSameLine()) {
expectedIndent--
val byKeywordOnSameLine = n.pairedLeftBrace()
?.leaves(forward = false)
?.takeWhile { !it.isWhiteSpaceWithNewline() }
?.firstOrNull { it.elementType == BY_KEYWORD }
val byKeywordOnSameLine = pairedLeftBrace?.prevLeafOnSameLine(BY_KEYWORD)
if (byKeywordOnSameLine != null &&
byKeywordOnSameLine.prevLeaf()?.isWhiteSpaceWithNewline() == true &&
!byKeywordOnSameLine.isPartOf(DELEGATED_SUPER_TYPE_ENTRY)
Expand Down Expand Up @@ -1111,4 +1115,24 @@ class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast {
}
return null
}

private fun ASTNode.prevLeafOnSameLine(prevLeafType: IElementType): ASTNode? {
return leaves(forward = false)
.takeWhile { !it.isWhiteSpaceWithNewline() }
.firstOrNull { it.elementType == prevLeafType }
}

private fun ASTNode?.isAfterLambdaArgumentOnSameLine(): Boolean {
return this?.prevLeafOnSameLine(RBRACE)?.nextCodeLeaf()?.treeParent?.elementType == VALUE_ARGUMENT_LIST
}

private fun ASTNode.hasLineBreak(vararg ignoreElementTypes: IElementType): Boolean {
if (isWhiteSpaceWithNewline()) return true
return if (ignoreElementTypes.isEmpty()) {
textContains('\n')
} else {
elementType !in ignoreElementTypes &&
children().any { c -> c.textContains('\n') && c.elementType !in ignoreElementTypes }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -914,4 +914,65 @@ internal class IndentationRuleTest {
)
).isEmpty()
}

// https://github.com/pinterest/ktlint/issues/764
@Test
fun `lint value argument list with lambda`() {
assertThat(
IndentationRule().lint(
"""
fun test(i: Int, f: (Int) -> Unit) {
f(i)
}

fun main() {
test(1, f = {
println(it)
})
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun `lint value argument list with two lambdas`() {
assertThat(
IndentationRule().lint(
"""
fun test(f: () -> Unit, g: () -> Unit) {
f()
g()
}

fun main() {
test({
println(1)
}, {
println(2)
})
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun `lint value argument list with anonymous function`() {
assertThat(
IndentationRule().lint(
"""
fun test(i: Int, f: (Int) -> Unit) {
f(i)
}

fun main() {
test(1, fun(it: Int) {
println(it)
})
}
""".trimIndent()
)
).isEmpty()
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
fun main() {
f(
a, b,
{
// body
},
c, d
)
f(a, b, {
// body
}, c, d)

fn(
a,
Expand Down