forked from detekt/detekt
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Validate yaml configurations by comparing their structure - detekt#516 (
detekt#1998) * Validate yaml configurations by comparing their structure - detekt#516 * Towards supporting composite config validation * Generalize config validation interface * Fix formatting * Delete non existing rule configurations * Support excluding config properties from validation * Use settings#info to print invalid properties * Exclude common rule properties by default from validation * Regenerate default configuration * Rename validation message functions
- Loading branch information
1 parent
5f49a72
commit 8dd4f33
Showing
54 changed files
with
686 additions
and
38 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/CommaSeparatedPattern.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package io.gitlab.arturbosch.detekt.api.internal | ||
|
|
||
| import io.gitlab.arturbosch.detekt.api.SplitPattern | ||
|
|
||
| class CommaSeparatedPattern(text: String, delimiters: String = ",") : | ||
| SplitPattern(text, delimiters, false) { | ||
|
|
||
| fun mapToRegex(): Set<Regex> = mapAll(::Regex).toSet() | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
76 changes: 76 additions & 0 deletions
76
...-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/ValidatableConfiguration.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package io.gitlab.arturbosch.detekt.api.internal | ||
|
|
||
| import io.gitlab.arturbosch.detekt.api.Config | ||
| import io.gitlab.arturbosch.detekt.api.Notification | ||
| import io.gitlab.arturbosch.detekt.api.YamlConfig | ||
|
|
||
| interface ValidatableConfiguration { | ||
|
|
||
| fun validate(baseline: Config, excludePatterns: Set<Regex>): List<Notification> | ||
| } | ||
|
|
||
| /** | ||
| * 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. | ||
| */ | ||
| const val DEFAULT_PROPERTY_EXCLUDES = ".*>.*>excludes,.*>.*>includes,.*>.*>active,.*>.*>autoCorrect" | ||
|
|
||
| @Suppress("UNCHECKED_CAST", "ComplexMethod") | ||
| fun validateConfig( | ||
| config: Config, | ||
| baseline: Config, | ||
| excludePatterns: Set<Regex> = CommaSeparatedPattern(DEFAULT_PROPERTY_EXCLUDES).mapToRegex() | ||
| ): List<Notification> { | ||
| require(baseline != Config.empty) { "Cannot validate configuration based on an empty baseline config." } | ||
| require(baseline is YamlConfig) { "Only supported baseline config is the YamlConfig." } | ||
|
|
||
| if (config == Config.empty) { | ||
| return emptyList() | ||
| } | ||
|
|
||
| val notifications = mutableListOf<Notification>() | ||
|
|
||
| fun testKeys(current: Map<String, Any>, base: Map<String, Any>, parentPath: String?) { | ||
| for (prop in current.keys) { | ||
|
|
||
| val propertyPath = "${if (parentPath == null) "" else "$parentPath>"}$prop" | ||
|
|
||
| if (excludePatterns.any { it.matches(propertyPath) }) { | ||
| continue | ||
| } | ||
|
|
||
| if (!base.contains(prop)) { | ||
| notifications.add(propertyDoesNotExists(propertyPath)) | ||
| } | ||
|
|
||
| val next = current[prop] as? Map<String, Any> | ||
| val nextBase = base[prop] as? Map<String, Any> | ||
|
|
||
| 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'.") |
156 changes: 156 additions & 0 deletions
156
detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/internal/ConfigValidationSpec.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| package io.gitlab.arturbosch.detekt.api.internal | ||
|
|
||
| import io.gitlab.arturbosch.detekt.api.CompositeConfig | ||
| import io.gitlab.arturbosch.detekt.api.Config | ||
| import io.gitlab.arturbosch.detekt.test.yamlConfig | ||
| import org.assertj.core.api.Assertions.assertThat | ||
| import org.spekframework.spek2.Spek | ||
| import org.spekframework.spek2.style.specification.describe | ||
|
|
||
| internal class ConfigValidationSpec : Spek({ | ||
|
|
||
| describe("validate configuration file") { | ||
|
|
||
| val baseline = yamlConfig("config_validation/baseline.yml") | ||
|
|
||
| it("passes for same config test") { | ||
| val result = validateConfig(baseline, baseline) | ||
| assertThat(result).isEmpty() | ||
| } | ||
|
|
||
| it("passes for properties which may appear on rules but may be not present in default config") { | ||
| val result = validateConfig( | ||
| yamlConfig("config_validation/default-excluded-properties.yml"), | ||
| baseline | ||
| ) | ||
| assertThat(result).isEmpty() | ||
| } | ||
|
|
||
| it("reports different rule set name") { | ||
| val result = validateConfig( | ||
| yamlConfig("config_validation/other-ruleset-name.yml"), | ||
| baseline | ||
| ) | ||
| assertThat(result).contains(propertyDoesNotExists("code-smell")) | ||
| } | ||
|
|
||
| it("reports different nested property names") { | ||
| val result = validateConfig( | ||
| yamlConfig("config_validation/other-nested-property-names.yml"), | ||
| baseline | ||
| ) | ||
| assertThat(result).contains( | ||
| propertyDoesNotExists("complexity>LongLongMethod"), | ||
| propertyDoesNotExists("complexity>LongParameterList>enabled"), | ||
| propertyDoesNotExists("complexity>LargeClass>howMany"), | ||
| propertyDoesNotExists("complexity>InnerMap>InnerKey"), | ||
| propertyDoesNotExists("complexity>InnerMap>Inner2>nestedActive") | ||
| ) | ||
| } | ||
|
|
||
| it("reports different rule set name") { | ||
| val result = validateConfig( | ||
| yamlConfig("config_validation/no-nested-config.yml"), | ||
| baseline | ||
| ) | ||
| assertThat(result).contains( | ||
| nestedConfigurationExpected("complexity"), | ||
| nestedConfigurationExpected("style>WildcardImport") | ||
| ) | ||
| } | ||
|
|
||
| it("reports unexpected nested configs") { | ||
| // note that the baseline config is now the first argument | ||
| val result = validateConfig(baseline, yamlConfig("config_validation/no-value.yml")) | ||
| assertThat(result).contains( | ||
| unexpectedNestedConfiguration("style"), | ||
| unexpectedNestedConfiguration("comments") | ||
| ) | ||
| } | ||
|
|
||
| describe("validate composite configurations") { | ||
|
|
||
| it("passes for same left, right and baseline config") { | ||
| val result = validateConfig(CompositeConfig(baseline, baseline), baseline) | ||
| assertThat(result).isEmpty() | ||
| } | ||
|
|
||
| it("passes for empty configs") { | ||
| val result = validateConfig(CompositeConfig(Config.empty, Config.empty), baseline) | ||
| assertThat(result).isEmpty() | ||
| } | ||
|
|
||
| it("finds accumulated errors") { | ||
| val result = validateConfig( | ||
| CompositeConfig( | ||
| yamlConfig("config_validation/other-nested-property-names.yml"), | ||
| yamlConfig("config_validation/no-nested-config.yml") | ||
| ), | ||
| baseline | ||
| ) | ||
|
|
||
| assertThat(result).contains( | ||
| nestedConfigurationExpected("complexity"), | ||
| nestedConfigurationExpected("style>WildcardImport"), | ||
| propertyDoesNotExists("complexity>LongLongMethod"), | ||
| propertyDoesNotExists("complexity>LongParameterList>enabled"), | ||
| propertyDoesNotExists("complexity>LargeClass>howMany"), | ||
| propertyDoesNotExists("complexity>InnerMap>InnerKey"), | ||
| propertyDoesNotExists("complexity>InnerMap>Inner2>nestedActive") | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| describe("configure additional exclude paths") { | ||
|
|
||
| fun patterns(str: String) = CommaSeparatedPattern(str).mapToRegex() | ||
|
|
||
| it("does not report any complexity properties") { | ||
| val result = validateConfig( | ||
| yamlConfig("config_validation/other-nested-property-names.yml"), | ||
| baseline, | ||
| patterns("complexity") | ||
| ) | ||
| assertThat(result).isEmpty() | ||
| } | ||
|
|
||
| it("does not report 'complexity>LargeClass>howMany'") { | ||
| val result = validateConfig( | ||
| yamlConfig("config_validation/other-nested-property-names.yml"), | ||
| baseline, | ||
| patterns(".*>.*>howMany") | ||
| ) | ||
|
|
||
| assertThat(result).contains( | ||
| propertyDoesNotExists("complexity>LongLongMethod"), | ||
| propertyDoesNotExists("complexity>LongParameterList>enabled"), | ||
| propertyDoesNotExists("complexity>InnerMap>InnerKey"), | ||
| propertyDoesNotExists("complexity>InnerMap>Inner2>nestedActive") | ||
| ) | ||
|
|
||
| assertThat(result).doesNotContain( | ||
| propertyDoesNotExists("complexity>LargeClass>howMany") | ||
| ) | ||
| } | ||
|
|
||
| it("does not report '.*>InnerMap'") { | ||
| val result = validateConfig( | ||
| yamlConfig("config_validation/other-nested-property-names.yml"), | ||
| baseline, | ||
| patterns(".*>InnerMap") | ||
| ) | ||
|
|
||
| assertThat(result).contains( | ||
| propertyDoesNotExists("complexity>LargeClass>howMany"), | ||
| propertyDoesNotExists("complexity>LongLongMethod"), | ||
| propertyDoesNotExists("complexity>LongParameterList>enabled") | ||
| ) | ||
|
|
||
| assertThat(result).doesNotContain( | ||
| propertyDoesNotExists("complexity>InnerMap>InnerKey"), | ||
| propertyDoesNotExists("complexity>InnerMap>Inner2>nestedActive") | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| }) |
Oops, something went wrong.