diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRule.kt index 648b94f57a..3c9c9e5dad 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRule.kt @@ -2,7 +2,6 @@ package com.github.shyiko.ktlint.ruleset.standard import com.github.shyiko.ktlint.core.Rule import org.jetbrains.kotlin.com.intellij.lang.ASTNode -import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet import org.jetbrains.kotlin.lexer.KtTokens.ABSTRACT_KEYWORD import org.jetbrains.kotlin.lexer.KtTokens.ACTUAL_KEYWORD @@ -29,13 +28,16 @@ import org.jetbrains.kotlin.lexer.KtTokens.SEALED_KEYWORD import org.jetbrains.kotlin.lexer.KtTokens.SUSPEND_KEYWORD import org.jetbrains.kotlin.lexer.KtTokens.TAILREC_KEYWORD import org.jetbrains.kotlin.lexer.KtTokens.VARARG_KEYWORD +import org.jetbrains.kotlin.psi.KtAnnotationEntry import org.jetbrains.kotlin.psi.KtDeclarationModifierList +import org.jetbrains.kotlin.psi.stubs.elements.KtStubElementTypes.ANNOTATION_ENTRY import java.util.Arrays class ModifierOrderRule : Rule("modifier-order") { - // subset of KtTokens.MODIFIER_KEYWORDS_ARRAY + // subset of KtTokens.MODIFIER_KEYWORDS_ARRAY (+ annotations entries) private val order = arrayOf( + ANNOTATION_ENTRY, PUBLIC_KEYWORD, PROTECTED_KEYWORD, PRIVATE_KEYWORD, INTERNAL_KEYWORD, EXPECT_KEYWORD, ACTUAL_KEYWORD, FINAL_KEYWORD, OPEN_KEYWORD, ABSTRACT_KEYWORD, SEALED_KEYWORD, CONST_KEYWORD, @@ -66,16 +68,26 @@ class ModifierOrderRule : Rule("modifier-order") { val modifierArr = node.getChildren(tokenSet) val sorted = modifierArr.copyOf().apply { sortWith(compareBy { order.indexOf(it.elementType) }) } if (!Arrays.equals(modifierArr, sorted)) { + // Since annotations can be fairly lengthy and/or span multiple lines we are + // squashing them into a single placeholder text to guarantee a single line output emit(node.startOffset, "Incorrect modifier order (should be \"${ - sorted.map { it.text }.joinToString(" ") + squashAnnotations(sorted).joinToString(" ") }\")", true) if (autoCorrect) { modifierArr.forEachIndexed { i, n -> - // fixme: find a better way (node type is now potentially out of sync) - (n.psi as LeafPsiElement).rawReplaceWithText(sorted[i].text) + node.replaceChild(n, sorted[i].clone() as ASTNode) } } } } } + + private fun squashAnnotations(sorted: Array): List { + val nonAnnotationModifiers = sorted.filter { it.psi !is KtAnnotationEntry } + return if (nonAnnotationModifiers.size != sorted.size) { + listOf("@Annotation...") + nonAnnotationModifiers.map { it.text } + } else { + nonAnnotationModifiers.map { it.text } + } + } } diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRuleTest.kt index 9d24596af0..24372bbc4b 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRuleTest.kt @@ -13,7 +13,7 @@ class ModifierOrderRuleTest { // pretty much every line below should trip an error assertThat(ModifierOrderRule().lint( """ - abstract open class A { // open is here for test purposes only, otherwise it's redundant + abstract @Deprecated open class A { // open is here for test purposes only, otherwise it's redundant open protected val v = "" open suspend internal fun f(v: Any): Any = "" lateinit public var lv: String @@ -25,6 +25,16 @@ class ModifierOrderRuleTest { suspend override fun f(v: Any): Any = "" tailrec override fun findFixPoint(x: Double): Double = if (x == Math.cos(x)) x else findFixPoint(Math.cos(x)) + override @Annotation fun getSomething() = "" + override @Annotation suspend public @Woohoo(data = "woohoo") fun doSomething() = "" + @A + @B(v = [ + "foo", + "baz", + "bar" + ]) + @C + suspend public fun returnsSomething() = "" companion object { const internal val V = "" @@ -32,7 +42,7 @@ class ModifierOrderRuleTest { } """.trimIndent() )).isEqualTo(listOf( - LintError(1, 1, "modifier-order", "Incorrect modifier order (should be \"open abstract\")"), + LintError(1, 1, "modifier-order", "Incorrect modifier order (should be \"@Annotation... open abstract\")"), LintError(2, 5, "modifier-order", "Incorrect modifier order (should be \"protected open\")"), LintError(3, 5, "modifier-order", "Incorrect modifier order (should be \"internal open suspend\")"), LintError(4, 5, "modifier-order", "Incorrect modifier order (should be \"public lateinit\")"), @@ -40,7 +50,10 @@ class ModifierOrderRuleTest { LintError(9, 5, "modifier-order", "Incorrect modifier order (should be \"public override\")"), LintError(10, 5, "modifier-order", "Incorrect modifier order (should be \"override suspend\")"), LintError(11, 5, "modifier-order", "Incorrect modifier order (should be \"override tailrec\")"), - LintError(15, 8, "modifier-order", "Incorrect modifier order (should be \"internal const\")") + LintError(13, 5, "modifier-order", "Incorrect modifier order (should be \"@Annotation... override\")"), + LintError(14, 5, "modifier-order", "Incorrect modifier order (should be \"@Annotation... public override suspend\")"), + LintError(15, 5, "modifier-order", "Incorrect modifier order (should be \"@Annotation... public suspend\")"), + LintError(25, 8, "modifier-order", "Incorrect modifier order (should be \"internal const\")") )) } @@ -48,7 +61,7 @@ class ModifierOrderRuleTest { fun testFormat() { assertThat(ModifierOrderRule().format( """ - abstract open class A { // open is here for test purposes only, otherwise it's redundant + abstract @Deprecated open class A { // open is here for test purposes only, otherwise it's redundant open protected val v = "" open suspend internal fun f(v: Any): Any = "" lateinit public var lv: String @@ -60,6 +73,16 @@ class ModifierOrderRuleTest { suspend override fun f(v: Any): Any = "" tailrec override fun findFixPoint(x: Double): Double = if (x == Math.cos(x)) x else findFixPoint(Math.cos(x)) + override @Annotation fun getSomething() = "" + suspend @Annotation override public @Woohoo(data = "woohoo") fun doSomething() = "" + @A + @B(v = [ + "foo", + "baz", + "bar" + ]) + @C + suspend public fun returnsSomething() = "" companion object { const internal val V = "" @@ -68,7 +91,7 @@ class ModifierOrderRuleTest { """ )).isEqualTo( """ - open abstract class A { // open is here for test purposes only, otherwise it's redundant + @Deprecated open abstract class A { // open is here for test purposes only, otherwise it's redundant protected open val v = "" internal open suspend fun f(v: Any): Any = "" public lateinit var lv: String @@ -80,6 +103,16 @@ class ModifierOrderRuleTest { override suspend fun f(v: Any): Any = "" override tailrec fun findFixPoint(x: Double): Double = if (x == Math.cos(x)) x else findFixPoint(Math.cos(x)) + @Annotation override fun getSomething() = "" + @Annotation @Woohoo(data = "woohoo") public override suspend fun doSomething() = "" + @A + @B(v = [ + "foo", + "baz", + "bar" + ]) + @C + public suspend fun returnsSomething() = "" companion object { internal const val V = ""