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

Allow to enable allRules = true and buildUponDefaultConfig = true #6844

Merged
merged 7 commits into from
Mar 14, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ interface SetupContext : PropertiesAware {
val configUris: Collection<URI>

/**
* Configuration which is used to setup detekt.
* Configuration which is used to set up detekt.
*/
val config: Config

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.gitlab.arturbosch.detekt.core

import io.github.detekt.psi.absolutePath
import io.github.detekt.tooling.api.spec.ProcessingSpec
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.CompilerResources
import io.gitlab.arturbosch.detekt.api.Config
Expand All @@ -20,13 +19,8 @@ import io.gitlab.arturbosch.detekt.api.internal.whichDetekt
import io.gitlab.arturbosch.detekt.api.internal.whichJava
import io.gitlab.arturbosch.detekt.api.internal.whichOS
import io.gitlab.arturbosch.detekt.api.ruleId
import io.gitlab.arturbosch.detekt.core.config.AllRulesConfig
import io.gitlab.arturbosch.detekt.core.config.DisabledAutoCorrectConfig
import io.gitlab.arturbosch.detekt.core.config.validation.DeprecatedRule
import io.gitlab.arturbosch.detekt.core.config.validation.loadDeprecations
import io.gitlab.arturbosch.detekt.core.rules.associateRuleIdsToRuleSetIds
import io.gitlab.arturbosch.detekt.core.suppressors.buildSuppressors
import io.gitlab.arturbosch.detekt.core.tooling.getDefaultConfiguration
import io.gitlab.arturbosch.detekt.core.util.isActiveOrDefault
import io.gitlab.arturbosch.detekt.core.util.shouldAnalyzeFile
import org.jetbrains.kotlin.config.languageVersionSettings
Expand All @@ -42,9 +36,6 @@ internal class Analyzer(
private val providers: List<RuleSetProvider>,
private val processors: List<FileProcessListener>
) {

private val config: Config = settings.spec.workaroundConfiguration(settings.config)

fun run(
ktFiles: Collection<KtFile>,
bindingContext: BindingContext = BindingContext.EMPTY
Expand Down Expand Up @@ -108,7 +99,7 @@ internal class Analyzer(
compilerResources: CompilerResources
): Map<RuleSet.Id, List<Finding2>> {
val activeRuleSetsToRuleSetConfigs = providers.asSequence()
.map { it to config.subConfig(it.ruleSetId.value) }
.map { it to settings.config.subConfig(it.ruleSetId.value) }
.filter { (_, ruleSetConfig) -> ruleSetConfig.isActiveOrDefault(true) }
.map { (provider, ruleSetConfig) -> provider.instance() to ruleSetConfig }
.filter { (_, ruleSetConfig) -> ruleSetConfig.shouldAnalyzeFile(file) }
Expand Down Expand Up @@ -157,7 +148,7 @@ internal class Analyzer(

private fun warnAboutEnabledRequiresTypeResolutionRules() {
providers.asSequence()
.map { it to config.subConfig(it.ruleSetId.value) }
.map { it to settings.config.subConfig(it.ruleSetId.value) }
.filter { (_, ruleSetConfig) -> ruleSetConfig.isActiveOrDefault(true) }
.map { (provider, ruleSetConfig) -> provider.instance() to ruleSetConfig }
.flatMap { (ruleSet, ruleSetConfig) ->
Expand Down Expand Up @@ -200,31 +191,6 @@ private fun throwIllegalStateException(file: KtFile, error: Throwable): Nothing
throw IllegalStateException(message, error)
}

internal fun ProcessingSpec.workaroundConfiguration(config: Config): Config = with(configSpec) {
var declaredConfig: Config? = when {
configPaths.isNotEmpty() -> config
resources.isNotEmpty() -> config
useDefaultConfig -> config
else -> null
}

if (rulesSpec.activateAllRules) {
val defaultConfig = getDefaultConfiguration()
val deprecatedRules = loadDeprecations().filterIsInstance<DeprecatedRule>().toSet()
declaredConfig = AllRulesConfig(
originalConfig = declaredConfig ?: defaultConfig,
defaultConfig = defaultConfig,
deprecatedRules = deprecatedRules
)
}

if (!rulesSpec.autoCorrect) {
declaredConfig = DisabledAutoCorrectConfig(declaredConfig ?: getDefaultConfiguration())
}

return declaredConfig ?: getDefaultConfiguration()
}

private fun Finding.toFinding2(rule: Finding2.RuleInfo, severity: Severity): Finding2 {
return when (this) {
is CorrectableCodeSmell -> Finding2Impl(rule, entity, message, references, severity, autoCorrectEnabled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import java.net.URI
*/
class ProcessingSettings(
val spec: ProcessingSpec,
override val config: Config
override val config: Config,
) : AutoCloseable,
Closeable,
LoggingAware by LoggingFacade(spec.loggingSpec),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,38 +7,37 @@

@Suppress("UNCHECKED_CAST")
internal data class AllRulesConfig(
private val originalConfig: Config,
private val defaultConfig: Config,
private val deprecatedRules: Set<DeprecatedRule> = emptySet(),
private val wrapped: Config,
private val deprecatedRules: Set<DeprecatedRule>,
override val parent: Config? = null,
) : Config, ValidatableConfiguration {

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

override fun subConfig(key: String) =
AllRulesConfig(originalConfig.subConfig(key), defaultConfig.subConfig(key), deprecatedRules, this)
AllRulesConfig(wrapped.subConfig(key), deprecatedRules, this)

override fun subConfigKeys(): Set<String> {
return originalConfig.subConfigKeys() + defaultConfig.subConfigKeys()
return wrapped.subConfigKeys()
}

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

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

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

Check warning on line 40 in detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/AllRulesConfig.kt

View check run for this annotation

Codecov / codecov/patch

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/AllRulesConfig.kt#L40

Added line #L40 was not covered by tests

private fun isDeprecated(): Boolean = deprecatedRules.any { parentPath == it.toPath() }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,23 @@ import io.github.detekt.tooling.api.spec.ConfigSpec
import io.github.detekt.tooling.api.spec.ProcessingSpec
import io.github.detekt.utils.openSafeStream
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.core.tooling.getDefaultConfiguration
import java.net.URI
import java.net.URL
import java.nio.file.FileSystemNotFoundException
import java.nio.file.FileSystems
import java.nio.file.Path

internal fun ProcessingSpec.loadConfiguration(): Config = with(configSpec) {
var declaredConfig: Config? = when {
return when {
configPaths.isNotEmpty() -> parsePathConfig(configPaths)
resources.isNotEmpty() -> parseResourceConfig(resources)
else -> null
else -> Config.empty
}

if (useDefaultConfig) {
declaredConfig = if (declaredConfig == null) {
getDefaultConfiguration()
} else {
CompositeConfig(declaredConfig, getDefaultConfiguration())
}
}

return declaredConfig ?: getDefaultConfiguration()
}

private fun parseResourceConfig(urls: Collection<URL>): Config =
if (urls.size == 1) {
urls.first().openSafeStream().reader().use(YamlConfig::load)
urls.single().openSafeStream().reader().use(YamlConfig::load)
} else {
urls.asSequence()
.map { it.openSafeStream().reader().use(YamlConfig::load) }
Expand All @@ -40,7 +29,7 @@ private fun parseResourceConfig(urls: Collection<URL>): Config =

private fun parsePathConfig(paths: Collection<Path>): Config =
if (paths.size == 1) {
YamlConfig.load(paths.first())
YamlConfig.load(paths.single())
} else {
paths.asSequence()
.map { YamlConfig.load(it) }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
package io.gitlab.arturbosch.detekt.core.tooling

import io.github.detekt.tooling.api.spec.ProcessingSpec
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.core.ProcessingSettings
import io.gitlab.arturbosch.detekt.core.baseline.DETEKT_BASELINE_CREATION_KEY
import io.gitlab.arturbosch.detekt.core.baseline.DETEKT_BASELINE_PATH_KEY
import io.gitlab.arturbosch.detekt.core.config.AllRulesConfig
import io.gitlab.arturbosch.detekt.core.config.CompositeConfig
import io.gitlab.arturbosch.detekt.core.config.DisabledAutoCorrectConfig
import io.gitlab.arturbosch.detekt.core.config.loadConfiguration
import io.gitlab.arturbosch.detekt.core.config.validation.DeprecatedRule
import io.gitlab.arturbosch.detekt.core.config.validation.loadDeprecations
import io.gitlab.arturbosch.detekt.core.reporting.DETEKT_OUTPUT_REPORT_BASE_PATH_KEY
import io.gitlab.arturbosch.detekt.core.reporting.DETEKT_OUTPUT_REPORT_PATHS_KEY
import io.gitlab.arturbosch.detekt.core.util.MONITOR_PROPERTY_KEY
Expand All @@ -13,7 +19,9 @@ import io.gitlab.arturbosch.detekt.core.util.PerformanceMonitor.Phase

internal fun <R> ProcessingSpec.withSettings(execute: ProcessingSettings.() -> R): R {
val monitor = PerformanceMonitor()
val configuration = monitor.measure(Phase.LoadConfig) { loadConfiguration() }
val configuration = monitor.measure(Phase.LoadConfig) {
workaroundConfiguration(loadConfiguration())
}
val settings = monitor.measure(Phase.CreateSettings) {
ProcessingSettings(this, configuration).apply {
baselineSpec.path?.let { register(DETEKT_BASELINE_PATH_KEY, it) }
Expand All @@ -31,3 +39,22 @@ internal fun <R> ProcessingSpec.withSettings(execute: ProcessingSettings.() -> R
}
return result
}

internal fun ProcessingSpec.workaroundConfiguration(config: Config): Config {
var declaredConfig: Config = config

if (rulesSpec.activateAllRules) {
val deprecatedRules = loadDeprecations().filterIsInstance<DeprecatedRule>().toSet()
declaredConfig = AllRulesConfig(declaredConfig, deprecatedRules)
}

if (!rulesSpec.autoCorrect) {
declaredConfig = DisabledAutoCorrectConfig(declaredConfig)
}

if (configSpec.useDefaultConfig) {
declaredConfig = CompositeConfig(declaredConfig, getDefaultConfiguration())
}

return declaredConfig
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,53 +10,23 @@ import org.junit.jupiter.api.Test
class AllRulesConfigSpec {
private val emptyYamlConfig = yamlConfigFromContent("")

@Nested
inner class ValueVerification {

@Test
fun verifyValue() {
val subject = AllRulesConfig(
originalConfig = yamlConfigFromContent(
"""
style:
MaxLineLength:
maxLineLength: 100
""".trimIndent()
),
defaultConfig = emptyYamlConfig,
)

val subConfig = subject.subConfig("style")
.subConfig("MaxLineLength")
assertThat(subConfig.valueOrDefault("maxLineLength", 0)).isEqualTo(100)
assertThat(subConfig.valueOrNull<Int>("maxLineLength")).isEqualTo(100)
}

@Test
fun verifyValueOverride() {
val subject = AllRulesConfig(
originalConfig = yamlConfigFromContent(
"""
@Test
fun verifyValue() {
val subject = AllRulesConfig(
wrapped = yamlConfigFromContent(
"""
style:
MaxLineLength:
maxLineLength: 100
""".trimIndent()
),
defaultConfig = yamlConfigFromContent(
"""
style:
MaxLineLength:
maxLineLength: 120
""".trimIndent()
),
)

val actual = subject.subConfig("style")
.subConfig("MaxLineLength")
.valueOrDefault("maxLineLength", 0)
""".trimIndent()
),
deprecatedRules = emptySet(),
)

assertThat(actual).isEqualTo(100)
}
val subConfig = subject.subConfig("style")
.subConfig("MaxLineLength")
assertThat(subConfig.valueOrDefault("maxLineLength", 0)).isEqualTo(100)
assertThat(subConfig.valueOrNull<Int>("maxLineLength")).isEqualTo(100)
}

@Nested
Expand All @@ -73,8 +43,8 @@ class AllRulesConfigSpec {
@Test
fun `is derived from the original config`() {
val subject = AllRulesConfig(
originalConfig = rulesetConfig,
defaultConfig = emptyYamlConfig,
wrapped = rulesetConfig,
deprecatedRules = emptySet(),
)
val actual = subject.parentPath
assertThat(actual).isEqualTo(rulesetId)
Expand All @@ -83,11 +53,11 @@ class AllRulesConfigSpec {
@Test
fun `is derived from the default config if unavailable in original config`() {
val subject = AllRulesConfig(
originalConfig = emptyYamlConfig,
defaultConfig = rulesetConfig,
wrapped = emptyYamlConfig,
deprecatedRules = emptySet(),
)
val actual = subject.parentPath
assertThat(actual).isEqualTo(rulesetId)
assertThat(actual).isEqualTo(null)
}
}

Expand All @@ -104,8 +74,8 @@ class AllRulesConfigSpec {
@Test
fun `is the parent`() {
val subject = AllRulesConfig(
originalConfig = rulesetConfig,
defaultConfig = emptyYamlConfig,
wrapped = rulesetConfig,
deprecatedRules = emptySet(),
)
val actual = subject.subConfig("style").parent
assertThat(actual).isEqualTo(subject)
Expand All @@ -114,8 +84,8 @@ class AllRulesConfigSpec {
@Test
fun `is the parent for all subConfig`() {
val subject = AllRulesConfig(
originalConfig = rulesetConfig,
defaultConfig = emptyYamlConfig,
wrapped = rulesetConfig,
deprecatedRules = emptySet(),
)

assertThat(subject.subConfigKeys()).contains("style")
Expand All @@ -128,8 +98,7 @@ class AllRulesConfigSpec {
@Test
fun `rule is active if not deprecated`() {
val subject = AllRulesConfig(
originalConfig = emptyYamlConfig,
defaultConfig = emptyYamlConfig,
wrapped = emptyYamlConfig,
deprecatedRules = emptySet()
)
.subConfig("ruleset")
Expand All @@ -143,9 +112,8 @@ class AllRulesConfigSpec {
@Test
fun `rule is inactive if deprecated`() {
val subject = AllRulesConfig(
originalConfig = emptyYamlConfig,
defaultConfig = emptyYamlConfig,
deprecatedRules = setOf(DeprecatedRule("ruleset", "ARule", ""))
wrapped = emptyYamlConfig,
deprecatedRules = setOf(DeprecatedRule("ruleset", "ARule", "")),
)
.subConfig("ruleset")
.subConfig("ARule")
Expand Down