Entity.KtElement is not longer nullable

detekt · May 8, 2024 · 726c394 · 726c394
1 parent e79ba7f
commit 726c394
Show file tree

Hide file tree

Showing 16 changed files with 34 additions and 36 deletions.
diff --git a/detekt-api/build.gradle.kts b/detekt-api/build.gradle.kts
@@ -12,6 +12,7 @@ dependencies {
 
     testImplementation(projects.detektTest)
     testImplementation(libs.assertj)
+    testFixturesImplementation(projects.detektTestUtils)
 }
 
 detekt {

diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Entity.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Entity.kt
@@ -15,7 +15,7 @@ class Entity(
     val name: String,
     val signature: String,
     val location: Location,
-    val ktElement: KtElement?
+    val ktElement: KtElement
 ) : Compactable {
 
     override fun compact(): String = "[$name] at ${location.compact()}"

diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/CodeSmellSpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/CodeSmellSpec.kt
@@ -1,5 +1,6 @@
 package io.gitlab.arturbosch.detekt.api
 
+import io.github.detekt.test.utils.internal.FakeKtElement
 import io.gitlab.arturbosch.detekt.test.location
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
@@ -20,16 +21,15 @@ class CodeSmellSpec {
                     text = TextLocation(0, 0),
                     path = Path("/").absolute().resolve("Users/tester/detekt/TestFile.kt"),
                 ),
-                ktElement = null
+                ktElement = FakeKtElement()
             ),
             message = "TestMessage"
         )
 
         assertThat(codeSmell.toString()).isEqualTo(
             "CodeSmell(entity=Entity(name=TestEntity, signature=TestEntitySignature, " +
-                "location=Location(source=1:1, endSource=1:1, text=0:0, " +
-                "path=${codeSmell.location.path}), ktElement=null), message=TestMessage, " +
-                "references=[])"
+                "location=Location(source=1:1, endSource=1:1, text=0:0, path=${codeSmell.location.path})," +
+                "ktElement=FakeKtElement), message=TestMessage, references=[])"
         )
     }
 }
diff --git a/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/CorrectableCodeSmellSpec.kt b/detekt-api/src/test/kotlin/io/gitlab/arturbosch/detekt/api/CorrectableCodeSmellSpec.kt
@@ -19,7 +19,7 @@ class CorrectableCodeSmellSpec {
             "CorrectableCodeSmell(autoCorrectEnabled=true, " +
                 "entity=Entity(name=TestEntity, signature=TestEntitySignature, " +
                 "location=Location(source=1:1, endSource=1:1, text=0:0, " +
-                "path=${codeSmell.location.path}), ktElement=null), " +
+                "path=${codeSmell.location.path}), ktElement=FakeKtElement), " +
                 "message=TestMessage, references=[])"
         )
     }

diff --git a/detekt-api/src/testFixtures/kotlin/io/gitlab/arturbosch/detekt/test/TestFactory.kt b/detekt-api/src/testFixtures/kotlin/io/gitlab/arturbosch/detekt/test/TestFactory.kt
@@ -1,5 +1,6 @@
 package io.gitlab.arturbosch.detekt.test
 
+import io.github.detekt.test.utils.internal.FakeKtElement
 import io.gitlab.arturbosch.detekt.api.Entity
 import io.gitlab.arturbosch.detekt.api.Issue
 import io.gitlab.arturbosch.detekt.api.Location
@@ -86,7 +87,7 @@ fun createIssueForRelativePath(
 fun createEntity(
     signature: String = "TestEntitySignature",
     location: Location = createLocation(),
-    ktElement: KtElement? = null,
+    ktElement: KtElement = FakeKtElement(),
 ) = Entity(
     name = "TestEntity",
     signature = signature,

diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt
@@ -116,7 +116,7 @@ internal class Analyzer(
         return (correctableRules + otherRules).flatMap { (ruleInfo, rule) ->
             rule.visitFile(file, bindingContext, compilerResources)
                 .filterNot {
-                    it.entity.ktElement?.isSuppressedBy(ruleInfo.id, rule.aliases, ruleInfo.ruleSetId) == true
+                    it.entity.ktElement.isSuppressedBy(ruleInfo.id, rule.aliases, ruleInfo.ruleSetId)
                 }
                 .filterSuppressedFindings(rule, bindingContext)
                 .map { it.toIssue(ruleInfo, rule.computeSeverity()) }

diff --git a/...core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/AnnotationSuppressor.kt b/...core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/AnnotationSuppressor.kt
@@ -21,8 +21,7 @@ internal fun annotationSuppressorFactory(rule: Rule, bindingContext: BindingCont
     return if (annotations.isNotEmpty()) {
         Suppressor { finding ->
             val element = finding.entity.ktElement
-            element != null &&
-                element.isAnnotatedWith(AnnotationExcluder(element.containingKtFile, annotations, bindingContext))
+            element.isAnnotatedWith(AnnotationExcluder(element.containingKtFile, annotations, bindingContext))
         }
     } else {
         null

diff --git a/...t-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/FunctionSuppressor.kt b/...t-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/FunctionSuppressor.kt
@@ -25,8 +25,7 @@ internal fun functionSuppressorFactory(rule: Rule, bindingContext: BindingContex
         .map(FunctionMatcher::fromFunctionSignature)
     return if (functionMatchers.isNotEmpty()) {
         Suppressor { finding ->
-            val element = finding.entity.ktElement
-            element != null && functionSuppressor(element, bindingContext, functionMatchers)
+            functionSuppressor(finding.entity.ktElement, bindingContext, functionMatchers)
         }
     } else {
         null

diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/Builders.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/Builders.kt
@@ -1,5 +1,6 @@
 package io.gitlab.arturbosch.detekt.core.suppressors
 
+import io.github.detekt.test.utils.internal.FakeKtElement
 import io.gitlab.arturbosch.detekt.api.CodeSmell
 import io.gitlab.arturbosch.detekt.api.Entity
 import io.gitlab.arturbosch.detekt.api.Finding
@@ -17,7 +18,7 @@ private fun buildEmptyEntity(): Entity = Entity(
     name = "",
     signature = "",
     location = createLocation(""),
-    ktElement = null,
+    ktElement = FakeKtElement(),
 )
 
 internal fun buildRule(

diff --git a/detekt-report-html/src/main/kotlin/io/github/detekt/report/html/HtmlOutputReport.kt b/detekt-report-html/src/main/kotlin/io/github/detekt/report/html/HtmlOutputReport.kt
@@ -169,11 +169,9 @@ class HtmlOutputReport : BuiltInOutputReport, OutputReport() {
             span("message") { text(issue.message) }
         }
 
-        val psiFile = issue.entity.ktElement?.containingFile
-        if (psiFile != null) {
-            val lineSequence = psiFile.text.splitToSequence('\n')
-            snippetCode(issue.ruleInfo.id, lineSequence, issue.location.source, issue.location.text.length())
-        }
+        val psiFile = issue.entity.ktElement.containingFile
+        val lineSequence = psiFile.text.splitToSequence('\n')
+        snippetCode(issue.ruleInfo.id, lineSequence, issue.location.source, issue.location.text.length())
     }
 
     private fun getComplexityMetrics(detektion: Detektion): List<String> {

diff --git a/detekt-report-md/src/main/kotlin/io/github/detekt/report/md/MdOutputReport.kt b/detekt-report-md/src/main/kotlin/io/github/detekt/report/md/MdOutputReport.kt
@@ -155,13 +155,9 @@ private fun MarkdownContent.renderIssue(issue: Issue, basePath: Path?): String {
         ""
     }
 
-    val psiFile = issue.entity.ktElement?.containingFile
-    val snippet = if (psiFile != null) {
-        val lineSequence = psiFile.text.splitToSequence('\n')
-        snippetCode(lineSequence, issue.location.source)
-    } else {
-        ""
-    }
+    val psiFile = issue.entity.ktElement.containingFile
+    val lineSequence = psiFile.text.splitToSequence('\n')
+    val snippet = snippetCode(lineSequence, issue.location.source)
 
     return "$location\n$message\n$snippet"
 }

diff --git a/...-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/VarCouldBeValSpec.kt b/...-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/VarCouldBeValSpec.kt
@@ -261,7 +261,7 @@ class VarCouldBeValSpec(val env: KotlinCoreEnvironment) {
 
             assertThat(lint).hasSize(1)
             with(lint[0].entity) {
-                assertThat(ktElement?.text).isEqualTo("var shadowed = 1")
+                assertThat(ktElement.text).isEqualTo("var shadowed = 1")
             }
         }
     }
@@ -307,7 +307,7 @@ class VarCouldBeValSpec(val env: KotlinCoreEnvironment) {
                 }
             """.trimIndent()
             with(subject.compileAndLintWithContext(env, code)[0]) {
-                assertThat(entity.ktElement?.text).isEqualTo("var myVar = value")
+                assertThat(entity.ktElement.text).isEqualTo("var myVar = value")
             }
         }
     }

diff --git a/.../test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesLoopsSpec.kt b/.../test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesLoopsSpec.kt
@@ -61,7 +61,7 @@ class MandatoryBracesLoopsSpec {
             val findings = subject.compileAndLint(code)
 
             assertThat(findings).hasSize(1)
-            assertThat(findings[0].entity.ktElement?.text).isEqualTo("println(i)")
+            assertThat(findings[0].entity.ktElement.text).isEqualTo("println(i)")
         }
 
         @Test
@@ -234,7 +234,7 @@ class MandatoryBracesLoopsSpec {
             val findings = subject.compileAndLint(code)
 
             assertThat(findings).hasSize(1)
-            assertThat(findings[0].entity.ktElement?.text).isEqualTo("println()")
+            assertThat(findings[0].entity.ktElement.text).isEqualTo("println()")
         }
 
         @Test
@@ -307,7 +307,7 @@ class MandatoryBracesLoopsSpec {
             val findings = subject.compileAndLint(code)
 
             assertThat(findings).hasSize(1)
-            assertThat(findings[0].entity.ktElement?.text).isEqualTo("println()")
+            assertThat(findings[0].entity.ktElement.text).isEqualTo("println()")
         }
 
         @Test

diff --git a/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/internal/FakeKtElement.kt b/detekt-test-utils/src/main/kotlin/io/github/detekt/test/utils/internal/FakeKtElement.kt
@@ -243,4 +243,8 @@ class FakeKtElement(private val psiFile: PsiFile = FakePsiFile("")) : KtElement
     override fun textMatches(p0: PsiElement): Boolean = false
 
     override fun textToCharArray(): CharArray = "".toCharArray()
+
+    override fun toString(): String {
+        return "FakeKtElement"
+    }
 }
diff --git a/detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/FindingsAssertions.kt b/detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/FindingsAssertions.kt
@@ -85,11 +85,12 @@ class FindingsAssert(actual: List<Finding>) :
                 return this
             } else {
                 failWithMessage("Expected ${expected.size} findings but was 0")
+                // This should never execute. `failWithMessage` always throws an exception but the kotlin compiled
+                // doesn't know that. So this line below helps it.
+                error("This should never execute, if you find this please open an issue with a reproducer")
             }
         }
-        val code = requireNotNull(finding?.entity?.ktElement?.run { containingKtFile.text }) {
-            "Finding expected to provide a KtElement."
-        }
+        val code = finding.entity.ktElement.containingKtFile.text
 
         val textLocations = expected.map { snippet ->
             val index = code.indexOf(snippet)

diff --git a/detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/RuleExtensions.kt b/detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/RuleExtensions.kt
@@ -65,7 +65,5 @@ fun Rule.compileAndLintWithContext(
 fun Rule.lint(ktFile: KtFile): List<Finding> = visitFile(ktFile).filterSuppressed(this)
 
 private fun List<Finding>.filterSuppressed(rule: Rule): List<Finding> {
-    return filterNot {
-        it.entity.ktElement?.isSuppressedBy(rule.ruleId, rule.aliases, RuleSet.Id("NoARuleSetId")) == true
-    }
+    return filterNot { it.entity.ktElement.isSuppressedBy(rule.ruleId, rule.aliases, RuleSet.Id("NoARuleSetId")) }
 }