Skip to content

Commit

Permalink
Fix false positive with value argument list has lambda (indent) (#1176
Browse files Browse the repository at this point in the history
)
  • Loading branch information
t-kameyama authored Jun 25, 2021
1 parent 5e3e058 commit aa47c9e
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 19 deletions.
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

0 comments on commit aa47c9e

Please sign in to comment.