Skip to content

Commit

Permalink
Remove messageOrDescription (#6771)
Browse files Browse the repository at this point in the history
* Always use the same default message

* Don't create findings without message on tests

* Make CodeSmell properites final

* Remove messageOrDescription
  • Loading branch information
BraisGabin committed Dec 30, 2023
1 parent 4dc2182 commit a3df60a
Show file tree
Hide file tree
Showing 19 changed files with 46 additions and 54 deletions.
8 changes: 3 additions & 5 deletions detekt-api/api/detekt-api.api
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@ public class io/gitlab/arturbosch/detekt/api/CodeSmell : io/gitlab/arturbosch/de
public fun compact ()Ljava/lang/String;
public fun compactWithSignature ()Ljava/lang/String;
public fun getCharPosition ()Lio/gitlab/arturbosch/detekt/api/TextLocation;
public fun getEntity ()Lio/gitlab/arturbosch/detekt/api/Entity;
public final fun getEntity ()Lio/gitlab/arturbosch/detekt/api/Entity;
public fun getFile ()Ljava/lang/String;
public fun getId ()Ljava/lang/String;
public final fun getIssue ()Lio/gitlab/arturbosch/detekt/api/Issue;
public fun getLocation ()Lio/gitlab/arturbosch/detekt/api/Location;
public fun getMessage ()Ljava/lang/String;
public fun getReferences ()Ljava/util/List;
public final fun getMessage ()Ljava/lang/String;
public final fun getReferences ()Ljava/util/List;
public fun getSeverity ()Lio/gitlab/arturbosch/detekt/api/Severity;
public fun getSignature ()Ljava/lang/String;
public fun getStartPosition ()Lio/gitlab/arturbosch/detekt/api/SourceLocation;
public fun messageOrDescription ()Ljava/lang/String;
public fun toString ()Ljava/lang/String;
}

Expand Down Expand Up @@ -221,7 +220,6 @@ public abstract interface class io/gitlab/arturbosch/detekt/api/Finding : io/git
public abstract fun getMessage ()Ljava/lang/String;
public abstract fun getReferences ()Ljava/util/List;
public abstract fun getSeverity ()Lio/gitlab/arturbosch/detekt/api/Severity;
public abstract fun messageOrDescription ()Ljava/lang/String;
}

public final class io/gitlab/arturbosch/detekt/api/Finding$DefaultImpls {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ package io.gitlab.arturbosch.detekt.api
*/
open class CodeSmell(
final override val issue: Issue,
override val entity: Entity,
override val message: String,
override val references: List<Entity> = emptyList()
final override val entity: Entity,
final override val message: String,
final override val references: List<Entity> = emptyList()
) : Finding {
init {
require(message.isNotBlank()) { "The message should not be empty" }
}

internal var internalSeverity: Severity? = null
override val severity: Severity
Expand All @@ -32,8 +35,6 @@ open class CodeSmell(
"severity=$severity, " +
"id='$id')"
}

override fun messageOrDescription(): String = message.ifEmpty { issue.description }
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ interface Finding : Compactable, HasEntity {
val message: String
val severity: Severity
get() = Severity.DEFAULT

/**
* Explanation why this finding was raised.
*/
fun messageOrDescription(): String
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fun createCorrectableFinding(
fun createFinding(
issue: Issue,
entity: Entity,
message: String = entity.signature,
message: String = "TestMessage",
severity: Severity = Severity.ERROR
) = object : CodeSmell(
issue = issue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ fun Finding.renderAsCompilerWarningMessage(): Pair<String, CompilerMessageLocati
)
}

return "$id: ${messageOrDescription()}" to sourceLocation
return "$id: $message" to sourceLocation
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fun Detektion.filterAutoCorrectedIssues(config: Config): Map<RuleSetId, List<Fin
}

private fun Finding.truncatedMessage(): String {
val message = messageOrDescription()
val message = message
.replace(messageReplacementRegex, " ")
.trim()
return when {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class LiteFindingsReport : AbstractFindingsReport() {
override fun render(findings: Map<RuleSetId, List<Finding>>): String {
return buildString {
findings.values.flatten().forEach { finding ->
append("${finding.location.compact()}: ${finding.messageOrDescription()} [${finding.issue.id}]")
append("${finding.location.compact()}: ${finding.message} [${finding.issue.id}]")
appendLine()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ private class CustomRuleSetProvider : RuleSetProvider {
}

private class MaxLineLength(config: Config) : Rule(config) {
override val issue = Issue(this::class.java.simpleName, "")
override val issue = Issue(this::class.java.simpleName, "TestDescription")
private val lengthThreshold: Int = valueOrDefault("maxLineLength", 10)
override fun visitKtFile(file: KtFile) {
super.visitKtFile(file)
Expand All @@ -258,7 +258,7 @@ private class MaxLineLength(config: Config) : Rule(config) {

@RequiresTypeResolution
private class RequiresTypeResolutionMaxLineLength(config: Config) : Rule(config) {
override val issue = Issue(this::class.java.simpleName, "")
override val issue = Issue(this::class.java.simpleName, "TestDescription")
private val lengthThreshold: Int = valueOrDefault("maxLineLength", 10)
override fun visitKtFile(file: KtFile) {
super.visitKtFile(file)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ class TestProvider2(override val ruleSetId: String = "Test2") : RuleSetProvider
class FindName(config: Config) : Rule(config) {
override val issue: Issue = Issue(javaClass.simpleName, "")
override fun visitClassOrObject(classOrObject: KtClassOrObject) {
report(CodeSmell(issue, Entity.atName(classOrObject), message = ""))
report(CodeSmell(issue, Entity.atName(classOrObject), message = "TestMessage"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,14 @@ private class TestLM(config: Config = Config.empty) : Rule(config) {
val start = Location.startLineAndColumn(function.funKeyword!!).line
val end = Location.startLineAndColumn(function.lastBlockStatementOrThis()).line
val offset = end - start
if (offset > 10) report(CodeSmell(issue, Entity.from(function), message = ""))
if (offset > 10) report(CodeSmell(issue, Entity.from(function), message = "TestMessage"))
}
}
private class TestLPL(config: Config = Config.empty) : Rule(config) {
override val issue = Issue("LongParameterList", "")
override fun visitNamedFunction(function: KtNamedFunction) {
val size = function.valueParameters.size
if (size > 5) report(CodeSmell(issue, Entity.from(function), message = ""))
if (size > 5) report(CodeSmell(issue, Entity.from(function), message = "TestMessage"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import kotlin.io.path.Path
internal fun buildFinding(element: KtElement?): Finding = CodeSmell(
issue = Issue("RuleName", ""),
entity = element?.let { Entity.from(element) } ?: buildEmptyEntity(),
message = "",
message = "TestMessage",
)

private fun buildEmptyEntity(): Entity = Entity(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class SuppressorsSpec {
val noIgnorableCodeSmell = CodeSmell(
issue = ARule().issue,
entity = Entity.from(compileContentForTest("""fun foo() = Unit""".trimIndent())),
message = ""
message = "TestMessage"
)

val ignorableCodeSmell = CodeSmell(
Expand All @@ -32,7 +32,7 @@ class SuppressorsSpec {
""".trimIndent()
)
),
message = ""
message = "TestMessage"
)

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private fun createTestDetektionWithMultipleSmells(): Detektion {
createFinding(issueA, entity1, "Message finding 1"),
createFinding(issueA, entity2, "Message finding 2")
),
"Section 2" to listOf(createFinding(issueB, entity3, ""))
"Section 2" to listOf(createFinding(issueB, entity3, "Message finding 3"))
)
}

Expand Down Expand Up @@ -257,7 +257,7 @@ private fun createTestDetektionFromRelativePath(): Detektion {
createFinding(issueA, entity1, "Message finding 1"),
createFinding(issueA, entity2, "Message finding 2")
),
"Section 2" to listOf(createFinding(issueB, entity3, ""))
"Section 2" to listOf(createFinding(issueB, entity3, "Message finding 3"))
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ <h3>Section 2: 1</h3>
<summary class="rule-container"><span class="rule">id_b: 1 </span><span class="description">Description id_b</span></summary>
<a href="https://detekt.dev/docs/rules/section 2#id_b">Documentation</a>
<ul>
<li><span class="location">src/main/com/sample/Sample3.kt:33:3</span></li>
<li><span class="location">src/main/com/sample/Sample3.kt:33:3</span><span class="message">Message finding 3</span></li>
</ul>
</details>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ private fun createTestDetektionWithMultipleSmells(): Detektion {
createFinding(issueA, entity2, "Message finding 2")
),
"Section-2" to listOf(
createFinding(issueB, entity3, "")
createFinding(issueB, entity3, "Message finding 3")
)
).also {
it.putUserData(complexityKey, 10)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ private fun Finding.toResult(ruleSetId: RuleSetId): io.github.detekt.sarif4k.Res
ruleID = "detekt.$ruleSetId.$id",
level = severity.toResultLevel(),
locations = (listOf(location) + references.map { it.location }).map { it.toLocation(code) }.distinct().toList(),
message = Message(text = messageOrDescription())
message = Message(text = message)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class XmlOutputReport : BuiltInOutputReport, OutputReport() {
"\t<error line=\"${it.location.source.line.toXmlString()}\"",
"column=\"${it.location.source.column.toXmlString()}\"",
"severity=\"${it.severityLabel.toXmlString()}\"",
"message=\"${it.messageOrDescription().toXmlString()}\"",
"message=\"${it.message.toXmlString()}\"",
"source=\"${"detekt.${it.id.toXmlString()}"}\" />"
).joinToString(separator = " ")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class XmlOutputFormatSpec {

@Test
fun `renders one reported issue in single file`() {
val smell = CodeSmell(Issue("id_a", ""), entity1, message = "")
val smell = CodeSmell(Issue("id_a", ""), entity1, message = "TestMessage")

val result = outputFormat.render(TestDetektion(smell))

Expand All @@ -66,7 +66,7 @@ class XmlOutputFormatSpec {
<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="4.3">
<file name="src/main/com/sample/Sample1.kt">
$TAB<error line="11" column="1" severity="error" message="" source="detekt.id_a" />
$TAB<error line="11" column="1" severity="error" message="TestMessage" source="detekt.id_a" />
</file>
</checkstyle>
""".trimIndent()
Expand All @@ -75,8 +75,8 @@ class XmlOutputFormatSpec {

@Test
fun `renders two reported issues in single file`() {
val smell1 = CodeSmell(Issue("id_a", ""), entity1, message = "")
val smell2 = CodeSmell(Issue("id_b", ""), entity1, message = "")
val smell1 = CodeSmell(Issue("id_a", ""), entity1, message = "TestMessage")
val smell2 = CodeSmell(Issue("id_b", ""), entity1, message = "TestMessage")

val result = outputFormat.render(TestDetektion(smell1, smell2))

Expand All @@ -85,8 +85,8 @@ class XmlOutputFormatSpec {
<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="4.3">
<file name="src/main/com/sample/Sample1.kt">
$TAB<error line="11" column="1" severity="error" message="" source="detekt.id_a" />
$TAB<error line="11" column="1" severity="error" message="" source="detekt.id_b" />
$TAB<error line="11" column="1" severity="error" message="TestMessage" source="detekt.id_a" />
$TAB<error line="11" column="1" severity="error" message="TestMessage" source="detekt.id_b" />
</file>
</checkstyle>
""".trimIndent()
Expand All @@ -95,8 +95,8 @@ class XmlOutputFormatSpec {

@Test
fun `renders one reported issue across multiple files`() {
val smell1 = CodeSmell(Issue("id_a", ""), entity1, message = "")
val smell2 = CodeSmell(Issue("id_a", ""), entity2, message = "")
val smell1 = CodeSmell(Issue("id_a", ""), entity1, message = "TestMessage")
val smell2 = CodeSmell(Issue("id_a", ""), entity2, message = "TestMessage")

val result = outputFormat.render(TestDetektion(smell1, smell2))

Expand All @@ -105,10 +105,10 @@ class XmlOutputFormatSpec {
<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="4.3">
<file name="src/main/com/sample/Sample1.kt">
$TAB<error line="11" column="1" severity="error" message="" source="detekt.id_a" />
$TAB<error line="11" column="1" severity="error" message="TestMessage" source="detekt.id_a" />
</file>
<file name="src/main/com/sample/Sample2.kt">
$TAB<error line="22" column="2" severity="error" message="" source="detekt.id_a" />
$TAB<error line="22" column="2" severity="error" message="TestMessage" source="detekt.id_a" />
</file>
</checkstyle>
""".trimIndent()
Expand Down Expand Up @@ -147,10 +147,10 @@ class XmlOutputFormatSpec {

@Test
fun `renders two reported issues across multiple files`() {
val smell1 = CodeSmell(Issue("id_a", ""), entity1, message = "")
val smell2 = CodeSmell(Issue("id_b", ""), entity1, message = "")
val smell3 = CodeSmell(Issue("id_a", ""), entity2, message = "")
val smell4 = CodeSmell(Issue("id_b", ""), entity2, message = "")
val smell1 = CodeSmell(Issue("id_a", ""), entity1, message = "TestMessage")
val smell2 = CodeSmell(Issue("id_b", ""), entity1, message = "TestMessage")
val smell3 = CodeSmell(Issue("id_a", ""), entity2, message = "TestMessage")
val smell4 = CodeSmell(Issue("id_b", ""), entity2, message = "TestMessage")

val result = outputFormat.render(
TestDetektion(
Expand All @@ -166,12 +166,12 @@ class XmlOutputFormatSpec {
<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="4.3">
<file name="src/main/com/sample/Sample1.kt">
$TAB<error line="11" column="1" severity="error" message="" source="detekt.id_a" />
$TAB<error line="11" column="1" severity="error" message="" source="detekt.id_b" />
$TAB<error line="11" column="1" severity="error" message="TestMessage" source="detekt.id_a" />
$TAB<error line="11" column="1" severity="error" message="TestMessage" source="detekt.id_b" />
</file>
<file name="src/main/com/sample/Sample2.kt">
$TAB<error line="22" column="2" severity="error" message="" source="detekt.id_a" />
$TAB<error line="22" column="2" severity="error" message="" source="detekt.id_b" />
$TAB<error line="22" column="2" severity="error" message="TestMessage" source="detekt.id_a" />
$TAB<error line="22" column="2" severity="error" message="TestMessage" source="detekt.id_b" />
</file>
</checkstyle>
""".trimIndent()
Expand All @@ -198,7 +198,7 @@ class XmlOutputFormatSpec {
<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="4.3">
<file name="src/main/com/sample/Sample1.kt">
$TAB<error line="${finding.location.source.line}" column="${finding.location.source.column}" severity="$xmlSeverity" message="${finding.messageOrDescription()}" source="detekt.${finding.id}" />
$TAB<error line="${finding.location.source.line}" column="${finding.location.source.column}" severity="$xmlSeverity" message="${finding.message}" source="detekt.${finding.id}" />
</file>
</checkstyle>
""".trimIndent()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,4 @@ open class ThresholdedCodeSmell(
references = references
) {
override fun compact(): String = "$id - $metric - ${entity.compact()}"

override fun messageOrDescription(): String = message.ifEmpty { issue.description }
}

0 comments on commit a3df60a

Please sign in to comment.