Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning for domain name #696

Merged
merged 11 commits into from
Jan 19, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ data class CommonConfiguration(private val configuration: Map<String, String>?)
/**
* 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")
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<RulesConfig>) : Rule("package-naming") {
private var isFixMode: Boolean = false
private lateinit var emitWarn: EmitType
Expand All @@ -46,32 +46,30 @@ class PackageNaming(private val configRules: List<RulesConfig>) : 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?)")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,21 +263,23 @@ class FileStructureRule(private val configRules: List<RulesConfig>) : Rule("file
else -> (headerKdoc ?: copyrightComment) to firstCodeNode
}

@Suppress("TYPE_ALIAS")
@Suppress("TYPE_ALIAS", "UnsafeCallOnNullableType")
private fun regroupImports(imports: List<KtImportDirective>): List<List<KtImportDirective>> {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class PackageNamingWarnTest : LintTestBase(::PackageNaming) {
private val rulesConfigList: List<RulesConfig> = listOf(
RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to "org.cqfn.diktat"))
)
private val rulesConfigListEmptyDomainName: List<RulesConfig> = listOf(
RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to ""))
)

@Test
@Tag(WarningNames.PACKAGE_NAME_MISSING)
Expand Down Expand Up @@ -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
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<RulesConfig> = listOf(
RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to ""))
)

@Test
@Tag(WarningNames.FILE_CONTAINS_ONLY_COMMENTS)
Expand Down Expand Up @@ -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
)
}
}