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
2 changes: 2 additions & 0 deletions diktat-rules/src/main/kotlin/generated/WarningNames.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package generated
import kotlin.String

public object WarningNames {
public const val MISSING_DOMAIN_NAME: String = "MISSING_DOMAIN_NAME"
kentr0w marked this conversation as resolved.
Show resolved Hide resolved

public const val PACKAGE_NAME_MISSING: String = "PACKAGE_NAME_MISSING"

public const val PACKAGE_NAME_INCORRECT_CASE: String = "PACKAGE_NAME_INCORRECT_CASE"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,35 @@ class PackageNaming(private val configRules: List<RulesConfig>) : Rule("package-
emitWarn = emit

val configuration by configRules.getCommonConfiguration()
domainName = configuration.also {
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?)")
log.error("Not able to find configuration file")
}
}
.domainName

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
}

// 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)
.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
}

// 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.get() == 1) {
kentr0w marked this conversation as resolved.
Show resolved Hide resolved
kentr0w marked this conversation as resolved.
Show resolved Hide resolved
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(domainName!!.split(PACKAGE_SEPARATOR).map(Name::identifier))
kentr0w marked this conversation as resolved.
Show resolved Hide resolved
?.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,10 @@ class PackageNamingWarnTest : LintTestBase(::PackageNaming) {
private val rulesConfigList: List<RulesConfig> = listOf(
RulesConfig("DIKTAT_COMMON", true, mapOf("domainName" to "org.cqfn.diktat"))
)
private val rulesConfigListWithoutDomainName: List<RulesConfig> = emptyList()
kentr0w marked this conversation as resolved.
Show resolved Hide resolved
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 +233,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()} myProjectMain.kotlin.org.cqfn.diktat.example", true),
fileName = "/home/testu/project/src/myProjectMain/kotlin/org/cqfn/diktat/example/Example.kt",
kentr0w marked this conversation as resolved.
Show resolved Hide resolved
rulesConfigList = rulesConfigListEmptyDomainName
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ 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 rulesConfigListWithoutDomainName: List<RulesConfig> = emptyList()

@Test
@Tag(WarningNames.FILE_CONTAINS_ONLY_COMMENTS)
Expand Down Expand Up @@ -226,4 +227,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 no domain name in config`() {
kentr0w marked this conversation as resolved.
Show resolved Hide resolved
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 = rulesConfigListWithoutDomainName
)
}
}