Enable more rules from naming rule set for detekt code base (#3996)

* Enable more naming rules for detekt code base

* Fix NoNameShowing violations

* Exclude buildSrc and kotlin scripts from InvalidPackageDeclaration

* Improve fix of NoNameShadowing

Co-authored-by: Markus Schwarz <post@markus-schwarz.net>
This commit is contained in:
marschwar
2021-08-06 17:54:14 +02:00
committed by GitHub
parent 23ffe7c6f4
commit eb141e9a4d
6 changed files with 52 additions and 29 deletions

View File

@@ -100,6 +100,19 @@ formatting:
SpacingBetweenDeclarationsWithComments:
active: true
naming:
InvalidPackageDeclaration:
active: true
excludes: ['**/buildSrc/**/*.kt', '**/*.kts']
NoNameShadowing:
active: true
NonBooleanPropertyPrefixedWithIs:
active: true
VariableMaxLength:
active: true
VariableMinLength:
active: true
potential-bugs:
UnsafeCast:
active: true

View File

@@ -68,10 +68,10 @@ class YamlConfig internal constructor(
*
* Note the reader will be consumed and closed.
*/
fun load(reader: Reader): Config = reader.buffered().use {
fun load(reader: Reader): Config = reader.buffered().use { bufferedReader ->
val map: Map<*, *>? = runCatching {
@Suppress("USELESS_CAST") // runtime inference bug
Yaml().loadAs(it, Map::class.java) as Map<*, *>?
Yaml().loadAs(bufferedReader, Map::class.java) as Map<*, *>?
}.getOrElse { throw Config.InvalidConfigurationError(it) }
if (map == null) {
YamlConfig(emptyMap())

View File

@@ -67,28 +67,32 @@ internal class DetektAndroid(private val project: Project) {
fun registerTasks(extension: DetektExtension) {
// There is not a single Android plugin, but each registers an extension based on BaseExtension,
// so we catch them all by looking for this one
project.afterEvaluate {
val baseExtension = project.extensions.findByType(BaseExtension::class.java)
baseExtension?.let {
val bootClasspath = project.files(baseExtension.bootClasspath)
project.afterEvaluate { evaluatedProject ->
val baseExtensionOrNull = evaluatedProject.extensions.findByType(BaseExtension::class.java)
baseExtensionOrNull?.let { baseExtension ->
val bootClasspath = evaluatedProject.files(baseExtension.bootClasspath)
baseExtension.variants
?.matching { !extension.matchesIgnoredConfiguration(it) }
?.all { variant ->
project.registerAndroidDetektTask(bootClasspath, extension, variant).also { provider ->
evaluatedProject.registerAndroidDetektTask(bootClasspath, extension, variant).also { provider ->
mainTaskProvider.dependsOn(provider)
}
project.registerAndroidCreateBaselineTask(bootClasspath, extension, variant)
evaluatedProject.registerAndroidCreateBaselineTask(bootClasspath, extension, variant)
.also { provider ->
mainBaselineTaskProvider.dependsOn(provider)
}
variant.testVariants
.filter { !extension.matchesIgnoredConfiguration(it) }
.forEach { testVariant ->
project.registerAndroidDetektTask(bootClasspath, extension, testVariant)
evaluatedProject.registerAndroidDetektTask(bootClasspath, extension, testVariant)
.also { provider ->
testTaskProvider.dependsOn(provider)
}
project.registerAndroidCreateBaselineTask(bootClasspath, extension, testVariant)
evaluatedProject.registerAndroidCreateBaselineTask(
bootClasspath,
extension,
testVariant
)
.also { provider ->
testBaselineTaskProvider.dependsOn(provider)
}

View File

@@ -25,8 +25,8 @@ internal class DetektMultiplatform(private val project: Project) {
private fun Project.registerMultiplatformTasks(extension: DetektExtension) {
// We need another project.afterEvaluate as the Android target is attached on
// a project.afterEvaluate inside AGP. We should further investigate and potentially remove this.
project.afterEvaluate {
val kmpExtension = project.extensions.getByType(KotlinMultiplatformExtension::class.java)
project.afterEvaluate { evaluatedProject ->
val kmpExtension = evaluatedProject.extensions.getByType(KotlinMultiplatformExtension::class.java)
kmpExtension.targets.all { target ->
target.compilations.forEach { compilation ->
@@ -36,9 +36,9 @@ internal class DetektMultiplatform(private val project: Project) {
val inputSource = compilation.kotlinSourceSets
.map { it.kotlin.sourceDirectories }
.fold(project.files() as FileCollection) { collection, next -> collection.plus(next) }
.fold(evaluatedProject.files() as FileCollection) { collection, next -> collection.plus(next) }
val detektTaskProvider = project.registerDetektTask(
val detektTaskProvider = evaluatedProject.registerDetektTask(
DetektPlugin.DETEKT_TASK_NAME + taskSuffix,
extension
) {
@@ -53,7 +53,7 @@ internal class DetektMultiplatform(private val project: Project) {
} else {
extension.baseline?.takeIf { it.exists() }
}?.let { baselineFile ->
baseline.set(layout.file(project.provider { baselineFile }))
baseline.set(layout.file(evaluatedProject.provider { baselineFile }))
}
reports = extension.reports
reports.xml.setDefaultIfUnset(File(extension.reportsDir, compilation.name + ".xml"))
@@ -69,7 +69,9 @@ internal class DetektMultiplatform(private val project: Project) {
tasks.matching { it.name == LifecycleBasePlugin.CHECK_TASK_NAME }
.configureEach { it.dependsOn(detektTaskProvider) }
project.registerCreateBaselineTask(DetektPlugin.BASELINE_TASK_NAME + taskSuffix, extension) {
evaluatedProject.registerCreateBaselineTask(
DetektPlugin.BASELINE_TASK_NAME + taskSuffix, extension
) {
setSource(inputSource)
if (runWithTypeResolution) {
classpath.setFrom(inputSource, compilation.compileDependencyFiles)
@@ -79,7 +81,7 @@ internal class DetektMultiplatform(private val project: Project) {
} else {
extension.baseline
}
baseline.set(project.layout.file(project.provider { variantBaselineFile }))
baseline.set(evaluatedProject.layout.file(evaluatedProject.provider { variantBaselineFile }))
description = "Creates detekt baseline for ${target.name} and source set ${compilation.name}"
if (runWithTypeResolution) {

View File

@@ -31,19 +31,14 @@ class ForbiddenClassName(config: Config = Config.empty) : Rule(config) {
}
override fun visitClassOrObject(classOrObject: KtClassOrObject) {
val name = classOrObject.name
val forbiddenEntries = name?.let { forbiddenName.filter { name.contains(it, ignoreCase = true) } }
val name = classOrObject.name ?: return
val forbiddenEntries = forbiddenName.filter { name.contains(it, ignoreCase = true) }
if (forbiddenEntries?.isNotEmpty() == true) {
var message = "Class name $name is forbidden as it contains:"
forbiddenEntries.forEach { message += " $it," }
message.trimEnd { it == ',' }
report(CodeSmell(issue, Entity.atName(classOrObject), message))
if (forbiddenEntries.isEmpty()) {
return
}
}
companion object {
const val FORBIDDEN_NAME = "forbiddenName"
val message = "Class name $name is forbidden as it contains: ${forbiddenEntries.joinToString(", ")}"
report(CodeSmell(issue, Entity.atName(classOrObject), message))
}
}

View File

@@ -1,12 +1,13 @@
package io.gitlab.arturbosch.detekt.rules.naming
import io.gitlab.arturbosch.detekt.rules.naming.ForbiddenClassName.Companion.FORBIDDEN_NAME
import io.gitlab.arturbosch.detekt.test.TestConfig
import io.gitlab.arturbosch.detekt.test.compileAndLint
import org.assertj.core.api.Assertions.assertThat
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
private const val FORBIDDEN_NAME = "forbiddenName"
class ForbiddenClassNameSpec : Spek({
describe("ForbiddenClassName rule") {
@@ -55,5 +56,13 @@ class ForbiddenClassNameSpec : Spek({
)
.hasSize(2)
}
it("should report all forbidden names in message") {
val code = """
class TestManager {}"""
val actual = ForbiddenClassName(TestConfig(mapOf(FORBIDDEN_NAME to "Test, Manager, Provider")))
.compileAndLint(code)
assertThat(actual.first().message)
.isEqualTo("Class name TestManager is forbidden as it contains: Test, Manager")
}
}
})