Skip to content

Commit c9bedd9

Browse files
Improve the "DataClassFunction" rule
1 parent a2ed77e commit c9bedd9

File tree

4 files changed

+85
-8
lines changed

4 files changed

+85
-8
lines changed

src/main/kotlin/com/github/ivy/explicit/IvyExplicitRuleSetProvider.kt

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.github.ivy.explicit
22

3+
import com.github.ivy.explicit.rule.DataClassFunctionsRule
34
import io.gitlab.arturbosch.detekt.api.Config
45
import io.gitlab.arturbosch.detekt.api.RuleSet
56
import io.gitlab.arturbosch.detekt.api.RuleSetProvider

src/main/kotlin/com/github/ivy/explicit/DataClassFunctionsRule.kt src/main/kotlin/com/github/ivy/explicit/rule/DataClassFunctionsRule.kt

+19-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1-
package com.github.ivy.explicit
1+
package com.github.ivy.explicit.rule
22

3+
import com.github.ivy.explicit.util.FunctionMessage
34
import io.gitlab.arturbosch.detekt.api.*
5+
import io.gitlab.arturbosch.detekt.rules.isOverride
46
import org.jetbrains.kotlin.psi.KtClass
57
import org.jetbrains.kotlin.psi.KtNamedFunction
68

79
class DataClassFunctionsRule(config: Config) : Rule(config) {
10+
private val functionMessage: FunctionMessage by lazy { FunctionMessage() }
11+
812
override val issue = Issue(
913
id = "DataClassFunctions",
1014
severity = Severity.Maintainability,
@@ -18,16 +22,28 @@ class DataClassFunctionsRule(config: Config) : Rule(config) {
1822
if (klass.isData()) {
1923
klass.body?.declarations
2024
?.filterIsInstance<KtNamedFunction>()
25+
?.filter {
26+
// Functions overrides are fine
27+
!it.isOverride()
28+
}
2129
?.forEach { function ->
2230
report(
2331
CodeSmell(
2432
issue,
2533
Entity.from(function),
26-
message = "Data class '${klass.name}' should not contain functions. " +
27-
"Found: function ${function.name}()."
34+
message = failureMessage(klass, function)
2835
)
2936
)
3037
}
3138
}
3239
}
40+
41+
private fun failureMessage(
42+
klass: KtClass,
43+
function: KtNamedFunction
44+
): String = buildString {
45+
append("Data class '${klass.name}' should not contain functions. ")
46+
append("Data classes should only model data and not define behavior. ")
47+
append("Found: function '${functionMessage.signature(function)}'.")
48+
}
3349
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package com.github.ivy.explicit.util
2+
3+
import org.jetbrains.kotlin.psi.KtNamedFunction
4+
5+
class FunctionMessage {
6+
fun signature(function: KtNamedFunction): String {
7+
// Extract the function name
8+
val fName = function.name ?: ""
9+
10+
// Build the parameter list string
11+
val inputParams = function.valueParameters.joinToString(separator = ", ") { param ->
12+
"${param.name}: ${param.typeReference?.text}"
13+
}
14+
15+
// Get the return type, considering both explicit and inferred types
16+
val returnType = try {
17+
when {
18+
function.typeReference != null -> function.typeReference!!.text
19+
function.hasBlockBody() -> "Unit" // Assume Unit for block bodies without a return type
20+
function.hasDeclaredReturnType() -> function.typeReference!!.text // Explicit return type
21+
else -> null
22+
}
23+
} catch (e: Exception) {
24+
null
25+
}
26+
// Construct and return the formatted string
27+
return "$fName($inputParams)" + if (returnType != null) {
28+
": $returnType"
29+
} else ""
30+
}
31+
}

src/test/kotlin/com/github/ivy/explicit/DataClassFunctionsRuleTest.kt src/test/kotlin/com/github/ivy/explicit/rule/DataClassFunctionsRuleTest.kt

+34-5
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
package com.github.ivy.explicit
1+
package com.github.ivy.explicit.rule
22

33
import io.gitlab.arturbosch.detekt.api.Config
44
import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
55
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
66
import io.kotest.matchers.collections.shouldHaveSize
7-
import io.kotest.matchers.string.shouldNotBeBlank
7+
import io.kotest.matchers.shouldBe
88
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
99
import org.junit.jupiter.api.Test
1010

1111
@KotlinCoreEnvironmentTest
1212
internal class DataClassFunctionsRuleTest(private val env: KotlinCoreEnvironment) {
1313

1414
@Test
15-
fun `reports data class having functions`() {
15+
fun `reports data class having one function`() {
1616
val code = """
1717
data class A(
1818
val x: Int
@@ -23,7 +23,9 @@ internal class DataClassFunctionsRuleTest(private val env: KotlinCoreEnvironment
2323
val findings = DataClassFunctionsRule(Config.empty).compileAndLintWithContext(env, code)
2424
findings shouldHaveSize 1
2525
val message = findings.first().message
26-
message.shouldNotBeBlank()
26+
message shouldBe """
27+
Data class 'A' should not contain functions. Data classes should only model data and not define behavior. Found: function 'a()'.
28+
""".trimIndent()
2729
}
2830

2931
@Test
@@ -38,7 +40,34 @@ internal class DataClassFunctionsRuleTest(private val env: KotlinCoreEnvironment
3840
}
3941

4042
@Test
41-
fun `doesn't report data class with companion object`() {
43+
fun `doesn't report data class with override functions`() {
44+
val code = """
45+
data class DisplayLoan(
46+
val loan: Loan,
47+
val amountPaid: Double,
48+
val currencyCode: String? = getDefaultFIATCurrency().currencyCode,
49+
val formattedDisplayText: String = "",
50+
val percentPaid: Double = 0.0
51+
) : Reorderable {
52+
override fun getItemOrderNum(): Double {
53+
return loan.orderNum
54+
}
55+
56+
override fun withNewOrderNum(newOrderNum: Double): Reorderable {
57+
return this.copy(
58+
loan = loan.copy(
59+
orderNum = newOrderNum
60+
)
61+
)
62+
}
63+
}
64+
"""
65+
val findings = DataClassFunctionsRule(Config.empty).compileAndLintWithContext(env, code)
66+
findings shouldHaveSize 0
67+
}
68+
69+
@Test
70+
fun `doesn't report data class with functions in companion object`() {
4271
val code = """
4372
data class A(
4473
val x: Int

0 commit comments

Comments
 (0)