Allow to enable allRules = true and buildUponDefaultConfig = true (…

…#6844)

* Remove default value

* Improve WorkaroundConfigurationKtSpec.kt

* Use single instead of first

* Simplify code

* Move the Composite config down

* Enable to use defaultConfig with allRules

* Move code away from Analyzer

detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/SetupContext.kt

@@ -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
 

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt

@@ -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
@@ -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
@@ -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
@@ -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) }
@@ -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) ->
@@ -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)

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/ProcessingSettings.kt

@@ -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),

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

@@ -7,38 +7,37 @@ 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 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)
 
     private fun isDeprecated(): Boolean = deprecatedRules.any { parentPath == it.toPath() }
 

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/Configurations.kt

@@ -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) }
@@ -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) }

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/tooling/ProcessingSpecSettingsBridge.kt

@@ -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
@@ -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) }
@@ -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
+}

detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/AllRulesConfigSpec.kt

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