Repository: detekt/detekt (kotlin)

Issue:
`OutputReport` is too complex
`OutputReport` is a bit too complex. It has two different values: `ending` and `id`. Usually the `id` is: `HtmlReport` and `ending` is `html`.

The problem is that `ending` then is used in the `cli` as a type of id. And for the case of `checkstyle` this is a problem because the ending is `xml` but we want the id to be `checkstyle`.

My proposal is to rename `ending` to `extension` or `fileExtension` to be clear the usage for it. And then change `id` from `HtmlReport` to just `html`.

Pull request:
Closes #8594

Change the `id` of our `OutputReport` so they don't have two different ids. This also aligns with the SarifReport. After this change we don't need `ending` for anything.

Patch:
diff --git a/detekt-api/api/detekt-api.api b/detekt-api/api/detekt-api.api
index eb821e7d4451..ec227f716f46 100644
--- a/detekt-api/api/detekt-api.api
+++ b/detekt-api/api/detekt-api.api
@@ -193,7 +193,6 @@ public final class dev/detekt/api/Notification$Level : java/lang/Enum {
 }
 
 public abstract interface class dev/detekt/api/OutputReport : dev/detekt/api/Extension {
-	public abstract fun getEnding ()Ljava/lang/String;
 	public abstract fun render (Ldev/detekt/api/Detektion;)Ljava/lang/String;
 }
 
diff --git a/detekt-api/src/main/kotlin/dev/detekt/api/OutputReport.kt b/detekt-api/src/main/kotlin/dev/detekt/api/OutputReport.kt
index f07b935f5d49..16ce1f99318e 100644
--- a/detekt-api/src/main/kotlin/dev/detekt/api/OutputReport.kt
+++ b/detekt-api/src/main/kotlin/dev/detekt/api/OutputReport.kt
@@ -5,12 +5,6 @@ package dev.detekt.api
  * which is written inside a file.
  */
 interface OutputReport : Extension {
-
-    /**
-     * Supported ending of this report type.
-     */
-    val ending: String
-
     /**
      * Defines the translation process of detekt's result into a string.
      */
diff --git a/detekt-api/src/main/kotlin/dev/detekt/api/internal/BuiltInOutputReport.kt b/detekt-api/src/main/kotlin/dev/detekt/api/internal/BuiltInOutputReport.kt
deleted file mode 100644
index f39b2132458e..000000000000
--- a/detekt-api/src/main/kotlin/dev/detekt/api/internal/BuiltInOutputReport.kt
+++ /dev/null
@@ -1,12 +0,0 @@
-package dev.detekt.api.internal
-
-interface BuiltInOutputReport {
-    /**
-     * The reportId allows you to specify a custom report name for your [dev.detekt.api.OutputReport]
-     * if it differs from the default ending determined by the [dev.detekt.api.OutputReport.ending].
-     * For instance, a "Checkstyle" report might need a .xml extension file, but report id will be
-     * `checkstyle`
-     */
-    val reportId: String?
-        get() = null
-}
diff --git a/detekt-core/src/main/kotlin/dev/detekt/core/reporting/OutputFacade.kt b/detekt-core/src/main/kotlin/dev/detekt/core/reporting/OutputFacade.kt
index 039db7e67c01..2364a0d8802c 100644
--- a/detekt-core/src/main/kotlin/dev/detekt/core/reporting/OutputFacade.kt
+++ b/detekt-core/src/main/kotlin/dev/detekt/core/reporting/OutputFacade.kt
@@ -45,7 +45,7 @@ class OutputFacade(
     private fun handleOutputReports(result: Detektion) {
         val extensions = loadExtensions<OutputReport>(settings)
         for (report in extensions) {
-            val filePath = reports[defaultReportMapping(report)]?.path
+            val filePath = reports[report.id]?.path
             if (filePath != null) {
                 report.write(filePath, result)
                 result.add(Notification("Successfully generated ${report.id} at ${filePath.toUri()}", Level.Error))
diff --git a/detekt-core/src/main/kotlin/dev/detekt/core/reporting/Reporting.kt b/detekt-core/src/main/kotlin/dev/detekt/core/reporting/Reporting.kt
index 8a95666e532d..5a5e9f530afe 100644
--- a/detekt-core/src/main/kotlin/dev/detekt/core/reporting/Reporting.kt
+++ b/detekt-core/src/main/kotlin/dev/detekt/core/reporting/Reporting.kt
@@ -1,13 +1,8 @@
 package dev.detekt.core.reporting
 
 import dev.detekt.api.Issue
-import dev.detekt.api.OutputReport
-import dev.detekt.api.internal.BuiltInOutputReport
 import java.nio.file.Path
 
-internal fun defaultReportMapping(report: OutputReport) =
-    if (report is BuiltInOutputReport) (report.reportId ?: report.ending) else report.id
-
 internal fun printIssues(issues: Map<String, List<Issue>>, basePath: Path): String =
     buildString {
         issues.forEach { (key, nestedIssues) ->
diff --git a/detekt-report-checkstyle/src/main/kotlin/dev/detekt/report/xml/CheckstyleOutputReport.kt b/detekt-report-checkstyle/src/main/kotlin/dev/detekt/report/xml/CheckstyleOutputReport.kt
index d898990187f7..22559de66a35 100644
--- a/detekt-report-checkstyle/src/main/kotlin/dev/detekt/report/xml/CheckstyleOutputReport.kt
+++ b/detekt-report-checkstyle/src/main/kotlin/dev/detekt/report/xml/CheckstyleOutputReport.kt
@@ -3,7 +3,6 @@ package dev.detekt.report.xml
 import dev.detekt.api.Detektion
 import dev.detekt.api.Issue
 import dev.detekt.api.OutputReport
-import dev.detekt.api.internal.BuiltInOutputReport
 import dev.detekt.api.suppressed
 import java.util.Locale
 import kotlin.io.path.invariantSeparatorsPathString
@@ -11,11 +10,8 @@ import kotlin.io.path.invariantSeparatorsPathString
 /**
  * Contains rule violations in an XML format. The report follows the structure of a Checkstyle report.
  */
-class CheckstyleOutputReport : BuiltInOutputReport, OutputReport {
-
-    override val id: String = "CheckstyleOutputReport"
-    override val reportId: String = "checkstyle"
-    override val ending = "xml"
+class CheckstyleOutputReport : OutputReport {
+    override val id: String = "checkstyle"
 
     private val Issue.severityLabel: String
         get() = severity.name.lowercase(Locale.US)
diff --git a/detekt-report-html/src/main/kotlin/dev/detekt/report/html/HtmlOutputReport.kt b/detekt-report-html/src/main/kotlin/dev/detekt/report/html/HtmlOutputReport.kt
index faf9a82c8a65..8a5b71242fb9 100644
--- a/detekt-report-html/src/main/kotlin/dev/detekt/report/html/HtmlOutputReport.kt
+++ b/detekt-report-html/src/main/kotlin/dev/detekt/report/html/HtmlOutputReport.kt
@@ -8,7 +8,6 @@ import dev.detekt.api.RuleInstance
 import dev.detekt.api.RuleSet
 import dev.detekt.api.SetupContext
 import dev.detekt.api.TextLocation
-import dev.detekt.api.internal.BuiltInOutputReport
 import dev.detekt.api.internal.whichDetekt
 import dev.detekt.api.suppressed
 import dev.detekt.metrics.ComplexityReportGenerator
@@ -47,10 +46,8 @@ private const val PLACEHOLDER_DATE = "@@@date@@@"
 /**
  * Contains rule violations and metrics formatted in a human friendly way, so that it can be inspected in a web browser.
  */
-class HtmlOutputReport : BuiltInOutputReport, OutputReport {
-
-    override val id: String = "HtmlOutputReport"
-    override val ending = "html"
+class HtmlOutputReport : OutputReport {
+    override val id: String = "html"
 
     private lateinit var basePath: Path
     override fun init(context: SetupContext) {
diff --git a/detekt-report-markdown/src/main/kotlin/dev/detekt/report/markdown/MarkdownOutputReport.kt b/detekt-report-markdown/src/main/kotlin/dev/detekt/report/markdown/MarkdownOutputReport.kt
index 2480382406cb..9a56136b3389 100644
--- a/detekt-report-markdown/src/main/kotlin/dev/detekt/report/markdown/MarkdownOutputReport.kt
+++ b/detekt-report-markdown/src/main/kotlin/dev/detekt/report/markdown/MarkdownOutputReport.kt
@@ -7,7 +7,6 @@ import dev.detekt.api.ProjectMetric
 import dev.detekt.api.RuleInstance
 import dev.detekt.api.SetupContext
 import dev.detekt.api.SourceLocation
-import dev.detekt.api.internal.BuiltInOutputReport
 import dev.detekt.api.internal.whichDetekt
 import dev.detekt.api.suppressed
 import dev.detekt.metrics.ComplexityReportGenerator
@@ -37,10 +36,8 @@ private const val EXTRA_LINES_IN_SNIPPET = 3
 /**
  * Contains rule violations in Markdown format report.
  */
-class MarkdownOutputReport : BuiltInOutputReport, OutputReport {
-    override val id: String = "MarkdownOutputReport"
-    override val reportId: String = "markdown"
-    override val ending: String = "md"
+class MarkdownOutputReport : OutputReport {
+    override val id: String = "markdown"
 
     private lateinit var basePath: Path
     override fun init(context: SetupContext) {
diff --git a/detekt-report-sarif/src/main/kotlin/dev/detekt/report/sarif/SarifOutputReport.kt b/detekt-report-sarif/src/main/kotlin/dev/detekt/report/sarif/SarifOutputReport.kt
index f1e414e1fab4..65e4e06adae4 100644
--- a/detekt-report-sarif/src/main/kotlin/dev/detekt/report/sarif/SarifOutputReport.kt
+++ b/detekt-report-sarif/src/main/kotlin/dev/detekt/report/sarif/SarifOutputReport.kt
@@ -4,7 +4,6 @@ import dev.detekt.api.Config
 import dev.detekt.api.Detektion
 import dev.detekt.api.OutputReport
 import dev.detekt.api.SetupContext
-import dev.detekt.api.internal.BuiltInOutputReport
 import io.github.detekt.sarif4k.Run
 import io.github.detekt.sarif4k.SarifSchema210
 import io.github.detekt.sarif4k.SarifSerializer
@@ -14,9 +13,7 @@ import io.github.detekt.sarif4k.Version
 
 const val SRCROOT = "%SRCROOT%"
 
-class SarifOutputReport : BuiltInOutputReport, OutputReport {
-
-    override val ending: String = "sarif"
+class SarifOutputReport : OutputReport {
     override val id: String = "sarif"
 
     private lateinit var config: Config


Test patch:
diff --git a/detekt-core/src/test/kotlin/dev/detekt/core/reporting/OutputReportsSpec.kt b/detekt-core/src/test/kotlin/dev/detekt/core/reporting/OutputReportsSpec.kt
index 78d4e69aca57..fe7d99437ed6 100644
--- a/detekt-core/src/test/kotlin/dev/detekt/core/reporting/OutputReportsSpec.kt
+++ b/detekt-core/src/test/kotlin/dev/detekt/core/reporting/OutputReportsSpec.kt
@@ -22,10 +22,9 @@ class OutputReportsSpec {
     @Nested
     inner class `arguments for spec` {
 
-        private val reportUnderTest = TestOutputReport::class.java.simpleName
         private val reports = ReportsSpecBuilder().apply {
             report { "checkstyle" to Path("/tmp/path1") }
-            report { reportUnderTest to Path("/tmp/path3") }
+            report { "yml" to Path("/tmp/path3") }
             report { "html" to Path("D:_Gradle\\xxx\\xxx\\build\\reports\\detekt\\detekt.html") }
             report { "markdown" to Path("/tmp/path4") }
         }.build().reports.toList()
@@ -38,21 +37,21 @@ class OutputReportsSpec {
         @Test
         fun `it should properly parse Checkstyle report entry`() {
             val checkstyleReport = reports[0]
-            assertThat(checkstyleReport.type).isEqualTo(defaultReportMapping(CheckstyleOutputReport()))
+            assertThat(checkstyleReport.type).isEqualTo(CheckstyleOutputReport().id)
             assertThat(checkstyleReport.path).isEqualTo(Path("/tmp/path1"))
         }
 
         @Test
         fun `it should properly parse custom report entry`() {
             val customReport = reports[1]
-            assertThat(customReport.type).isEqualTo(reportUnderTest)
+            assertThat(customReport.type).isEqualTo(TestOutputReport().id)
             assertThat(customReport.path).isEqualTo(Path("/tmp/path3"))
         }
 
         @Test
         fun `it should properly parse HTML report entry`() {
             val htmlReport = reports[2]
-            assertThat(htmlReport.type).isEqualTo(defaultReportMapping(HtmlOutputReport()))
+            assertThat(htmlReport.type).isEqualTo(HtmlOutputReport().id)
             assertThat(htmlReport.path).isEqualTo(
                 Path("D:_Gradle\\xxx\\xxx\\build\\reports\\detekt\\detekt.html")
             )
@@ -61,15 +60,14 @@ class OutputReportsSpec {
         @Test
         fun `it should properly parse Markdown report entry`() {
             val markdownReport = reports[3]
-            assertThat(markdownReport.type).isEqualTo(defaultReportMapping(MarkdownOutputReport()))
+            assertThat(markdownReport.type).isEqualTo(MarkdownOutputReport().id)
             assertThat(markdownReport.path).isEqualTo(Path("/tmp/path4"))
         }
 
         @Nested
         inner class `default report ids` {
-
             private val extensions = createProcessingSettings().use { loadExtensions<OutputReport>(it) }
-            private val extensionsIds = extensions.mapTo(HashSet()) { defaultReportMapping(it) }
+            private val extensionsIds = extensions.mapTo(HashSet()) { it.id }
 
             @Test
             fun `should be able to convert to output reports`() {
@@ -80,18 +78,7 @@ class OutputReportsSpec {
             fun `should recognize custom output format`() {
                 assertThat(reports).haveExactly(
                     1,
-                    Condition(
-                        { it.type == reportUnderTest },
-                        "Corresponds exactly to the test output report."
-                    )
-                )
-
-                assertThat(extensions).haveExactly(
-                    1,
-                    Condition(
-                        { it is TestOutputReport && it.ending == "yml" },
-                        "Is exactly the test output report."
-                    )
+                    Condition({ it.type == "yml" }, "Corresponds exactly to the test output report.")
                 )
             }
         }
@@ -116,9 +103,7 @@ class OutputReportsSpec {
 }
 
 class TestOutputReport : OutputReport {
-
-    override val id: String = "TestOutputReport"
-    override val ending: String = "yml"
+    override val id: String = "yml"
 
     override fun render(detektion: Detektion) = throw UnsupportedOperationException("not implemented")
 }