Repository: analysis-dev/diktat (kotlin)

Issue:
Reuse com.pinterest.ktlint.core.Rule.VisitorModifier.RunAfterRule to keep order of rules
Ordering was changed in https://github.com/pinterest/ktlint/issues/1478: previously ktlint kept order of rules as ResultSetProvider returned it.
But now `ktlink` sorts rules by ID.

We found duplication and invalid order in rule id in scope of #1306.
Now prefix of ID which was introduced as tactic solution for ordering, can be removed

Pull request:
### What's done:
* added a wrapper for rule with c.p.k.c.Rule.VisitorModifier.RunAfterRule is pointed to prev rule

This pull request closes #1309

Patch:
diff --git a/.git-hooks/commit-msg.sh b/.git-hooks/commit-msg.sh
index f465ed4d47..91a120259f 100644
--- a/.git-hooks/commit-msg.sh
+++ b/.git-hooks/commit-msg.sh
@@ -2,8 +2,9 @@
 
 commit_pattern="(Merge (remote-tracking )?branch|### What's done:)"
 error_msg="Your commit message doesn't match the pattern $commit_pattern. Please fix it."
+commit_count="$(git rev-list --count master..HEAD 2> /dev/null)"
 
-if [[ ! $( cat "$1" ) =~ $commit_pattern ]]
+if [[ $commit_count = "0" && ! $( cat "$1" ) =~ $commit_pattern ]]
 then
     echo "$error_msg"
     exit 1
diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
index 214975c24d..9bd00bcbd9 100644
--- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
+++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
@@ -3,6 +3,7 @@ package org.cqfn.diktat.ruleset.rules
 import org.cqfn.diktat.common.config.rules.DIKTAT_COMMON
 import org.cqfn.diktat.common.config.rules.RulesConfig
 import org.cqfn.diktat.common.config.rules.RulesConfigReader
+import org.cqfn.diktat.ruleset.constants.EmitType
 import org.cqfn.diktat.ruleset.constants.Warnings
 import org.cqfn.diktat.ruleset.dummy.DummyWarning
 import org.cqfn.diktat.ruleset.rules.chapter1.FileNaming
@@ -79,8 +80,10 @@ import org.cqfn.diktat.ruleset.rules.chapter6.classes.SingleConstructorRule
 import org.cqfn.diktat.ruleset.rules.chapter6.classes.SingleInitRule
 import org.cqfn.diktat.ruleset.rules.chapter6.classes.StatelessClassesRule
 
+import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.RuleSet
 import com.pinterest.ktlint.core.RuleSetProvider
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.jetbrains.kotlin.org.jline.utils.Levenshtein
 import org.slf4j.LoggerFactory
 
@@ -226,10 +229,12 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
             .map {
                 it.invoke(configRules)
             }
-            .toTypedArray()
+        val orderedRules = rules.mapIndexed { index, rule ->
+            if (index != 0) OrderedRule(rule, rules[index - 1]) else rule
+        }
         return RuleSet(
             DIKTAT_RULE_SET_ID,
-            *rules
+            rules = orderedRules.toTypedArray()
         )
     }
 
@@ -262,7 +267,43 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
 
     private fun resolveConfigFileFromSystemProperty(): String? = System.getProperty(DIKTAT_CONF_PROPERTY)
 
+    /**
+     * This is a wrapper around Ktlint Rule which adjusts visitorModifiers to keep order with prevRule
+     * Added as a workaround after introducing a new logic for sorting KtLint Rules: https://github.com/pinterest/ktlint/issues/1478
+     *
+     * @property rule KtLink Rule which this class wraps
+     *
+     * @param prevRule previous KtLink Rule, the wrapped rule is called after prevRule
+     */
+    internal class OrderedRule(val rule: Rule, prevRule: Rule) : Rule(rule.id, adjustVisitorModifiers(rule, prevRule)) {
+        /**
+         * Delegating a call of this method
+         */
+        override fun visit(
+            node: ASTNode,
+            autoCorrect: Boolean,
+            emit: EmitType
+        ) {
+            rule.visit(node, autoCorrect, emit)
+        }
+    }
+
     companion object {
         private val log = LoggerFactory.getLogger(DiktatRuleSetProvider::class.java)
+
+        private fun adjustVisitorModifiers(rule: Rule, prevRule: Rule): Set<Rule.VisitorModifier> {
+            val visitorModifiers: Set<Rule.VisitorModifier> = rule.visitorModifiers
+            require(visitorModifiers.none { it is Rule.VisitorModifier.RunAfterRule }) {
+                "Rule ${rule.id} already contains VisitorModifier.RunAfterRule"
+            }
+            require(rule.id != prevRule.id) {
+                "PrevRule has same ID as rule: ${rule.id}"
+            }
+            return visitorModifiers + Rule.VisitorModifier.RunAfterRule(
+                ruleId = prevRule.id,
+                loadOnlyWhenOtherRuleIsLoaded = false,
+                runOnlyWhenOtherRuleIsEnabled = false
+            )
+        }
     }
 }


Test patch:
diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/DiktatRuleSetProvider4Test.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/DiktatRuleSetProvider4Test.kt
index 5661b44b08..1007c609ec 100644
--- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/DiktatRuleSetProvider4Test.kt
+++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/util/DiktatRuleSetProvider4Test.kt
@@ -6,12 +6,14 @@ package org.cqfn.diktat.util
 
 import org.cqfn.diktat.common.config.rules.RulesConfig
 import org.cqfn.diktat.common.config.rules.RulesConfigReader
+import org.cqfn.diktat.ruleset.constants.EmitType
 import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
 import org.cqfn.diktat.ruleset.rules.DiktatRuleSetProvider
 
 import com.pinterest.ktlint.core.Rule
 import com.pinterest.ktlint.core.RuleSet
 import com.pinterest.ktlint.core.RuleSetProvider
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
 import org.junit.jupiter.api.Assertions
 import org.junit.jupiter.api.Test
 
@@ -40,10 +42,85 @@ class DiktatRuleSetProviderTest {
             .filter { it.isFile }
             .map { it.nameWithoutExtension }
             .filterNot { it in ignoreFile }
-        val rulesName = DiktatRuleSetProvider().get().map { it::class.simpleName!! }.filter { it != "DummyWarning" }
+        val rulesName = DiktatRuleSetProvider().get()
+            .onEachIndexed { index, rule ->
+                if (index != 0) {
+                    Assertions.assertTrue(
+                        rule.visitorModifiers.any { it is Rule.VisitorModifier.RunAfterRule },
+                        "Rule ${rule.id} doesn't contain Rule.VisitorModifier.RunAfterRule"
+                    )
+                }
+            }
+            .map { (it as? DiktatRuleSetProvider.OrderedRule)?.rule ?: it }
+            .map { it::class.simpleName!! }
+            .filter { it != "DummyWarning" }
         Assertions.assertEquals(filesName.sorted().toList(), rulesName.sorted())
     }
 
+    @Test
+    fun `check OrderedRule with VisitorModifier RunAfterRule`() {
+        val rule = object : Rule("rule") {
+            override fun visit(
+                node: ASTNode,
+                autoCorrect: Boolean,
+                emit: EmitType
+            ) {
+                // do nothing
+            }
+        }
+        Assertions.assertThrows(IllegalArgumentException::class.java) {
+            DiktatRuleSetProvider.OrderedRule(rule, rule)
+        }
+
+        val ruleWithRunAfterRule = object : Rule("invalid-rule", setOf(VisitorModifier.RunAfterRule("another-rule"))) {
+            override fun visit(
+                node: ASTNode,
+                autoCorrect: Boolean,
+                emit: EmitType
+            ) {
+                // do nothing
+            }
+        }
+        Assertions.assertThrows(IllegalArgumentException::class.java) {
+            DiktatRuleSetProvider.OrderedRule(ruleWithRunAfterRule, rule)
+        }
+    }
+
+    @Test
+    fun `check OrderedRule`() {
+        val rule1 = object : Rule("rule-first") {
+            override fun visit(
+                node: ASTNode,
+                autoCorrect: Boolean,
+                emit: EmitType
+            ) {
+                // do nothing
+            }
+        }
+        val rule2 = object : Rule("rule-second") {
+            override fun visit(
+                node: ASTNode,
+                autoCorrect: Boolean,
+                emit: EmitType
+            ) {
+                // do nothing
+            }
+        }
+
+        val orderedRule = DiktatRuleSetProvider.OrderedRule(rule2, rule1)
+        orderedRule.visitorModifiers
+            .filterIsInstance<Rule.VisitorModifier.RunAfterRule>()
+            .also {
+                Assertions.assertEquals(1, it.size,
+                    "Found invalid count of Rule.VisitorModifier.RunAfterRule")
+            }
+            .first()
+            .let {
+                Assertions.assertEquals(rule1.id, it.ruleId,
+                    "Invalid ruleId in Rule.VisitorModifier.RunAfterRule")
+            }
+    }
+
     companion object {
         private val ignoreFile = listOf("DiktatRuleSetProvider", "DiktatRule")
     }