Repository: detekt/detekt (kotlin)

Issue:
Support dynamic defaults when using --build-upon-default-config
## Expected Behavior
I can use `--build-upon-default-config`/`buildUponDefaultConfig` to set the default config values, while also honouring parameters with dynamic defaults like the max line length setting in detekt-formatting.

## Current Behavior
Using `--build-upon-default-config` effectively sets max line length to 120, also sets a single default value for other config items with different settings for Android vs non-Android in detekt-formatting to whatever is used in the detekt default config file.

## Context
detekt-formatting wraps ktlint, which sets different defaults for rules based on the code style that's set. The most obvious difference is in max line length - `ktlint_official` sets to 140, `intellij_idea` is not set, and `android_studio` sets to 100.

This is generally fine if not using detekt's defaults, but when we do, it's set to 100 if using the Android code style and 120 if not.

But this dynamic default in the rule is not honoured when using `--build-upon-default-config`/`buildUponDefaultConfig` since the default config file sets max line length to 120 everywhere.

It's very reasonable to build upon the default config, but doing so should not disregard the dynamic nature of the rule config for these rules.

It would be ideal if we can use `--build-upon-default-config` while also setting the values of parameters with dynamic defaults dynamically.

Pull request:
This fixes #7972 

I guess this is not so obvious, why this fixes the issue. 

**Current behavior** with `--build-upon-default-config`

When calculating the value for `ArgumentListWrapping.maxLineLength`, we correctly determine if the android configuration should be used or not and the correct value is chosen. But because there is a default value in the (default) yaml file, that value is always preferred over the one that was calculated. 

```kotlin
       // ConfigProperty:104-106
        val isAndroid = getValueOrDefault(rulesetConfig, "android", false)
        val value = if (isAndroid) defaultAndroidValue else defaultValue
        return transform(getValueOrDefault(thisRef.config, property.name, value))
```

By not putting a default value into the generated default yaml files when there are multiple options, the correct default value is calculated, depending on the `android` setting of the rule set.

**How to test?**
If this is the solution, we can agree on, I will try to create a test. In the meantime, please verify the behavior by:
* run ` ./gradlew :detekt-cli:runWithArgsFile` . There should be no issues
* add `android: true` to the formatting rule set configuration in `config/detekt/detekt.yml`
* run ` ./gradlew :detekt-cli:runWithArgsFile` again. There are now more than 4000 issues due to the different defaults.

Is there anything, that I am missing? Any downside to this solution?

Patch:
diff --git a/detekt-formatting/src/main/resources/config/config.yml b/detekt-formatting/src/main/resources/config/config.yml
index 9a25b7601b3a..068974ca4552 100644
--- a/detekt-formatting/src/main/resources/config/config.yml
+++ b/detekt-formatting/src/main/resources/config/config.yml
@@ -13,14 +13,14 @@ formatting:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     ignoreRuleParameterThreshold: 8
   BackingPropertyNaming:
     active: true
   BinaryExpressionWrapping:
     active: true
     autoCorrect: true
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     indentSize: 4
   BlankLineBeforeDeclaration:
     active: true
@@ -36,7 +36,7 @@ formatting:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     forceMultilineWhenChainOperatorCountGreaterOrEqualThan: 4
   ChainWrapping:
     active: true
@@ -48,7 +48,7 @@ formatting:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
   CommentSpacing:
     active: true
     autoCorrect: true
@@ -63,12 +63,12 @@ formatting:
   ContextReceiverListWrapping:
     active: true
     autoCorrect: true
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     indentSize: 4
   ContextReceiverMapping:
     active: true
     autoCorrect: true
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     indentSize: 4
   EnumEntryNameCase:
     active: true
@@ -96,24 +96,24 @@ formatting:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
   FunctionLiteral:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
   FunctionName:
     active: false
   FunctionReturnTypeSpacing:
     active: true
     autoCorrect: true
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
   FunctionSignature:
     active: true
     autoCorrect: true
     forceMultilineWhenParameterCountGreaterOrEqualThan: 2147483647
     functionBodyExpressionWrapping: 'default'
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     indentSize: 4
   FunctionStartOfBodySpacing:
     active: true
@@ -135,7 +135,7 @@ formatting:
   ImportOrdering:
     active: true
     autoCorrect: true
-    layout: '*,java.**,javax.**,kotlin.**,^'
+    # layout: If the 'android' ruleset property is set to true, the default is '*', otherwise '*,java.**,javax.**,kotlin.**,^'.
   Indentation:
     active: true
     autoCorrect: true
@@ -150,7 +150,7 @@ formatting:
   MaximumLineLength:
     active: true
     aliases: ['MaxLineLength']
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     ignoreBackTickedIdentifier: false
   MixedConditionOperators:
     active: false
@@ -236,17 +236,17 @@ formatting:
   ParameterListSpacing:
     active: true
     autoCorrect: true
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
   ParameterListWrapping:
     active: true
     autoCorrect: true
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
     indentSize: 4
   ParameterWrapping:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
   PropertyName:
     active: true
     constantNamingStyle: 'screaming_snake_case'
@@ -254,7 +254,7 @@ formatting:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
   SpacingAroundAngleBrackets:
     active: true
     autoCorrect: true
@@ -315,11 +315,11 @@ formatting:
   TrailingCommaOnCallSite:
     active: true
     autoCorrect: true
-    useTrailingCommaOnCallSite: true
+    # useTrailingCommaOnCallSite: If the 'android' ruleset property is set to true, the default is 'false', otherwise 'true'.
   TrailingCommaOnDeclarationSite:
     active: true
     autoCorrect: true
-    useTrailingCommaOnDeclarationSite: true
+    # useTrailingCommaOnDeclarationSite: If the 'android' ruleset property is set to true, the default is 'false', otherwise 'true'.
   TryCatchFinallySpacing:
     active: true
     autoCorrect: true
@@ -351,4 +351,4 @@ formatting:
     active: true
     autoCorrect: true
     indentSize: 4
-    maxLineLength: 120
+    # maxLineLength: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
diff --git a/detekt-generator/src/main/kotlin/dev/detekt/generator/printer/defaultconfig/RuleSetConfigPrinter.kt b/detekt-generator/src/main/kotlin/dev/detekt/generator/printer/defaultconfig/RuleSetConfigPrinter.kt
index fb83a126fefa..145a8f6f929a 100644
--- a/detekt-generator/src/main/kotlin/dev/detekt/generator/printer/defaultconfig/RuleSetConfigPrinter.kt
+++ b/detekt-generator/src/main/kotlin/dev/detekt/generator/printer/defaultconfig/RuleSetConfigPrinter.kt
@@ -6,6 +6,7 @@ import dev.detekt.generator.collection.Rule
 import dev.detekt.generator.collection.RuleSetPage
 import dev.detekt.generator.collection.RuleSetProvider
 import dev.detekt.utils.YamlNode
+import dev.detekt.utils.comment
 import dev.detekt.utils.keyValue
 import dev.detekt.utils.node
 
@@ -51,5 +52,16 @@ internal fun YamlNode.printRule(rule: Rule) {
 internal fun YamlNode.printConfiguration(configuration: Configuration) {
     if (configuration.isDeprecated()) return
 
-    configuration.defaultValue.printAsYaml(configuration.name, this)
+    val hasDeviatingAndroidDefault = configuration.defaultAndroidValue != null &&
+        configuration.defaultValue != configuration.defaultAndroidValue
+
+    if (hasDeviatingAndroidDefault) {
+        val description =
+            "If the 'android' ruleset property is set to true, " +
+                "the default is '${configuration.defaultAndroidValue.getPlainValue()}', " +
+                "otherwise '${configuration.defaultValue.getPlainValue()}'."
+        comment("${configuration.name}: $description")
+    } else {
+        configuration.defaultValue.printAsYaml(configuration.name, this)
+    }
 }
diff --git a/detekt-utils/src/main/kotlin/dev/detekt/utils/Yaml.kt b/detekt-utils/src/main/kotlin/dev/detekt/utils/Yaml.kt
index 736d5da2e15f..232059314ece 100644
--- a/detekt-utils/src/main/kotlin/dev/detekt/utils/Yaml.kt
+++ b/detekt-utils/src/main/kotlin/dev/detekt/utils/Yaml.kt
@@ -39,6 +39,10 @@ inline fun YamlNode.keyValue(comment: String = "", keyValue: () -> Pair<String,
     }
 }
 
+fun YamlNode.comment(comment: String = "") {
+    append("# $comment")
+}
+
 fun YamlNode.list(name: String, list: List<String>) {
     if (list.isEmpty()) {
         keyValue { name to EMPTY_LIST }
@@ -84,6 +88,7 @@ private fun String.ensureQuoted(): String =
             endsWith(SINGLE_QUOTE) ||
             startsWith(DOUBLE_QUOTE) &&
             endsWith(DOUBLE_QUOTE) -> this
+
         else -> quoted()
     }
 


Test patch:
diff --git a/detekt-api/src/test/kotlin/dev/detekt/api/ConfigPropertySpec.kt b/detekt-api/src/test/kotlin/dev/detekt/api/ConfigPropertySpec.kt
index a8f3192f8232..d2eb1e14cbe9 100644
--- a/detekt-api/src/test/kotlin/dev/detekt/api/ConfigPropertySpec.kt
+++ b/detekt-api/src/test/kotlin/dev/detekt/api/ConfigPropertySpec.kt
@@ -6,6 +6,10 @@ import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.assertj.core.api.Assertions.tuple
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.Arguments
+import org.junit.jupiter.params.provider.Arguments.argumentSet
+import org.junit.jupiter.params.provider.MethodSource
 import java.util.concurrent.atomic.AtomicInteger
 
 class ConfigPropertySpec {
@@ -544,6 +548,61 @@ class ConfigPropertySpec {
             }
         }
     }
+
+    @Nested
+    inner class `With android variant` {
+
+        private val androidRulesetConfig = TestConfig("android" to true)
+        private val nonAndroidRulesetConfig = TestConfig("android" to false)
+        private val androidUndefinedRulesetConfig = TestConfig()
+
+        @Test
+        fun `uses default if android property of ruleset is not defined`() {
+            val subject = object : AndroidTestRule(androidUndefinedRulesetConfig) {
+                val configValue: String by configWithAndroidVariants("default", "android")
+            }
+
+            assertThat(subject.configValue).isEqualTo("default")
+        }
+
+        @Test
+        fun `uses default if ruleset is non android`() {
+            val subject = object : AndroidTestRule(nonAndroidRulesetConfig) {
+                val configValue: String by configWithAndroidVariants("default", "android")
+            }
+
+            assertThat(subject.configValue).isEqualTo("default")
+        }
+
+        @Test
+        fun `uses android default if ruleset is android`() {
+            val subject = object : AndroidTestRule(androidRulesetConfig) {
+                val configValue: String by configWithAndroidVariants("default", "android")
+            }
+
+            assertThat(subject.configValue).isEqualTo("android")
+        }
+
+        @ParameterizedTest
+        @MethodSource("rulesetConfigs")
+        fun `always uses explicitly defined value from config`(rulesetConfig: Config) {
+            val explicitlyConfiguredValue = "other"
+            val subject = object : AndroidTestRule(rulesetConfig, "configValue" to explicitlyConfiguredValue) {
+                val configValue: String by configWithAndroidVariants("default", "android")
+            }
+
+            assertThat(subject.configValue).isEqualTo(explicitlyConfiguredValue)
+        }
+
+        fun rulesetConfigs(): List<Arguments.ArgumentSet> = listOf(
+            argumentSet("ruleset has android = true", androidRulesetConfig),
+            argumentSet("ruleset has android = false", nonAndroidRulesetConfig),
+            argumentSet("ruleset has no value for android property", androidUndefinedRulesetConfig)
+        )
+
+        private open inner class AndroidTestRule(rulesetConfig: Config, vararg ruleConfig: Pair<String, Any>) :
+            Rule(TestConfig(parent = rulesetConfig, *ruleConfig), "description")
+    }
 }
 
-open class TestRule(vararg data: Pair<String, Any>) : Rule(TestConfig(*data), "description")
+private open class TestRule(vararg data: Pair<String, Any>) : Rule(TestConfig(*data), "description")
diff --git a/detekt-generator/src/test/kotlin/dev/detekt/generator/printer/defaultconfig/ConfigPrinterSpec.kt b/detekt-generator/src/test/kotlin/dev/detekt/generator/printer/defaultconfig/ConfigPrinterSpec.kt
index 5f1e9f82398f..448e8b2ed50c 100644
--- a/detekt-generator/src/test/kotlin/dev/detekt/generator/printer/defaultconfig/ConfigPrinterSpec.kt
+++ b/detekt-generator/src/test/kotlin/dev/detekt/generator/printer/defaultconfig/ConfigPrinterSpec.kt
@@ -29,7 +29,7 @@ class ConfigPrinterSpec {
                 conf3:
                   - 'a'
                   - 'b'
-                conf5: 120
+                # conf5: If the 'android' ruleset property is set to true, the default is '100', otherwise '120'.
               EqualsNull:
                 active: false
               NoUnitKeyword: