From 63dc501b8ca42d3f847ce0584eb376c15e08b1fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sat, 16 Oct 2021 00:36:04 +0200 Subject: [PATCH] Split InvalidPackageDeclaration to create MissingPackageDeclaration (#4149) --- .../main/resources/default-detekt-config.yml | 4 +- .../printer/defaultconfig/Exclusion.kt | 2 +- .../rules/bugs/MissingPackageDeclaration.kt | 38 +++++++++++++++++++ .../detekt/rules/bugs/PotentialBugProvider.kt | 1 + .../bugs/MissingPackageDeclarationSpec.kt | 31 +++++++++++++++ .../rules/naming/InvalidPackageDeclaration.kt | 32 ++++++---------- .../naming/InvalidPackageDeclarationSpec.kt | 8 ---- 7 files changed, 85 insertions(+), 31 deletions(-) create mode 100644 detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclaration.kt create mode 100644 detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclarationSpec.kt diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 9a262d956..764f9946a 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -447,7 +447,6 @@ naming: ignoreOverridden: true InvalidPackageDeclaration: active: false - excludes: ['**/*.kts'] rootPackage: '' MatchingDeclarationName: active: true @@ -556,6 +555,9 @@ potential-bugs: ignoreOnClassesPattern: '' MapGetWithNotNullAssertionOperator: active: false + MissingPackageDeclaration: + active: false + excludes: ['**/*.kts'] MissingWhenCase: active: true allowElseExpression: true diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt index 067b00129..53a42e4d5 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt +++ b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/Exclusion.kt @@ -48,7 +48,7 @@ private object TestExclusions : Exclusions() { private object KotlinScriptExclusions : Exclusions() { override val pattern = "['**/*.kts']" - override val rules = setOf("InvalidPackageDeclaration") + override val rules = setOf("MissingPackageDeclaration") } private object LibraryExclusions : Exclusions() { diff --git a/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclaration.kt b/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclaration.kt new file mode 100644 index 000000000..f2af0241d --- /dev/null +++ b/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclaration.kt @@ -0,0 +1,38 @@ +package io.gitlab.arturbosch.detekt.rules.bugs + +import io.gitlab.arturbosch.detekt.api.CodeSmell +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Debt +import io.gitlab.arturbosch.detekt.api.Entity +import io.gitlab.arturbosch.detekt.api.Issue +import io.gitlab.arturbosch.detekt.api.Rule +import io.gitlab.arturbosch.detekt.api.Severity +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.KtPackageDirective + +/** + * Reports when the package declaration is missing. + */ +class MissingPackageDeclaration(config: Config = Config.empty) : Rule(config) { + + override val issue = Issue( + javaClass.simpleName, + Severity.Maintainability, + "Kotlin source files should define a package", + debt = Debt.FIVE_MINS + ) + + private var packageDeclaration: KtPackageDirective? = null + + override fun visitPackageDirective(directive: KtPackageDirective) { + super.visitPackageDirective(directive) + packageDeclaration = directive + } + + override fun postVisit(root: KtFile) { + super.postVisit(root) + if (packageDeclaration?.text.isNullOrBlank()) { + report(CodeSmell(issue, Entity.from(root), "The file does not contain a package declaration.")) + } + } +} diff --git a/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt b/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt index 858be7c57..8e0f255dc 100644 --- a/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt +++ b/detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt @@ -32,6 +32,7 @@ class PotentialBugProvider : DefaultRuleSetProvider { IteratorNotThrowingNoSuchElementException(config), LateinitUsage(config), MapGetWithNotNullAssertionOperator(config), + MissingPackageDeclaration(config), MissingWhenCase(config), RedundantElseInWhen(config), UnconditionalJumpStatementInLoop(config), diff --git a/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclarationSpec.kt b/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclarationSpec.kt new file mode 100644 index 000000000..78cce479b --- /dev/null +++ b/detekt-rules-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingPackageDeclarationSpec.kt @@ -0,0 +1,31 @@ +package io.gitlab.arturbosch.detekt.rules.bugs + +import io.gitlab.arturbosch.detekt.test.assertThat +import io.gitlab.arturbosch.detekt.test.compileAndLint +import org.spekframework.spek2.Spek +import org.spekframework.spek2.style.specification.describe + +internal class MissingPackageDeclarationSpec : Spek({ + + describe("MissingPackageDeclaration rule") { + + it("should pass if package declaration is declared") { + val code = """ + package foo.bar + + class C + """ + val findings = MissingPackageDeclaration().compileAndLint(code) + + assertThat(findings).isEmpty() + } + + it("should report if package declaration is missing") { + val code = "class C" + + val findings = MissingPackageDeclaration().compileAndLint(code) + + assertThat(findings).hasSize(1) + } + } +}) diff --git a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclaration.kt b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclaration.kt index ce5102e41..1b6ff531c 100644 --- a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclaration.kt +++ b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclaration.kt @@ -11,11 +11,10 @@ import io.gitlab.arturbosch.detekt.api.Severity import io.gitlab.arturbosch.detekt.api.config import io.gitlab.arturbosch.detekt.api.internal.Configuration import org.jetbrains.kotlin.psi.KtElement -import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.KtPackageDirective /** - * Reports when the package declaration is missing or the file location does not match the declared package. + * Reports when the file location does not match the declared package. */ class InvalidPackageDeclaration(config: Config = Config.empty) : Rule(config) { @@ -29,21 +28,11 @@ class InvalidPackageDeclaration(config: Config = Config.empty) : Rule(config) { @Configuration("if specified this part of the package structure is ignored") private val rootPackage: String by config("") - private var packageDeclaration: KtPackageDirective? = null - override fun visitPackageDirective(directive: KtPackageDirective) { super.visitPackageDirective(directive) - packageDeclaration = directive - } - - override fun postVisit(root: KtFile) { - super.postVisit(root) - val packageDeclaration = packageDeclaration - val declaredPath = packageDeclaration?.packageNames?.map(KtElement::getText)?.toNormalizedForm() - if (declaredPath.isNullOrBlank()) { - root.reportInvalidPackageDeclaration("The file does not contain a package declaration.") - } else { - val normalizedFilePath = root.absolutePath().parent.toNormalizedForm() + val declaredPath = directive.packageNames.map(KtElement::getText).toNormalizedForm() + if (declaredPath.isNotBlank()) { + val normalizedFilePath = directive.containingKtFile.absolutePath().parent.toNormalizedForm() val normalizedRootPackage = packageNameToNormalizedForm(rootPackage) val expectedPath = if (normalizedRootPackage.isBlank()) { @@ -54,16 +43,17 @@ class InvalidPackageDeclaration(config: Config = Config.empty) : Rule(config) { val isInRootPackage = expectedPath.isBlank() if (!isInRootPackage && !normalizedFilePath.endsWith(expectedPath)) { - packageDeclaration - .reportInvalidPackageDeclaration("The package declaration does not match the actual file location.") + report( + CodeSmell( + issue, + Entity.from(directive), + "The package declaration does not match the actual file location.", + ) + ) } } } - private fun KtElement.reportInvalidPackageDeclaration(message: String) { - report(CodeSmell(issue, Entity.from(this), message)) - } - private fun Iterable.toNormalizedForm() = joinToString("|") private fun packageNameToNormalizedForm(packageName: String) = packageName.split('.').toNormalizedForm() diff --git a/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclarationSpec.kt b/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclarationSpec.kt index 3dcaf8e97..4b2475fa2 100644 --- a/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclarationSpec.kt +++ b/detekt-rules-naming/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/naming/InvalidPackageDeclarationSpec.kt @@ -28,14 +28,6 @@ internal class InvalidPackageDeclarationSpec : Spek({ assertThat(findings).isEmpty() } - it("should report if package declaration is missing") { - val ktFile = compileContentForTest("class C") - - val findings = InvalidPackageDeclaration().lint(ktFile) - - assertThat(findings).hasSize(1) - } - it("should report if package declaration does not match source location") { val source = "package foo\n\nclass C"