diff --git a/detekt-api/api/detekt-api.api b/detekt-api/api/detekt-api.api index dabd9594cd3..8405d730980 100644 --- a/detekt-api/api/detekt-api.api +++ b/detekt-api/api/detekt-api.api @@ -616,3 +616,31 @@ public abstract interface annotation class io/gitlab/arturbosch/detekt/api/Unsta public abstract fun reason ()Ljava/lang/String; } +public final class io/gitlab/arturbosch/detekt/api/ValueWithReason { + public fun (Ljava/lang/String;Ljava/lang/String;)V + public synthetic fun (Ljava/lang/String;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun component1 ()Ljava/lang/String; + public final fun component2 ()Ljava/lang/String; + public final fun copy (Ljava/lang/String;Ljava/lang/String;)Lio/gitlab/arturbosch/detekt/api/ValueWithReason; + public static synthetic fun copy$default (Lio/gitlab/arturbosch/detekt/api/ValueWithReason;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lio/gitlab/arturbosch/detekt/api/ValueWithReason; + public fun equals (Ljava/lang/Object;)Z + public final fun getReason ()Ljava/lang/String; + public final fun getValue ()Ljava/lang/String; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + +public final class io/gitlab/arturbosch/detekt/api/ValuesWithReason : java/lang/Iterable, kotlin/jvm/internal/markers/KMappedMarker { + public final fun copy (Ljava/util/List;)Lio/gitlab/arturbosch/detekt/api/ValuesWithReason; + public static synthetic fun copy$default (Lio/gitlab/arturbosch/detekt/api/ValuesWithReason;Ljava/util/List;ILjava/lang/Object;)Lio/gitlab/arturbosch/detekt/api/ValuesWithReason; + public fun equals (Ljava/lang/Object;)Z + public fun hashCode ()I + public fun iterator ()Ljava/util/Iterator; + public fun toString ()Ljava/lang/String; +} + +public final class io/gitlab/arturbosch/detekt/api/ValuesWithReasonKt { + public static final fun valuesWithReason (Ljava/util/List;)Lio/gitlab/arturbosch/detekt/api/ValuesWithReason; + public static final fun valuesWithReason ([Lkotlin/Pair;)Lio/gitlab/arturbosch/detekt/api/ValuesWithReason; +} + diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt index b519b819e43..056cd9f6eaa 100644 --- a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt +++ b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ConfigProperty.kt @@ -111,14 +111,8 @@ fun configWithAndroidVariants( private fun getValueOrDefault(configAware: ConfigAware, propertyName: String, defaultValue: T): T { @Suppress("UNCHECKED_CAST") return when (defaultValue) { - is List<*> -> { - if (defaultValue.all { it is String }) { - val defaultValueAsListOfStrings = defaultValue as List - configAware.valueOrDefaultCommaSeparated(propertyName, defaultValueAsListOfStrings) as T - } else { - error("Only lists of strings are supported. '$propertyName' is invalid. ") - } - } + is ValuesWithReason -> configAware.getValuesWithReasonOrDefault(propertyName, defaultValue) as T + is List<*> -> configAware.getListOrDefault(propertyName, defaultValue) as T is String, is Boolean, is Int -> configAware.valueOrDefault(propertyName, defaultValue) @@ -129,6 +123,45 @@ private fun getValueOrDefault(configAware: ConfigAware, propertyName: } } +private fun ConfigAware.getListOrDefault(propertyName: String, defaultValue: List<*>): List { + return if (defaultValue.all { it is String }) { + @Suppress("UNCHECKED_CAST") + val defaultValueAsListOfStrings = defaultValue as List + valueOrDefaultCommaSeparated(propertyName, defaultValueAsListOfStrings) + } else { + error("Only lists of strings are supported. '$propertyName' is invalid. ") + } +} + +private fun ConfigAware.getValuesWithReasonOrDefault( + propertyName: String, + defaultValue: ValuesWithReason +): ValuesWithReason { + val valuesAsList: List<*> = valueOrNull(propertyName) ?: return defaultValue + if (valuesAsList.all { it is String }) { + return ValuesWithReason(values = valuesAsList.map { ValueWithReason(it as String) }) + } + if (valuesAsList.all { it is Map<*, *> }) { + return ValuesWithReason( + valuesAsList + .map { it as Map<*, *> } + .map { dict -> + try { + ValueWithReason( + value = dict["value"] as String, + reason = dict["reason"] as String? + ) + } catch (e: ClassCastException) { + throw Config.InvalidConfigurationError(e) + } catch (@Suppress("TooGenericExceptionCaught") e: NullPointerException) { + throw Config.InvalidConfigurationError(e) + } + } + ) + } + error("Only lists of strings or maps with keys 'value' and 'reason' are supported. '$propertyName' is invalid.") +} + private abstract class MemoizedConfigProperty : ReadOnlyProperty { private var value: U? = null diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ValuesWithReason.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ValuesWithReason.kt new file mode 100644 index 00000000000..7fe929f2c17 --- /dev/null +++ b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/ValuesWithReason.kt @@ -0,0 +1,42 @@ +package io.gitlab.arturbosch.detekt.api + +/** + * This factory method can be used by rule authors to specify one or many configuration values along with an + * explanation for each value. For example: + * + * ```kotlin + * @Configuration("imports which should not be used") + * private val imports: ValuesWithReason by config( + * valuesWithReason("org.junit.Test" to "Do not use Junit4. Use org.junit.jupiter.api.Test instead.") + * ) + * ``` + * + * Note that the [config] property delegate only supports the factory methods when defining [ValuesWithReason]. + */ +fun valuesWithReason(vararg values: Pair): ValuesWithReason { + return valuesWithReason(values.map { ValueWithReason(it.first, it.second) }) +} + +/** + * This factory method can be used by rule authors to specify one or many configuration values along with an + * explanation for each value. + * + * Note that the [config] property delegate only supports the factory methods when defining [ValuesWithReason]. + */ +fun valuesWithReason(values: List): ValuesWithReason { + return ValuesWithReason(values) +} + +/** + * [ValuesWithReason] is essentially the same as [List] of [ValueWithReason]. Due to type erasure we cannot use the + * list directly. Instances of this type should always created using the [valuesWithReason] factory method. + */ +data class ValuesWithReason internal constructor(private val values: List) : + Iterable by values + +/** + * A ValueWithReason represents a single configuration value that may have an explanation as to why it is used. + * @property value the actual value that is configured + * @property reason an optional explanation for the configured value + */ +data class ValueWithReason(val value: String, val reason: String? = null) diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt index 1bab275bc11..af82c994357 100644 --- a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt +++ b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/ConfigPropertySpec.kt @@ -3,6 +3,7 @@ package io.gitlab.arturbosch.detekt.api import io.gitlab.arturbosch.detekt.test.TestConfig import org.assertj.core.api.Assertions.assertThat 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 java.util.concurrent.atomic.AtomicInteger @@ -65,6 +66,104 @@ class ConfigPropertySpec { assertThat(subject.present).isEqualTo(configValue) } } + + @Nested + inner class `ValuesWithReason property` { + private val defaultValue = valuesWithReason("aValue" to "aReason") + + @Nested + inner class `value defined as list` { + private val subject = object : TestConfigAware("present" to listOf("a", "b", "c")) { + val present: ValuesWithReason by config(defaultValue) + val notPresent: ValuesWithReason by config(defaultValue) + } + + @Test + fun `uses the value provided in config if present`() { + assertThat(subject.present) + .extracting(ValueWithReason::value, ValueWithReason::reason) + .containsExactly(tuple("a", null), tuple("b", null), tuple("c", null)) + } + + @Test + fun `uses the default value if not present`() { + assertThat(subject.notPresent).isEqualTo(defaultValue) + } + } + + @Nested + inner class `value defined as list of maps` { + private val subject = object : TestConfigAware( + "present" to listOf( + mapOf("value" to "a", "reason" to "reasonA"), + mapOf("value" to "b", "reason" to null), + mapOf("value" to "c"), + ) + ) { + val present: ValuesWithReason by config(defaultValue) + val notPresent: ValuesWithReason by config(defaultValue) + } + + @Test + fun `uses the value provided in config if present`() { + assertThat(subject.present) + .extracting(ValueWithReason::value, ValueWithReason::reason) + .containsExactly( + tuple("a", "reasonA"), + tuple("b", null), + tuple("c", null) + ) + } + + @Test + fun `uses the default value if not present`() { + assertThat(subject.notPresent).isEqualTo(defaultValue) + } + } + + @Nested + inner class `value defined as list of maps with invalid data` { + + @Test + fun `value missing`() { + assertThatThrownBy { + object : TestConfigAware( + "present" to listOf( + mapOf("reason" to "reason") + ) + ) { + val present: ValuesWithReason by config(defaultValue) + }.present + }.isInstanceOf(Config.InvalidConfigurationError::class.java) + } + + @Test + fun `value with an invalid type`() { + assertThatThrownBy { + object : TestConfigAware( + "present" to listOf( + mapOf("value" to 42, "reason" to "reason") + ) + ) { + val present: ValuesWithReason by config(defaultValue) + }.present + }.isInstanceOf(Config.InvalidConfigurationError::class.java) + } + + @Test + fun `reason with an invalid type`() { + assertThatThrownBy { + object : TestConfigAware( + "present" to listOf( + mapOf("value" to "a", "reason" to 42) + ) + ) { + val present: ValuesWithReason by config(defaultValue) + }.present + }.isInstanceOf(Config.InvalidConfigurationError::class.java) + } + } + } } @Nested diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfigSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfigSpec.kt index 8531378fe4f..8c45bc8c762 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfigSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfigSpec.kt @@ -126,6 +126,46 @@ class YamlConfigSpec { assertThat(config).isNotNull } + @Nested + inner class `Values with reason` { + private val config = YamlConfig.load(resourceAsPath("values-with-reason.yml")) + + @Test + fun `can be parsed`() { + assertThat(config).isNotNull + } + + @Test + fun `supports lists`() { + val actualAsList: List<*>? = config + .subConfig("style") + .subConfig("AsList") + .valueOrNull("values") + assertThat(actualAsList).hasSize(3) + } + + @Test + fun `supports dictionaries`() { + val actualAsMap: List>? = config + .subConfig("style") + .subConfig("AsListOfMaps") + .valueOrNull("values") + assertThat(actualAsMap) + .hasSize(3) + } + + @Test + fun `supports empty dictionaries`() { + val actualAsMap: List>? = config + .subConfig("style") + .subConfig("EmptyListOfMaps") + .valueOrNull("values") + assertThat(actualAsMap) + .isNotNull + .isEmpty() + } + } + @Test fun `throws an exception on an non-existing file`() { val path = Paths.get("doesNotExist.yml") diff --git a/detekt-core/src/test/resources/values-with-reason.yml b/detekt-core/src/test/resources/values-with-reason.yml new file mode 100644 index 00000000000..ea8fecd04c4 --- /dev/null +++ b/detekt-core/src/test/resources/values-with-reason.yml @@ -0,0 +1,15 @@ +style: + AsList: + values: + - a + - b + - c + AsListOfMaps: + values: + - value: a + reason: reason A + - value: b + - value: c + reason: reason C + EmptyListOfMaps: + values: [] diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/Configuration.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/Configuration.kt index 4773e3e25c0..7b77f8a184e 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/Configuration.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/Configuration.kt @@ -8,8 +8,4 @@ data class Configuration( val deprecated: String? ) { fun isDeprecated() = deprecated != null - - fun isDefaultValueNonEmptyList() = defaultValue.isNonEmptyList() - - fun getDefaultValueAsList(): List = defaultValue.getAsList() } diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt index 680d37b9809..86e36b87da4 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt @@ -1,5 +1,7 @@ package io.gitlab.arturbosch.detekt.generator.collection +import io.gitlab.arturbosch.detekt.api.ValueWithReason +import io.gitlab.arturbosch.detekt.api.valuesWithReason import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.ConfigWithAndroidVariantsSupport.ANDROID_VARIANTS_DELEGATE_NAME import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.ConfigWithAndroidVariantsSupport.DEFAULT_ANDROID_VALUE_ARGUMENT_NAME import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.ConfigWithAndroidVariantsSupport.isAndroidVariantConfigDelegate @@ -9,7 +11,12 @@ import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.C import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.DefaultValueSupport.getAndroidDefaultValue import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.DefaultValueSupport.getDefaultValue import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.DefaultValueSupport.toDefaultValueIfLiteral +import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.StringListSupport.getListDefaultOrNull +import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.StringListSupport.hasListDeclaration +import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.ValuesWithReasonSupport.getValuesWithReasonDefaultOrNull +import io.gitlab.arturbosch.detekt.generator.collection.ConfigurationCollector.ValuesWithReasonSupport.hasValuesWithReasonDeclaration import io.gitlab.arturbosch.detekt.generator.collection.exception.InvalidDocumentationException +import org.jetbrains.kotlin.psi.KtBinaryExpression import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtCallableReferenceExpression import org.jetbrains.kotlin.psi.KtConstantExpression @@ -56,8 +63,13 @@ class ConfigurationCollector { } private fun KtProperty.getConstantValue(): DefaultValue? { + if (hasValuesWithReasonDeclaration()) { + return getValuesWithReasonDefaultOrNull() + ?: invalidDocumentation { "Invalid declaration of values with reasons default for property '$text'" } + } if (hasListDeclaration()) { - return DefaultValue.of(getListDeclaration().valueArguments.map { it.text.withoutQuotes() }) + return getListDefaultOrNull(emptyMap()) + ?: invalidDocumentation { "Invalid declaration of string list default for property '$text'" } } return findDescendantOfType()?.toDefaultValueIfLiteral() @@ -125,15 +137,9 @@ class ConfigurationCollector { } fun KtExpression.toDefaultValue(constantsByName: Map): DefaultValue { - val listDeclarationForDefault = getListDeclarationOrNull() - if (listDeclarationForDefault != null) { - val listValues = listDeclarationForDefault.valueArguments.map { - (constantsByName[it.text]?.getAsPlainString() ?: it.text.withoutQuotes()) - } - return DefaultValue.of(listValues) - } - - return toDefaultValueIfLiteral() + return getValuesWithReasonDefaultOrNull() + ?: getListDefaultOrNull(constantsByName) + ?: toDefaultValueIfLiteral() ?: constantsByName[text.withoutQuotes()] ?: error("$text is neither a literal nor a constant") } @@ -176,6 +182,63 @@ class ConfigurationCollector { delegate?.expression?.referenceExpression()?.text == ANDROID_VARIANTS_DELEGATE_NAME } + private object ValuesWithReasonSupport { + private const val VALUES_WITH_REASON_FACTORY_METHOD = "valuesWithReason" + + fun KtElement.getValuesWithReasonDefaultOrNull(): DefaultValue? { + return getValuesWithReasonDeclarationOrNull() + ?.valueArguments + ?.map(::toValueWithReason) + ?.let { DefaultValue.of(valuesWithReason(it)) } + } + + fun KtElement.getValuesWithReasonDeclarationOrNull(): KtCallExpression? = + findDescendantOfType { it.isValuesWithReasonDeclaration() } + + fun KtCallExpression.isValuesWithReasonDeclaration(): Boolean { + return referenceExpression()?.text == VALUES_WITH_REASON_FACTORY_METHOD + } + + fun KtProperty.hasValuesWithReasonDeclaration(): Boolean = + anyDescendantOfType { it.isValuesWithReasonDeclaration() } + + private fun toValueWithReason(arg: KtValueArgument): ValueWithReason { + val keyToValue = arg.children.first() as? KtBinaryExpression + return keyToValue?.let { + ValueWithReason( + value = it.left?.text?.withoutQuotes() + ?: error("left side of value with reason argument is null"), + reason = it.right?.text?.withoutQuotes() + ?: error("right side of value with reason argument is null") + ) + } ?: error("invalid value argument '${arg.text}'") + } + } + + private object StringListSupport { + private const val LIST_OF = "listOf" + private const val EMPTY_LIST = "emptyList" + private val LIST_CREATORS = setOf(LIST_OF, EMPTY_LIST) + + fun KtElement.getListDefaultOrNull(constantsByName: Map): DefaultValue? { + return getListDeclarationOrNull()?.valueArguments?.map { + (constantsByName[it.text]?.getPlainValue() ?: it.text.withoutQuotes()) + }?.let { DefaultValue.of(it) } + } + + fun KtElement.getListDeclarationOrNull(): KtCallExpression? = + findDescendantOfType { it.isListDeclaration() } + + fun KtProperty.hasListDeclaration(): Boolean = + anyDescendantOfType { it.isListDeclaration() } + + fun KtElement.getListDeclaration(): KtCallExpression = + checkNotNull(getListDeclarationOrNull()) + + fun KtCallExpression.isListDeclaration() = + referenceExpression()?.text in LIST_CREATORS + } + companion object { private const val SIMPLE_DELEGATE_NAME = "config" private val DELEGATE_NAMES = listOf( @@ -184,25 +247,10 @@ class ConfigurationCollector { ANDROID_VARIANTS_DELEGATE_NAME ) private const val DEFAULT_VALUE_ARGUMENT_NAME = "defaultValue" - private const val LIST_OF = "listOf" - private const val EMPTY_LIST = "emptyList" - private val LIST_CREATORS = setOf(LIST_OF, EMPTY_LIST) - - private fun KtElement.getListDeclaration(): KtCallExpression = - checkNotNull(getListDeclarationOrNull()) - - private fun KtElement.getListDeclarationOrNull(): KtCallExpression? = - findDescendantOfType { it.isListDeclaration() } private fun KtProperty.isInitializedWithConfigDelegate(): Boolean = delegate?.expression?.referenceExpression()?.text in DELEGATE_NAMES - private fun KtProperty.hasListDeclaration(): Boolean = - anyDescendantOfType { it.isListDeclaration() } - - private fun KtCallExpression.isListDeclaration() = - referenceExpression()?.text in LIST_CREATORS - private fun KtElement.invalidDocumentation(message: () -> String): Nothing { throw InvalidDocumentationException("[${containingFile.name}] ${message.invoke()}") } diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/DefaultValue.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/DefaultValue.kt index d04ba305563..f3b481b2371 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/DefaultValue.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/DefaultValue.kt @@ -1,38 +1,79 @@ package io.gitlab.arturbosch.detekt.generator.collection +import io.gitlab.arturbosch.detekt.api.ValuesWithReason +import io.gitlab.arturbosch.detekt.generator.out.YamlNode +import io.gitlab.arturbosch.detekt.generator.out.keyValue +import io.gitlab.arturbosch.detekt.generator.out.list +import io.gitlab.arturbosch.detekt.generator.out.listOfMaps + sealed interface DefaultValue { - fun isNonEmptyList(): Boolean = false - fun getAsList(): List = error("default value is not a list") - fun getAsPlainString(): String = toString() - fun getQuotedIfNecessary(): String = getAsPlainString() + + fun getPlainValue(): String + fun printAsYaml(name: String, yaml: YamlNode) + fun printAsMarkdownCode(): String companion object { fun of(defaultValue: String): DefaultValue = StringDefault(defaultValue) fun of(defaultValue: Boolean): DefaultValue = BooleanDefault(defaultValue) fun of(defaultValue: Int): DefaultValue = IntegerDefault(defaultValue) fun of(defaultValue: List): DefaultValue = StringListDefault(defaultValue) + fun of(defaultValue: ValuesWithReason): DefaultValue = ValuesWithReasonDefault(defaultValue) } } private data class StringDefault(private val defaultValue: String) : DefaultValue { private val quoted = "'$defaultValue'" - override fun getAsPlainString(): String = defaultValue - override fun getQuotedIfNecessary(): String = quoted + override fun printAsYaml(name: String, yaml: YamlNode) { + yaml.keyValue { name to quoted } + } + + override fun printAsMarkdownCode(): String = quoted + + override fun getPlainValue(): String = defaultValue } private data class BooleanDefault(private val defaultValue: Boolean) : DefaultValue { - override fun getAsPlainString(): String = defaultValue.toString() + override fun getPlainValue(): String = defaultValue.toString() + override fun printAsYaml(name: String, yaml: YamlNode) { + yaml.keyValue { name to defaultValue.toString() } + } + + override fun printAsMarkdownCode(): String = defaultValue.toString() } private data class IntegerDefault(private val defaultValue: Int) : DefaultValue { - override fun getAsPlainString(): String = defaultValue.toString() + override fun getPlainValue(): String = defaultValue.toString() + override fun printAsYaml(name: String, yaml: YamlNode) { + yaml.keyValue { name to defaultValue.toString() } + } + + override fun printAsMarkdownCode(): String = defaultValue.toString() } private data class StringListDefault(private val defaultValue: List) : DefaultValue { private val quoted: String = defaultValue.map { "'$it'" }.toString() + override fun printAsYaml(name: String, yaml: YamlNode) { + yaml.list(name, defaultValue) + } - override fun isNonEmptyList(): Boolean = defaultValue.isNotEmpty() - override fun getAsList(): List = defaultValue.ifEmpty { error("default value is an empty list") } - override fun getAsPlainString(): String = defaultValue.toString() - override fun getQuotedIfNecessary(): String = quoted + override fun printAsMarkdownCode(): String = quoted + override fun getPlainValue(): String { + error("there is no plain string representation for list defaults") + } +} + +private data class ValuesWithReasonDefault(private val defaultValue: ValuesWithReason) : DefaultValue { + override fun getPlainValue(): String { + error("there is no plain string representation for values with reason defaults") + } + + override fun printAsYaml(name: String, yaml: YamlNode) { + val asMap: List> = + defaultValue.map { mapOf("value" to it.value, "reason" to it.reason) } + yaml.listOfMaps(name, asMap) + } + + override fun printAsMarkdownCode(): String { + return defaultValue.map { "'${it.value}'" }.toString() + } } diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/out/Yaml.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/out/Yaml.kt index 6807507c7a9..100f1cbf7ba 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/out/Yaml.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/out/Yaml.kt @@ -55,15 +55,44 @@ inline fun YamlNode.keyValue(comment: String = "", keyValue: () -> Pair) { - append("$name:") - list.forEach { - append("$SINGLE_INDENT- ${it.quotedForList()}") + if (list.isEmpty()) { + keyValue { name to EMPTY_LIST } + } else { + append("$name:") + list.forEach { + append("${SINGLE_INDENT}${LIST_PREFIX}${it.ensureQuoted()}") + } + } +} + +fun YamlNode.listOfMaps(name: String, maps: List>) { + val noneEmptyMaps = maps.filter { it.isNotEmpty() } + if (noneEmptyMaps.isEmpty()) { + list(name, emptyList()) + } else { + node(name) { + maps.forEach { map(it) } + } } } +private fun YamlNode.map(map: Map) { + map.entries + .filter { it.value != null } + .sortedBy { it.key } + .forEachIndexed { index, (key, value) -> + val prefix = if (index == 0) { + LIST_PREFIX + } else { + SINGLE_INDENT + } + keyValue { "$prefix$key" to (value?.ensureQuoted() ?: error("value cannot be null")) } + } +} + inline fun YamlNode.yaml(yaml: () -> String): Unit = append(yaml()) -private fun String.quotedForList(): String { +private fun String.ensureQuoted(): String { return when { isBlank() -> quoted() startsWith(SINGLE_QUOTE) && endsWith(SINGLE_QUOTE) || @@ -77,3 +106,5 @@ private fun String.quoted() = "'$this'" private const val SINGLE_INDENT = " " private const val SINGLE_QUOTE = "'" private const val DOUBLE_QUOTE = "\"" +private const val EMPTY_LIST = "[]" +private const val LIST_PREFIX = "- " diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinter.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinter.kt index 56f2ffe2c67..558ab6b62b4 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinter.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinter.kt @@ -18,8 +18,8 @@ internal object RuleConfigurationPrinter : DocumentationPrinter() + val result = yaml { list("key", given) } + val expected = "key: []" + assertThat(result).isEqualTo(expected) + } @Test fun `renders single element`() { @@ -74,4 +82,134 @@ class YamlSpec { assertThat(result).isEqualTo(expected) } } + + @Nested + inner class ListOfMaps { + + @Test + fun `renders an empty list of maps`() { + val given = emptyList>() + val result = yaml { listOfMaps("key", given) } + val expected = "key: []" + assertThat(result).isEqualTo(expected) + } + + @Test + fun `renders an list of empty maps`() { + val given = listOf>(emptyMap(), emptyMap()) + val result = yaml { listOfMaps("key", given) } + val expected = "key: []" + assertThat(result).isEqualTo(expected) + } + + @Test + fun `renders single map with single element`() { + val given = listOf(mapOf("name" to "value")) + val result = yaml { listOfMaps("key", given) } + val expected = """key: + | - name: 'value' + """.trimMargin() + assertThat(result).isEqualTo(expected) + } + + @Test + fun `renders single map with multiple elements`() { + val given = listOf( + mapOf( + "name1" to "value 1", + "name2" to "value 2", + "name3" to "value 3" + ) + ) + val result = yaml { listOfMaps("key", given) } + val expected = """key: + | - name1: 'value 1' + | name2: 'value 2' + | name3: 'value 3' + """.trimMargin() + assertThat(result).isEqualTo(expected) + } + + @Test + fun `renders multiple maps with multiple elements omitting empty maps`() { + val given = listOf( + mapOf( + "name1" to "value 1", + "name2" to "value 2" + ), + emptyMap(), + mapOf( + "name3" to "value 3" + ), + mapOf( + "name4" to "value 4", + "name5" to "value 5" + ) + ) + val result = yaml { listOfMaps("key", given) } + val expected = """key: + | - name1: 'value 1' + | name2: 'value 2' + | - name3: 'value 3' + | - name4: 'value 4' + | name5: 'value 5' + """.trimMargin() + assertThat(result).isEqualTo(expected) + } + + @Test + fun `sorts entries by key name`() { + val given = listOf( + mapOf( + "z" to "value", + "a" to "value", + "x" to "value", + "b" to "value", + ), + ) + val result = yaml { listOfMaps("key", given) } + val expected = """key: + | - a: 'value' + | b: 'value' + | x: 'value' + | z: 'value' + """.trimMargin() + assertThat(result).isEqualTo(expected) + } + + @Test + fun `omits entries with null value`() { + val given = listOf( + mapOf( + "a" to "value", + "b" to null, + "c" to "value", + ), + ) + val result = yaml { listOfMaps("key", given) } + val expected = """key: + | - a: 'value' + | c: 'value' + """.trimMargin() + assertThat(result).isEqualTo(expected) + } + + @Test + fun `quotes values if necessary`() { + val given = listOf( + mapOf( + "name1" to "'already quoted'", + "name2" to "\"also quoted\"", + "name3" to "should be quoted" + ) + ) + val result = yaml { listOfMaps("key", given) } + val expected = """key: + | - name1: 'already quoted' + | name2: "also quoted" + | name3: 'should be quoted' + """.trimMargin() + assertThat(result).isEqualTo(expected) + } + } } diff --git a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinterTest.kt b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinterTest.kt index 509a54dc49e..f25819866a8 100644 --- a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinterTest.kt +++ b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RuleConfigurationPrinterTest.kt @@ -1,5 +1,6 @@ package io.gitlab.arturbosch.detekt.generator.printer +import io.gitlab.arturbosch.detekt.api.valuesWithReason import io.gitlab.arturbosch.detekt.generator.collection.Configuration import io.gitlab.arturbosch.detekt.generator.collection.DefaultValue import org.assertj.core.api.Assertions.assertThat @@ -52,6 +53,28 @@ internal class RuleConfigurationPrinterTest { assertThat(actual).contains("""* ``configName`` (default: ``['a', 'b', 'c']``)""") } + @Test + fun `empty string list default`() { + val config = configTemplate.copy(defaultValue = DefaultValue.of(emptyList())) + val actual = RuleConfigurationPrinter.print(listOf(config)) + assertThat(actual).contains("""* ``configName`` (default: ``[]``)""") + } + + @Test + fun `values with reason default`() { + val config = configTemplate.copy( + defaultValue = DefaultValue.of( + valuesWithReason( + "a" to "reason for a", + "b" to "reason for b", + "c" to null + ) + ) + ) + val actual = RuleConfigurationPrinter.print(listOf(config)) + assertThat(actual).contains("""* ``configName`` (default: ``['a', 'b', 'c']``)""") + } + @Test fun `with android default`() { val config = configTemplate.copy( diff --git a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/RuleSetConfigPrinterTest.kt b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/RuleSetConfigPrinterTest.kt index 0cfa1fc5e4d..35532232259 100644 --- a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/RuleSetConfigPrinterTest.kt +++ b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/RuleSetConfigPrinterTest.kt @@ -1,6 +1,7 @@ package io.gitlab.arturbosch.detekt.generator.printer.defaultconfig import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.valuesWithReason import io.gitlab.arturbosch.detekt.generator.collection.Active import io.gitlab.arturbosch.detekt.generator.collection.Configuration import io.gitlab.arturbosch.detekt.generator.collection.DefaultValue @@ -200,6 +201,35 @@ internal class RuleSetConfigPrinterTest { """.trimMargin() assertThat(actual).isEqualTo(expected) } + + @Test + fun `empty ValuesWithReason default value uses empty list syntax`() { + val given = configurationTemplate.copy(defaultValue = DefaultValue.of(valuesWithReason())) + val actual = yaml { printConfiguration(given) } + assertThat(actual).isEqualTo("name: []") + } + + @Test + fun `ValuesWithReason default value block syntax`() { + val given = configurationTemplate.copy( + defaultValue = DefaultValue.of( + valuesWithReason( + "a" to "reason a", + "b" to null, + "c" to "reason c", + ) + ) + ) + val actual = yaml { printConfiguration(given) } + val expected = """name: + | - reason: 'reason a' + | value: 'a' + | - value: 'b' + | - reason: 'reason c' + | value: 'c' + """.trimMargin() + assertThat(actual).isEqualTo(expected) + } } } } diff --git a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/util/CollectorTestExtensions.kt b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/util/CollectorTestExtensions.kt index 9e6c6e8ff06..419a62a8cd1 100644 --- a/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/util/CollectorTestExtensions.kt +++ b/detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/util/CollectorTestExtensions.kt @@ -2,8 +2,9 @@ package io.gitlab.arturbosch.detekt.generator.util import io.github.detekt.test.utils.compileContentForTest import io.gitlab.arturbosch.detekt.generator.collection.Collector +import org.intellij.lang.annotations.Language -fun Collector.run(code: String): List { +fun Collector.run(@Language("kotlin") code: String): List { val ktFile = compileContentForTest(code.trimIndent()) visit(ktFile) return items