Xml Report Merger now merges duplicate smells across input report files

detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMerger.kt

@@ -2,6 +2,7 @@ package io.gitlab.arturbosch.detekt.report
 
 import org.w3c.dom.Document
 import org.w3c.dom.Node
+import org.w3c.dom.NodeList
 import java.io.File
 import javax.xml.parsers.DocumentBuilderFactory
 import javax.xml.transform.OutputKeys
@@ -16,42 +17,133 @@ object XmlReportMerger {
 
     private val documentBuilder by lazy { DocumentBuilderFactory.newInstance().newDocumentBuilder() }
 
-    fun merge(inputs: Collection<File>, output: File) {
-        val document = documentBuilder.newDocument().apply {
-            xmlStandalone = true
-            val checkstyleNode = createElement("checkstyle")
-            checkstyleNode.setAttribute("version", "4.3")
-            appendChild(checkstyleNode)
-        }
-        inputs.filter { it.exists() }.forEach {
-            importNodesFromInput(it, document)
-        }
+    fun merge(reportFiles: Collection<File>, output: File) {
+        val distinctErrorsBySourceFile = DetektCheckstyleReports(reportFiles)
+            .parseCheckstyleToSourceFileNodes()
+            .distinctErrorsGroupedBySourceFile()
+
+        val mergedCheckstyle = createMergedCheckstyle(distinctErrorsBySourceFile)
+
         TransformerFactory.newInstance().newTransformer().run {
             setOutputProperty(OutputKeys.INDENT, "yes")
             setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2")
-            transform(DOMSource(document), StreamResult(output.writer()))
+            transform(DOMSource(mergedCheckstyle), StreamResult(output.writer()))
         }
     }
 
-    private fun importNodesFromInput(input: File, document: Document) {
-        val checkstyleNode = documentBuilder.parse(input.inputStream()).documentElement.also { removeWhitespaces(it) }
-        (0 until checkstyleNode.childNodes.length).forEach {
-            val node = checkstyleNode.childNodes.item(it)
-            document.documentElement.appendChild(document.importNode(node, true))
+    private fun createMergedCheckstyle(distinctErrorsBySourceFile: Map<String, List<Node>>): Document {
+        val mergedDocument = documentBuilder.newDocument().apply {
+            xmlStandalone = true
         }
+        val mergedCheckstyleNode = mergedDocument.createElement("checkstyle")
+        mergedCheckstyleNode.setAttribute("version", "4.3")
+        mergedDocument.appendChild(mergedCheckstyleNode)
+
+        distinctErrorsBySourceFile.forEach { (fileName, errorNodes) ->
+            mergedCheckstyleNode.appendChild(
+                mergedDocument.createElement("file").apply {
+                    setAttribute("name", fileName)
+                    errorNodes.forEach {
+                        appendChild(mergedDocument.importNode(it, true))
+                    }
+                }
+            )
+        }
+        return mergedDocument
+    }
+
+    /** A list of checkstyle xml files written by Detekt */
+    private class DetektCheckstyleReports(private val files: Collection<File>) {
+
+        /**
+         * Parses a list of `file` nodes matching the following topology
+         *
+         * ```xml
+         * <checkstyle>
+         *   <file/>
+         * </checkstyle>
+         * ```
+         *
+         * @see CheckstyleSourceFileNodes
+         */
+        fun parseCheckstyleToSourceFileNodes() =
+            CheckstyleSourceFileNodes(
+                files.filter { reportFile -> reportFile.exists() }
+                    .flatMap { existingReportFile ->
+                        val checkstyleNode = documentBuilder.parse(existingReportFile.inputStream())
+                        val sourceFileNodes = checkstyleNode.documentElement.childNodes.asSequence().filterWhitespace()
+                        sourceFileNodes
+                    }
+            )
     }
 
     /**
-     * Use code instead of XSLT to exclude whitespaces.
+     * A list of checkstyle `file` nodes that may contain 0 to many `error` nodes
+     *
+     * ```xml
+     * <file>
+     *     <error>
+     * </file>
+     * ```
      */
-    private fun removeWhitespaces(node: Node) {
-        (node.childNodes.length - 1 downTo 0).forEach { idx ->
-            val childNode = node.childNodes.item(idx)
-            if (childNode.nodeType == Node.TEXT_NODE && childNode.textContent.isBlank()) {
-                node.removeChild(childNode)
+    private class CheckstyleSourceFileNodes(private val nodes: List<Node>) {
+
+        /** Returns a map containing only distinct error nodes, grouped by file name */
+        fun distinctErrorsGroupedBySourceFile() = nodes
+            .flatMap { fileNode ->
+                val fileNameAttribute = fileNode.attributes.getNamedItem("name").nodeValue
+                val errorNodes = fileNode.childNodes.asSequence().filterWhitespace()
+                errorNodes.map { errorNode ->
+                    CheckstyleErrorNodeWithFileData(
+                        errorID = errorID(fileNameAttribute, errorNode),
+                        fileName = fileNameAttribute,
+                        errorNode = errorNode
+                    )
+                }
+            }
+            .distinctBy { it.errorID }
+            .groupBy({ it.fileName }, { it.errorNode })
+
+        private fun errorID(fileNameAttribute: String, errorNode: Node): Any {
+            // error nodes are expected to take the form of at least <error line="#" column="#" source="ruleName"/>
+            val line = errorNode.attributes.getNamedItem("line")?.nodeValue
+            val column = errorNode.attributes.getNamedItem("column")?.nodeValue
+            val source = errorNode.attributes.getNamedItem("source")?.nodeValue
+
+            return if (line != null && column != null && source != null) {
+                // data class provides convenient hashCode/equals based on these attributes
+                ErrorID(fileName = fileNameAttribute, line = line, column = column, source = source)
             } else {
-                removeWhitespaces(childNode)
+                // if the error node does not contain the expected attributes,
+                // use org.w3c.dom.Node's more strict hashCode/equals method to determine error uniqueness
+                errorNode
             }
         }
+
+        private class CheckstyleErrorNodeWithFileData(
+            val errorID: Any,
+            val fileName: String,
+            val errorNode: Node
+        )
+
+        private data class ErrorID(
+            val fileName: String,
+            val line: String,
+            val column: String,
+            val source: String
+        )
+    }
+
+    /**
+     * Use code instead of XSLT to exclude whitespaces.
+     */
+    private fun Sequence<Node>.filterWhitespace(): Sequence<Node> = asSequence().filterNot {
+        it.nodeType == Node.TEXT_NODE && it.textContent.isBlank()
+    }
+
+    private fun NodeList.asSequence() = sequence {
+        for (index in 0 until length) {
+            yield(item(index))
+        }
     }
 }

detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMergerSpec.kt

@@ -9,7 +9,7 @@ private const val TAB = "\t"
 class XmlReportMergerSpec {
 
     @Test
-    fun `passes for same files`() {
+    fun `passes for no overlapping errors`() {
         val file1 = File.createTempFile("detekt1", "xml").apply {
             writeText(
                 """
@@ -49,4 +49,78 @@ class XmlReportMergerSpec {
         """.trimIndent()
         assertThat(output.readText()).isEqualToIgnoringNewLines(expectedText)
     }
+
+    @Test
+    fun `passes for all overlapping errors`() {
+        val text = """
+            <?xml version="1.0" encoding="utf-8"?>
+            <checkstyle version="4.3">
+            <file name="Sample1.kt">
+            $TAB<error line="1" column="1" severity="warning" message="TestMessage" source="detekt.id_a" />
+            </file>
+            </checkstyle>
+        """.trimIndent()
+        val file1 = File.createTempFile("detekt1", "xml").apply {
+            writeText(text)
+        }
+        val file2 = File.createTempFile("detekt2", "xml").apply {
+            writeText(text)
+        }
+        val output = File.createTempFile("output", "xml")
+        XmlReportMerger.merge(setOf(file1, file2), output)
+
+        val expectedText = """
+            <?xml version="1.0" encoding="UTF-8"?><checkstyle version="4.3">
+              <file name="Sample1.kt">
+                <error column="1" line="1" message="TestMessage" severity="warning" source="detekt.id_a"/>
+              </file>
+            </checkstyle>
+        """.trimIndent()
+        assertThat(output.readText()).isEqualToIgnoringNewLines(expectedText)
+    }
+
+    @Test
+    fun `passes for some overlapping errors`() {
+        val file1 = File.createTempFile("detekt1", "xml").apply {
+            writeText(
+                """
+                <?xml version="1.0" encoding="utf-8"?>
+                <checkstyle version="4.3">
+                <file name="Sample1.kt">
+                $TAB<error line="1" column="1" severity="warning" message="TestMessage" source="detekt.id_a" />
+                </file>
+                <file name="Sample2.kt">
+                $TAB<error line="1" column="1" severity="warning" message="TestMessage" source="detekt.id_b" />
+                </file>
+                </checkstyle>
+                """.trimIndent()
+            )
+        }
+        val file2 = File.createTempFile("detekt2", "xml").apply {
+            writeText(
+                """
+                <?xml version="1.0" encoding="utf-8"?>
+                <checkstyle version="4.3">
+                <file name="Sample2.kt">
+                $TAB<error line="1" column="1" severity="warning" message="TestMessage" source="detekt.id_b" />
+                </file>
+                </checkstyle>
+                """.trimIndent()
+            )
+        }
+        val output = File.createTempFile("output", "xml")
+        XmlReportMerger.merge(setOf(file1, file2), output)
+
+        val expectedText = """
+            <?xml version="1.0" encoding="UTF-8"?><checkstyle version="4.3">
+              <file name="Sample1.kt">
+                <error column="1" line="1" message="TestMessage" severity="warning" source="detekt.id_a"/>
+              </file>
+              <file name="Sample2.kt">
+                <error column="1" line="1" message="TestMessage" severity="warning" source="detekt.id_b"/>
+              </file>
+            </checkstyle>
+        """.trimIndent()
+        assertThat(output.readText()).isEqualToIgnoringNewLines(expectedText)
+    }
 }

detekt-report-md/src/test/kotlin/io/github/detekt/report/md/MdOutputReportSpec.kt

@@ -14,18 +14,44 @@ import io.gitlab.arturbosch.detekt.test.TestDetektion
 import io.gitlab.arturbosch.detekt.test.createEntity
 import io.gitlab.arturbosch.detekt.test.createFinding
 import io.gitlab.arturbosch.detekt.test.createIssue
+import io.mockk.clearStaticMockk
 import io.mockk.every
 import io.mockk.mockk
+import io.mockk.mockkStatic
 import org.assertj.core.api.Assertions.assertThat
 import org.jetbrains.kotlin.com.intellij.psi.PsiFile
 import org.jetbrains.kotlin.psi.KtElement
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
+import java.time.OffsetDateTime
+import java.time.ZoneOffset
 
 class MdOutputReportSpec {
     private val mdReport = MdOutputReport()
     private val detektion = createTestDetektionWithMultipleSmells()
     private val result = mdReport.render(detektion)
 
+    @BeforeEach
+    fun setup() {
+        mockkStatic(OffsetDateTime::class)
+        every { OffsetDateTime.now(ZoneOffset.UTC) } returns OffsetDateTime.of(
+            2000, // year
+            1, // month
+            1, // dayOfMonth
+            0, // hour
+            0, // minute
+            0, // second
+            0, // nanoOfSecond
+            ZoneOffset.UTC // offset
+        )
+    }
+
+    @AfterEach
+    fun teardown() {
+        clearStaticMockk(OffsetDateTime::class)
+    }
+
     @Test
     fun `renders Markdown structure correctly`() {
         assertThat(result).contains("Metrics")