Add endColumn/endLine to SARIF region (#5011)

* Add endColumn/endLine to SARIF region * Add fixes Replace toSet() with distinct(), get rid of null check. * Get rid of 'mockk' * Fix line order
detekt · Jul 1, 2022 · b49aea7 · b49aea7
1 parent fbb913b
commit b49aea7
Show file tree

Hide file tree

Showing 3 changed files with 159 additions and 24 deletions.
diff --git a/detekt-report-sarif/build.gradle.kts b/detekt-report-sarif/build.gradle.kts
@@ -10,4 +10,5 @@ dependencies {
     testImplementation(projects.detektTestUtils)
     testImplementation(testFixtures(projects.detektApi))
     testImplementation(libs.assertj)
+    testImplementation(projects.detektTest)
 }
diff --git a/detekt-report-sarif/src/main/kotlin/io/github/detekt/report/sarif/Results.kt b/detekt-report-sarif/src/main/kotlin/io/github/detekt/report/sarif/Results.kt
@@ -23,26 +23,43 @@ private fun SeverityLevel.toResultLevel() = when (this) {
     SeverityLevel.INFO -> Level.Note
 }
 
-private fun Finding.toResult(ruleSetId: RuleSetId) = io.github.detekt.sarif4k.Result(
-    ruleID = "detekt.$ruleSetId.$id",
-    level = severity.toResultLevel(),
-    locations = (listOf(location) + references.map { it.location }).map(Location::toLocation).toSet().toList(),
-    message = Message(text = messageOrDescription())
-)
-
-private fun Location.toLocation() = io.github.detekt.sarif4k.Location(
-    physicalLocation = PhysicalLocation(
-        region = Region(
-            startLine = source.line.toLong(),
-            startColumn = source.column.toLong(),
-        ),
-        artifactLocation = if (filePath.relativePath != null) {
-            ArtifactLocation(
-                uri = filePath.relativePath?.toUnifiedString(),
-                uriBaseID = SRCROOT
-            )
-        } else {
-            ArtifactLocation(uri = filePath.absolutePath.toUnifiedString())
-        }
+private fun Finding.toResult(ruleSetId: RuleSetId): io.github.detekt.sarif4k.Result {
+    val code = entity.ktElement?.containingFile?.text
+
+    return io.github.detekt.sarif4k.Result(
+        ruleID = "detekt.$ruleSetId.$id",
+        level = severity.toResultLevel(),
+        locations = (listOf(location) + references.map { it.location }).map { it.toLocation(code) }.distinct().toList(),
+        message = Message(text = messageOrDescription())
+    )
+}
+
+private fun Location.toLocation(code: String?): io.github.detekt.sarif4k.Location {
+    var endLine: Long? = null
+    var endColumn: Long? = null
+
+    if (code != null) {
+        val snippet = code.take(text.end).split('\n')
+        endLine = snippet.size.toLong()
+        endColumn = snippet.last().length.toLong() + 1
+    }
+
+    return io.github.detekt.sarif4k.Location(
+        physicalLocation = PhysicalLocation(
+            region = Region(
+                startLine = source.line.toLong(),
+                startColumn = source.column.toLong(),
+                endLine = endLine,
+                endColumn = endColumn
+            ),
+            artifactLocation = if (filePath.relativePath != null) {
+                ArtifactLocation(
+                    uri = filePath.relativePath?.toUnifiedString(),
+                    uriBaseID = SRCROOT
+                )
+            } else {
+                ArtifactLocation(uri = filePath.absolutePath.toUnifiedString())
+            }
+        )
     )
-)
+}
diff --git a/detekt-report-sarif/src/test/kotlin/io/github/detekt/report/sarif/SarifOutputReportSpec.kt b/detekt-report-sarif/src/test/kotlin/io/github/detekt/report/sarif/SarifOutputReportSpec.kt
@@ -3,16 +3,26 @@ package io.github.detekt.report.sarif
 import io.github.detekt.test.utils.readResourceContent
 import io.github.detekt.tooling.api.VersionProvider
 import io.gitlab.arturbosch.detekt.api.CodeSmell
+import io.gitlab.arturbosch.detekt.api.Debt
+import io.gitlab.arturbosch.detekt.api.Entity
 import io.gitlab.arturbosch.detekt.api.Finding
+import io.gitlab.arturbosch.detekt.api.Issue
+import io.gitlab.arturbosch.detekt.api.Location
+import io.gitlab.arturbosch.detekt.api.Rule
+import io.gitlab.arturbosch.detekt.api.Severity
 import io.gitlab.arturbosch.detekt.api.SeverityLevel
+import io.gitlab.arturbosch.detekt.api.SourceLocation
+import io.gitlab.arturbosch.detekt.api.TextLocation
 import io.gitlab.arturbosch.detekt.api.UnstableApi
 import io.gitlab.arturbosch.detekt.api.internal.whichOS
 import io.gitlab.arturbosch.detekt.test.EmptySetupContext
 import io.gitlab.arturbosch.detekt.test.TestDetektion
+import io.gitlab.arturbosch.detekt.test.compileAndLint
 import io.gitlab.arturbosch.detekt.test.createEntity
 import io.gitlab.arturbosch.detekt.test.createFindingForRelativePath
 import io.gitlab.arturbosch.detekt.test.createIssue
 import org.assertj.core.api.Assertions.assertThat
+import org.jetbrains.kotlin.psi.KtClassOrObject
 import org.junit.jupiter.api.Test
 import java.nio.file.Paths
 
@@ -66,10 +76,117 @@ class SarifOutputReportSpec {
 
         assertThat(report).isEqualToIgnoringWhitespace(systemAwareExpectedReport)
     }
+
+    @Test
+    fun `region should be bounded with word`() {
+        /**
+         * Region constraints in Snippet for word 'TestClass'
+         *  @sample Snippet.code
+         */
+        val startLine = 3
+        val startColumn = 7
+        val endLine = 3
+        val endColumn = 15
+
+        val refEntity = TestRule().compileAndLint(Snippet.code).first().entity
+        val location = Location(
+            SourceLocation(startLine, startColumn),
+            TextLocation(
+                startLine + (startColumn - 1) * Snippet.lineLength,
+                endColumn + (endLine - 1) * Snippet.lineLength
+            ),
+            filePath = refEntity.location.filePath
+        )
+
+        val result = TestDetektion(
+            createFinding(
+                ruleName = "TestSmellB",
+                entity = refEntity.copy(location = location),
+                severity = SeverityLevel.WARNING
+            )
+        )
+
+        val report = SarifOutputReport()
+            .apply { init(EmptySetupContext()) }
+            .render(result)
+
+        assertThat(report)
+            .containsIgnoringWhitespaces(constrainRegion(startLine, startColumn, endLine, endColumn))
+    }
+
+    @Test
+    fun `region should be bounded with block`() {
+        /**
+         * Region constraints in Snippet for curly braces
+         *  @sample Snippet.code
+         */
+        val startLine = 3
+        val startColumn = 17
+        val endLine = 5
+        val endColumn = 1
+
+        val refEntity = TestRule().compileAndLint(Snippet.code).first().entity
+        val location = Location(
+            SourceLocation(startLine, startColumn),
+            TextLocation(
+                startLine + (startColumn - 1) * Snippet.lineLength,
+                endColumn + (endLine - 1) * Snippet.lineLength
+            ),
+            filePath = refEntity.location.filePath
+        )
+
+        val result = TestDetektion(
+            createFinding(
+                ruleName = "TestSmellB",
+                entity = refEntity.copy(location = location),
+                severity = SeverityLevel.WARNING
+            )
+        )
+
+        val report = SarifOutputReport()
+            .apply { init(EmptySetupContext()) }
+            .render(result)
+
+        assertThat(report)
+            .containsIgnoringWhitespaces(constrainRegion(startLine, startColumn, endLine, endColumn))
+    }
+}
+
+private object Snippet {
+    // Each line of code is 50 chars long
+    const val lineLength = 50
+    val code = """
+        // 4567890123456789012345678901234567890123456789
+        // 0000001111111111222222222233333333334444444444
+        class TestClass { ///////////////////////////////
+            val greeting: String = "Hello, World!"///////
+        }////////////////////////////////////////////////
+    """.trimIndent()
+}
+
+private fun constrainRegion(startLine: Int, startColumn: Int, endLine: Int, endColumn: Int) = """
+    "region": {
+      "endColumn": ${endColumn + 1},
+      "endLine": $endLine,
+      "startColumn": $startColumn,
+      "startLine": $startLine
+    }            
+"""
+
+class TestRule : Rule() {
+    override val issue = Issue(javaClass.simpleName, Severity.Warning, "", Debt.FIVE_MINS)
+
+    override fun visitClassOrObject(classOrObject: KtClassOrObject) {
+        report(CodeSmell(issue, Entity.atName(classOrObject), message = "Error"))
+    }
 }
 
-private fun createFinding(ruleName: String, severity: SeverityLevel): Finding {
-    return object : CodeSmell(createIssue(ruleName), createEntity("TestFile.kt"), "TestMessage") {
+private fun createFinding(
+    ruleName: String,
+    severity: SeverityLevel,
+    entity: Entity = createEntity("TestFile.kt")
+): Finding {
+    return object : CodeSmell(createIssue(ruleName), entity, "TestMessage") {
         override val severity: SeverityLevel
             get() = severity
     }