diff --git a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMerger.kt b/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMerger.kt index 2bfa89ce47b..daebbbae9ff 100644 --- a/detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMerger.kt +++ b/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,132 @@ object XmlReportMerger { private val documentBuilder by lazy { DocumentBuilderFactory.newInstance().newDocumentBuilder() } - fun merge(inputs: Collection, 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, 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>): 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) { + + /** + * Parses a list of `file` nodes matching the following topology + * + * ```xml + * + * + * + * ``` + * + * @see CheckstyleSourceFileNodes + */ + fun parseCheckstyleToSourceFileNodes() = + CheckstyleSourceFileNodes( + files.filter { reportFile -> reportFile.exists() } + .flatMap { existingReportFile -> + val checkstyleNode = documentBuilder.parse(existingReportFile.inputStream()) + checkstyleNode.documentElement.childNodes.asSequence().filterWhitespace() + } + ) } /** - * Use code instead of XSLT to exclude whitespaces. + * A list of checkstyle `file` nodes that may contain 0 to many `error` nodes + * + * ```xml + * + * + * + * ``` */ - 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) { + + /** 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 + 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.filterWhitespace(): Sequence = asSequence().filterNot { + it.nodeType == Node.TEXT_NODE && it.textContent.isBlank() + } + + private fun NodeList.asSequence() = sequence { + for (index in 0 until length) { + yield(item(index)) + } } } diff --git a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMergerSpec.kt b/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMergerSpec.kt index 6a22dea2bfc..4f34c691675 100644 --- a/detekt-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMergerSpec.kt +++ b/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 = """ + + + + $TAB + + + """.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 = """ + + + + + + """.trimIndent() + assertThat(output.readText()).isEqualToIgnoringNewLines(expectedText) + } + + @Test + fun `passes for some overlapping errors`() { + val file1 = File.createTempFile("detekt1", "xml").apply { + writeText( + """ + + + + $TAB + + + $TAB + + + """.trimIndent() + ) + } + val file2 = File.createTempFile("detekt2", "xml").apply { + writeText( + """ + + + + $TAB + + + """.trimIndent() + ) + } + val output = File.createTempFile("output", "xml") + XmlReportMerger.merge(setOf(file1, file2), output) + + val expectedText = """ + + + + + + + + + """.trimIndent() + assertThat(output.readText()).isEqualToIgnoringNewLines(expectedText) + } } diff --git a/detekt-report-md/src/test/kotlin/io/github/detekt/report/md/MdOutputReportSpec.kt b/detekt-report-md/src/test/kotlin/io/github/detekt/report/md/MdOutputReportSpec.kt index 378d6643605..cf8d96b56e2 100644 --- a/detekt-report-md/src/test/kotlin/io/github/detekt/report/md/MdOutputReportSpec.kt +++ b/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")