From a17ad9f1ca5a63120bcca0b26f8e6b46b4a61534 Mon Sep 17 00:00:00 2001 From: Sha Sha Chu Date: Mon, 22 Feb 2021 10:45:09 -0800 Subject: [PATCH 1/5] Fixing bug with experiment:annotation-spacing-rule with comments --- .../experimental/AnnotationSpacingRule.kt | 46 +++++--- .../experimental/AnnotationSpacingRuleTest.kt | 101 +++++++++++++++++- 2 files changed, 128 insertions(+), 19 deletions(-) diff --git a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt index 8a3ecebfbd..0a168c14ce 100644 --- a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt +++ b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt @@ -1,15 +1,10 @@ package com.pinterest.ktlint.ruleset.experimental import com.pinterest.ktlint.core.Rule -import com.pinterest.ktlint.core.ast.ElementType -import com.pinterest.ktlint.core.ast.isPartOf -import com.pinterest.ktlint.core.ast.isPartOfComment -import com.pinterest.ktlint.core.ast.isWhiteSpace -import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline -import com.pinterest.ktlint.core.ast.nextLeaf -import com.pinterest.ktlint.core.ast.nextSibling +import com.pinterest.ktlint.core.ast.* import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace +import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl import org.jetbrains.kotlin.psi.KtAnnotationEntry @@ -17,15 +12,14 @@ import org.jetbrains.kotlin.psi.psiUtil.children import org.jetbrains.kotlin.psi.psiUtil.endOffset /** - * Ensures there are not multiple line breaks between annotations and declarations. + * Ensures annotations occur immediately prior to the annotated construct * * https://kotlinlang.org/docs/reference/coding-conventions.html#annotation-formatting */ class AnnotationSpacingRule : Rule("annotation-spacing") { companion object { - const val fileAnnotationsLineBreaks = - "There should not be empty lines between an annotation and the object that it's annotating" + const val ERROR_MESSAGE = "Annotations should occur immediately before the annotated construct" } override fun visit( @@ -62,21 +56,36 @@ class AnnotationSpacingRule : Rule("annotation-spacing") { { !it.isWhiteSpace() && it.textLength > 0 && - !(it.isPartOfComment() /* && it.lineNumber() == lineNumber*/) && !it.isPartOf(ElementType.FILE_ANNOTATION_LIST) }, { - val s = it.text - // Ensure at least one occurrence of two line breaks - s.indexOf("\n") != s.lastIndexOf("\n") + // Disallow multiple white spaces as well as comments + if (it.psi is PsiWhiteSpace) { + val s = it.text + // Ensure at least one occurrence of two line breaks + s.indexOf("\n") != s.lastIndexOf("\n") + } else it.isPartOfComment() } ) if (next != null) { if (node.elementType != ElementType.FILE_ANNOTATION_LIST) { val psi = node.psi - emit(psi.endOffset - 1, fileAnnotationsLineBreaks, true) + emit(psi.endOffset - 1, ERROR_MESSAGE, true) if (autoCorrect) { - removeExtraLineBreaks(node) + // Special-case autocorrection when the annotation is separated from the annotated construct + // by a comment: we need to swap the order of the comment and the annotation + if (next.isPartOfComment()) { + // Remove the annotation and the following whitespace + val nextSibling = node.nextSibling { it.isWhiteSpace() } + node.treeParent.removeChild(node) + nextSibling?.treeParent?.removeChild(nextSibling) + // Insert the annotation prior to the annotated construct + val space = PsiWhiteSpaceImpl("\n") + next.treeParent.addChild(space, next.nextCodeSibling()) + next.treeParent.addChild(node, space) + } else { + removeExtraLineBreaks(node) + } } } } @@ -84,7 +93,7 @@ class AnnotationSpacingRule : Rule("annotation-spacing") { // Check to make sure there are multi breaks between annotations if (whiteSpaces.any { psi -> psi.textToCharArray().filter { it == '\n' }.count() > 1 }) { val psi = node.psi - emit(psi.endOffset - 1, fileAnnotationsLineBreaks, true) + emit(psi.endOffset - 1, ERROR_MESSAGE, true) if (autoCorrect) { removeIntraLineBreaks(node, annotations.last()) } @@ -114,6 +123,9 @@ class AnnotationSpacingRule : Rule("annotation-spacing") { return null } + private fun ASTNode?.isWhiteSpaceWithMultipleNewlines() = + this != null && elementType == ElementType.WHITE_SPACE && text.contains("\n\n") + private fun removeExtraLineBreaks(node: ASTNode) { val next = node.nextSibling { it.isWhiteSpaceWithNewline() diff --git a/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRuleTest.kt b/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRuleTest.kt index e259a9dc59..82e438d006 100644 --- a/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRuleTest.kt +++ b/ktlint-ruleset-experimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRuleTest.kt @@ -2,7 +2,6 @@ package com.pinterest.ktlint.ruleset.experimental import com.pinterest.ktlint.core.KtLint import com.pinterest.ktlint.core.LintError -import com.pinterest.ktlint.ruleset.experimental.AnnotationSpacingRule.Companion.fileAnnotationsLineBreaks import com.pinterest.ktlint.test.format import com.pinterest.ktlint.test.lint import java.util.ArrayList @@ -37,7 +36,7 @@ class AnnotationSpacingRuleTest { ) ).isEqualTo( listOf( - LintError(1, 9, "annotation-spacing", fileAnnotationsLineBreaks) + LintError(1, 9, "annotation-spacing", AnnotationSpacingRule.ERROR_MESSAGE) ) ) } @@ -328,4 +327,102 @@ class AnnotationSpacingRuleTest { it.ruleId == "experimental:argument-list-wrapping" } } + + @Test + fun `annotations should not be separated by comments from the annotated construct`() { + val code = + """ + @Suppress("DEPRECATION") @Hello + /** + * block comment + */ + class Foo { + } + """.trimIndent() + assertThat( + AnnotationSpacingRule().lint(code) + ).isEqualTo( + listOf( + LintError(1, 31, "annotation-spacing", AnnotationSpacingRule.ERROR_MESSAGE) + ) + ) + } + + @Test + fun `annotations should be moved after comments`() { + val code = + """ + @Suppress("DEPRECATION") @Hello + /** + * block comment + */ + class Foo { + } + """.trimIndent() + assertThat( + AnnotationSpacingRule().format(code) + ).isEqualTo( + """ + /** + * block comment + */ + @Suppress("DEPRECATION") @Hello + class Foo { + } + """.trimIndent() + ) + + val codeEOL = + """ + @Suppress("DEPRECATION") @Hello + // hello + class Foo { + } + """.trimIndent() + assertThat( + AnnotationSpacingRule().format(codeEOL) + ).isEqualTo( + """ + // hello + @Suppress("DEPRECATION") @Hello + class Foo { + } + """.trimIndent() + ) + } + + @Test + fun `preceding whitespaces are preserved`() { + val code = + """ + package a.b.c + + val hello = 5 + + + @Suppress("DEPRECATION") @Hello + /** + * block comment + */ + class Foo { + } + """.trimIndent() + assertThat( + AnnotationSpacingRule().format(code) + ).isEqualTo( + """ + package a.b.c + + val hello = 5 + + + /** + * block comment + */ + @Suppress("DEPRECATION") @Hello + class Foo { + } + """.trimIndent() + ) + } } From 14d5ae9aef4d8fb2daf2921f9640fdf9d67fe808 Mon Sep 17 00:00:00 2001 From: Sha Sha Chu Date: Mon, 22 Feb 2021 10:48:45 -0800 Subject: [PATCH 2/5] updating changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99a62d2e8a..8dbcf98b52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Fix formatting with comments (`colon-spacing`) ([#1057](https://github.com/pinterest/ktlint/issues/1057)) - Fix IndexOutOfBoundsException in `argument-list-wrapping-rule` formatting file with many corrections ([#1081](https://github.com/pinterest/ktlint/issues/1081)) - Fix formatting in arguments (`multiline-if-else`) ([#1079](https://github.com/pinterest/ktlint/issues/1079)) +- Fix experimental:annotation-spacing-rule autocorrection with comments ### Changed - Update Gradle shadow plugin to `6.1.0` version From 9e81b6e2b82ef3ecbb78220e74a487c531125d38 Mon Sep 17 00:00:00 2001 From: Sha Sha Chu Date: Mon, 22 Feb 2021 10:51:03 -0800 Subject: [PATCH 3/5] deleting unused function --- .../ktlint/ruleset/experimental/AnnotationSpacingRule.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt index 0a168c14ce..75b9a0f7bd 100644 --- a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt +++ b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt @@ -123,9 +123,6 @@ class AnnotationSpacingRule : Rule("annotation-spacing") { return null } - private fun ASTNode?.isWhiteSpaceWithMultipleNewlines() = - this != null && elementType == ElementType.WHITE_SPACE && text.contains("\n\n") - private fun removeExtraLineBreaks(node: ASTNode) { val next = node.nextSibling { it.isWhiteSpaceWithNewline() From 655e12894d5e941a0324e86fe06f06ca267d06e5 Mon Sep 17 00:00:00 2001 From: Sha Sha Chu Date: Mon, 22 Feb 2021 10:52:51 -0800 Subject: [PATCH 4/5] fix imports --- .../ruleset/experimental/AnnotationSpacingRule.kt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt index 75b9a0f7bd..48ba99dbfe 100644 --- a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt +++ b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt @@ -1,10 +1,16 @@ package com.pinterest.ktlint.ruleset.experimental import com.pinterest.ktlint.core.Rule -import com.pinterest.ktlint.core.ast.* import org.jetbrains.kotlin.com.intellij.lang.ASTNode import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace -import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafElement +import com.pinterest.ktlint.core.ast.ElementType +import com.pinterest.ktlint.core.ast.isPartOf +import com.pinterest.ktlint.core.ast.isPartOfComment +import com.pinterest.ktlint.core.ast.isWhiteSpace +import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline +import com.pinterest.ktlint.core.ast.nextLeaf +import com.pinterest.ktlint.core.ast.nextSibling +import com.pinterest.ktlint.core.ast.nextCodeSibling import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl import org.jetbrains.kotlin.psi.KtAnnotationEntry From ce275f0fbb2d410bdaffb53d0bec841547b1d4b7 Mon Sep 17 00:00:00 2001 From: Sha Sha Chu Date: Mon, 22 Feb 2021 12:44:28 -0800 Subject: [PATCH 5/5] fix build --- .../ktlint/ruleset/experimental/AnnotationSpacingRule.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt index 48ba99dbfe..41f061f12e 100644 --- a/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt +++ b/ktlint-ruleset-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationSpacingRule.kt @@ -1,16 +1,16 @@ package com.pinterest.ktlint.ruleset.experimental import com.pinterest.ktlint.core.Rule -import org.jetbrains.kotlin.com.intellij.lang.ASTNode -import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace import com.pinterest.ktlint.core.ast.ElementType import com.pinterest.ktlint.core.ast.isPartOf import com.pinterest.ktlint.core.ast.isPartOfComment import com.pinterest.ktlint.core.ast.isWhiteSpace import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline +import com.pinterest.ktlint.core.ast.nextCodeSibling import com.pinterest.ktlint.core.ast.nextLeaf import com.pinterest.ktlint.core.ast.nextSibling -import com.pinterest.ktlint.core.ast.nextCodeSibling +import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl import org.jetbrains.kotlin.psi.KtAnnotationEntry