From b096a5b2eec805a44e0389eb30ef461974813725 Mon Sep 17 00:00:00 2001 From: JelloRanger Date: Sat, 7 Apr 2018 18:30:04 -0700 Subject: [PATCH 1/4] Modify ModifierOrderRuleTest to include annotations --- .../ruleset/standard/ModifierOrderRuleTest.kt | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) 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..1e99ae9321 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,10 @@ 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() = "" + @AnnotationAboveLine + fun returnsSomething() = "" companion object { const internal val V = "" @@ -32,7 +36,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 \"@Deprecated 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 +44,9 @@ 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 @Woohoo(data = \"woohoo\") public override suspend\")"), + LintError(19, 8, "modifier-order", "Incorrect modifier order (should be \"internal const\")") )) } @@ -48,7 +54,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 +66,10 @@ 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() = "" + @AnnotationAboveLine + fun returnsSomething() = "" companion object { const internal val V = "" @@ -68,7 +78,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 +90,10 @@ 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() = "" + @AnnotationAboveLine + fun returnsSomething() = "" companion object { internal const val V = "" From fd2bc0e87c34bc51090885d90e645f7d565a704f Mon Sep 17 00:00:00 2001 From: JelloRanger Date: Sat, 7 Apr 2018 18:34:57 -0700 Subject: [PATCH 2/4] Fix ModifierOrderRule to include annotations --- .../ktlint/ruleset/standard/ModifierOrderRule.kt | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) 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..ba07b25346 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,7 @@ 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.impl.source.tree.PsiWhiteSpaceImpl 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 @@ -30,12 +30,14 @@ 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.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, @@ -70,9 +72,12 @@ class ModifierOrderRule : Rule("modifier-order") { sorted.map { it.text }.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.removeRange(node.firstChildNode, node.lastChildNode.treeNext) + modifierArr.forEachIndexed { i, _ -> + node.addChild(sorted[i], null) + if (i != sorted.size - 1) { + node.addChild(PsiWhiteSpaceImpl(" "), null) + } } } } From 9a2191c6e798785d22494b7525ea0b4589d0f0e3 Mon Sep 17 00:00:00 2001 From: JelloRanger Date: Mon, 9 Apr 2018 00:08:23 -0700 Subject: [PATCH 3/4] Fix ModifierRule autoFormat by replacing children elements - The assumption that whitespace always separates modifiers isn't true since annotations can be on separate lines. - Added test case to catch previous error --- .../ruleset/standard/ModifierOrderRule.kt | 9 +---- .../ruleset/standard/ModifierOrderRuleTest.kt | 38 +++++++++++++++---- 2 files changed, 33 insertions(+), 14 deletions(-) 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 ba07b25346..25c5d035b6 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.PsiWhiteSpaceImpl 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 @@ -72,12 +71,8 @@ class ModifierOrderRule : Rule("modifier-order") { sorted.map { it.text }.joinToString(" ") }\")", true) if (autoCorrect) { - node.removeRange(node.firstChildNode, node.lastChildNode.treeNext) - modifierArr.forEachIndexed { i, _ -> - node.addChild(sorted[i], null) - if (i != sorted.size - 1) { - node.addChild(PsiWhiteSpaceImpl(" "), null) - } + modifierArr.forEachIndexed { i, n -> + node.replaceChild(n, sorted[i].clone() as ASTNode) } } } 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 1e99ae9321..6f1724c9b9 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 @@ -27,8 +27,14 @@ class ModifierOrderRuleTest { = if (x == Math.cos(x)) x else findFixPoint(Math.cos(x)) override @Annotation fun getSomething() = "" override @Annotation suspend public @Woohoo(data = "woohoo") fun doSomething() = "" - @AnnotationAboveLine - fun returnsSomething() = "" + @A + @B(v = [ + "foo", + "baz", + "bar" + ]) + @C + suspend public fun returnsSomething() = "" companion object { const internal val V = "" @@ -46,7 +52,13 @@ class ModifierOrderRuleTest { LintError(11, 5, "modifier-order", "Incorrect modifier order (should be \"override tailrec\")"), LintError(13, 5, "modifier-order", "Incorrect modifier order (should be \"@Annotation override\")"), LintError(14, 5, "modifier-order", "Incorrect modifier order (should be \"@Annotation @Woohoo(data = \"woohoo\") public override suspend\")"), - LintError(19, 8, "modifier-order", "Incorrect modifier order (should be \"internal const\")") + LintError(15, 5, "modifier-order", """Incorrect modifier order (should be "@A @B(v = [ + | "foo", + | "baz", + | "bar" + | ]) @C public suspend") + """.trimMargin()), + LintError(25, 8, "modifier-order", "Incorrect modifier order (should be \"internal const\")") )) } @@ -68,8 +80,14 @@ class ModifierOrderRuleTest { = if (x == Math.cos(x)) x else findFixPoint(Math.cos(x)) override @Annotation fun getSomething() = "" suspend @Annotation override public @Woohoo(data = "woohoo") fun doSomething() = "" - @AnnotationAboveLine - fun returnsSomething() = "" + @A + @B(v = [ + "foo", + "baz", + "bar" + ]) + @C + suspend public fun returnsSomething() = "" companion object { const internal val V = "" @@ -92,8 +110,14 @@ class ModifierOrderRuleTest { = if (x == Math.cos(x)) x else findFixPoint(Math.cos(x)) @Annotation override fun getSomething() = "" @Annotation @Woohoo(data = "woohoo") public override suspend fun doSomething() = "" - @AnnotationAboveLine - fun returnsSomething() = "" + @A + @B(v = [ + "foo", + "baz", + "bar" + ]) + @C + public suspend fun returnsSomething() = "" companion object { internal const val V = "" From 9a0aa1587e2d40ebb9f6c7324048e4b5072d433b Mon Sep 17 00:00:00 2001 From: JelloRanger Date: Tue, 10 Apr 2018 23:20:11 -0700 Subject: [PATCH 4/4] Change ModifierOrderRule to squash annotations in error output - This is the guarantee that lengthy annotations or annotations with new lines don't cause an individual error to span multiple lines --- .../ktlint/ruleset/standard/ModifierOrderRule.kt | 14 +++++++++++++- .../ruleset/standard/ModifierOrderRuleTest.kt | 13 ++++--------- 2 files changed, 17 insertions(+), 10 deletions(-) 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 25c5d035b6..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 @@ -28,6 +28,7 @@ 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 @@ -67,8 +68,10 @@ 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 -> @@ -78,4 +81,13 @@ class ModifierOrderRule : Rule("modifier-order") { } } } + + 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 6f1724c9b9..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 @@ -42,7 +42,7 @@ class ModifierOrderRuleTest { } """.trimIndent() )).isEqualTo(listOf( - LintError(1, 1, "modifier-order", "Incorrect modifier order (should be \"@Deprecated 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\")"), @@ -50,14 +50,9 @@ 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(13, 5, "modifier-order", "Incorrect modifier order (should be \"@Annotation override\")"), - LintError(14, 5, "modifier-order", "Incorrect modifier order (should be \"@Annotation @Woohoo(data = \"woohoo\") public override suspend\")"), - LintError(15, 5, "modifier-order", """Incorrect modifier order (should be "@A @B(v = [ - | "foo", - | "baz", - | "bar" - | ]) @C public suspend") - """.trimMargin()), + 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\")") )) }