Skip to content

Commit

Permalink
* Do not add a trailing comma in a function call if a newline charact…
Browse files Browse the repository at this point in the history
…er is only found inside a multiline argument. Only in case a newline character is found between arguments, this should be taken into account to add a trailing comma. (#1648)

* Refactor and cleanup unused branches in isMultiline method

Closes #1642
  • Loading branch information
paul-dingemans authored Sep 22, 2022
1 parent 70d0542 commit 3e663e6
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 34 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](https://semver.org/).
### Fixed

* Let a rule process all nodes even in case the rule is suppressed for a node so that the rule can update the internal state ([#1644](https://github.com/pinterest/ktlint/issue/1644))
* Do not add a trailing comma in case a multiline function call argument is found but no newline between the arguments `trailing-comma-on-call-site` ([#1642](https://github.com/pinterest/ktlint/issue/1642))

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ public class IndentationRule :
private fun adjustExpectedIndentInsideQualifiedExpression(n: ASTNode, ctx: IndentContext) {
val p = n.parent({
it.treeParent.elementType != DOT_QUALIFIED_EXPRESSION && it.treeParent.elementType != SAFE_ACCESS_EXPRESSION
},) ?: return
}) ?: return
val nextSibling = n.treeNext
if (!ctx.ignored.contains(p) && nextSibling != null) {
if (p.treeParent.elementType == PROPERTY_DELEGATE &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.api.EditorConfigProperties
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.COLLECTION_LITERAL_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.INDICES
import com.pinterest.ktlint.core.ast.ElementType.TYPE_ARGUMENT_LIST
import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT
import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT_LIST
import com.pinterest.ktlint.core.ast.children
import com.pinterest.ktlint.core.ast.containsLineBreakInRange
import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline
import com.pinterest.ktlint.core.ast.prevCodeLeaf
import com.pinterest.ktlint.core.ast.prevLeaf
import kotlin.properties.Delegates
Expand All @@ -14,14 +19,7 @@ import org.ec4j.core.model.PropertyType.PropertyValueParser
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression
import org.jetbrains.kotlin.psi.KtDestructuringDeclaration
import org.jetbrains.kotlin.psi.KtFunctionLiteral
import org.jetbrains.kotlin.psi.KtPsiFactory
import org.jetbrains.kotlin.psi.KtValueArgumentList
import org.jetbrains.kotlin.psi.KtWhenEntry
import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
import org.jetbrains.kotlin.psi.psiUtil.nextLeaf

/**
Expand Down Expand Up @@ -64,10 +62,10 @@ public class TrailingCommaOnCallSiteRule :
// Keep processing of element types in sync with Intellij Kotlin formatting settings.
// https://github.com/JetBrains/intellij-kotlin/blob/master/formatter/src/org/jetbrains/kotlin/idea/formatter/trailingComma/util.kt
when (node.elementType) {
ElementType.COLLECTION_LITERAL_EXPRESSION -> visitCollectionLiteralExpression(node, autoCorrect, emit)
ElementType.INDICES -> visitIndices(node, autoCorrect, emit)
ElementType.TYPE_ARGUMENT_LIST -> visitTypeList(node, autoCorrect, emit)
ElementType.VALUE_ARGUMENT_LIST -> visitValueList(node, autoCorrect, emit)
COLLECTION_LITERAL_EXPRESSION -> visitCollectionLiteralExpression(node, autoCorrect, emit)
INDICES -> visitIndices(node, autoCorrect, emit)
TYPE_ARGUMENT_LIST -> visitTypeList(node, autoCorrect, emit)
VALUE_ARGUMENT_LIST -> visitValueList(node, autoCorrect, emit)
else -> Unit
}
}
Expand Down Expand Up @@ -149,7 +147,7 @@ public class TrailingCommaOnCallSiteRule :
val prevLeaf = inspectNode.prevLeaf()
val trailingCommaNode = prevLeaf.findPreviousTrailingCommaNodeOrNull()
val trailingCommaState = when {
isMultiline(psi) -> if (trailingCommaNode != null) TrailingCommaState.EXISTS else TrailingCommaState.MISSING
this.isMultiline() -> if (trailingCommaNode != null) TrailingCommaState.EXISTS else TrailingCommaState.MISSING
else -> if (trailingCommaNode != null) TrailingCommaState.REDUNDANT else TrailingCommaState.NOT_EXISTS
}
when (trailingCommaState) {
Expand Down Expand Up @@ -189,22 +187,18 @@ public class TrailingCommaOnCallSiteRule :
}
}

private fun isMultiline(element: PsiElement): Boolean = when {
element.parent is KtFunctionLiteral -> isMultiline(element.parent)
element is KtFunctionLiteral -> containsLineBreakInRange(element.valueParameterList!!, element.arrow!!)
element is KtWhenEntry -> containsLineBreakInRange(element.firstChild, element.arrow!!)
element is KtDestructuringDeclaration -> containsLineBreakInRange(element.lPar!!, element.rPar!!)
element is KtValueArgumentList && element.children.size == 1 && element.anyDescendantOfType<KtCollectionLiteralExpression>() -> {
// special handling for collection literal
// @Annotation([
// "something",
// ])
val lastChild = element.collectDescendantsOfType<KtCollectionLiteralExpression>().last()
containsLineBreakInLeavesRange(lastChild.rightBracket!!, element.rightParenthesis!!)
private fun ASTNode.isMultiline(): Boolean =
if (elementType == VALUE_ARGUMENT_LIST) {
hasAtLeastOneArgument() && hasAtLeastOneWhiteSpaceWithNewLine()
} else {
textContains('\n')
}
element is KtValueArgumentList && element.arguments.isEmpty() -> false
else -> element.textContains('\n')
}

private fun ASTNode.hasAtLeastOneWhiteSpaceWithNewLine() =
children().any { it.isWhiteSpaceWithNewline() }

private fun ASTNode.hasAtLeastOneArgument() =
children().any { it.elementType == VALUE_ARGUMENT }

private fun ASTNode?.findPreviousTrailingCommaNodeOrNull(): ASTNode? {
var node = this
Expand Down Expand Up @@ -277,10 +271,10 @@ public class TrailingCommaOnCallSiteRule :
)

private val TYPES_ON_CALL_SITE = TokenSet.create(
ElementType.COLLECTION_LITERAL_EXPRESSION,
ElementType.INDICES,
ElementType.TYPE_ARGUMENT_LIST,
ElementType.VALUE_ARGUMENT_LIST,
COLLECTION_LITERAL_EXPRESSION,
INDICES,
TYPE_ARGUMENT_LIST,
VALUE_ARGUMENT_LIST,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -475,9 +475,39 @@ class TrailingCommaOnCallSiteRuleTest {
fun `Issue 1602 - Given that an empty argument list with a comment, dot add a trailing comma`() {
val code =
"""
val foo = setOf<Int>(
val foo1 = setOf<Int>(
// some comment
)
val foo2 = setOf<Int>(
)
val foo1 = setOf<Int>()
""".trimIndent()
ruleAssertThat(code)
.withEditorConfigOverride(allowTrailingCommaOnCallSiteProperty to true)
.hasNoLintViolations()
}

@Test
fun `Issue 1642 - Given a multiline argument then ignore the newline character and do not add a trailing comma`() {
val code =
"""
fun main() {
bar(object : Foo {
override fun foo() {
TODO("Not yet implemented")
}
})
bar("foo", object : Foo {
override fun foo() {
TODO("Not yet implemented")
}
})
bar(object : Foo {
override fun foo() {
TODO("Not yet implemented")
}
}, "foo")
}
""".trimIndent()
ruleAssertThat(code)
.withEditorConfigOverride(allowTrailingCommaOnCallSiteProperty to true)
Expand Down

0 comments on commit 3e663e6

Please sign in to comment.