From 9297528c7860361ccac2e2530cc522a7c61d3cba Mon Sep 17 00:00:00 2001 From: Markus Schwarz Date: Mon, 18 Jul 2022 01:12:28 +0200 Subject: [PATCH 1/6] Add exhaustiveness check for config validation --- .../detekt/core/config/AllRulesConfig.kt | 2 + .../detekt/core/config/CompositeConfig.kt | 2 + .../detekt/core/config/ConfigValidators.kt | 39 ---- .../core/config/DisabledAutoCorrectConfig.kt | 2 + .../detekt/core/config/ValidateConfig.kt | 153 --------------- .../detekt/core/config/YamlConfig.kt | 2 + .../validation/AbstractYamlConfigValidator.kt | 32 ++++ .../config/validation/ConfigValidation.kt | 99 ++++++++++ .../DefaultPropertiesConfigValidator.kt | 4 +- .../DeprecatedPropertiesConfigValidator.kt | 43 +++++ .../core/config/validation/Deprecations.kt | 19 ++ .../InvalidPropertiesConfigValidator.kt | 94 +++++++++ .../validation/MissingRulesConfigValidator.kt | 94 +++++++++ .../ValidatableConfiguration.kt | 2 +- .../config/validation/ValidationSettings.kt | 7 + .../detekt/core/tooling/Lifecycle.kt | 2 +- .../main/resources/default-detekt-config.yml | 1 + .../config/DefaultConfigValidationSpec.kt | 20 -- .../AbstractYamlConfigValidatorSpec.kt | 75 ++++++++ .../CheckConfigurationSpec.kt | 4 +- .../validation/DefaultConfigValidationSpec.kt | 24 +++ ...DeprecatedPropertiesConfigValidatorSpec.kt | 33 ++++ .../InvalidPropertiesConfigValidatorSpec.kt} | 179 +++++++----------- .../MissingRulesConfigValidatorSpec.kt | 48 +++++ .../config/validation/ValidateConfigSpec.kt | 36 ++++ ...tlab.arturbosch.detekt.api.ConfigValidator | 2 +- ...tlab.arturbosch.detekt.api.RuleSetProvider | 2 +- .../resources/config_validation/baseline.yml | 14 +- .../deprecated-properties.yml | 3 + .../exhaustiveness-check-disabled.yml | 6 + .../exhaustiveness-check-successful.yml | 17 ++ .../exhaustiveness-check-with-error.yml | 21 ++ .../printer/defaultconfig/ConfigPrinter.kt | 1 + 33 files changed, 746 insertions(+), 336 deletions(-) delete mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ConfigValidators.kt delete mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/ValidateConfig.kt create mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidator.kt create mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ConfigValidation.kt rename detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/{ => validation}/DefaultPropertiesConfigValidator.kt (91%) create mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidator.kt create mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/Deprecations.kt create mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidator.kt create mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidator.kt rename detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/{ => validation}/ValidatableConfiguration.kt (79%) create mode 100644 detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidationSettings.kt delete mode 100644 detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/DefaultConfigValidationSpec.kt create mode 100644 detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidatorSpec.kt rename detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/{ => validation}/CheckConfigurationSpec.kt (98%) create mode 100644 detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DefaultConfigValidationSpec.kt create mode 100644 detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidatorSpec.kt rename detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/{ValidateConfigSpec.kt => validation/InvalidPropertiesConfigValidatorSpec.kt} (58%) create mode 100644 detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidatorSpec.kt create mode 100644 detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidateConfigSpec.kt create mode 100644 detekt-core/src/test/resources/config_validation/deprecated-properties.yml create mode 100644 detekt-core/src/test/resources/config_validation/exhaustiveness-check-disabled.yml create mode 100644 detekt-core/src/test/resources/config_validation/exhaustiveness-check-successful.yml create mode 100644 detekt-core/src/test/resources/config_validation/exhaustiveness-check-with-error.yml 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..85f63606c90 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidator.kt @@ -0,0 +1,32 @@ +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( + private val excludePatterns: Set = emptySet(), +) : 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( + warningsAsErrors = config.subConfig("config").valueOrDefault("warningsAsErrors", false), + checkExhaustiveness = config.subConfig("config").valueOrDefault("checkExhaustiveness", false), + excludePatterns = excludePatterns, + ) + + 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..9a2d28f2d22 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ConfigValidation.kt @@ -0,0 +1,99 @@ +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.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 validators: List = listOf( + InvalidPropertiesConfigValidator(baseline, excludePatterns), + DeprecatedPropertiesConfigValidator(), + MissingRulesConfigValidator(baseline, excludePatterns) + ) + + return validators.flatMap { it.validate(configToValidate) } +} + +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/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..3a0563d263e --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidator.kt @@ -0,0 +1,43 @@ +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 : 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, settings.warningsAsErrors) } + } + + @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, + reportAsError: Boolean, + ): Notification { + val prop = propertyPath.joinToString(">") + return SimpleNotification( + "Property '$prop' is deprecated. $deprecationDescription.", + if (reportAsError) Notification.Level.Error else 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..dbd184d4dcc --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/Deprecations.kt @@ -0,0 +1,19 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.github.detekt.utils.openSafeStream +import java.util.Properties + +internal val deprecatedProperties: Map by lazy(::loadDeprecations) + +private 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..1e6e7a55537 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidator.kt @@ -0,0 +1,94 @@ +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, + excludePatterns: Set, +) : AbstractYamlConfigValidator(excludePatterns) { + + override fun validate( + configToValidate: YamlConfig, + settings: ValidationSettings + ): Collection { + return testKeys(configToValidate.properties, baseline.properties, null, settings) + } + + private fun testKeys( + configToValidate: Map, + baseline: Map, + parentPath: String?, + settings: ValidationSettings + ): List { + val notifications = mutableListOf() + for (prop in configToValidate.keys) { + val propertyPath = "${if (parentPath == null) "" else "$parentPath>"}$prop" + val isExcluded = settings.excludePatterns.any { it.matches(propertyPath) } + val isDeprecated = deprecatedProperties.containsKey(propertyPath) + if (isExcluded || isDeprecated) { + continue + } + notifications.addAll( + checkProp( + propertyName = prop, + propertyPath = propertyPath, + configToValidate = configToValidate, + baseline = baseline, + settings = settings + ) + ) + } + return notifications + } + + @Suppress("UNCHECKED_CAST") + private fun checkProp( + propertyName: String, + propertyPath: String, + configToValidate: Map, + baseline: Map, + settings: ValidationSettings + ): List { + if (!baseline.contains(propertyName)) { + return listOf(propertyDoesNotExists(propertyPath)) + } + if (configToValidate[propertyName] is String && baseline[propertyName] is List<*>) { + return listOf(propertyShouldBeAnArray(propertyPath, settings.warningsAsErrors)) + } + + 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, settings) + 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, + 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/validation/MissingRulesConfigValidator.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidator.kt new file mode 100644 index 00000000000..dbd807a2705 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/MissingRulesConfigValidator.kt @@ -0,0 +1,94 @@ +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, + excludePatterns: Set, +) : AbstractYamlConfigValidator(excludePatterns) { + + override fun validate( + configToValidate: YamlConfig, + settings: ValidationSettings + ): Collection { + if (!settings.checkExhaustiveness) { + return emptyList() + } + return defaultRuleSetNames.flatMap { ruleSet -> validateRuleSet(ruleSet, configToValidate, settings) } + } + + @Suppress("UNCHECKED_CAST") + private fun validateRuleSet( + ruleSet: String, + configToValidate: YamlConfig, + settings: ValidationSettings + ): List { + val configPropertiesToValidate = configToValidate.properties + val ruleSetConfigToValidate = configPropertiesToValidate[ruleSet] as? Map + val ruleSetConfigFromBaseline = baseline.properties[ruleSet] as? Map + return when { + ruleSetConfigFromBaseline == null -> emptyList() + ruleSetConfigToValidate == null -> listOf(ruleSetMissing(ruleSet, settings)) + else -> checkForMissingRules(ruleSet, ruleSetConfigToValidate, ruleSetConfigFromBaseline, settings) + } + } + + private fun checkForMissingRules( + ruleSetName: String, + ruleSetConfigToValidate: Map, + ruleSetConfigFromBaseline: Map?, + settings: ValidationSettings + ): List { + if (ruleSetConfigFromBaseline == null) { + return emptyList() + } + if (ruleSetConfigToValidate[Config.ACTIVE_KEY] == false) { + return emptyList() + } + + return ruleSetConfigFromBaseline.keys + .filter { ruleNameCandidate -> + settings.excludePatterns.none { it.matches("$ruleSetName>$ruleNameCandidate") } + } + .filter { ruleName -> !ruleSetConfigToValidate.containsKey(ruleName) } + .map { ruleName -> ruleMissing(ruleName, ruleSetName, settings) } + } + + private fun ruleMissing( + ruleName: String, + ruleSetName: String, + settings: ValidationSettings, + ): Notification = + SimpleNotification( + "Rule '$ruleName' from the '$ruleSetName' rule set is missing in the configuration.", + if (settings.warningsAsErrors) Notification.Level.Error else Notification.Level.Warning, + ) + + private fun ruleSetMissing( + ruleSetName: String, + settings: ValidationSettings, + ): Notification = + SimpleNotification( + "Rule set '$ruleSetName' is missing in the configuration.", + if (settings.warningsAsErrors) Notification.Level.Error else Notification.Level.Warning, + ) + + 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..b99d78b6721 --- /dev/null +++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidationSettings.kt @@ -0,0 +1,7 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +internal data class ValidationSettings( + val warningsAsErrors: Boolean = false, + val checkExhaustiveness: Boolean = false, + val excludePatterns: Set = emptySet(), +) 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 462058ca590..4fc47f0abb4 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..c0cb48cd700 --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/AbstractYamlConfigValidatorSpec.kt @@ -0,0 +1,75 @@ +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 and given exclude patterns`() { + val excludePatterns = setOf(".*".toRegex()) + val validator = SettingsCapturingValidatorAbstract(excludePatterns) + val config = yamlConfigFromContent("") + + validator.validate(config) + + val expectedDefaultSettings = ValidationSettings( + warningsAsErrors = false, + checkExhaustiveness = false, + excludePatterns + ) + assertThat(validator.validationSettings).isEqualTo(expectedDefaultSettings) + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `extract warningsAsErrors settings from config`(configValue: Boolean) { + val excludePatterns = emptySet() + val config = yamlConfigFromContent( + """ + config: + warningsAsErrors: $configValue + """.trimIndent() + ) + val validator = SettingsCapturingValidatorAbstract(excludePatterns) + + validator.validate(config) + + assertThat(validator.validationSettings.warningsAsErrors).isEqualTo(configValue) + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `extract checkExhaustiveness settings from config`(configValue: Boolean) { + val excludePatterns = emptySet() + val config = yamlConfigFromContent( + """ + config: + checkExhaustiveness: $configValue + """.trimIndent() + ) + val validator = SettingsCapturingValidatorAbstract(excludePatterns) + + validator.validate(config) + + assertThat(validator.validationSettings.checkExhaustiveness).isEqualTo(configValue) + } + + private class SettingsCapturingValidatorAbstract( + excludePatterns: Set, + ) : AbstractYamlConfigValidator(excludePatterns) { + 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..d8625d45064 --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidatorSpec.kt @@ -0,0 +1,33 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +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.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +internal class DeprecatedPropertiesConfigValidatorSpec { + + private val subject = DeprecatedPropertiesConfigValidator() + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `reports a deprecated property as a warning or error`(warningsAsErrors: Boolean) { + val settings = ValidationSettings(warningsAsErrors = warningsAsErrors) + val config = yamlConfigFromContent( + """ + naming: + FunctionParameterNaming: + ignoreOverriddenFunctions: '' + """.trimIndent() + ) as YamlConfig + + val result = subject.validate(config, settings) + + assertThat(result).hasSize(1) + val notification = result.first() + assertThat(notification.isError).isEqualTo(warningsAsErrors) + assertThat(notification.message).contains("naming>FunctionParameterNaming>ignoreOverriddenFunctions") + assertThat(notification.message).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 58% 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 a67f6d93e70..21eaec017ff 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,49 @@ -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.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.propertyShouldBeAnArray +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 baseline = yamlConfig("config_validation/baseline.yml") as YamlConfig + val defaultExcludePatterns = CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() + private val subject = InvalidPropertiesConfigValidator(baseline, 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 +55,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 +64,13 @@ 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, + defaultExcludePatterns + ) + val result = subject.validate(baseline) assertThat(result).contains( unexpectedNestedConfiguration("style"), unexpectedNestedConfiguration("comments") @@ -80,27 +78,32 @@ 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.") - } + @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() + ) - @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).contains( + propertyShouldBeAnArray( + "style>MagicNumber>ignoreNumbers", + reportAsError = warningsAsErrors + ) + ) } @Nested @@ -108,24 +111,23 @@ 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( @@ -147,20 +149,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, 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, patterns(".*>.*>howMany")) + val result = subject.validate( + yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig ) assertThat(result).contains( @@ -178,10 +179,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, patterns(".*>InnerMap")) + val result = subject.validate( + yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig ) assertThat(result).contains( @@ -196,55 +196,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..26547d838c5 --- /dev/null +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/ValidateConfigSpec.kt @@ -0,0 +1,36 @@ +package io.gitlab.arturbosch.detekt.core.config.validation + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.test.TestConfig +import io.gitlab.arturbosch.detekt.test.yamlConfig +import org.assertj.core.api.Assertions.assertThatIllegalArgumentException +import org.assertj.core.api.Assertions.assertThatIllegalStateException +import org.junit.jupiter.api.Test + +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.") + } +} 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 9131be47088..fdb0e155690 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 @@ -39,6 +39,7 @@ object ConfigPrinter : DocumentationPrinter> { 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() From 4e257f081b3345d8f16f6bf4ee35a51dc0b24922 Mon Sep 17 00:00:00 2001 From: Markus Schwarz Date: Tue, 19 Jul 2022 15:50:45 +0200 Subject: [PATCH 2/6] fix ArgumentListWrapping --- .../config/validation/InvalidPropertiesConfigValidatorSpec.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt index 21eaec017ff..0982699f0e0 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt @@ -126,9 +126,7 @@ internal class InvalidPropertiesConfigValidatorSpec { val result = CompositeConfig( yamlConfig("config_validation/other-nested-property-names.yml"), yamlConfig("config_validation/no-nested-config.yml") - ).validate( - baseline, emptySet() - ) + ).validate(baseline, emptySet()) assertThat(result).contains( nestedConfigurationExpected("complexity"), From ca49994ae8a9fbb4dd3a6bcc2bbacef03247669b Mon Sep 17 00:00:00 2001 From: Markus Schwarz Date: Thu, 21 Jul 2022 00:45:12 +0200 Subject: [PATCH 3/6] inject deprecated properties to validators --- .../core/config/validation/ConfigValidation.kt | 5 +++-- .../DeprecatedPropertiesConfigValidator.kt | 4 +++- .../detekt/core/config/validation/Deprecations.kt | 4 +--- .../validation/InvalidPropertiesConfigValidator.kt | 3 ++- .../DeprecatedPropertiesConfigValidatorSpec.kt | 4 +++- .../InvalidPropertiesConfigValidatorSpec.kt | 12 +++++++----- 6 files changed, 19 insertions(+), 13 deletions(-) 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 index 9a2d28f2d22..30c4702725f 100644 --- 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 @@ -82,9 +82,10 @@ private fun validateYamlConfig( baseline: YamlConfig, excludePatterns: Set ): List { + val deprecatedProperties = loadDeprecations() val validators: List = listOf( - InvalidPropertiesConfigValidator(baseline, excludePatterns), - DeprecatedPropertiesConfigValidator(), + InvalidPropertiesConfigValidator(baseline, deprecatedProperties.keys, excludePatterns), + DeprecatedPropertiesConfigValidator(deprecatedProperties), MissingRulesConfigValidator(baseline, excludePatterns) ) 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 index 3a0563d263e..e5a91aed952 100644 --- 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 @@ -4,7 +4,9 @@ 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 : AbstractYamlConfigValidator() { +internal class DeprecatedPropertiesConfigValidator( + private val deprecatedProperties: Map, +) : AbstractYamlConfigValidator() { override fun validate( configToValidate: YamlConfig, settings: ValidationSettings 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 index dbd184d4dcc..869b745f041 100644 --- 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 @@ -3,9 +3,7 @@ package io.gitlab.arturbosch.detekt.core.config.validation import io.github.detekt.utils.openSafeStream import java.util.Properties -internal val deprecatedProperties: Map by lazy(::loadDeprecations) - -private fun loadDeprecations(): Map { +internal fun loadDeprecations(): Map { return ValidationSettings::class.java.classLoader .getResource("deprecation.properties")!! .openSafeStream() 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 index 1e6e7a55537..b11363b3a9a 100644 --- 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 @@ -6,6 +6,7 @@ import io.gitlab.arturbosch.detekt.core.config.YamlConfig internal class InvalidPropertiesConfigValidator( private val baseline: YamlConfig, + private val deprecatedProperties: Set, excludePatterns: Set, ) : AbstractYamlConfigValidator(excludePatterns) { @@ -26,7 +27,7 @@ internal class InvalidPropertiesConfigValidator( for (prop in configToValidate.keys) { val propertyPath = "${if (parentPath == null) "" else "$parentPath>"}$prop" val isExcluded = settings.excludePatterns.any { it.matches(propertyPath) } - val isDeprecated = deprecatedProperties.containsKey(propertyPath) + val isDeprecated = deprecatedProperties.contains(propertyPath) if (isExcluded || isDeprecated) { continue } 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 index d8625d45064..d0a0141953c 100644 --- 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 @@ -7,8 +7,10 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource internal class DeprecatedPropertiesConfigValidatorSpec { + private val deprecatedProperties = + mapOf("naming>FunctionParameterNaming>ignoreOverriddenFunctions" to "Use `ignoreOverridden` instead") - private val subject = DeprecatedPropertiesConfigValidator() + private val subject = DeprecatedPropertiesConfigValidator(deprecatedProperties) @ParameterizedTest @ValueSource(booleans = [true, false]) diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt index 0982699f0e0..075ed2405f6 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt @@ -18,9 +18,10 @@ import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource internal class InvalidPropertiesConfigValidatorSpec { + private val deprecatedProperties = setOf("complexity>LongParameterList>threshold") private val baseline = yamlConfig("config_validation/baseline.yml") as YamlConfig - val defaultExcludePatterns = CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() - private val subject = InvalidPropertiesConfigValidator(baseline, defaultExcludePatterns) + private val defaultExcludePatterns = CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() + private val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, defaultExcludePatterns) @Test fun `passes for same config test`() { @@ -68,6 +69,7 @@ internal class InvalidPropertiesConfigValidatorSpec { val subject = InvalidPropertiesConfigValidator( yamlConfig("config_validation/no-value.yml") as YamlConfig, + deprecatedProperties, defaultExcludePatterns ) val result = subject.validate(baseline) @@ -147,7 +149,7 @@ internal class InvalidPropertiesConfigValidatorSpec { @Test fun `does not report any complexity properties`() { - val subject = InvalidPropertiesConfigValidator(baseline, patterns("complexity")) + val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, patterns("complexity")) val result = subject.validate( yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig, @@ -157,7 +159,7 @@ internal class InvalidPropertiesConfigValidatorSpec { @Test fun `does not report 'complexity_LargeClass_howMany'`() { - val subject = InvalidPropertiesConfigValidator(baseline, patterns(".*>.*>howMany")) + val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, patterns(".*>.*>howMany")) val result = subject.validate( yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig ) @@ -177,7 +179,7 @@ internal class InvalidPropertiesConfigValidatorSpec { @Test @DisplayName("does not report .*>InnerMap") fun `does not report innerMap`() { - val subject = InvalidPropertiesConfigValidator(baseline, patterns(".*>InnerMap")) + val subject = InvalidPropertiesConfigValidator(baseline, deprecatedProperties, patterns(".*>InnerMap")) val result = subject.validate( yamlConfig("config_validation/other-nested-property-names.yml") as YamlConfig ) From 28a962694caf1d760fecd0e23eb68d842354ab04 Mon Sep 17 00:00:00 2001 From: Markus Schwarz Date: Thu, 21 Jul 2022 16:09:45 +0200 Subject: [PATCH 4/6] move converting warning level up the chain --- .../validation/AbstractYamlConfigValidator.kt | 6 +-- .../config/validation/ConfigValidation.kt | 26 +++++++++-- .../DeprecatedPropertiesConfigValidator.kt | 5 +-- .../InvalidPropertiesConfigValidator.kt | 24 ++++------ .../validation/MissingRulesConfigValidator.kt | 45 +++++++------------ .../config/validation/ValidationSettings.kt | 2 - .../AbstractYamlConfigValidatorSpec.kt | 25 +---------- ...DeprecatedPropertiesConfigValidatorSpec.kt | 23 +++++----- .../InvalidPropertiesConfigValidatorSpec.kt | 23 ++++------ .../config/validation/ValidateConfigSpec.kt | 27 +++++++++++ 10 files changed, 101 insertions(+), 105 deletions(-) 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 index 85f63606c90..93424d31792 100644 --- 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 @@ -6,7 +6,7 @@ import io.gitlab.arturbosch.detekt.api.Notification import io.gitlab.arturbosch.detekt.core.config.YamlConfig internal abstract class AbstractYamlConfigValidator( - private val excludePatterns: Set = emptySet(), + val excludePatterns: Set = emptySet(), ) : ConfigValidator { override fun validate(config: Config): Collection { @@ -17,9 +17,7 @@ internal abstract class AbstractYamlConfigValidator( "Only supported config is the $yamlConfigClass. Actual type is $actualClass" } val settings = ValidationSettings( - warningsAsErrors = config.subConfig("config").valueOrDefault("warningsAsErrors", false), - checkExhaustiveness = config.subConfig("config").valueOrDefault("checkExhaustiveness", false), - excludePatterns = excludePatterns, + config.subConfig("config").valueOrDefault("checkExhaustiveness", false), ) return validate(config, settings) 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 index 30c4702725f..3e6051077e3 100644 --- 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 @@ -4,6 +4,8 @@ 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 @@ -83,18 +85,34 @@ private fun validateYamlConfig( 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) } + 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) { - Notification.Level.Error -> message.red() - Notification.Level.Warning -> message.yellow() - Notification.Level.Info -> message + 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/validation/DeprecatedPropertiesConfigValidator.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/DeprecatedPropertiesConfigValidator.kt index e5a91aed952..a77d6c8b119 100644 --- 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 @@ -15,7 +15,7 @@ internal class DeprecatedPropertiesConfigValidator( return deprecatedProperties .map { (path, description) -> path.split(">") to description } .filter { (path, _) -> configAsMap.hasValue(path) } - .map { (path, description) -> createNotification(path, description, settings.warningsAsErrors) } + .map { (path, description) -> createNotification(path, description) } } @Suppress("UNCHECKED_CAST") @@ -34,12 +34,11 @@ internal class DeprecatedPropertiesConfigValidator( private fun createNotification( propertyPath: List, deprecationDescription: String, - reportAsError: Boolean, ): Notification { val prop = propertyPath.joinToString(">") return SimpleNotification( "Property '$prop' is deprecated. $deprecationDescription.", - if (reportAsError) Notification.Level.Error else Notification.Level.Warning, + Notification.Level.Warning, ) } } 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 index b11363b3a9a..41405e2d5b6 100644 --- 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 @@ -14,19 +14,18 @@ internal class InvalidPropertiesConfigValidator( configToValidate: YamlConfig, settings: ValidationSettings ): Collection { - return testKeys(configToValidate.properties, baseline.properties, null, settings) + return testKeys(configToValidate.properties, baseline.properties) } private fun testKeys( configToValidate: Map, baseline: Map, - parentPath: String?, - settings: ValidationSettings + parentPath: String? = null ): List { val notifications = mutableListOf() for (prop in configToValidate.keys) { val propertyPath = "${if (parentPath == null) "" else "$parentPath>"}$prop" - val isExcluded = settings.excludePatterns.any { it.matches(propertyPath) } + val isExcluded = excludePatterns.any { it.matches(propertyPath) } val isDeprecated = deprecatedProperties.contains(propertyPath) if (isExcluded || isDeprecated) { continue @@ -36,8 +35,7 @@ internal class InvalidPropertiesConfigValidator( propertyName = prop, propertyPath = propertyPath, configToValidate = configToValidate, - baseline = baseline, - settings = settings + baseline = baseline ) ) } @@ -49,14 +47,13 @@ internal class InvalidPropertiesConfigValidator( propertyName: String, propertyPath: String, configToValidate: Map, - baseline: Map, - settings: ValidationSettings + baseline: Map ): List { if (!baseline.contains(propertyName)) { return listOf(propertyDoesNotExists(propertyPath)) } if (configToValidate[propertyName] is String && baseline[propertyName] is List<*>) { - return listOf(propertyShouldBeAnArray(propertyPath, settings.warningsAsErrors)) + return listOf(propertyShouldBeAnArray(propertyPath)) } val next = configToValidate[propertyName] as? Map @@ -67,7 +64,7 @@ internal class InvalidPropertiesConfigValidator( baseline.contains(propertyName) && next != null && nextBase == null -> listOf(unexpectedNestedConfiguration(propertyPath)) next != null && nextBase != null -> - testKeys(next, nextBase, propertyPath, settings) + testKeys(next, nextBase, propertyPath) else -> emptyList() } } @@ -83,13 +80,10 @@ internal class InvalidPropertiesConfigValidator( internal fun unexpectedNestedConfiguration(prop: String): Notification = SimpleNotification("Unexpected nested config for '$prop'.") - internal fun propertyShouldBeAnArray( - prop: String, - reportAsError: Boolean, - ): Notification = + internal fun propertyShouldBeAnArray(prop: String): Notification = SimpleNotification( "Property '$prop' should be a YAML array instead of a comma-separated String.", - if (reportAsError) Notification.Level.Error else Notification.Level.Warning, + 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 index dbd807a2705..00f71ce5cdf 100644 --- 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 @@ -20,68 +20,55 @@ internal class MissingRulesConfigValidator( if (!settings.checkExhaustiveness) { return emptyList() } - return defaultRuleSetNames.flatMap { ruleSet -> validateRuleSet(ruleSet, configToValidate, settings) } + return defaultRuleSetNames.flatMap { ruleSet -> validateRuleSet(ruleSet, configToValidate) } } - @Suppress("UNCHECKED_CAST") private fun validateRuleSet( ruleSet: String, configToValidate: YamlConfig, - settings: ValidationSettings ): List { - val configPropertiesToValidate = configToValidate.properties - val ruleSetConfigToValidate = configPropertiesToValidate[ruleSet] as? Map - val ruleSetConfigFromBaseline = baseline.properties[ruleSet] as? Map + val ruleSetConfigToValidate = configToValidate.getSubMapOrNull(ruleSet) + val ruleSetConfigFromBaseline = baseline.getSubMapOrNull(ruleSet) return when { ruleSetConfigFromBaseline == null -> emptyList() - ruleSetConfigToValidate == null -> listOf(ruleSetMissing(ruleSet, settings)) - else -> checkForMissingRules(ruleSet, ruleSetConfigToValidate, ruleSetConfigFromBaseline, settings) + ruleSetConfigToValidate == null -> listOf(ruleSetMissing(ruleSet)) + else -> checkForMissingRules(ruleSet, ruleSetConfigToValidate, ruleSetConfigFromBaseline) } } private fun checkForMissingRules( ruleSetName: String, ruleSetConfigToValidate: Map, - ruleSetConfigFromBaseline: Map?, - settings: ValidationSettings + ruleSetConfigFromBaseline: Map, ): List { - if (ruleSetConfigFromBaseline == null) { - return emptyList() - } if (ruleSetConfigToValidate[Config.ACTIVE_KEY] == false) { return emptyList() } return ruleSetConfigFromBaseline.keys - .filter { ruleNameCandidate -> - settings.excludePatterns.none { it.matches("$ruleSetName>$ruleNameCandidate") } - } + .filter { ruleNameCandidate -> excludePatterns.none { it.matches("$ruleSetName>$ruleNameCandidate") } } .filter { ruleName -> !ruleSetConfigToValidate.containsKey(ruleName) } - .map { ruleName -> ruleMissing(ruleName, ruleSetName, settings) } + .map { ruleName -> ruleMissing(ruleName, ruleSetName) } } - private fun ruleMissing( - ruleName: String, - ruleSetName: String, - settings: ValidationSettings, - ): Notification = + private fun ruleMissing(ruleName: String, ruleSetName: String): Notification = SimpleNotification( "Rule '$ruleName' from the '$ruleSetName' rule set is missing in the configuration.", - if (settings.warningsAsErrors) Notification.Level.Error else Notification.Level.Warning, + Notification.Level.Warning, ) - private fun ruleSetMissing( - ruleSetName: String, - settings: ValidationSettings, - ): Notification = + private fun ruleSetMissing(ruleSetName: String): Notification = SimpleNotification( "Rule set '$ruleSetName' is missing in the configuration.", - if (settings.warningsAsErrors) Notification.Level.Error else Notification.Level.Warning, + 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 val defaultRuleSetNames: List by lazy(Companion::loadDefaultRuleSets) private fun loadDefaultRuleSets(): List { return ServiceLoader.load( RuleSetProvider::class.java, 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 index b99d78b6721..a1ddf9d4a42 100644 --- 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 @@ -1,7 +1,5 @@ package io.gitlab.arturbosch.detekt.core.config.validation internal data class ValidationSettings( - val warningsAsErrors: Boolean = false, val checkExhaustiveness: Boolean = false, - val excludePatterns: Set = emptySet(), ) 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 index c0cb48cd700..d8ca84712db 100644 --- 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 @@ -18,29 +18,8 @@ internal class AbstractYamlConfigValidatorSpec { validator.validate(config) - val expectedDefaultSettings = ValidationSettings( - warningsAsErrors = false, - checkExhaustiveness = false, - excludePatterns - ) - assertThat(validator.validationSettings).isEqualTo(expectedDefaultSettings) - } - - @ParameterizedTest - @ValueSource(booleans = [true, false]) - fun `extract warningsAsErrors settings from config`(configValue: Boolean) { - val excludePatterns = emptySet() - val config = yamlConfigFromContent( - """ - config: - warningsAsErrors: $configValue - """.trimIndent() - ) - val validator = SettingsCapturingValidatorAbstract(excludePatterns) - - validator.validate(config) - - assertThat(validator.validationSettings.warningsAsErrors).isEqualTo(configValue) + assertThat(validator.validationSettings.checkExhaustiveness).isFalse() + assertThat(validator.excludePatterns).isEqualTo(excludePatterns) } @ParameterizedTest 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 index d0a0141953c..6dd5ff709ff 100644 --- 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 @@ -1,10 +1,10 @@ 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.params.ParameterizedTest -import org.junit.jupiter.params.provider.ValueSource +import org.junit.jupiter.api.Test internal class DeprecatedPropertiesConfigValidatorSpec { private val deprecatedProperties = @@ -12,10 +12,9 @@ internal class DeprecatedPropertiesConfigValidatorSpec { private val subject = DeprecatedPropertiesConfigValidator(deprecatedProperties) - @ParameterizedTest - @ValueSource(booleans = [true, false]) - fun `reports a deprecated property as a warning or error`(warningsAsErrors: Boolean) { - val settings = ValidationSettings(warningsAsErrors = warningsAsErrors) + @Test + fun `reports a deprecated property as a warning`() { + val settings = ValidationSettings() val config = yamlConfigFromContent( """ naming: @@ -26,10 +25,12 @@ internal class DeprecatedPropertiesConfigValidatorSpec { val result = subject.validate(config, settings) - assertThat(result).hasSize(1) - val notification = result.first() - assertThat(notification.isError).isEqualTo(warningsAsErrors) - assertThat(notification.message).contains("naming>FunctionParameterNaming>ignoreOverriddenFunctions") - assertThat(notification.message).contains("Use `ignoreOverridden` instead") + 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/validation/InvalidPropertiesConfigValidatorSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt index 075ed2405f6..93617c8f048 100644 --- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt +++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/config/validation/InvalidPropertiesConfigValidatorSpec.kt @@ -1,12 +1,12 @@ 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.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.propertyShouldBeAnArray 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 @@ -14,8 +14,6 @@ import org.assertj.core.api.Assertions.assertThat 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 internal class InvalidPropertiesConfigValidatorSpec { private val deprecatedProperties = setOf("complexity>LongParameterList>threshold") @@ -85,13 +83,10 @@ internal class InvalidPropertiesConfigValidatorSpec { assertThat(result).isEmpty() } - @ParameterizedTest - @ValueSource(booleans = [true, false]) - fun `reports a string that should be an array as a warning`(warningsAsErrors: Boolean) { + @Test + fun `reports a string that should be an array as a warning`() { val config = yamlConfigFromContent( """ - config: - warningsAsErrors: $warningsAsErrors style: MagicNumber: ignoreNumbers: '-1,0,1,2' @@ -100,12 +95,12 @@ internal class InvalidPropertiesConfigValidatorSpec { val result = subject.validate(config) - assertThat(result).contains( - propertyShouldBeAnArray( - "style>MagicNumber>ignoreNumbers", - reportAsError = warningsAsErrors - ) - ) + 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 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 index 26547d838c5..91b4cd7c64c 100644 --- 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 @@ -1,11 +1,16 @@ 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") @@ -33,4 +38,26 @@ internal class ValidateConfigSpec { 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) + } + } } From d7e88b7a1c34f87a8cffb3a6ea70be9af2c0d97e Mon Sep 17 00:00:00 2001 From: Markus Schwarz Date: Thu, 28 Jul 2022 08:44:54 +0200 Subject: [PATCH 5/6] remove exclusion pattern from abstract validator --- .../validation/AbstractYamlConfigValidator.kt | 4 +--- .../validation/InvalidPropertiesConfigValidator.kt | 4 ++-- .../validation/MissingRulesConfigValidator.kt | 4 ++-- .../validation/AbstractYamlConfigValidatorSpec.kt | 13 ++++--------- 4 files changed, 9 insertions(+), 16 deletions(-) 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 index 93424d31792..9bb1270cf09 100644 --- 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 @@ -5,9 +5,7 @@ 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( - val excludePatterns: Set = emptySet(), -) : ConfigValidator { +internal abstract class AbstractYamlConfigValidator : ConfigValidator { override fun validate(config: Config): Collection { require(config is YamlConfig) { 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 index 41405e2d5b6..f2c7c68528e 100644 --- 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 @@ -7,8 +7,8 @@ import io.gitlab.arturbosch.detekt.core.config.YamlConfig internal class InvalidPropertiesConfigValidator( private val baseline: YamlConfig, private val deprecatedProperties: Set, - excludePatterns: Set, -) : AbstractYamlConfigValidator(excludePatterns) { + private val excludePatterns: Set, +) : AbstractYamlConfigValidator() { override fun validate( configToValidate: YamlConfig, 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 index 00f71ce5cdf..fe22e8ab2ff 100644 --- 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 @@ -10,8 +10,8 @@ import java.util.ServiceLoader internal class MissingRulesConfigValidator( private val baseline: YamlConfig, - excludePatterns: Set, -) : AbstractYamlConfigValidator(excludePatterns) { + private val excludePatterns: Set, +) : AbstractYamlConfigValidator() { override fun validate( configToValidate: YamlConfig, 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 index d8ca84712db..8edd9df6035 100644 --- 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 @@ -11,37 +11,32 @@ import org.junit.jupiter.params.provider.ValueSource internal class AbstractYamlConfigValidatorSpec { @Test - fun `use default validation settings and given exclude patterns`() { - val excludePatterns = setOf(".*".toRegex()) - val validator = SettingsCapturingValidatorAbstract(excludePatterns) + fun `use default validation settings`() { + val validator = SettingsCapturingValidatorAbstract() val config = yamlConfigFromContent("") validator.validate(config) assertThat(validator.validationSettings.checkExhaustiveness).isFalse() - assertThat(validator.excludePatterns).isEqualTo(excludePatterns) } @ParameterizedTest @ValueSource(booleans = [true, false]) fun `extract checkExhaustiveness settings from config`(configValue: Boolean) { - val excludePatterns = emptySet() val config = yamlConfigFromContent( """ config: checkExhaustiveness: $configValue """.trimIndent() ) - val validator = SettingsCapturingValidatorAbstract(excludePatterns) + val validator = SettingsCapturingValidatorAbstract() validator.validate(config) assertThat(validator.validationSettings.checkExhaustiveness).isEqualTo(configValue) } - private class SettingsCapturingValidatorAbstract( - excludePatterns: Set, - ) : AbstractYamlConfigValidator(excludePatterns) { + private class SettingsCapturingValidatorAbstract() : AbstractYamlConfigValidator() { lateinit var validationSettings: ValidationSettings override fun validate( configToValidate: YamlConfig, From c364d04f68eebf8c9d7544799c62117d6b5788ca Mon Sep 17 00:00:00 2001 From: Markus Schwarz Date: Thu, 28 Jul 2022 10:00:21 +0200 Subject: [PATCH 6/6] fix EmptyDefaultConstructor --- .../core/config/validation/AbstractYamlConfigValidatorSpec.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 8edd9df6035..641293a6c69 100644 --- 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 @@ -36,7 +36,7 @@ internal class AbstractYamlConfigValidatorSpec { assertThat(validator.validationSettings.checkExhaustiveness).isEqualTo(configValue) } - private class SettingsCapturingValidatorAbstract() : AbstractYamlConfigValidator() { + private class SettingsCapturingValidatorAbstract : AbstractYamlConfigValidator() { lateinit var validationSettings: ValidationSettings override fun validate( configToValidate: YamlConfig,