From a9e5a56c449d411cd43403bac92109b3b19daf7e Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 22 Oct 2020 13:35:35 +0300 Subject: [PATCH] Validate warning names in config file on startup (#396) ### What's done: * Added validation logic * Added tests --- .../cqfn/diktat/ruleset/constants/Warnings.kt | 6 ++ .../ruleset/rules/DiktatRuleSetProvider.kt | 26 ++++++- .../diktat/ruleset/rules/PackageNaming.kt | 2 +- .../src/main/resources/diktat-analysis.yml | 8 +-- .../diktat/ruleset/smoke/DiktatSmokeTest.kt | 14 ++-- .../smoke/RulesConfigValidationTest.kt | 70 +++++++++++++++++++ .../ruleset/utils/VariablesSearchTest.kt | 2 +- .../VariablesWithAssignmentsSearchTest.kt | 6 +- .../smoke/src/main/kotlin/Bug1Expected.kt | 6 +- .../smoke/src/main/kotlin/Example1Expected.kt | 6 +- .../smoke/src/main/kotlin/Example2Expected.kt | 6 +- 11 files changed, 119 insertions(+), 33 deletions(-) create mode 100644 diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/RulesConfigValidationTest.kt diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index 582d570d1b..e5d19f6069 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -162,4 +162,10 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S autoFix() } } + + companion object { + val names by lazy { + values().map { it.name } + } + } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index cabeb6afde..7602a99da4 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -2,7 +2,10 @@ package org.cqfn.diktat.ruleset.rules import com.pinterest.ktlint.core.RuleSet import com.pinterest.ktlint.core.RuleSetProvider +import org.cqfn.diktat.common.config.rules.DIKTAT_COMMON +import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.common.config.rules.RulesConfigReader +import org.cqfn.diktat.ruleset.constants.Warnings import org.cqfn.diktat.ruleset.rules.calculations.AccurateCalculationsRule import org.cqfn.diktat.ruleset.rules.comments.CommentsRule import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule @@ -11,12 +14,14 @@ import org.cqfn.diktat.ruleset.rules.files.FileSize import org.cqfn.diktat.ruleset.rules.files.FileStructureRule import org.cqfn.diktat.ruleset.rules.files.IndentationRule import org.cqfn.diktat.ruleset.rules.files.NewlinesRule -import org.cqfn.diktat.ruleset.rules.kdoc.CommentsFormatting import org.cqfn.diktat.ruleset.rules.identifiers.LocalVariablesRule +import org.cqfn.diktat.ruleset.rules.kdoc.CommentsFormatting import org.cqfn.diktat.ruleset.rules.kdoc.KdocComments import org.cqfn.diktat.ruleset.rules.kdoc.KdocFormatting import org.cqfn.diktat.ruleset.rules.kdoc.KdocMethods +import org.jetbrains.kotlin.org.jline.utils.Levenshtein import org.slf4j.LoggerFactory +import java.io.File /** * this constant will be used everywhere in the code to mark usage of Diktat ruleset @@ -24,9 +29,20 @@ import org.slf4j.LoggerFactory const val DIKTAT_RULE_SET_ID = "diktat-ruleset" class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analysis.yml") : RuleSetProvider { + @Suppress("LongMethod") override fun get(): RuleSet { log.debug("Will run $DIKTAT_RULE_SET_ID with $diktatConfigFile (it can be placed to the run directory or the default file from resources will be used)") - val configRules = RulesConfigReader(javaClass.classLoader).readResource(diktatConfigFile) ?: listOf() + if (!File(diktatConfigFile).exists()) { + log.warn("Configuration file $diktatConfigFile not found in file system, the file included in jar will be used. " + + "Some configuration options will be disabled or substituted with defaults. " + + "Custom configuration file should be placed in diktat working directory if run from CLI " + + "or provided as configuration options in plugins." + ) + } + val configRules = RulesConfigReader(javaClass.classLoader) + .readResource(diktatConfigFile) + ?.onEach(::validate) + ?: emptyList() val rules = listOf( ::CommentsRule, ::KdocComments, @@ -81,6 +97,12 @@ class DiktatRuleSetProvider(private val diktatConfigFile: String = "diktat-analy ) } + private fun validate(config: RulesConfig) = + require(config.name == DIKTAT_COMMON || config.name in Warnings.names) { + val closestMatch = Warnings.names.minBy { Levenshtein.distance(it, config.name) } + "Warning name <${config.name}> in configuration file is invalid, did you mean <$closestMatch>?" + } + companion object { private val log = LoggerFactory.getLogger(DiktatRuleSetProvider::class.java) } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/PackageNaming.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/PackageNaming.kt index 2e3c710c87..95767a1e66 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/PackageNaming.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/PackageNaming.kt @@ -53,7 +53,7 @@ class PackageNaming(private val configRules: List) : Rule("package- if (domainNameConfiguration == null) { log.error("Not able to find an external configuration for domain name in the common configuration (is it missing in yml config?)") } - val configuration = configRules.getCommonConfiguration().value + val configuration by configRules.getCommonConfiguration() domainName = configuration.domainName if (node.elementType == PACKAGE_DIRECTIVE) { diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index fa368a8de8..66559c1800 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -38,16 +38,16 @@ configuration: {} - name: PACKAGE_NAME_INCORRECT_CASE enabled: true - configuration: {} + configuration: {} # configuration domainName is taken from DIKTAT_COMMON - name: PACKAGE_NAME_INCORRECT_PREFIX - enabled: true + enabled: false configuration: {} - name: PACKAGE_NAME_INCORRECT_SYMBOLS enabled: true configuration: {} - name: PACKAGE_NAME_INCORRECT_PATH - enabled: true - configuration: {} + enabled: false + configuration: {} # configuration domainName is taken from DIKTAT_COMMON - name: PACKAGE_NAME_MISSING enabled: true configuration: {} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt index 4e2b6f2ad5..96dbb1c2be 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt @@ -18,7 +18,7 @@ import java.io.File // fixme: run as a separate maven goal/module? class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin", - { DiktatRuleSetProvider() }, + { DiktatRuleSetProvider("../diktat-analysis.yml") }, // using same yml config as for diktat code style check { lintError, _ -> unfixedLintErrors.add(lintError) }, null ) { @@ -38,12 +38,12 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin", unfixedLintErrors.assertEquals( LintError(1, 1, "$DIKTAT_RULE_SET_ID:file-naming", "${FILE_NAME_INCORRECT.warnText()} Example1Test.kt_copy", true), // todo this is a false one LintError(1, 1, "$DIKTAT_RULE_SET_ID:file-naming", "${FILE_NAME_MATCH_CLASS.warnText()} Example1Test.kt_copy vs Example", true), // todo this is a false one - LintError(1, 1, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false), - LintError(4, 17, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_TOP_LEVEL.warnText()} Example", false), - LintError(6, 13, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_CLASS_ELEMENTS.warnText()} isValid", false), - LintError(10, 3, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_CLASS_ELEMENTS.warnText()} foo", false), // todo what's with offset? - LintError(11, 7, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false), - LintError(15, 14, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false) + LintError(3, 6, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_TOP_LEVEL.warnText()} Example", false), + LintError(3, 26, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_CLASS_ELEMENTS.warnText()} isValid", false), + LintError(6, 9, "$DIKTAT_RULE_SET_ID:kdoc-comments", "${MISSING_KDOC_CLASS_ELEMENTS.warnText()} foo", false), // todo what's with offset? + LintError(10, 10, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false), + LintError(12, 3, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false), + LintError(17, 15, "$DIKTAT_RULE_SET_ID:kdoc-formatting", "${KDOC_NO_EMPTY_TAGS.warnText()} @return", false) ) } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/RulesConfigValidationTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/RulesConfigValidationTest.kt new file mode 100644 index 0000000000..5da5da443b --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/RulesConfigValidationTest.kt @@ -0,0 +1,70 @@ +package org.cqfn.diktat.ruleset.smoke + +import com.charleskorn.kaml.InvalidPropertyValueException +import org.cqfn.diktat.ruleset.rules.DiktatRuleSetProvider +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Disabled +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.io.File +import java.lang.IllegalArgumentException + +class RulesConfigValidationTest { + private lateinit var file: File + + @BeforeEach + fun `prepare temporary file`() { + file = createTempFile() + } + + @AfterEach + fun `clear temporary file`() { + file.delete() + } + + @Test + fun `should throw error if name is missing in Warnings`() { + file.writeText( + """ + |- name: MISSING_DOC_TOP_LEVEL + | enabled: true + | configuration: {} + """.trimMargin() + ) + val e = assertThrows { + DiktatRuleSetProvider(file.absolutePath).get() + } + Assertions.assertEquals("Warning name in configuration file is invalid, did you mean ?", e.message) + } + + @Test + fun `should throw error on invalid yml config`() { + file.writeText( + """ + |- name: PACKAGE_NAME_MISSING + | enabled: true + | configuration: + """.trimMargin() + ) + assertThrows { + DiktatRuleSetProvider(file.absolutePath).get() + } + } + + @Test + @Disabled("https://github.com/cqfn/diKTat/issues/395") + fun `should throw error on invalid configuration section`() { + file.writeText( + """ + |- name: TOO_LONG_FUNCTION + | enabled: true + | configuration: + | maxFunctionLength: 1o + | isIncludeHeader: Fslse + """.trimMargin() + ) + DiktatRuleSetProvider(file.absolutePath).get() + } +} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt index 8b1430039e..8b61264520 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesSearchTest.kt @@ -23,7 +23,7 @@ class VariablesSearchTest { o = 17 } } - """.trimIndent(), 0) { node, counter -> + """.trimIndent(), 0) { node, _ -> if (node.elementType != ElementType.FILE) { val thrown = Assertions.assertThrows(IllegalArgumentException::class.java) { val variablesSearchAbstract: VariablesSearch = Mockito.mock(VariablesSearch::class.java, Mockito.CALLS_REAL_METHODS) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt index dca992f238..4d7951514b 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/utils/VariablesWithAssignmentsSearchTest.kt @@ -21,7 +21,7 @@ class VariablesWithAssignmentsSearchTest { o = 17 } } - """.trimIndent(), 0) { node, counter -> + """.trimIndent(), 0) { node, _ -> if (node.elementType == FILE) { val vars = node.findAllVariablesWithAssignments().mapKeys { it.key.text } val keys = vars.keys @@ -47,7 +47,7 @@ class VariablesWithAssignmentsSearchTest { } } } - """.trimIndent(), 0) { node, counter -> + """.trimIndent(), 0) { node, _ -> if (node.elementType == FILE) { val vars = node.findAllVariablesWithAssignments().mapKeys { it.key.text } val keys = vars.keys @@ -66,7 +66,7 @@ class VariablesWithAssignmentsSearchTest { var a = 1 a++ } - """.trimIndent(), 0) { node, counter -> + """.trimIndent(), 0) { node, _ -> if (node.elementType == FILE) { val vars = node.findAllVariablesWithAssignments().mapKeys { it.key.text } val keys = vars.keys diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt index 7907519ebb..3b6710ac3b 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Bug1Expected.kt @@ -1,8 +1,4 @@ -/* - * Copyright (c) Your Company Name Here. 2010-2020 - */ - -package your.name.here +package org.cqfn.diktat /** * @param foo diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt index 127c8e0b4e..6d20644362 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example1Expected.kt @@ -1,8 +1,4 @@ -/* - * Copyright (c) Your Company Name Here. 2010-2020 - */ - -package your.name.here +package org.cqfn.diktat class Example { @get:JvmName("getIsValid") diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Expected.kt index 38e7103c3d..672b9623e3 100644 --- a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Expected.kt +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example2Expected.kt @@ -1,8 +1,4 @@ -/* - * Copyright (c) Your Company Name Here. 2010-2020 - */ - -package your.name.here +package org.cqfn.diktat import org.slf4j.LoggerFactory