Repository: detekt/detekt (kotlin)

Issue:
Refactor how we handle `Baseline`
The code that handles the baseline feature is a bit messy. We have 3 different files that could be simplified in only one:

- `BaselineResultMapping`
- `BaselineFilteredResult`
- `BaselineFacade`

In all that code we are using the concept of `Detektion` that we don't need at all. My idea would be to merge all the logic in one single file `BaselineResultMapping` and rewrite the tests to check the cases. They aren't that much so we can take that as a single unit.

Pull request:
Merge `BaselineResultMapping` `BaselineFilteredResult` and `BaselineFacade` into `BaselineResultMapping` and also merge corresponding test specs as described here #6882.

Close #6882

Patch:
diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacade.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacade.kt
deleted file mode 100644
index be0125f56fba..000000000000
--- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacade.kt
+++ /dev/null
@@ -1,36 +0,0 @@
-package io.gitlab.arturbosch.detekt.core.baseline
-
-import io.gitlab.arturbosch.detekt.api.Detektion
-import io.gitlab.arturbosch.detekt.api.Issue
-import java.nio.file.Path
-import kotlin.io.path.createDirectories
-import kotlin.io.path.exists
-import kotlin.io.path.isRegularFile
-
-class BaselineFacade {
-
-    fun transformResult(baselineFile: Path, result: Detektion): Detektion {
-        return if (baselineExists(baselineFile)) {
-            BaselineFilteredResult(result, DefaultBaseline.load(baselineFile))
-        } else {
-            result
-        }
-    }
-
-    fun createOrUpdate(baselineFile: Path, issues: List<Issue>) {
-        val ids = issues.map { it.baselineId }.toSortedSet()
-        val oldBaseline = if (baselineExists(baselineFile)) {
-            DefaultBaseline.load(baselineFile)
-        } else {
-            DefaultBaseline(emptySet(), emptySet())
-        }
-        val baselineFormat = BaselineFormat()
-        val baseline = baselineFormat.of(oldBaseline.manuallySuppressedIssues, ids)
-        if (oldBaseline != baseline) {
-            baselineFile.parent?.createDirectories()
-            baselineFormat.write(baselineFile, baseline)
-        }
-    }
-
-    private fun baselineExists(baseline: Path) = baseline.exists() && baseline.isRegularFile()
-}
diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResult.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResult.kt
deleted file mode 100644
index 4b831fad2c3f..000000000000
--- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResult.kt
+++ /dev/null
@@ -1,13 +0,0 @@
-package io.gitlab.arturbosch.detekt.core.baseline
-
-import io.github.detekt.tooling.api.Baseline
-import io.gitlab.arturbosch.detekt.api.Detektion
-import io.gitlab.arturbosch.detekt.api.Issue
-
-internal class BaselineFilteredResult(
-    result: Detektion,
-    private val baseline: Baseline,
-) : Detektion by result {
-
-    override val issues: List<Issue> = result.issues.filterNot { baseline.contains(it.baselineId) }
-}
diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMapping.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMapping.kt
index 1094b46748c0..eb13956fc4e8 100644
--- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMapping.kt
+++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMapping.kt
@@ -4,8 +4,10 @@ import io.gitlab.arturbosch.detekt.api.Issue
 import io.gitlab.arturbosch.detekt.api.ReportingExtension
 import io.gitlab.arturbosch.detekt.api.SetupContext
 import io.gitlab.arturbosch.detekt.api.getOrNull
-import io.gitlab.arturbosch.detekt.core.DetektResult
 import java.nio.file.Path
+import kotlin.io.path.createDirectories
+import kotlin.io.path.exists
+import kotlin.io.path.isRegularFile
 
 class BaselineResultMapping : ReportingExtension {
 
@@ -31,12 +33,36 @@ class BaselineResultMapping : ReportingExtension {
     private fun List<Issue>.transformWithBaseline(
         baselinePath: Path,
     ): List<Issue> {
-        val facade = BaselineFacade()
-
         if (createBaseline) {
-            facade.createOrUpdate(baselinePath, this)
+            createOrUpdate(baselinePath, this)
         }
 
-        return facade.transformResult(baselinePath, DetektResult(this)).issues
+        return filterByBaseline(baselinePath, this)
     }
+
+    fun filterByBaseline(baselineFile: Path, issues: List<Issue>): List<Issue> {
+        return if (baselineExists(baselineFile)) {
+            val baseline = DefaultBaseline.load(baselineFile)
+            issues.filterNot { baseline.contains(it.baselineId) }
+        } else {
+            issues
+        }
+    }
+
+    fun createOrUpdate(baselineFile: Path, issues: List<Issue>) {
+        val ids = issues.map { it.baselineId }.toSortedSet()
+        val oldBaseline = if (baselineExists(baselineFile)) {
+            DefaultBaseline.load(baselineFile)
+        } else {
+            DefaultBaseline(emptySet(), emptySet())
+        }
+        val baselineFormat = BaselineFormat()
+        val baseline = baselineFormat.of(oldBaseline.manuallySuppressedIssues, ids)
+        if (oldBaseline != baseline) {
+            baselineFile.parent?.createDirectories()
+            baselineFormat.write(baselineFile, baseline)
+        }
+    }
+
+    private fun baselineExists(baseline: Path) = baseline.exists() && baseline.isRegularFile()
 }


Test patch:
diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacadeSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacadeSpec.kt
deleted file mode 100644
index dbc2c1646b58..000000000000
--- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacadeSpec.kt
+++ /dev/null
@@ -1,86 +0,0 @@
-package io.gitlab.arturbosch.detekt.core.baseline
-
-import io.github.detekt.test.utils.createTempDirectoryForTest
-import io.github.detekt.test.utils.resourceAsPath
-import io.gitlab.arturbosch.detekt.test.TestDetektion
-import io.gitlab.arturbosch.detekt.test.createIssue
-import org.assertj.core.api.Assertions.assertThat
-import org.junit.jupiter.api.AfterEach
-import org.junit.jupiter.api.Test
-import kotlin.io.path.copyTo
-import kotlin.io.path.deleteIfExists
-
-class BaselineFacadeSpec {
-
-    private val baselineFile = createTempDirectoryForTest("baseline_format").resolve("baseline.xml")
-    private val validBaseline = resourceAsPath("/baseline_feature/valid-baseline.xml")
-
-    @AfterEach
-    fun tearDown() {
-        baselineFile.deleteIfExists()
-    }
-
-    @Test
-    fun `returns a BaselineFilteredResult when the baseline exists`() {
-        val detektion = BaselineFacade().transformResult(validBaseline, TestDetektion())
-
-        assertThat(detektion).isInstanceOf(BaselineFilteredResult::class.java)
-    }
-
-    @Test
-    fun `returns the same detektion when the baseline doesn't exist`() {
-        val initialDetektion = TestDetektion()
-        val detektion = BaselineFacade().transformResult(baselineFile, initialDetektion)
-
-        assertThat(detektion).isEqualTo(initialDetektion)
-    }
-
-    @Test
-    fun `doesn't create a baseline file without issues`() {
-        BaselineFacade().createOrUpdate(baselineFile, emptyList())
-
-        assertThat(baselineFile).doesNotExist()
-    }
-
-    @Test
-    fun `creates on top of an existing a baseline file without issues`() {
-        validBaseline.copyTo(baselineFile)
-
-        BaselineFacade().createOrUpdate(baselineFile, emptyList())
-
-        assertThat(baselineFile).hasContent(
-            """
-                <?xml version="1.0" ?>
-                <SmellBaseline>
-                  <ManuallySuppressedIssues>
-                    <ID>LongParameterList:Signature</ID>
-                    <ID>LongMethod:Signature</ID>
-                  </ManuallySuppressedIssues>
-                  <CurrentIssues></CurrentIssues>
-                </SmellBaseline>
-            """.trimIndent()
-        )
-    }
-
-    @Test
-    fun `creates on top of an existing a baseline file with issues`() {
-        validBaseline.copyTo(baselineFile)
-
-        BaselineFacade().createOrUpdate(baselineFile, listOf(createIssue()))
-
-        assertThat(baselineFile).hasContent(
-            """
-                <?xml version="1.0" ?>
-                <SmellBaseline>
-                  <ManuallySuppressedIssues>
-                    <ID>LongParameterList:Signature</ID>
-                    <ID>LongMethod:Signature</ID>
-                  </ManuallySuppressedIssues>
-                  <CurrentIssues>
-                    <ID>TestSmell:TestEntitySignature</ID>
-                  </CurrentIssues>
-                </SmellBaseline>
-            """.trimIndent()
-        )
-    }
-}
diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResultSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResultSpec.kt
deleted file mode 100644
index 497b449d0d61..000000000000
--- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResultSpec.kt
+++ /dev/null
@@ -1,32 +0,0 @@
-package io.gitlab.arturbosch.detekt.core.baseline
-
-import io.github.detekt.test.utils.resourceAsPath
-import io.gitlab.arturbosch.detekt.test.TestDetektion
-import io.gitlab.arturbosch.detekt.test.createEntity
-import io.gitlab.arturbosch.detekt.test.createIssue
-import org.assertj.core.api.Assertions.assertThat
-import org.junit.jupiter.api.Test
-
-class BaselineFilteredResultSpec {
-
-    private val baselineFile = resourceAsPath("/baseline_feature/valid-baseline.xml")
-
-    private val result = TestDetektion(
-        createIssue("LongParameterList", createEntity(signature = "Signature")),
-        createIssue("LongMethod", createEntity(signature = "Signature")),
-        createIssue("FeatureEnvy", createEntity(signature = "Signature")),
-    )
-
-    @Test
-    fun `does return the same issue on empty baseline`() {
-        val actual = BaselineFilteredResult(result, DefaultBaseline(emptySet(), emptySet()))
-        assertThat(actual.issues).hasSize(3)
-    }
-
-    @Test
-    fun `filters with an existing baseline file`() {
-        val baseline = DefaultBaseline.load(baselineFile)
-        val actual = BaselineFilteredResult(result, baseline)
-        assertThat(actual.issues).isEmpty()
-    }
-}
diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMappingSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMappingSpec.kt
index ee8b4604db17..4e1e01fb6c98 100644
--- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMappingSpec.kt
+++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMappingSpec.kt
@@ -26,7 +26,19 @@ class BaselineResultMappingSpec {
         createIssue(
             ruleInfo = createRuleInfo("SomeIssueId", "RuleSet"),
             entity = createEntity(signature = "SomeSignature"),
-        )
+        ),
+        createIssue(
+            ruleName = "LongParameterList",
+            entity = createEntity(signature = "Signature")
+        ),
+        createIssue(
+            ruleName = "LongMethod",
+            entity = createEntity(signature = "Signature")
+        ),
+        createIssue(
+            ruleName = "FeatureEnvy",
+            entity = createEntity(signature = "Signature")
+        ),
     )
 
     @AfterEach
@@ -112,6 +124,94 @@ class BaselineResultMappingSpec {
         val changed = DefaultBaseline.load(baselineFile)
         assertThat(existing).isNotEqualTo(changed)
     }
+
+    @Test
+    fun `returns a filtered issues list when the baseline exists`() {
+        existingBaselineFile.copyTo(baselineFile)
+
+        val mapping = resultMapping(
+            baselineFile = baselineFile,
+            createBaseline = true,
+        )
+
+        val filtered = mapping.filterByBaseline(baselineFile, issues)
+
+        assertThat(issues).isNotEqualTo(filtered)
+    }
+
+    @Test
+    fun `returns the same issues list when the baseline doesn't exist`() {
+        val mapping = resultMapping(
+            baselineFile = baselineFile,
+            createBaseline = false,
+        )
+
+        val filtered = mapping.filterByBaseline(baselineFile, issues)
+
+        assertThat(issues).isEqualTo(filtered)
+    }
+
+    @Test
+    fun `doesn't create a baseline file without issues`() {
+        val mapping = resultMapping(
+            baselineFile = baselineFile,
+            createBaseline = false,
+        )
+
+        mapping.createOrUpdate(baselineFile, emptyList())
+
+        assertThat(baselineFile).doesNotExist()
+    }
+
+    @Test
+    fun `creates on top of an existing a baseline file without issues`() {
+        existingBaselineFile.copyTo(baselineFile)
+        val mapping = resultMapping(
+            baselineFile = baselineFile,
+            createBaseline = true,
+        )
+
+        mapping.createOrUpdate(baselineFile, emptyList())
+
+        assertThat(baselineFile).hasContent(
+            """
+                <?xml version="1.0" ?>
+                <SmellBaseline>
+                  <ManuallySuppressedIssues>
+                    <ID>LongParameterList:Signature</ID>
+                    <ID>LongMethod:Signature</ID>
+                  </ManuallySuppressedIssues>
+                  <CurrentIssues></CurrentIssues>
+                </SmellBaseline>
+            """.trimIndent()
+        )
+    }
+
+    @Test
+    fun `creates on top of an existing a baseline file with issues`() {
+        existingBaselineFile.copyTo(baselineFile)
+        val mapping = resultMapping(
+            baselineFile = baselineFile,
+            createBaseline = true,
+        )
+
+        mapping.createOrUpdate(baselineFile, listOf(createIssue()))
+
+        assertThat(baselineFile).hasContent(
+            """
+                <?xml version="1.0" ?>
+                <SmellBaseline>
+                  <ManuallySuppressedIssues>
+                    <ID>LongParameterList:Signature</ID>
+                    <ID>LongMethod:Signature</ID>
+                  </ManuallySuppressedIssues>
+                  <CurrentIssues>
+                    <ID>TestSmell:TestEntitySignature</ID>
+                  </CurrentIssues>
+                </SmellBaseline>
+            """.trimIndent()
+        )
+    }
 }
 
 private fun resultMapping(baselineFile: Path?, createBaseline: Boolean?) =