From 250a3e467d2624a123175f6a392c8742e6dd81e0 Mon Sep 17 00:00:00 2001 From: Sha Sha Chu Date: Tue, 23 Feb 2021 10:02:29 -0800 Subject: [PATCH] Annotation spacing fix (#1087) * Fixing bug with experiment:annotation-spacing-rule with comments * updating changelog * deleting unused function * fix imports * fix build --- CHANGELOG.md | 1 + .../experimental/AnnotationSpacingRule.kt | 35 ++++-- .../experimental/AnnotationSpacingRuleTest.kt | 101 +++++++++++++++++- 3 files changed, 125 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1965256fbe..ab9741b94e 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 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..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 @@ -6,6 +6,7 @@ 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 org.jetbrains.kotlin.com.intellij.lang.ASTNode @@ -17,15 +18,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 +62,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 +99,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()) } 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() + ) + } }