diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/AllRulesConfig.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/AllRulesConfig.kt index 26d8e5e88a7..9522973cd8d 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/AllRulesConfig.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/AllRulesConfig.kt @@ -1,6 +1,8 @@ package io.gitlab.arturbosch.detekt.core.config import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.core.config.validation.ValidatableConfiguration +import io.gitlab.arturbosch.detekt.core.config.validation.validateConfig @Suppress("UNCHECKED_CAST") internal data class AllRulesConfig( diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/CompositeConfig.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/CompositeConfig.kt index 90aeca227a7..9e7075d85e7 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/CompositeConfig.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/CompositeConfig.kt @@ -2,6 +2,8 @@ package io.gitlab.arturbosch.detekt.core.config import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.core.config.validation.ValidatableConfiguration +import io.gitlab.arturbosch.detekt.core.config.validation.validateConfig /** * Wraps two different configuration which should be considered when retrieving properties. diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ConfigValidators.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ConfigValidators.kt deleted file mode 100644 index bd00be46dc2..00000000000 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ConfigValidators.kt +++ /dev/null @@ -1,39 +0,0 @@ -package io.gitlab.arturbosch.detekt.core.config - -import io.github.detekt.tooling.api.InvalidConfig -import io.gitlab.arturbosch.detekt.api.Config -import io.gitlab.arturbosch.detekt.api.ConfigValidator -import io.gitlab.arturbosch.detekt.api.Notification -import io.gitlab.arturbosch.detekt.core.NL -import io.gitlab.arturbosch.detekt.core.ProcessingSettings -import io.gitlab.arturbosch.detekt.core.extensions.loadExtensions -import io.gitlab.arturbosch.detekt.core.reporting.red -import io.gitlab.arturbosch.detekt.core.reporting.yellow - -internal fun checkConfiguration(settings: ProcessingSettings, baseline: Config) { - var shouldValidate = settings.spec.configSpec.shouldValidateBeforeAnalysis - if (shouldValidate == null) { - val props = settings.config.subConfig("config") - shouldValidate = props.valueOrDefault("validation", true) - } - if (shouldValidate) { - val validators = - loadExtensions(settings) + DefaultPropertiesConfigValidator(settings, baseline) - val notifications = validators.flatMap { it.validate(settings.config) } - notifications.map(Notification::message).forEach(settings::info) - val errors = notifications.filter(Notification::isError) - if (errors.isNotEmpty()) { - val problems = notifications.joinToString(NL) { "\t- ${it.renderMessage()}" } - val propsString = if (errors.size == 1) "property" else "properties" - val title = "Run failed with ${errors.size} invalid config $propsString.".red() - throw InvalidConfig("$title$NL$problems") - } - } -} - -internal fun Notification.renderMessage(): String = - when (level) { - Notification.Level.Error -> message.red() - Notification.Level.Warning -> message.yellow() - Notification.Level.Info -> message - } diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/DisabledAutoCorrectConfig.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/DisabledAutoCorrectConfig.kt index 193751a0039..6e7bffae848 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/DisabledAutoCorrectConfig.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/DisabledAutoCorrectConfig.kt @@ -2,6 +2,8 @@ package io.gitlab.arturbosch.detekt.core.config import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.core.config.validation.ValidatableConfiguration +import io.gitlab.arturbosch.detekt.core.config.validation.validateConfig @Suppress("UNCHECKED_CAST") class DisabledAutoCorrectConfig(private val wrapped: Config) : Config, ValidatableConfiguration { diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidateConfig.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidateConfig.kt deleted file mode 100644 index 4f6dfe62d52..00000000000 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidateConfig.kt +++ /dev/null @@ -1,153 +0,0 @@ -package io.gitlab.arturbosch.detekt.core.config - -import io.github.detekt.utils.openSafeStream -import io.gitlab.arturbosch.detekt.api.Config -import io.gitlab.arturbosch.detekt.api.Notification -import io.gitlab.arturbosch.detekt.api.internal.CommaSeparatedPattern -import io.gitlab.arturbosch.detekt.api.internal.SimpleNotification -import java.util.Properties - -/** - * Known existing properties on rule's which my be absent in the default-detekt-config.yml. - * - * We need to predefine them as the user may not have already declared an 'config'-block - * in the configuration and we want to validate the config by default. - */ -val DEFAULT_PROPERTY_EXCLUDES = setOf( - ".*>excludes", - ".*>includes", - ".*>active", - ".*>.*>excludes", - ".*>.*>includes", - ".*>.*>active", - ".*>.*>autoCorrect", - ".*>severity", - ".*>.*>severity", - "build>weights.*", - ".*>.*>ignoreAnnotated", - ".*>.*>ignoreFunction", -).joinToString(",") - -fun validateConfig( - config: Config, - baseline: Config, - excludePatterns: Set = CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() -): List = validateConfig( - config, - baseline, - ValidationSettings( - config.subConfig("config").valueOrDefault("warningsAsErrors", false), - excludePatterns, - ) -) - -internal data class ValidationSettings( - val warningsAsErrors: Boolean, - val excludePatterns: Set, -) - -@Suppress("UNCHECKED_CAST", "ComplexMethod") -internal fun validateConfig( - config: Config, - baseline: Config, - settings: ValidationSettings, -): List { - require(baseline != Config.empty) { "Cannot validate configuration based on an empty baseline config." } - require(baseline is YamlConfig) { - val yamlConfigClass = YamlConfig::class.simpleName - val actualClass = baseline.javaClass.simpleName - - "Only supported baseline config is the $yamlConfigClass. Actual type is $actualClass" - } - - if (config == Config.empty) { - return emptyList() - } - - val (warningsAsErrors, excludePatterns) = settings - val notifications = mutableListOf() - - fun getDeprecatedProperties(): List> { - return settings.javaClass.classLoader - .getResource("deprecation.properties")!! - .openSafeStream() - .use { inputStream -> - val prop = Properties().apply { load(inputStream) } - - prop.entries.map { entry -> - (entry.key as String).toRegex() to (entry.value as String) - } - } - } - - fun testKeys(current: Map, base: Map, parentPath: String?) { - for (prop in current.keys) { - val propertyPath = "${if (parentPath == null) "" else "$parentPath>"}$prop" - - val deprecationWarning = getDeprecatedProperties() - .find { (regex, _) -> regex.matches(propertyPath) } - ?.second - val isExcluded = excludePatterns.any { it.matches(propertyPath) } - - if (deprecationWarning != null) { - notifications.add(propertyIsDeprecated(propertyPath, deprecationWarning, warningsAsErrors)) - } - - if (deprecationWarning != null || isExcluded) { - continue - } - - if (!base.contains(prop)) { - notifications.add(propertyDoesNotExists(propertyPath)) - } else if (current[prop] is String && base[prop] is List<*>) { - notifications.add(propertyShouldBeAnArray(propertyPath, warningsAsErrors)) - } - - val next = current[prop] as? Map - val nextBase = base[prop] as? Map - - when { - next == null && nextBase != null -> notifications.add(nestedConfigurationExpected(propertyPath)) - base.contains(prop) && next != null && nextBase == null -> - notifications.add(unexpectedNestedConfiguration(propertyPath)) - next != null && nextBase != null -> testKeys(next, nextBase, propertyPath) - } - } - } - - when (config) { - is YamlConfig -> testKeys(config.properties, baseline.properties, null) - is ValidatableConfiguration -> notifications.addAll(config.validate(baseline, excludePatterns)) - else -> error("Unsupported config type for validation: '${config::class}'.") - } - - return notifications -} - -internal fun propertyDoesNotExists(prop: String): Notification = - SimpleNotification("Property '$prop' is misspelled or does not exist.") - -internal fun nestedConfigurationExpected(prop: String): Notification = - SimpleNotification("Nested config expected for '$prop'.") - -internal fun unexpectedNestedConfiguration(prop: String): Notification = - SimpleNotification("Unexpected nested config for '$prop'.") - -internal fun propertyIsDeprecated( - prop: String, - deprecationDescription: String, - reportAsError: Boolean, -): Notification = - SimpleNotification( - "Property '$prop' is deprecated. $deprecationDescription.", - if (reportAsError) Notification.Level.Error else Notification.Level.Warning, - ) - -internal fun propertyShouldBeAnArray( - prop: String, - reportAsError: Boolean, -): Notification = - SimpleNotification( - "Property '$prop' should be a YAML array instead of a comma-separated String.", - if (reportAsError) Notification.Level.Error else Notification.Level.Warning, - ) diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfig.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfig.kt index 24c9d5cd25e..b0ae1d6481d 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfig.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/YamlConfig.kt @@ -5,6 +5,8 @@ package io.gitlab.arturbosch.detekt.core.config import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.api.Config.Companion.CONFIG_SEPARATOR import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.core.config.validation.ValidatableConfiguration +import io.gitlab.arturbosch.detekt.core.config.validation.validateConfig import org.yaml.snakeyaml.Yaml import java.io.Reader import java.nio.file.Path diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidator.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidator.kt new file mode 100644 index 00000000000..9bb1270cf09 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidator.kt @@ -0,0 +1,28 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.ConfigValidator +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.core.config.YamlConfig + +internal abstract class AbstractYamlConfigValidator : ConfigValidator { + + override fun validate(config: Config): Collection { + require(config is YamlConfig) { + val yamlConfigClass = YamlConfig::class.simpleName + val actualClass = config.javaClass.simpleName + + "Only supported config is the $yamlConfigClass. Actual type is $actualClass" + } + val settings = ValidationSettings( + config.subConfig("config").valueOrDefault("checkExhaustiveness", false), + ) + + return validate(config, settings) + } + + abstract fun validate( + configToValidate: YamlConfig, + settings: ValidationSettings + ): Collection +} diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ConfigValidation.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ConfigValidation.kt new file mode 100644 index 00000000000..3e6051077e3 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ConfigValidation.kt @@ -0,0 +1,118 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.github.detekt.tooling.api.InvalidConfig +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.ConfigValidator +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.api.Notification.Level +import io.gitlab.arturbosch.detekt.api.internal.SimpleNotification +import io.gitlab.arturbosch.detekt.core.NL +import io.gitlab.arturbosch.detekt.core.ProcessingSettings +import io.gitlab.arturbosch.detekt.core.config.YamlConfig +import io.gitlab.arturbosch.detekt.core.extensions.loadExtensions +import io.gitlab.arturbosch.detekt.core.reporting.red +import io.gitlab.arturbosch.detekt.core.reporting.yellow + +/** + * Known existing properties on rule's which my be absent in the default-detekt-config.yml. + * + * We need to predefine them as the user may not have already declared an 'config'-block + * in the configuration and we want to validate the config by default. + */ +internal val DEFAULT_PROPERTY_EXCLUDES = setOf( + ".*>excludes", + ".*>includes", + ".*>active", + ".*>.*>excludes", + ".*>.*>includes", + ".*>.*>active", + ".*>.*>autoCorrect", + ".*>severity", + ".*>.*>severity", + "build>weights.*", + ".*>.*>ignoreAnnotated", + ".*>.*>ignoreFunction", +).joinToString(",") + +internal fun checkConfiguration(settings: ProcessingSettings, baseline: Config) { + var shouldValidate = settings.spec.configSpec.shouldValidateBeforeAnalysis + if (shouldValidate == null) { + val props = settings.config.subConfig("config") + shouldValidate = props.valueOrDefault("validation", true) + } + if (shouldValidate) { + val validators = + loadExtensions(settings) + DefaultPropertiesConfigValidator(settings, baseline) + val notifications = validators.flatMap { it.validate(settings.config) } + notifications.map(Notification::message).forEach(settings::info) + val errors = notifications.filter(Notification::isError) + if (errors.isNotEmpty()) { + val problems = notifications.joinToString(NL) { "\t- ${it.renderMessage()}" } + val propsString = if (errors.size == 1) "property" else "properties" + val title = "Run failed with ${errors.size} invalid config $propsString.".red() + throw InvalidConfig("$title$NL$problems") + } + } +} + +internal fun validateConfig( + config: Config, + baseline: Config, + excludePatterns: Set +): List { + require(baseline != Config.empty) { "Cannot validate configuration based on an empty baseline config." } + require(baseline is YamlConfig) { + val yamlConfigClass = YamlConfig::class.simpleName + val actualClass = baseline.javaClass.simpleName + + "Only supported baseline config is the $yamlConfigClass. Actual type is $actualClass" + } + + if (config == Config.empty) { + return emptyList() + } + + return when (config) { + is YamlConfig -> validateYamlConfig(config, baseline, excludePatterns) + is ValidatableConfiguration -> config.validate(baseline, excludePatterns) + else -> error("Unsupported config type for validation: '${config::class}'.") + } +} + +private fun validateYamlConfig( + configToValidate: YamlConfig, + baseline: YamlConfig, + excludePatterns: Set +): List { + val deprecatedProperties = loadDeprecations() + val warningsAsErrors = configToValidate + .subConfig("config") + .valueOrDefault("warningsAsErrors", false) + + val validators: List = listOf( + InvalidPropertiesConfigValidator(baseline, deprecatedProperties.keys, excludePatterns), + DeprecatedPropertiesConfigValidator(deprecatedProperties), + MissingRulesConfigValidator(baseline, excludePatterns) + ) + + return validators + .flatMap { it.validate(configToValidate) } + .map { notification -> + notification.transformIf(warningsAsErrors && notification.level == Level.Warning) { + SimpleNotification( + message = notification.message, + level = Level.Error + ) + } + } +} + +private fun T.transformIf(condition: Boolean, transform: () -> T): T = + if (condition) transform() else this + +internal fun Notification.renderMessage(): String = + when (level) { + Level.Error -> message.red() + Level.Warning -> message.yellow() + Level.Info -> message + } diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/DefaultPropertiesConfigValidator.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DefaultPropertiesConfigValidator.kt similarity index 91% rename from detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/DefaultPropertiesConfigValidator.kt rename to detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DefaultPropertiesConfigValidator.kt index 302b9c18033..7578ad2fa72 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/DefaultPropertiesConfigValidator.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DefaultPropertiesConfigValidator.kt @@ -1,4 +1,4 @@ -package io.gitlab.arturbosch.detekt.core.config +package io.gitlab.arturbosch.detekt.core.config.validation import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.api.ConfigValidator @@ -8,7 +8,7 @@ import io.gitlab.arturbosch.detekt.api.internal.DefaultRuleSetProvider import io.gitlab.arturbosch.detekt.core.ProcessingSettings import io.gitlab.arturbosch.detekt.core.rules.RuleSetLocator -class DefaultPropertiesConfigValidator( +internal class DefaultPropertiesConfigValidator( private val settings: ProcessingSettings, private val baseline: Config, ) : ConfigValidator { diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidator.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidator.kt new file mode 100644 index 00000000000..a77d6c8b119 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidator.kt @@ -0,0 +1,44 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.api.internal.SimpleNotification +import io.gitlab.arturbosch.detekt.core.config.YamlConfig + +internal class DeprecatedPropertiesConfigValidator( + private val deprecatedProperties: Map, +) : AbstractYamlConfigValidator() { + override fun validate( + configToValidate: YamlConfig, + settings: ValidationSettings + ): Collection { + val configAsMap = configToValidate.properties + return deprecatedProperties + .map { (path, description) -> path.split(">") to description } + .filter { (path, _) -> configAsMap.hasValue(path) } + .map { (path, description) -> createNotification(path, description) } + } + + @Suppress("UNCHECKED_CAST") + private fun Map.hasValue(propertyPath: List): Boolean { + if (propertyPath.isEmpty()) { + return false + } + if (propertyPath.size == 1) { + return this.containsKey(propertyPath.first()) + } + + val subMap = this[propertyPath.first()] as? Map ?: return false + return subMap.hasValue(propertyPath.drop(1)) + } + + private fun createNotification( + propertyPath: List, + deprecationDescription: String, + ): Notification { + val prop = propertyPath.joinToString(">") + return SimpleNotification( + "Property '$prop' is deprecated. $deprecationDescription.", + Notification.Level.Warning, + ) + } +} diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/Deprecations.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/Deprecations.kt new file mode 100644 index 00000000000..869b745f041 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/Deprecations.kt @@ -0,0 +1,17 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.github.detekt.utils.openSafeStream +import java.util.Properties + +internal fun loadDeprecations(): Map { + return ValidationSettings::class.java.classLoader + .getResource("deprecation.properties")!! + .openSafeStream() + .use { inputStream -> + val prop = Properties().apply { load(inputStream) } + + prop.entries.associate { entry -> + (entry.key as String) to (entry.value as String) + } + } +} diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidator.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidator.kt new file mode 100644 index 00000000000..f2c7c68528e --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidator.kt @@ -0,0 +1,89 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.api.internal.SimpleNotification +import io.gitlab.arturbosch.detekt.core.config.YamlConfig + +internal class InvalidPropertiesConfigValidator( + private val baseline: YamlConfig, + private val deprecatedProperties: Set, + private val excludePatterns: Set, +) : AbstractYamlConfigValidator() { + + override fun validate( + configToValidate: YamlConfig, + settings: ValidationSettings + ): Collection { + return testKeys(configToValidate.properties, baseline.properties) + } + + private fun testKeys( + configToValidate: Map, + baseline: Map, + parentPath: String? = null + ): List { + val notifications = mutableListOf() + for (prop in configToValidate.keys) { + val propertyPath = "${if (parentPath == null) "" else "$parentPath>"}$prop" + val isExcluded = excludePatterns.any { it.matches(propertyPath) } + val isDeprecated = deprecatedProperties.contains(propertyPath) + if (isExcluded || isDeprecated) { + continue + } + notifications.addAll( + checkProp( + propertyName = prop, + propertyPath = propertyPath, + configToValidate = configToValidate, + baseline = baseline + ) + ) + } + return notifications + } + + @Suppress("UNCHECKED_CAST") + private fun checkProp( + propertyName: String, + propertyPath: String, + configToValidate: Map, + baseline: Map + ): List { + if (!baseline.contains(propertyName)) { + return listOf(propertyDoesNotExists(propertyPath)) + } + if (configToValidate[propertyName] is String && baseline[propertyName] is List<*>) { + return listOf(propertyShouldBeAnArray(propertyPath)) + } + + val next = configToValidate[propertyName] as? Map + val nextBase = baseline[propertyName] as? Map + return when { + next == null && nextBase != null -> + listOf(nestedConfigurationExpected(propertyPath)) + baseline.contains(propertyName) && next != null && nextBase == null -> + listOf(unexpectedNestedConfiguration(propertyPath)) + next != null && nextBase != null -> + testKeys(next, nextBase, propertyPath) + else -> emptyList() + } + } + + companion object { + + internal fun propertyDoesNotExists(prop: String): Notification = + SimpleNotification("Property '$prop' is misspelled or does not exist.") + + internal fun nestedConfigurationExpected(prop: String): Notification = + SimpleNotification("Nested config expected for '$prop'.") + + internal fun unexpectedNestedConfiguration(prop: String): Notification = + SimpleNotification("Unexpected nested config for '$prop'.") + + internal fun propertyShouldBeAnArray(prop: String): Notification = + SimpleNotification( + "Property '$prop' should be a YAML array instead of a comma-separated String.", + Notification.Level.Warning, + ) + } +} diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidator.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidator.kt new file mode 100644 index 00000000000..fe22e8ab2ff --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidator.kt @@ -0,0 +1,81 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.api.RuleSetProvider +import io.gitlab.arturbosch.detekt.api.internal.DefaultRuleSetProvider +import io.gitlab.arturbosch.detekt.api.internal.SimpleNotification +import io.gitlab.arturbosch.detekt.core.config.YamlConfig +import java.util.ServiceLoader + +internal class MissingRulesConfigValidator( + private val baseline: YamlConfig, + private val excludePatterns: Set, +) : AbstractYamlConfigValidator() { + + override fun validate( + configToValidate: YamlConfig, + settings: ValidationSettings + ): Collection { + if (!settings.checkExhaustiveness) { + return emptyList() + } + return defaultRuleSetNames.flatMap { ruleSet -> validateRuleSet(ruleSet, configToValidate) } + } + + private fun validateRuleSet( + ruleSet: String, + configToValidate: YamlConfig, + ): List { + val ruleSetConfigToValidate = configToValidate.getSubMapOrNull(ruleSet) + val ruleSetConfigFromBaseline = baseline.getSubMapOrNull(ruleSet) + return when { + ruleSetConfigFromBaseline == null -> emptyList() + ruleSetConfigToValidate == null -> listOf(ruleSetMissing(ruleSet)) + else -> checkForMissingRules(ruleSet, ruleSetConfigToValidate, ruleSetConfigFromBaseline) + } + } + + private fun checkForMissingRules( + ruleSetName: String, + ruleSetConfigToValidate: Map, + ruleSetConfigFromBaseline: Map, + ): List { + if (ruleSetConfigToValidate[Config.ACTIVE_KEY] == false) { + return emptyList() + } + + return ruleSetConfigFromBaseline.keys + .filter { ruleNameCandidate -> excludePatterns.none { it.matches("$ruleSetName>$ruleNameCandidate") } } + .filter { ruleName -> !ruleSetConfigToValidate.containsKey(ruleName) } + .map { ruleName -> ruleMissing(ruleName, ruleSetName) } + } + + private fun ruleMissing(ruleName: String, ruleSetName: String): Notification = + SimpleNotification( + "Rule '$ruleName' from the '$ruleSetName' rule set is missing in the configuration.", + Notification.Level.Warning, + ) + + private fun ruleSetMissing(ruleSetName: String): Notification = + SimpleNotification( + "Rule set '$ruleSetName' is missing in the configuration.", + Notification.Level.Warning, + ) + + @Suppress("UNCHECKED_CAST") + private fun YamlConfig.getSubMapOrNull(propertyName: String) = properties[propertyName] as? Map + + companion object { + + private val defaultRuleSetNames: List by lazy(Companion::loadDefaultRuleSets) + private fun loadDefaultRuleSets(): List { + return ServiceLoader.load( + RuleSetProvider::class.java, + MissingRulesConfigValidator::class.java.classLoader + ) + .filterIsInstance() + .map { it.ruleSetId } + } + } +} diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidatableConfiguration.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidatableConfiguration.kt similarity index 79% rename from detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidatableConfiguration.kt rename to detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidatableConfiguration.kt index 78a97e9dea8..8b69fbfe475 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidatableConfiguration.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidatableConfiguration.kt @@ -1,4 +1,4 @@ -package io.gitlab.arturbosch.detekt.core.config +package io.gitlab.arturbosch.detekt.core.config.validation import io.gitlab.arturbosch.detekt.api.Config import io.gitlab.arturbosch.detekt.api.Notification diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidationSettings.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidationSettings.kt new file mode 100644 index 00000000000..a1ddf9d4a42 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidationSettings.kt @@ -0,0 +1,5 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +internal data class ValidationSettings( + val checkExhaustiveness: Boolean = false, +) diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/tooling/Lifecycle.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/tooling/Lifecycle.kt index c2070413a3c..5e04be5baac 100644 --- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/tooling/Lifecycle.kt +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/tooling/Lifecycle.kt @@ -11,7 +11,7 @@ import io.gitlab.arturbosch.detekt.core.Analyzer import io.gitlab.arturbosch.detekt.core.DetektResult import io.gitlab.arturbosch.detekt.core.FileProcessorLocator import io.gitlab.arturbosch.detekt.core.ProcessingSettings -import io.gitlab.arturbosch.detekt.core.config.checkConfiguration +import io.gitlab.arturbosch.detekt.core.config.validation.checkConfiguration import io.gitlab.arturbosch.detekt.core.extensions.handleReportingExtensions import io.gitlab.arturbosch.detekt.core.generateBindingContext import io.gitlab.arturbosch.detekt.core.reporting.OutputFacade diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 59de0ada0e4..219fcc7ad02 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -10,6 +10,7 @@ build: config: validation: true warningsAsErrors: false + checkExhaustiveness: false # when writing own rules with new properties, exclude the property path e.g.: 'my_rule_set,.*>.*>[my_property]' excludes: '' diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/DefaultConfigValidationSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/DefaultConfigValidationSpec.kt deleted file mode 100644 index c18e954802f..00000000000 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/DefaultConfigValidationSpec.kt +++ /dev/null @@ -1,20 +0,0 @@ -package io.gitlab.arturbosch.detekt.core.config - -import io.gitlab.arturbosch.detekt.test.yamlConfig -import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.Test - -class DefaultConfigValidationSpec { - - private val baseline = yamlConfig("default-detekt-config.yml") - - @Test - fun `is valid comparing itself`() { - assertThat(validateConfig(baseline, baseline)).isEmpty() - } - - @Test - fun `does not flag common known config sub sections`() { - assertThat(validateConfig(yamlConfig("common_known_sections.yml"), baseline)).isEmpty() - } -} diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidatorSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidatorSpec.kt new file mode 100644 index 00000000000..641293a6c69 --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidatorSpec.kt @@ -0,0 +1,49 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.core.config.YamlConfig +import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +internal class AbstractYamlConfigValidatorSpec { + + @Test + fun `use default validation settings`() { + val validator = SettingsCapturingValidatorAbstract() + val config = yamlConfigFromContent("") + + validator.validate(config) + + assertThat(validator.validationSettings.checkExhaustiveness).isFalse() + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `extract checkExhaustiveness settings from config`(configValue: Boolean) { + val config = yamlConfigFromContent( + """ + config: + checkExhaustiveness: $configValue + """.trimIndent() + ) + val validator = SettingsCapturingValidatorAbstract() + + validator.validate(config) + + assertThat(validator.validationSettings.checkExhaustiveness).isEqualTo(configValue) + } + + private class SettingsCapturingValidatorAbstract : AbstractYamlConfigValidator() { + lateinit var validationSettings: ValidationSettings + override fun validate( + configToValidate: YamlConfig, + settings: ValidationSettings + ): Collection { + validationSettings = settings + return emptyList() + } + } +} diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/CheckConfigurationSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/CheckConfigurationSpec.kt similarity index 98% rename from detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/CheckConfigurationSpec.kt rename to detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/CheckConfigurationSpec.kt index 9a0d468063c..95e50e18402 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/CheckConfigurationSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/CheckConfigurationSpec.kt @@ -1,4 +1,4 @@ -package io.gitlab.arturbosch.detekt.core.config +package io.gitlab.arturbosch.detekt.core.config.validation import io.github.detekt.test.utils.createTempDirectoryForTest import io.github.detekt.tooling.api.InvalidConfig @@ -15,7 +15,7 @@ import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent import org.assertj.core.api.Assertions.assertThatCode import org.junit.jupiter.api.Test -class SupportConfigValidationSpec { +class CheckConfigurationSpec { private val testDir = createTempDirectoryForTest("detekt-sample") private val spec = createNullLoggingSpec {} diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DefaultConfigValidationSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DefaultConfigValidationSpec.kt new file mode 100644 index 00000000000..2a9b3b5f775 --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DefaultConfigValidationSpec.kt @@ -0,0 +1,24 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.internal.CommaSeparatedPattern +import io.gitlab.arturbosch.detekt.test.yamlConfig +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class DefaultConfigValidationSpec { + + private val baseline = yamlConfig("default-detekt-config.yml") + private val defaultExcludePatterns = CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() + + @Test + fun `is valid comparing itself`() { + val actual = validateConfig(baseline, baseline, defaultExcludePatterns) + assertThat(actual).isEmpty() + } + + @Test + fun `does not flag common known config sub sections`() { + val actual = validateConfig(yamlConfig("common_known_sections.yml"), baseline, defaultExcludePatterns) + assertThat(actual).isEmpty() + } +} diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidatorSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidatorSpec.kt new file mode 100644 index 00000000000..6dd5ff709ff --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidatorSpec.kt @@ -0,0 +1,36 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.core.config.YamlConfig +import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +internal class DeprecatedPropertiesConfigValidatorSpec { + private val deprecatedProperties = + mapOf("naming>FunctionParameterNaming>ignoreOverriddenFunctions" to "Use `ignoreOverridden` instead") + + private val subject = DeprecatedPropertiesConfigValidator(deprecatedProperties) + + @Test + fun `reports a deprecated property as a warning`() { + val settings = ValidationSettings() + val config = yamlConfigFromContent( + """ + naming: + FunctionParameterNaming: + ignoreOverriddenFunctions: '' + """.trimIndent() + ) as YamlConfig + + val result = subject.validate(config, settings) + + assertThat(result).anySatisfy { notification -> + assertThat(notification.level) + .isEqualTo(Notification.Level.Warning) + assertThat(notification.message) + .contains("naming>FunctionParameterNaming>ignoreOverriddenFunctions") + .contains("Use `ignoreOverridden` instead") + } + } +} diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidateConfigSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt similarity index 52% rename from detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidateConfigSpec.kt rename to detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt index 6d88958c699..93617c8f048 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidateConfigSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt @@ -1,53 +1,48 @@ -package io.gitlab.arturbosch.detekt.core.config +package io.gitlab.arturbosch.detekt.core.config.validation import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Notification import io.gitlab.arturbosch.detekt.api.internal.CommaSeparatedPattern -import io.gitlab.arturbosch.detekt.test.TestConfig +import io.gitlab.arturbosch.detekt.core.config.CompositeConfig +import io.gitlab.arturbosch.detekt.core.config.YamlConfig +import io.gitlab.arturbosch.detekt.core.config.validation.InvalidPropertiesConfigValidator.Companion.nestedConfigurationExpected +import io.gitlab.arturbosch.detekt.core.config.validation.InvalidPropertiesConfigValidator.Companion.propertyDoesNotExists +import io.gitlab.arturbosch.detekt.core.config.validation.InvalidPropertiesConfigValidator.Companion.unexpectedNestedConfiguration import io.gitlab.arturbosch.detekt.test.yamlConfig import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.assertThatIllegalArgumentException -import org.assertj.core.api.Assertions.assertThatIllegalStateException import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test -import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.ValueSource -class ValidateConfigSpec { - - private val baseline = yamlConfig("config_validation/baseline.yml") +internal class InvalidPropertiesConfigValidatorSpec { + private val deprecatedProperties = setOf("complexity>LongParameterList>threshold") + private val baseline = yamlConfig("config_validation/baseline.yml") as YamlConfig + private val defaultExcludePatterns = CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() + private val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, defaultExcludePatterns) @Test fun `passes for same config test`() { - val result = validateConfig(baseline, baseline) + val result = subject.validate(baseline) assertThat(result).isEmpty() } @Test fun `passes for properties which may appear on rules and rule sets but may be not present in default config`() { - val result = validateConfig( - yamlConfig("config_validation/default-excluded-properties.yml"), - baseline - ) + val result = subject.validate(yamlConfig("config_validation/default-excluded-properties.yml")) assertThat(result).isEmpty() } @Test fun `reports different rule set name`() { - val result = validateConfig( - yamlConfig("config_validation/other-ruleset-name.yml"), - baseline - ) + val result = subject.validate(yamlConfig("config_validation/other-ruleset-name.yml")) assertThat(result).contains(propertyDoesNotExists("code-smell")) } @Test fun `reports different nested property names`() { - val result = validateConfig( - yamlConfig("config_validation/other-nested-property-names.yml"), - baseline - ) + val result = subject.validate(yamlConfig("config_validation/other-nested-property-names.yml")) + assertThat(result).contains( propertyDoesNotExists("complexity>LongLongMethod"), propertyDoesNotExists("complexity>LongParameterList>enabled"), @@ -59,10 +54,7 @@ class ValidateConfigSpec { @Test fun `reports nested configuration expected`() { - val result = validateConfig( - yamlConfig("config_validation/no-nested-config.yml"), - baseline - ) + val result = subject.validate(yamlConfig("config_validation/no-nested-config.yml")) assertThat(result).contains( nestedConfigurationExpected("complexity"), nestedConfigurationExpected("style>WildcardImport") @@ -71,8 +63,14 @@ class ValidateConfigSpec { @Test fun `reports unexpected nested configs`() { - // note that the baseline config is now the first argument - val result = validateConfig(baseline, yamlConfig("config_validation/no-value.yml")) + // note that the baseline config is now the config to validate + val subject = + InvalidPropertiesConfigValidator( + yamlConfig("config_validation/no-value.yml") as YamlConfig, + deprecatedProperties, + defaultExcludePatterns + ) + val result = subject.validate(baseline) assertThat(result).contains( unexpectedNestedConfiguration("style"), unexpectedNestedConfiguration("comments") @@ -80,27 +78,29 @@ class ValidateConfigSpec { } @Test - fun `returns an error for an invalid config type`() { - val invalidConfig = TestConfig() - assertThatIllegalStateException().isThrownBy { - validateConfig(invalidConfig, baseline) - }.withMessageStartingWith("Unsupported config type for validation") + fun `does not report missing property when it is deprecated`() { + val result = subject.validate(yamlConfig("config_validation/deprecated-properties.yml")) + assertThat(result).isEmpty() } @Test - fun `returns an error for an invalid baseline`() { - val invalidBaseline = TestConfig() - assertThatIllegalArgumentException().isThrownBy { - validateConfig(Config.empty, invalidBaseline) - }.withMessageStartingWith("Only supported baseline config is the YamlConfig.") - } + fun `reports a string that should be an array as a warning`() { + val config = yamlConfigFromContent( + """ + style: + MagicNumber: + ignoreNumbers: '-1,0,1,2' + """.trimIndent() + ) - @Test - fun `returns an error for an empty baseline`() { - val invalidBaseline = Config.empty - assertThatIllegalArgumentException().isThrownBy { - validateConfig(Config.empty, invalidBaseline) - }.withMessageStartingWith("Cannot validate configuration based on an empty baseline config.") + val result = subject.validate(config) + + assertThat(result).anySatisfy { notification -> + assertThat(notification.message) + .contains("style>MagicNumber>ignoreNumbers") + .contains("should be a YAML array instead of a comma-separated String") + assertThat(notification.level).isEqualTo(Notification.Level.Warning) + } } @Nested @@ -108,25 +108,22 @@ class ValidateConfigSpec { @Test fun `passes for same left, right and baseline config`() { - val result = validateConfig(CompositeConfig(baseline, baseline), baseline) + val result = CompositeConfig(baseline, baseline).validate(baseline, emptySet()) assertThat(result).isEmpty() } @Test fun `passes for empty configs`() { - val result = validateConfig(CompositeConfig(Config.empty, Config.empty), baseline) + val result = CompositeConfig(Config.empty, Config.empty).validate(baseline, emptySet()) assertThat(result).isEmpty() } @Test fun `finds accumulated errors`() { - val result = validateConfig( - CompositeConfig( - yamlConfig("config_validation/other-nested-property-names.yml"), - yamlConfig("config_validation/no-nested-config.yml") - ), - baseline - ) + val result = CompositeConfig( + yamlConfig("config_validation/other-nested-property-names.yml"), + yamlConfig("config_validation/no-nested-config.yml") + ).validate(baseline, emptySet()) assertThat(result).contains( nestedConfigurationExpected("complexity"), @@ -147,20 +144,19 @@ class ValidateConfigSpec { @Test fun `does not report any complexity properties`() { - val result = validateConfig( - yamlConfig("config_validation/other-nested-property-names.yml"), - baseline, - patterns("complexity") + val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, patterns("complexity")) + + val result = subject.validate( + yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig, ) assertThat(result).isEmpty() } @Test fun `does not report 'complexity_LargeClass_howMany'`() { - val result = validateConfig( - yamlConfig("config_validation/other-nested-property-names.yml"), - baseline, - patterns(".*>.*>howMany") + val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, patterns(".*>.*>howMany")) + val result = subject.validate( + yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig ) assertThat(result).contains( @@ -178,10 +174,9 @@ class ValidateConfigSpec { @Test @DisplayName("does not report .*>InnerMap") fun `does not report innerMap`() { - val result = validateConfig( - yamlConfig("config_validation/other-nested-property-names.yml"), - baseline, - patterns(".*>InnerMap") + val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, patterns(".*>InnerMap")) + val result = subject.validate( + yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig ) assertThat(result).contains( @@ -196,55 +191,4 @@ class ValidateConfigSpec { ) } } - - @Nested - inner class `deprecated configuration option` { - - @ParameterizedTest - @ValueSource(booleans = [true, false]) - fun `reports a deprecated property as a warning`(warningsAsErrors: Boolean) { - val config = yamlConfigFromContent( - """ - config: - warningsAsErrors: $warningsAsErrors - naming: - FunctionParameterNaming: - ignoreOverriddenFunctions: '' - """.trimIndent() - ) - - val result = validateConfig(config, config) - - assertThat(result).contains( - propertyIsDeprecated( - "naming>FunctionParameterNaming>ignoreOverriddenFunctions", - "Use `ignoreOverridden` instead", - reportAsError = warningsAsErrors - ) - ) - } - } - - @ParameterizedTest - @ValueSource(booleans = [true, false]) - fun `reports a string that should be an array as a warning`(warningsAsErrors: Boolean) { - val config = yamlConfigFromContent( - """ - config: - warningsAsErrors: $warningsAsErrors - style: - MagicNumber: - ignoreNumbers: '-1,0,1,2' - """.trimIndent() - ) - - val result = validateConfig(config, baseline) - - assertThat(result).contains( - propertyShouldBeAnArray( - "style>MagicNumber>ignoreNumbers", - reportAsError = warningsAsErrors - ) - ) - } } diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidatorSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidatorSpec.kt new file mode 100644 index 00000000000..9b862d83f92 --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidatorSpec.kt @@ -0,0 +1,48 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.internal.CommaSeparatedPattern +import io.gitlab.arturbosch.detekt.core.config.YamlConfig +import io.gitlab.arturbosch.detekt.test.yamlConfig +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +internal class MissingRulesConfigValidatorSpec { + private val baseline = yamlConfig("config_validation/baseline.yml") as YamlConfig + private val subject = MissingRulesConfigValidator( + baseline, + CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() + ) + + @Test + fun `do not check for exhaustiveness if disabled by config`() { + val config = yamlConfig("config_validation/exhaustiveness-check-disabled.yml") + + val result = subject.validate(config) + + assertThat(result).isEmpty() + } + + @Test + fun `do not report violations if all rules are mentioned or rule set is disabled`() { + val config = yamlConfig("config_validation/exhaustiveness-check-successful.yml") + + val result = subject.validate(config) + + assertThat(result).isEmpty() + } + + @Test + fun `report violations of missing rules and rule sets`() { + val config = yamlConfig("config_validation/exhaustiveness-check-with-error.yml") + + val result = subject.validate(config) + + assertThat(result) + .extracting("message") + .containsExactlyInAnyOrder( + "Rule set 'comments' is missing in the configuration.", + "Rule 'LongMethod' from the 'complexity' rule set is missing in the configuration.", + "Rule 'WildcardImport' from the 'style' rule set is missing in the configuration.", + ) + } +} diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidateConfigSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidateConfigSpec.kt new file mode 100644 index 00000000000..91b4cd7c64c --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidateConfigSpec.kt @@ -0,0 +1,63 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Notification +import io.gitlab.arturbosch.detekt.test.TestConfig +import io.gitlab.arturbosch.detekt.test.yamlConfig +import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatIllegalArgumentException +import org.assertj.core.api.Assertions.assertThatIllegalStateException +import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +internal class ValidateConfigSpec { + private val baseline = yamlConfig("config_validation/baseline.yml") + + @Test + fun `returns an error for an invalid config type`() { + val invalidConfig = TestConfig() + assertThatIllegalStateException().isThrownBy { + validateConfig(invalidConfig, baseline, emptySet()) + }.withMessageStartingWith("Unsupported config type for validation") + } + + @Test + fun `returns an error for an invalid baseline`() { + val invalidBaseline = TestConfig() + assertThatIllegalArgumentException().isThrownBy { + validateConfig(Config.empty, invalidBaseline, emptySet()) + }.withMessageStartingWith("Only supported baseline config is the YamlConfig.") + } + + @Test + fun `returns an error for an empty baseline`() { + val invalidBaseline = Config.empty + assertThatIllegalArgumentException().isThrownBy { + validateConfig(Config.empty, invalidBaseline, emptySet()) + }.withMessageStartingWith("Cannot validate configuration based on an empty baseline config.") + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `converts warnings to error if config is set up this way`(warningsAsErrors: Boolean) { + val config = yamlConfigFromContent( + """ + config: + warningsAsErrors: $warningsAsErrors + style: + MagicNumber: + ignoreNumbers: '-1,0,1,2' + """.trimIndent() + ) + + val result = validateConfig(config, baseline, emptySet()) + + val expectedLevel = if (warningsAsErrors) Notification.Level.Error else Notification.Level.Warning + assertThat(result).anySatisfy { notification -> + assertThat(notification.message).contains("style>MagicNumber") + assertThat(notification.level).isEqualTo(expectedLevel) + } + } +} diff --git a/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConfigValidator b/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConfigValidator index 67f0618954f..4b8243351cd 100644 --- a/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConfigValidator +++ b/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConfigValidator @@ -1 +1 @@ -io.gitlab.arturbosch.detekt.core.config.SampleConfigValidator +io.gitlab.arturbosch.detekt.core.config.validation.SampleConfigValidator diff --git a/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider b/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider index 02f24d5b65e..5767e4ad01f 100644 --- a/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider +++ b/detekt-core/src/test/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider @@ -1,3 +1,3 @@ io.gitlab.arturbosch.detekt.core.TestProvider io.gitlab.arturbosch.detekt.core.TestProvider2 -io.gitlab.arturbosch.detekt.core.config.SampleRuleProvider +io.gitlab.arturbosch.detekt.core.config.validation.SampleRuleProvider diff --git a/detekt-core/src/test/resources/config_validation/baseline.yml b/detekt-core/src/test/resources/config_validation/baseline.yml index dc814e5a41a..47e16f0e037 100644 --- a/detekt-core/src/test/resources/config_validation/baseline.yml +++ b/detekt-core/src/test/resources/config_validation/baseline.yml @@ -1,5 +1,15 @@ +build: + maxIssues: 1 config: warningsAsErrors: true + checkExhaustiveness: false +processors: + active: true +console-reports: + active: true +output-reports: + active: true + complexity: LongMethod: active: true @@ -17,6 +27,7 @@ complexity: active: true style: + active: true WildcardImport: active: true NoElseInWhenExpression: @@ -26,4 +37,5 @@ style: ignoreNumbers: ['-1', '0', '1', '2'] comments: - active: false + CommentOverPrivateProperty: + active: false diff --git a/detekt-core/src/test/resources/config_validation/deprecated-properties.yml b/detekt-core/src/test/resources/config_validation/deprecated-properties.yml new file mode 100644 index 00000000000..acd2cfcd348 --- /dev/null +++ b/detekt-core/src/test/resources/config_validation/deprecated-properties.yml @@ -0,0 +1,3 @@ +complexity: + LongParameterList: + threshold: 5 diff --git a/detekt-core/src/test/resources/config_validation/exhaustiveness-check-disabled.yml b/detekt-core/src/test/resources/config_validation/exhaustiveness-check-disabled.yml new file mode 100644 index 00000000000..cd71203b0a0 --- /dev/null +++ b/detekt-core/src/test/resources/config_validation/exhaustiveness-check-disabled.yml @@ -0,0 +1,6 @@ +config: + checkExhaustiveness: false + +complexity: +style: +comments: diff --git a/detekt-core/src/test/resources/config_validation/exhaustiveness-check-successful.yml b/detekt-core/src/test/resources/config_validation/exhaustiveness-check-successful.yml new file mode 100644 index 00000000000..bad46ef78fa --- /dev/null +++ b/detekt-core/src/test/resources/config_validation/exhaustiveness-check-successful.yml @@ -0,0 +1,17 @@ +config: + checkExhaustiveness: true + +complexity: + active: false + +style: + WildcardImport: + active: true + NoElseInWhenExpression: + active: true + MagicNumber: + active: true + +comments: + CommentOverPrivateProperty: + active: false diff --git a/detekt-core/src/test/resources/config_validation/exhaustiveness-check-with-error.yml b/detekt-core/src/test/resources/config_validation/exhaustiveness-check-with-error.yml new file mode 100644 index 00000000000..2088d0ee472 --- /dev/null +++ b/detekt-core/src/test/resources/config_validation/exhaustiveness-check-with-error.yml @@ -0,0 +1,21 @@ +config: + checkExhaustiveness: true + +complexity: + LongParameterList: + active: false + LargeClass: + active: false + InnerMap: + Inner1: + active: true + Inner2: + active: true + +style: + NoElseInWhenExpression: + active: true + MagicNumber: + active: true + +comments: diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/ConfigPrinter.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/ConfigPrinter.kt index 1029d93145d..10bf2f683ac 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/ConfigPrinter.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/ConfigPrinter.kt @@ -36,11 +36,12 @@ object ConfigPrinter : DocumentationPrinter> { """.trimIndent() private fun defaultConfigConfiguration(): String = """ - config: - validation: true - warningsAsErrors: false - # when writing own rules with new properties, exclude the property path e.g.: 'my_rule_set,.*>.*>[my_property]' - excludes: '' + config: + validation: true + warningsAsErrors: false + checkExhaustiveness: false + # when writing own rules with new properties, exclude the property path e.g.: 'my_rule_set,.*>.*>[my_property]' + excludes: '' """.trimIndent() private fun defaultProcessorsConfiguration(): String = """