Skip to content

Commit

Permalink
Xml Report Merger now merges duplicate smells across input report fil…
Browse files Browse the repository at this point in the history 
…es (#5033)

* Xml Report Merger now merges duplicate smells across input report files

* Mock time in MarkdownReportSpec for more consistent test results

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

Co-authored-by: Tim Oltjenbruns <tim.oltjenbruns@softvision.com>
Co-authored-by: Chao Zhang <zhangchao6865@gmail.com>
  • Loading branch information
@timothyolt @chao2zhang
3 people committed Jul 7, 2022
1 parent 46aa36f commit b6448e3
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 24 deletions.
137 changes: 114 additions & 23 deletions detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMerger.kt
View file
Expand Up @@ -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
Expand All @@ -16,42 +17,132 @@ 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())
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
* <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))
}
}
}
76 changes: 75 additions & 1 deletion ...t-gradle-plugin/src/test/kotlin/io/gitlab/arturbosch/detekt/report/XmlReportMergerSpec.kt
View file
Expand Up @@ -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(
"""
Expand Down Expand Up @@ -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)
}
}
26 changes: 26 additions & 0 deletions detekt-report-md/src/test/kotlin/io/github/detekt/report/md/MdOutputReportSpec.kt
View file
Expand Up @@ -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")
Expand Down

0 comments on commit b6448e3

Please sign in to comment.