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.

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

Reused c.p.k.c.Rule.VisitorModifier.RunAfterRule #1310

Merged
merged 16 commits into from
May 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion .git-hooks/commit-msg.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()
)
}

Expand Down Expand Up @@ -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
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
}
Expand Down