From 3a28022ffea0816e5640ed127c5c7fa22a5d68e1 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Mon, 19 Oct 2020 09:18:15 +0900 Subject: [PATCH] SpacingBetweenDeclarationsWithCommentsRule: fix false positives/negatives - Fix false positives when a comment is not between declarations (#865) - Fix false negatives when declarations is in class --- ...cingBetweenDeclarationsWithCommentsRule.kt | 13 ++- ...BetweenDeclarationsWithCommentsRuleTest.kt | 79 ++++++++++++++++++- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRule.kt b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRule.kt index c3f2056473..079daf4141 100644 --- a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRule.kt +++ b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRule.kt @@ -3,11 +3,14 @@ package com.pinterest.ktlint.ruleset.experimental import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType.FILE import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE +import com.pinterest.ktlint.core.ast.prevLeaf import com.pinterest.ktlint.core.ast.prevSibling import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiComment import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement +import org.jetbrains.kotlin.psi.KtDeclaration +import org.jetbrains.kotlin.psi.psiUtil.getPrevSiblingIgnoringWhitespaceAndComments /** * @see https://youtrack.jetbrains.com/issue/KT-35088 @@ -20,19 +23,23 @@ class SpacingBetweenDeclarationsWithCommentsRule : Rule("spacing-between-declara emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit ) { if (node is PsiComment) { - val prevSibling = node.parent.node.prevSibling { it.elementType != WHITE_SPACE } + val declaration = node.parent as? KtDeclaration ?: return + if (declaration.getPrevSiblingIgnoringWhitespaceAndComments() !is KtDeclaration) return + + val prevSibling = declaration.node.prevSibling { it.elementType != WHITE_SPACE } if (prevSibling != null && prevSibling.elementType != FILE && prevSibling !is PsiComment ) { - if (node.parent.prevSibling is PsiWhiteSpace && node.parent.prevSibling.text == "\n") { + if (declaration.prevSibling is PsiWhiteSpace && declaration.prevSibling.text.count { it == '\n' } < 2) { emit( node.startOffset, "Declarations and declarations with comments should have an empty space between.", true ) if (autoCorrect) { - (node.parent.prevSibling.node as LeafPsiElement).rawReplaceWithText("\n\n") + val indent = node.prevLeaf()?.text?.trim('\n') ?: "" + (declaration.prevSibling.node as LeafPsiElement).rawReplaceWithText("\n\n$indent") } } } diff --git a/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRuleTest.kt b/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRuleTest.kt index 24f8c0f3b1..6a0785c2bc 100644 --- a/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRuleTest.kt +++ b/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/SpacingBetweenDeclarationsWithCommentsRuleTest.kt @@ -119,13 +119,13 @@ class SpacingBetweenDeclarationsWithCommentsRuleTest { * Doc 1 */ fun one() = 1 - + /** * Doc 2 */ fun two() = 2 fun three() = 42 - + // comment fun four() = 44 """.trimIndent(), @@ -146,4 +146,79 @@ class SpacingBetweenDeclarationsWithCommentsRuleTest { ) ) } + + @Test + fun `not a declaration comment`() { + assertThat( + SpacingBetweenDeclarationsWithCommentsRule().lint( + """ + fun function() { + with(binding) { + loginButton.setOnClickListener { + // comment + showScreen() + } + } + } + """.trimIndent() + ) + ).isEmpty() + } + + @Test + fun `not a comment between declarations`() { + assertThat( + SpacingBetweenDeclarationsWithCommentsRule().lint( + """ + class C { + fun test() { + println() + // comment1 + fun one() = 1 + } + } + """.trimIndent() + ) + ).isEmpty() + } + + @Test + fun `declarations in class`() { + val actual = SpacingBetweenDeclarationsWithCommentsRule().format( + """ + class C { + /** + * Doc 1 + */ + fun one() = 1 + /** + * Doc 2 + */ + fun two() = 2 + fun three() = 42 + // comment + fun four() = 44 + } + """.trimIndent() + ) + assertThat(actual).isEqualTo( + """ + class C { + /** + * Doc 1 + */ + fun one() = 1 + + /** + * Doc 2 + */ + fun two() = 2 + fun three() = 42 + + // comment + fun four() = 44 + } + """.trimIndent() + ) + } }