Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ModifierOrderRule to include annotations as part of its checks #183

Merged
merged 4 commits into from
Apr 11, 2018
Merged

Update ModifierOrderRule to include annotations as part of its checks #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b096a5b
Modify ModifierOrderRuleTest to include annotations
JelloRanger Apr 8, 2018
fd2bc0e
Fix ModifierOrderRule to include annotations
JelloRanger Apr 8, 2018
9a2191c
Fix ModifierRule autoFormat by replacing children elements
JelloRanger Apr 9, 2018
9a0aa15
Change ModifierOrderRule to squash annotations in error output
JelloRanger Apr 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions ...t-standard/src/main/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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<ASTNode>): List<String> {
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 }
}
}
}
43 changes: 38 additions & 5 deletions ...andard/src/test/kotlin/com/github/shyiko/ktlint/ruleset/standard/ModifierOrderRuleTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,30 +25,43 @@ 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 = ""
}
}
""".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\")"),
LintError(5, 5, "modifier-order", "Incorrect modifier order (should be \"abstract tailrec\")"),
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\")")
))
}

@Test
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
Expand All @@ -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 = ""
Expand All @@ -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
Expand All @@ -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 = ""
Expand Down