Skip to content

Commit

Permalink
Handle comment between class/function name and parameterlist (#2541)
Browse files Browse the repository at this point in the history
Comments are not allowed between a class/function name and the parameter list.

* Insert newline after KDoc at correct position in the AST
* Report violation when parameter list is preceded by a comment

Closes #2535
paul-dingemans authored Feb 9, 2024
1 parent d38c899 commit 4c13743
Showing 4 changed files with 77 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ import com.pinterest.ktlint.rule.engine.core.api.indent
import com.pinterest.ktlint.rule.engine.core.api.nextLeaf
import com.pinterest.ktlint.rule.engine.core.api.noNewLineInClosedRange
import com.pinterest.ktlint.rule.engine.core.api.prevLeaf
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceBeforeMe
import com.pinterest.ktlint.rule.engine.core.api.upsertWhitespaceAfterMe
import com.pinterest.ktlint.ruleset.standard.StandardRule
import org.jetbrains.kotlin.com.intellij.lang.ASTNode

@@ -97,7 +97,7 @@ public class KdocWrappingRule :
) {
emit(startOffset, "A KDoc comment may not be followed by any other element on that same line", true)
if (autoCorrect) {
this.upsertWhitespaceBeforeMe(kdocCommentNode.indent())
kdocCommentNode.upsertWhitespaceAfterMe(kdocCommentNode.indent())
}
}

Original file line number Diff line number Diff line change
@@ -24,6 +24,8 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPE
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.MAX_LINE_LENGTH_PROPERTY
import com.pinterest.ktlint.rule.engine.core.api.firstChildLeafOrSelf
import com.pinterest.ktlint.rule.engine.core.api.indent
import com.pinterest.ktlint.rule.engine.core.api.isPartOfComment
import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace
import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpaceWithNewline
import com.pinterest.ktlint.rule.engine.core.api.leavesIncludingSelf
import com.pinterest.ktlint.rule.engine.core.api.nextLeaf
@@ -74,16 +76,12 @@ public class ParameterListWrappingRule :
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
when (node.elementType) {
NULLABLE_TYPE -> wrapNullableType(node, emit, autoCorrect)
VALUE_PARAMETER_LIST -> {
if (node.needToWrapParameterList()) {
wrapParameterList(node, emit, autoCorrect)
}
}
NULLABLE_TYPE -> visitNullableType(node, emit, autoCorrect)
VALUE_PARAMETER_LIST -> visitParameterList(node, emit, autoCorrect)
}
}

private fun wrapNullableType(
private fun visitNullableType(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean,
@@ -183,16 +181,27 @@ public class ParameterListWrappingRule :
.orEmpty()
.any { it.elementType == ElementType.ANNOTATION_ENTRY }

private fun wrapParameterList(
private fun visitParameterList(
node: ASTNode,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
autoCorrect: Boolean,
) {
node
.children()
.forEach { child -> wrapParameterInList(child, emit, autoCorrect) }
if (isPrecededByComment(node)) {
emit(node.startOffset, "Parameter list should not be preceded by a comment", false)
} else if (node.needToWrapParameterList()) {
node
.children()
.forEach { child -> wrapParameterInList(child, emit, autoCorrect) }
}
}

private fun isPrecededByComment(node: ASTNode) =
node
.prevLeaf { !it.isWhiteSpace() }
?.prevLeaf()
?.isPartOfComment()
?: false

private fun intendedIndent(child: ASTNode): String =
when {
// IDEA quirk:
Original file line number Diff line number Diff line change
@@ -111,4 +111,31 @@ class KdocWrappingRuleTest {
.hasLintViolation(2, 29, "A KDoc comment may not be followed by any other element on that same line")
.isFormattedAs(formattedCode)
}

@Test
fun `Issue 2535 - Given a class with a KDoc on the primary constructor instead of on the class name`() {
val code =
// Code sample below contains a code smell. The KDoc is at the wrong position.
"""
class ClassA
/**
* some comment
*/(paramA: String)
""".trimIndent()
val formattedCode =
// Code sample below contains a code smell. The KDoc is at the wrong position. The formatted code is not intended to show
// that this code is well formatted according to good coding practices. It merely is used to validate that the newline is
// inserted at the correct position in the AST so that no exception will be thrown in the ParameterListWrappingRule.
"""
class ClassA
/**
* some comment
*/
(paramA: String)
""".trimIndent()
kdocWrappingRuleAssertThat(code)
.addAdditionalRuleProvider { ParameterListWrappingRule() }
.hasLintViolation(4, 8, "A KDoc comment may not be followed by any other element on that same line")
.isFormattedAs(formattedCode)
}
}
Original file line number Diff line number Diff line change
@@ -702,4 +702,32 @@ class ParameterListWrappingRuleTest {
LintViolation(3, 61, "Exceeded max line length (60)", false),
).hasNoLintViolationsExceptInAdditionalRules()
}

@Test
fun `Issue 2535 - Given a class with a KDoc on the primary constructor instead of on the class name`() {
val code =
// Code sample below contains a code smell. The KDoc is at the wrong position.
"""
class ClassA
/**
* some comment
*/(paramA: String)
""".trimIndent()
val formattedCode =
// Code sample below contains a code smell. The KDoc is at the wrong position. The formatted code is not intended to show
// that this code is well formatted according to good coding practices. It merely is used to validate that the newline is
// inserted at the correct position in the AST so that no exception will be thrown in the ParameterListWrappingRule.
"""
class ClassA
/**
* some comment
*/
(paramA: String)
""".trimIndent()
parameterListWrappingRuleAssertThat(code)
.addAdditionalRuleProvider { KdocWrappingRule() }
.hasLintViolations(
LintViolation(4, 8, "Parameter list should not be preceded by a comment", false),
).isFormattedAs(formattedCode)
}
}

0 comments on commit 4c13743

Please sign in to comment.