Skip to content

Commit

Permalink
AnnotationRule: disallow empty line between annotation and annotated …
Browse files Browse the repository at this point in the history 
…target (#755)

* Create the test to ensure that stable indentation passes the requirement that the annotation is not separated from the object it is annotating

* Creating the test for the annotation that we want to make sure fails. Eg, this test should result in a failure since the @JvmField annotation is separated from the function by a blank line

* First pass at creating the rules for checking the line break - logic based on above rules for ensuring that we have a stable break with the list of annotations. Next step is to run unit test to make sure this triggers a failure

* Updating test to reflect the correct placement of the annotation so that we can ensure we see a valid break in the tests. Next step is to create a test for the auto correction

* Creating the test (red) that will fail currently, but ensure that we get a solid check for our issue once the autoformat is added to our code

* Updated the rule to get it working so that all the inclusions make sure we have all the fields input correctly and we remove redundant line breaks

* Adding extra tests and breaking our auto correction in testing multiple sources

* Ensure we do not apply this rule to @filename

* Ensure we fix the test so it satisfies all the issue we have seen so far

* Fixing ktlint issues

* Updating format on the kt files - strangely not being picked up earlier on ./gradlew :ktlint

* Adding additional test cases:

@JvmField @JvmStatic

fun foo() = Unit

// and

@JvmField
@JvmStatic

fun foo() = Unit

* Adding the test in as well as the piece for spaces between annotations

* Tweaking the measurements a bit so that we can handle all the edge cases

* Lint fixes

* Minor tweaks

Co-authored-by: Roman Zavarnitsyn <[email protected]>
  • Loading branch information
@AdamMTGreenberg @romtsn
AdamMTGreenberg and romtsn authored Jun 20, 2020
1 parent 42b555e commit 4dda69b
Show file tree
Hide file tree
Showing 2 changed files with 351 additions and 1 deletion.
104 changes: 103 additions & 1 deletion ...-experimental/src/main/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.core.ast.ElementType.TYPE_ARGUMENT_LIST
import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT
import com.pinterest.ktlint.core.ast.ElementType.VALUE_PARAMETER
import com.pinterest.ktlint.core.ast.children
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.lineNumber
import com.pinterest.ktlint.core.ast.nextLeaf
import com.pinterest.ktlint.core.ast.nextSibling
import com.pinterest.ktlint.core.ast.prevSibling
import com.pinterest.ktlint.core.ast.upsertWhitespaceBeforeMe
Expand All @@ -20,6 +21,7 @@ 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
import org.jetbrains.kotlin.psi.psiUtil.children
import org.jetbrains.kotlin.psi.psiUtil.endOffset
import org.jetbrains.kotlin.psi.psiUtil.getNextSiblingIgnoringWhitespaceAndComments
import org.jetbrains.kotlin.psi.psiUtil.nextLeaf
Expand All @@ -39,6 +41,8 @@ class AnnotationRule : Rule("annotation") {
"Annotations with parameters should all be placed on separate lines prior to the annotated construct"
const val fileAnnotationsShouldBeSeparated =
"File annotations should be separated from file contents with a blank line"
const val fileAnnotationsLineBreaks =
"There should not be empty lines between an annotation and the object that it's annotating"
}

override fun visit(
Expand Down Expand Up @@ -140,6 +144,67 @@ class AnnotationRule : Rule("annotation") {
}
}
}

// Check to make sure no trailing line breaks between annotation and object
val lineNumber = node.lineNumber()
val next = node.nextSiblingWithAtLeastOneOf(
{
!it.isWhiteSpace() &&
it.textLength > 0 &&
!(it.isPartOfComment() && it.lineNumber() == lineNumber) &&
!it.isPartOf(FILE_ANNOTATION_LIST)
},
{
val s = it.text
// Ensure at least one occurrence of two line breaks
s.indexOf("\n") != s.lastIndexOf("\n")
}
)
val nextLineNumber = next?.lineNumber()
if (lineNumber != null && nextLineNumber != null) {
val diff = nextLineNumber - lineNumber
// Ensure declaration is not on the same line, there is a line break in between, and it is not an
// annotation we explicitly want to have a line break between
if (diff > 1 && node.elementType != FILE_ANNOTATION_LIST) {
val psi = node.psi
emit(psi.endOffset - 1, fileAnnotationsLineBreaks, true)
if (autoCorrect) {
removeExtraLineBreaks(node)
}
}
}
if (whiteSpaces.isNotEmpty() && annotations.size > 1 && node.elementType != FILE_ANNOTATION_LIST) {
// 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)
if (autoCorrect) {
removeIntraLineBreaks(node, annotations.last())
}
}
}
}

private inline fun ASTNode.nextSiblingWithAtLeastOneOf(
p: (ASTNode) -> Boolean,
needsToOccur: (ASTNode) -> Boolean
): ASTNode? {
var n = this.treeNext
var occurrenceCount = 0
while (n != null) {
if (needsToOccur(n)) {
occurrenceCount++
}
if (p(n)) {
return if (occurrenceCount > 0) {
n
} else {
null
}
}
n = n.treeNext
}
return null
}

private fun getNewlineWithIndent(modifierListRoot: ASTNode): String {
Expand All @@ -155,8 +220,45 @@ class AnnotationRule : Rule("annotation") {
}
}

private fun removeExtraLineBreaks(node: ASTNode) {
val next = node.nextSibling {
it.isWhiteSpaceWithNewline()
} as? LeafPsiElement
if (next != null) {
rawReplaceExtraLineBreaks(next)
}
}

private fun rawReplaceExtraLineBreaks(leaf: LeafPsiElement) {
// Replace the extra white space with a single break
val text = leaf.text
val firstIndex = text.indexOf("\n") + 1
val replacementText = text.substring(0, firstIndex) +
text.substringAfter("\n").replace("\n", "")

leaf.rawReplaceWithText(replacementText)
}

private fun doesNotEndWithAComment(whiteSpaces: List<PsiWhiteSpace>): Boolean {
val lastNode = whiteSpaces.lastOrNull()?.nextLeaf()
return lastNode !is PsiComment || lastNode.nextLeaf()?.textContains('\n') == false
}

private fun removeIntraLineBreaks(
node: ASTNode,
last: KtAnnotationEntry
) {
val txt = node.text
// Pull the next before raw replace or it will blow up
val lNext = node.nextLeaf()
if (node is PsiWhiteSpaceImpl) {
if (txt.toCharArray().count { it == '\n' } > 1) {
rawReplaceExtraLineBreaks(node)
}
}

if (lNext != null && !last.text.endsWith(lNext.text)) {
removeIntraLineBreaks(lNext, last)
}
}
}
248 changes: 248 additions & 0 deletions ...erimental/src/test/kotlin/com/pinterest/ktlint/ruleset/experimental/AnnotationRuleTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -920,4 +920,252 @@ class AnnotationRuleTest {
)
).isEmpty()
}

@Test
fun `lint no empty lines between an annotation and object`() {
assertThat(
AnnotationRule().lint(
"""
@JvmField
fun foo() {}
""".trimIndent()
)
).isEmpty()
}

@Test
fun `lint there should not be empty lines between an annotation and object`() {
assertThat(
AnnotationRule().lint(
"""
@JvmField
fun foo() {}
""".trimIndent()
)
).isEqualTo(
listOf(
LintError(1, 9, "annotation", AnnotationRule.fileAnnotationsLineBreaks)
)
)
}

@Test
fun `lint there should not be empty lines between an annotation and object autocorrected`() {
val code =
"""
@JvmField
fun foo() {}
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField
fun foo() {}
""".trimIndent()
)
}

@Test
fun `lint there should not be empty lines between an annotation and object autocorrected with control`() {
val code =
"""
@JvmField
fun foo() {
@JvmStatic
val r = A()
}
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField
fun foo() {
@JvmStatic
val r = A()
}
""".trimIndent()
)
}

@Test
fun `lint there should not be empty lines between an annotation and object autocorrected multiple lines`() {
val code =
"""
@JvmField
fun foo() {}
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField
fun foo() {}
""".trimIndent()
)
}

@Test
fun `lint annotation on the same line remains there`() {
val code =
"""
@JvmField fun foo() {}
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField fun foo() {}
""".trimIndent()
)
}

@Test
fun `lint there should not be empty lines between multiple annotations`() {
val code =
"""
@JvmField @JvmStatic
fun foo() = Unit
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField @JvmStatic
fun foo() = Unit
""".trimIndent()
)
}

@Test
fun `lint there should not be empty lines between multiple annotations on multiple lines`() {
val code =
"""
@JvmField
@JvmStatic
fun foo() = Unit
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField
@JvmStatic
fun foo() = Unit
""".trimIndent()
)
}

@Test
fun `lint there should not be empty lines between multiple annotations with inline annotation`() {
val code =
"""
@JvmField
@JvmName
@JvmStatic fun foo() = Unit
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField
@JvmName
@JvmStatic
fun foo() = Unit
""".trimIndent()
)
}

@Test
fun `lint there should not be empty lines between two or more annotations`() {
val code =
"""
@JvmField
@JvmName
@JvmStatic
fun foo() = Unit
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField
@JvmName
@JvmStatic
fun foo() = Unit
""".trimIndent()
)
}

@Test
fun `lint there should not be empty lines between an annotation and object autocorrected multiple annotations`() {
val code =
"""
@JvmField
fun foo() {
@JvmStatic
val foo = Foo()
}
""".trimIndent()

assertThat(
AnnotationRule().format(code)
).isEqualTo(
"""
@JvmField
fun foo() {
@JvmStatic
val foo = Foo()
}
""".trimIndent()
)
}
}

0 comments on commit 4dda69b

Please sign in to comment.