Skip to content

Commit

Permalink
Consider deprecated rules as inactive when running allRules (#6381)
Browse files Browse the repository at this point in the history
  • Loading branch information
marschwar committed Sep 1, 2023
1 parent 5fc3df0 commit c0684d1
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import io.gitlab.arturbosch.detekt.api.internal.whichJava
import io.gitlab.arturbosch.detekt.api.internal.whichOS
import io.gitlab.arturbosch.detekt.core.config.AllRulesConfig
import io.gitlab.arturbosch.detekt.core.config.DisabledAutoCorrectConfig
import io.gitlab.arturbosch.detekt.core.config.validation.loadDeprecations
import io.gitlab.arturbosch.detekt.core.rules.associateRuleIdsToRuleSetIds
import io.gitlab.arturbosch.detekt.core.rules.isActive
import io.gitlab.arturbosch.detekt.core.rules.shouldAnalyzeFile
Expand Down Expand Up @@ -194,7 +195,11 @@ internal fun ProcessingSpec.workaroundConfiguration(config: Config): Config = wi

if (rulesSpec.activateAllRules) {
val defaultConfig = getDefaultConfiguration()
declaredConfig = AllRulesConfig(declaredConfig ?: defaultConfig, defaultConfig)
declaredConfig = AllRulesConfig(
originalConfig = declaredConfig ?: defaultConfig,
defaultConfig = defaultConfig,
deprecatedRuleIds = loadDeprecatedRuleIds()
)
}

if (!rulesSpec.autoCorrect) {
Expand All @@ -203,3 +208,10 @@ internal fun ProcessingSpec.workaroundConfiguration(config: Config): Config = wi

return declaredConfig ?: getDefaultConfiguration()
}

internal fun loadDeprecatedRuleIds(): Set<String> =
loadDeprecations().keys
.map { it.split(">") }
.filter { it.size == 2 }
.map { it.joinToString(" > ") }
.toSet()
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,38 @@ import io.gitlab.arturbosch.detekt.core.config.validation.validateConfig
@Suppress("UNCHECKED_CAST")
internal data class AllRulesConfig(
private val originalConfig: Config,
private val defaultConfig: Config
private val defaultConfig: Config,
private val deprecatedRuleIds: Set<String> = emptySet()
) : Config, ValidatableConfiguration {

override val parentPath: String?
get() = originalConfig.parentPath ?: defaultConfig.parentPath

override fun subConfig(key: String) =
AllRulesConfig(originalConfig.subConfig(key), defaultConfig.subConfig(key))
AllRulesConfig(
originalConfig = originalConfig.subConfig(key),
defaultConfig = defaultConfig.subConfig(key),
deprecatedRuleIds = deprecatedRuleIds
)

override fun <T : Any> valueOrDefault(key: String, default: T): T {
return when (key) {
Config.ACTIVE_KEY -> originalConfig.valueOrDefault(key, true) as T
Config.ACTIVE_KEY -> if (isDeprecated()) false as T else originalConfig.valueOrDefault(key, true) as T
else -> originalConfig.valueOrDefault(key, defaultConfig.valueOrDefault(key, default))
}
}

override fun <T : Any> valueOrNull(key: String): T? {
return when (key) {
Config.ACTIVE_KEY -> originalConfig.valueOrNull(key) ?: true as? T
Config.ACTIVE_KEY -> if (isDeprecated()) false as T else originalConfig.valueOrNull(key) ?: true as? T
else -> originalConfig.valueOrNull(key) ?: defaultConfig.valueOrNull(key)
}
}

override fun validate(baseline: Config, excludePatterns: Set<Regex>) =
validateConfig(originalConfig, baseline, excludePatterns)

private fun isDeprecated(): Boolean {
return parentPath?.let { deprecatedRuleIds.contains(it) } ?: false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package io.gitlab.arturbosch.detekt.core

import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test

class LoadDeprecatedRuleIdsSpec {
@Test
fun loadDeprecatedRules() {
// rule id as used by config path mechanism uses 'ruleSetId > ruleId' pattern (with space)
val ruleIdPattern = """^[-\w]+ > \w+$""".toRegex()

val actual = loadDeprecatedRuleIds()

assertThat(actual)
.isNotEmpty()
.allMatch { ruleId -> ruleIdPattern.matches(ruleId as String) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,24 @@ package io.gitlab.arturbosch.detekt.core.config

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.test.yamlConfig
import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test

class AllRulesConfigSpec {
private val emptyYamlConfig = yamlConfigFromContent("")

@Nested
inner class ParentPath {
private val rulesetId = "style"
private val rulesetConfig = yamlConfig("/configs/single-rule-in-style-ruleset.yml").subConfig(rulesetId)
private val emptyConfig = Config.empty

@Test
fun `is derived from the original config`() {
val subject = AllRulesConfig(
originalConfig = rulesetConfig,
defaultConfig = emptyConfig,
defaultConfig = emptyYamlConfig,
)
val actual = subject.parentPath
assertThat(actual).isEqualTo(rulesetId)
Expand All @@ -26,11 +28,45 @@ class AllRulesConfigSpec {
@Test
fun `is derived from the default config if unavailable in original config`() {
val subject = AllRulesConfig(
originalConfig = emptyConfig,
originalConfig = emptyYamlConfig,
defaultConfig = rulesetConfig,
)
val actual = subject.parentPath
assertThat(actual).isEqualTo(rulesetId)
}
}

@Nested
inner class DeactivateDeprecatedRule {

@Test
fun `rule is active if not deprecated`() {
val subject = AllRulesConfig(
originalConfig = emptyYamlConfig,
defaultConfig = emptyYamlConfig,
deprecatedRuleIds = emptySet()
)
.subConfig("ruleset")
.subConfig("ARule")

val actual = subject.valueOrDefault(Config.ACTIVE_KEY, false)

assertThat(actual).isTrue
}

@Test
fun `rule is inactive if deprecated`() {
val subject = AllRulesConfig(
originalConfig = emptyYamlConfig,
defaultConfig = emptyYamlConfig,
deprecatedRuleIds = setOf("ruleset > ARule")
)
.subConfig("ruleset")
.subConfig("ARule")

val actual = subject.valueOrDefault(Config.ACTIVE_KEY, false)

assertThat(actual).isFalse
}
}
}

0 comments on commit c0684d1

Please sign in to comment.