Skip to content

Commit

Permalink
Add new experimental trailing comma rule (#709) (#1032)
Browse files Browse the repository at this point in the history
* Add new experimental rule to remove trailing comma's from argument and values lists (#709)

* Remove trailing comma's from other types (#709)

* Remove trailing comma's from COLLECTION_LITERAL_EXPRESSION type (#709)

* Use IntelliJ IDEA '.editorconfig' properties to (dis)allow trailing comma's (#709)

* Replace KOTLIN_BOOLEAN_VALUE_PARSER with ec4j PropertyType.LowerCasingPropertyType

* Use setOf instead of mutableSetOf

* Rename the rule to trailing-comma to better reflect what it supposed to do

* Enhance EditorConfig properties representation

* Make trailing comma rule work like IntelliJ one

* Make it work

* Remove unneccesary assertion

* Add changelog entry

* Workaround ktlint bug

Co-authored-by: Paul Dingemans <pdingemans@bol.com>
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
  • Loading branch information
3 people committed Aug 8, 2021
1 parent 3eb3950 commit 98d1a8e
Show file tree
Hide file tree
Showing 7 changed files with 1,285 additions and 2 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
## Unreleased

### Added

- New `trailing-comma` rule ([#709](https://github.com/pinterest/ktlint/issues/709)) (prior art by [paul-dingemans](https://github.com/paul-dingemans))
### Fixed
- Fix false positive with lambda argument and call chain (`indent`) ([#1202](https://github.com/pinterest/ktlint/issues/1202))

Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ task ktlint(type: JavaExec, group: LifecycleBasePlugin.VERIFICATION_GROUP) {
description = "Check Kotlin code style."
classpath = configurations.ktlint
main = 'com.pinterest.ktlint.Main'
args '**/src/**/*.kt', '--baseline=ktlint-baseline.xml'
args '**/src/**/*.kt', '--baseline=ktlint-baseline.xml', '--verbose'
}

allprojects {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
import org.jetbrains.kotlin.psi.psiUtil.siblings

fun ASTNode.nextLeaf(includeEmpty: Boolean = false, skipSubtree: Boolean = false): ASTNode? {
var n = if (skipSubtree) this.lastChildLeafOrSelf().nextLeafAny() else this.nextLeafAny()
Expand Down Expand Up @@ -187,8 +188,10 @@ fun ASTNode.isPartOfString() =

fun ASTNode?.isWhiteSpace() =
this != null && elementType == WHITE_SPACE

fun ASTNode?.isWhiteSpaceWithNewline() =
this != null && elementType == WHITE_SPACE && textContains('\n')

fun ASTNode?.isWhiteSpaceWithoutNewline() =
this != null && elementType == WHITE_SPACE && !textContains('\n')

Expand All @@ -210,6 +213,7 @@ fun LeafElement.upsertWhitespaceBeforeMe(text: String): LeafElement {
}
}
}

fun LeafElement.upsertWhitespaceAfterMe(text: String): LeafElement {
val s = treeNext
return if (s != null && s.elementType == WHITE_SPACE) {
Expand Down Expand Up @@ -260,3 +264,26 @@ fun ASTNode.lineIndent(): String {
}
return ""
}

/**
* Print content of a node and the element type of the node, its parent and its direct children. Utility is meant to
* be used during development only. Please do not remove.
*/
@Suppress("unused")
fun ASTNode.logStructure(): ASTNode =
also {
println("Processing ${text.replaceTabAndNewline()} : Type $elementType with parent ${treeParent?.elementType} ")
children()
.toList()
.map {
println(" ${it.text.replaceTabAndNewline()} : Type ${it.elementType}")
}
}

private fun String.replaceTabAndNewline(): String =
replace("\t", "\\t").replace("\n", "\\n")

public fun containsLineBreakInRange(from: PsiElement, to: PsiElement): Boolean =
from.siblings(forward = true, withItself = true)
.takeWhile { !it.isEquivalentTo(to) }
.any { it.textContains('\n') }
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.pinterest.ktlint.ruleset.experimental

import com.pinterest.ktlint.core.RuleSet
import com.pinterest.ktlint.core.RuleSetProvider
import com.pinterest.ktlint.ruleset.experimental.trailingcomma.TrailingCommaRule

public class ExperimentalRuleSetProvider : RuleSetProvider {

Expand All @@ -11,6 +12,7 @@ public class ExperimentalRuleSetProvider : RuleSetProvider {
ArgumentListWrappingRule(),
MultiLineIfElseRule(),
NoEmptyFirstLineInMethodBlockRule(),
TrailingCommaRule(),
PackageNameRule(),
EnumEntryNameCaseRule(),
SpacingAroundDoubleColonRule(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,307 @@
package com.pinterest.ktlint.ruleset.experimental.trailingcomma

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.children
import com.pinterest.ktlint.core.ast.containsLineBreakInRange
import com.pinterest.ktlint.core.ast.isRoot
import com.pinterest.ktlint.core.ast.prevCodeLeaf
import com.pinterest.ktlint.core.ast.prevLeaf
import kotlin.properties.Delegates
import org.ec4j.core.model.PropertyType
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.KtWhenExpression
import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
import org.jetbrains.kotlin.utils.addToStdlib.cast

@OptIn(FeatureInAlphaState::class)
public class TrailingCommaRule :
Rule("trailing-comma"),
// runs last to ensure that the linebreaks are already inserted by the indent and other rules
Rule.Modifier.Last,
UsesEditorConfigProperties {

private var allowTrailingComma by Delegates.notNull<Boolean>()
private var allowTrailingCommaOnCallSite by Delegates.notNull<Boolean>()

override val editorConfigProperties: List<UsesEditorConfigProperties.EditorConfigProperty<*>> = listOf(
allowTrailingCommaProperty,
allowTrailingCommaOnCallSiteProperty
)

override fun visit(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
if (node.isRoot()) {
getEditorConfigValues(node)
return
}

// 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.DESTRUCTURING_DECLARATION -> visitDestructuringDeclaration(node, emit, autoCorrect)
ElementType.FUNCTION_LITERAL -> visitFunctionLiteral(node, emit, autoCorrect)
ElementType.TYPE_PARAMETER_LIST -> visitTypeList(node, emit, autoCorrect)
ElementType.VALUE_PARAMETER_LIST -> visitValueList(node, emit, autoCorrect)
ElementType.WHEN_ENTRY -> visitWhenEntry(node, emit, autoCorrect)
else -> Unit
}
when (node.elementType) {
ElementType.COLLECTION_LITERAL_EXPRESSION -> visitCollectionLiteralExpression(node, emit, autoCorrect)
ElementType.INDICES -> visitIndices(node, emit, autoCorrect)
ElementType.TYPE_ARGUMENT_LIST -> visitTypeList(node, emit, autoCorrect)
ElementType.VALUE_ARGUMENT_LIST -> visitValueList(node, emit, autoCorrect)
else -> Unit
}
}

private fun getEditorConfigValues(node: ASTNode) {
val editorConfig = node.getUserData(KtLint.EDITOR_CONFIG_PROPERTIES_USER_DATA_KEY)!!
allowTrailingComma = editorConfig.getEditorConfigValue(allowTrailingCommaProperty)
allowTrailingCommaOnCallSite = editorConfig.getEditorConfigValue(allowTrailingCommaOnCallSiteProperty)
}

private fun visitCollectionLiteralExpression(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
val inspectNode = node
.children()
.last { it.elementType == ElementType.RBRACKET }
node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
}

private fun visitDestructuringDeclaration(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
val inspectNode = node
.children()
.last { it.elementType == ElementType.RPAR }
node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
}

private fun visitFunctionLiteral(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
val inspectNode = node
.children()
.lastOrNull { it.elementType == ElementType.ARROW }
?: // lambda w/o an arrow -> no arguments -> no commas
return
node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
}

private fun visitIndices(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
val inspectNode = node
.children()
.last { it.elementType == ElementType.RBRACKET }
node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
}

private fun visitValueList(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
if (node.treeParent.elementType != ElementType.FUNCTION_LITERAL) {
val inspectNode = node
.children()
.last { it.elementType == ElementType.RPAR }
node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
}
}

private fun visitTypeList(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
val inspectNode = node
.children()
.first { it.elementType == ElementType.GT }
node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
}

private fun visitWhenEntry(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
val psi = node.psi
require(psi is KtWhenEntry)
if (psi.isElse || psi.parent.cast<KtWhenExpression>().leftParenthesis == null) {
// no commas for "else" or when there are no opening parenthesis for the when-expression
return
}

val inspectNode = node
.children()
.first { it.elementType == ElementType.ARROW }
node.reportAndCorrectTrailingCommaNodeBefore(inspectNode, emit, autoCorrect)
}

private fun ASTNode.reportAndCorrectTrailingCommaNodeBefore(
inspectNode: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean
) {
val prevLeaf = inspectNode.prevLeaf()
val trailingCommaNode = prevLeaf.findPreviousTrailingCommaNodeOrNull()
val trailingCommaState = when {
isMultiline(psi) -> if (trailingCommaNode != null) TrailingCommaState.EXISTS else TrailingCommaState.MISSING
else -> if (trailingCommaNode != null) TrailingCommaState.REDUNDANT else TrailingCommaState.NOT_EXISTS
}
val isTrailingCommaAllowed = when (elementType) {
in TYPES_ON_DECLARATION_SITE -> allowTrailingComma
in TYPES_ON_CALL_SITE -> allowTrailingCommaOnCallSite
else -> false
}
when (trailingCommaState) {
TrailingCommaState.EXISTS -> if (!isTrailingCommaAllowed) {
emit(
trailingCommaNode!!.startOffset,
"Unnecessary trailing comma before \"${inspectNode.text}\"",
true
)
if (autoCorrect) {
this.removeChild(trailingCommaNode)
}
}
TrailingCommaState.MISSING -> if (isTrailingCommaAllowed) {
val prevNode = inspectNode.prevCodeLeaf()!!
emit(
prevNode.startOffset + prevNode.textLength,
"Missing trailing comma before \"${inspectNode.text}\"",
true
)
if (autoCorrect) {
val comma = KtPsiFactory(prevNode.psi).createComma()
prevNode.psi.parent.addAfter(comma, prevNode.psi)
}
}
TrailingCommaState.REDUNDANT -> {
emit(
trailingCommaNode!!.startOffset,
"Unnecessary trailing comma before \"${inspectNode.text}\"",
true
)
if (autoCorrect) {
this.removeChild(trailingCommaNode)
}
}
TrailingCommaState.NOT_EXISTS -> Unit
}
}

private fun ASTNode?.findPreviousTrailingCommaNodeOrNull(): ASTNode? {
var node = this
while (node?.isIgnorable() == true) {
node = node.prevLeaf()
}
return if (node?.elementType == ElementType.COMMA) {
node
} else {
null
}
}

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.anyDescendantOfType<KtCollectionLiteralExpression>() -> {
// special handling for collection literal
// @Annotation([
// "something",
// )]
val lastChild = element.collectDescendantsOfType<KtCollectionLiteralExpression>().last()
containsLineBreakInRange(element.rightParenthesis!!, lastChild.rightBracket!!)
}
else -> element.textContains('\n')
}

private fun ASTNode.isIgnorable(): Boolean =
elementType == ElementType.WHITE_SPACE ||
elementType == ElementType.EOL_COMMENT ||
elementType == ElementType.BLOCK_COMMENT

public companion object {

private val TYPES_ON_DECLARATION_SITE = TokenSet.create(
ElementType.DESTRUCTURING_DECLARATION,
ElementType.FUNCTION_LITERAL,
ElementType.FUNCTION_TYPE,
ElementType.TYPE_PARAMETER_LIST,
ElementType.VALUE_PARAMETER_LIST,
ElementType.WHEN_ENTRY
)

private val TYPES_ON_CALL_SITE = TokenSet.create(
ElementType.COLLECTION_LITERAL_EXPRESSION,
ElementType.INDICES,
ElementType.TYPE_ARGUMENT_LIST,
ElementType.VALUE_ARGUMENT_LIST
)

internal const val ALLOW_TRAILING_COMMA_NAME = "ij_kotlin_allow_trailing_comma"
private const val ALLOW_TRAILING_COMMA_DESCRIPTION = "Defines whether a trailing comma (or no trailing comma)" +
"should be enforced on the defining side," +
"e.g. parameter-list, type-argument-list, lambda-value-parameters, enum-entries, etc."

private val BOOLEAN_VALUES_SET = setOf("true", "false")

public val allowTrailingCommaProperty: UsesEditorConfigProperties.EditorConfigProperty<Boolean> =
UsesEditorConfigProperties.EditorConfigProperty(
type = PropertyType.LowerCasingPropertyType(
ALLOW_TRAILING_COMMA_NAME,
ALLOW_TRAILING_COMMA_DESCRIPTION,
PropertyValueParser.BOOLEAN_VALUE_PARSER,
BOOLEAN_VALUES_SET
),
defaultValue = false
)

internal const val ALLOW_TRAILING_COMMA_ON_CALL_SITE_NAME = "ij_kotlin_allow_trailing_comma_on_call_site"
private const val ALLOW_TRAILING_COMMA_ON_CALL_SITE_DESCRIPTION =
"Defines whether a trailing comma (or no trailing comma)" +
"should be enforced on the calling side," +
"e.g. argument-list, when-entries, lambda-arguments, indices, etc."

public val allowTrailingCommaOnCallSiteProperty: UsesEditorConfigProperties.EditorConfigProperty<Boolean> =
UsesEditorConfigProperties.EditorConfigProperty(
type = PropertyType.LowerCasingPropertyType(
ALLOW_TRAILING_COMMA_ON_CALL_SITE_NAME,
ALLOW_TRAILING_COMMA_ON_CALL_SITE_DESCRIPTION,
PropertyValueParser.BOOLEAN_VALUE_PARSER,
BOOLEAN_VALUES_SET
),
defaultValue = false
)
}
}

0 comments on commit 98d1a8e

Please sign in to comment.