From b43fb712aeae80262280cba49906e38ba1fb0a9f Mon Sep 17 00:00:00 2001 From: Denis Kumar Date: Tue, 19 Jan 2021 11:47:01 +0300 Subject: [PATCH] Warning for domain name (#696) * Warning for domain name ### What's done: Handled case when domainName is empty in config --- .../common/config/rules/RulesConfigReader.kt | 4 +- .../diktat/ruleset/rules/PackageNaming.kt | 46 +++++++++---------- .../ruleset/rules/files/FileStructureRule.kt | 22 +++++---- .../ruleset/chapter1/PackageNamingWarnTest.kt | 17 +++++++ .../ruleset/chapter3/FileStructureRuleTest.kt | 20 ++++++++ 5 files changed, 73 insertions(+), 36 deletions(-) diff --git a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt index 376305439e..c2d2b51be4 100644 --- a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt +++ b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt @@ -112,8 +112,8 @@ data class CommonConfiguration(private val configuration: Map?) /** * Start of package name, which shoould be common, e.g. org.example.myproject */ - val domainName: String by lazy { - (configuration ?: emptyMap()).getOrDefault("domainName", "") + val domainName: String? by lazy { + configuration?.get("domainName") } /** 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 7be9e7c55f..a21e6fdd27 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 @@ -33,7 +33,7 @@ import java.util.concurrent.atomic.AtomicInteger * need to support autofixing of directories in the same way as package is named. For example if we have package name: * package a.b.c.D -> then class D should be placed in a/b/c/ directories */ -@Suppress("ForbiddenComment") +@Suppress("ForbiddenComment", "TOO_MANY_LINES_IN_LAMBDA") class PackageNaming(private val configRules: List) : Rule("package-naming") { private var isFixMode: Boolean = false private lateinit var emitWarn: EmitType @@ -46,32 +46,30 @@ class PackageNaming(private val configRules: List) : Rule("package- emitWarn = emit val configuration by configRules.getCommonConfiguration() - domainName = configuration.also { - if (it.isDefault && visitorCounter.incrementAndGet() == 1) { - log.error("Not able to find an external configuration for domain name in the common" + - " configuration (is it missing in yml config?)") - } - } - .domainName + configuration.domainName?.let { + domainName = it + if (node.elementType == PACKAGE_DIRECTIVE) { + val filePath = node.getRootNode().getFilePath() + // calculating package name based on the directory where the file is placed + val realPackageName = calculateRealPackageName(filePath) + + // if node isLeaf - this means that there is no package name declared + if (node.isLeaf() && !filePath.isKotlinScript()) { + warnAndFixMissingPackageName(node, realPackageName, filePath) + return + } - if (node.elementType == PACKAGE_DIRECTIVE) { - val filePath = node.getRootNode().getFilePath() - // calculating package name based on the directory where the file is placed - val realPackageName = calculateRealPackageName(filePath) + // getting all identifiers from existing package name into the list like [org, diktat, project] + val wordsInPackageName = node.findAllNodesWithSpecificType(IDENTIFIER) - // if node isLeaf - this means that there is no package name declared - if (node.isLeaf() && !filePath.isKotlinScript()) { - warnAndFixMissingPackageName(node, realPackageName, filePath) - return + // no need to check that packageIdentifiers is empty, because in this case parsing will fail + checkPackageName(wordsInPackageName, node) + // fix in checkFilePathMatchesWithPackageName is much more aggressive than fixes in checkPackageName, they can conflict + checkFilePathMatchesWithPackageName(wordsInPackageName, realPackageName, node) } - - // getting all identifiers from existing package name into the list like [org, diktat, project] - val wordsInPackageName = node.findAllNodesWithSpecificType(IDENTIFIER) - - // no need to check that packageIdentifiers is empty, because in this case parsing will fail - checkPackageName(wordsInPackageName, node) - // fix in checkFilePathMatchesWithPackageName is much more aggressive than fixes in checkPackageName, they can conflict - checkFilePathMatchesWithPackageName(wordsInPackageName, realPackageName, node) + } ?: if (visitorCounter.incrementAndGet() == 1) { + log.error("Not able to find an external configuration for domain" + + " name in the common configuration (is it missing in yml config?)") } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt index 25a85b8819..c28a72b364 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/files/FileStructureRule.kt @@ -263,21 +263,23 @@ class FileStructureRule(private val configRules: List) : Rule("file else -> (headerKdoc ?: copyrightComment) to firstCodeNode } - @Suppress("TYPE_ALIAS") + @Suppress("TYPE_ALIAS", "UnsafeCallOnNullableType") private fun regroupImports(imports: List): List> { val (android, notAndroid) = imports.partition { it.isStandard(StandardPlatforms.ANDROID) } - val (ownDomain, tmp) = notAndroid.partition { import -> - import - .importPath - ?.fqName - ?.pathSegments() - ?.zip(domainName.split(PACKAGE_SEPARATOR).map(Name::identifier)) - ?.all { it.first == it.second } - ?: false - } + val (ownDomain, tmp) = domainName?.let { + notAndroid.partition { import -> + import + .importPath + ?.fqName + ?.pathSegments() + ?.zip(it.split(PACKAGE_SEPARATOR).map(Name::identifier)) + ?.all { it.first == it.second } + ?: false + } + } ?: Pair(emptyList(), notAndroid) val (others, javaAndKotlin) = tmp.partition { !it.isStandard(StandardPlatforms.JAVA) && !it.isStandard(StandardPlatforms.KOTLIN) diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/PackageNamingWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/PackageNamingWarnTest.kt index 2a3924d558..cfcad912e9 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/PackageNamingWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter1/PackageNamingWarnTest.kt @@ -22,6 +22,9 @@ class PackageNamingWarnTest : LintTestBase(::PackageNaming) { private val rulesConfigList: List = listOf( RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to "org.cqfn.diktat")) ) + private val rulesConfigListEmptyDomainName: List = listOf( + RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to "")) + ) @Test @Tag(WarningNames.PACKAGE_NAME_MISSING) @@ -229,4 +232,18 @@ class PackageNamingWarnTest : LintTestBase(::PackageNaming) { rulesConfigList = rulesConfigList ) } + + @Test + @Tag(WarningNames.PACKAGE_NAME_INCORRECT_PATH) + fun `should warn if there is empty domain name`() { + lintMethod( + """ + |package org.cqfn.diktat + """.trimMargin(), + LintError(1, 9, ruleId, "${PACKAGE_NAME_INCORRECT_PREFIX.warnText()} ", true), + LintError(1, 9, ruleId, "${PACKAGE_NAME_INCORRECT_PATH.warnText()} org.cqfn.diktat.example", true), + fileName = "/home/testu/project/src/main/kotlin/org/cqfn/diktat/example/Example.kt", + rulesConfigList = rulesConfigListEmptyDomainName + ) + } } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt index 480c269d8e..e433e308df 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/FileStructureRuleTest.kt @@ -25,6 +25,9 @@ class FileStructureRuleTest : LintTestBase(::FileStructureRule) { RulesConfig(FILE_WILDCARD_IMPORTS.name, true, mapOf("allowedWildcards" to "org.cqfn.diktat.example.*, org.cqfn.diktat.ruleset.constants.Warnings.*")) ) + private val rulesConfigListEmptyDomainName: List = listOf( + RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to "")) + ) @Test @Tag(WarningNames.FILE_CONTAINS_ONLY_COMMENTS) @@ -226,4 +229,21 @@ class FileStructureRuleTest : LintTestBase(::FileStructureRule) { LintError(4, 1, ruleId, "${FILE_INCORRECT_BLOCKS_ORDER.warnText()} @file:Annotation", true) ) } + + @Test + @Tag(WarningNames.FILE_INCORRECT_BLOCKS_ORDER) + fun `test with empty domain name in config`() { + lintMethod( + """ + |package org.cqfn.diktat.example + | + |import org.cqfn.diktat.example.Foo + |import com.pinterest.ktlint.core.LintError + | + |class Example + """.trimMargin(), + LintError(3, 1, ruleId, "${FILE_UNORDERED_IMPORTS.warnText()} import com.pinterest.ktlint.core.LintError...", true), + rulesConfigList = rulesConfigListEmptyDomainName + ) + } }