Skip to content

Commit

Permalink
Add "UnnecessaryPassThroughClass" rule
Browse files Browse the repository at this point in the history
  • Loading branch information
ILIYANGERMANOV committed Feb 8, 2024
1 parent 4eb1df3 commit f67bdff
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package com.github.ivy.explicit

import com.github.ivy.explicit.rule.DataClassDefaultValuesRule
import com.github.ivy.explicit.rule.DataClassFunctionsRule
import com.github.ivy.explicit.rule.DataClassTypedIDsRule
import com.github.ivy.explicit.rule.NoImplicitFunctionReturnTypeRule
import com.github.ivy.explicit.rule.*
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
Expand All @@ -19,6 +16,7 @@ class IvyExplicitRuleSetProvider : RuleSetProvider {
DataClassDefaultValuesRule(config),
DataClassTypedIDsRule(config),
NoImplicitFunctionReturnTypeRule(config),
UnnecessaryPassThroughClassRule(config),
),
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package com.github.ivy.explicit.rule

import io.gitlab.arturbosch.detekt.api.*
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.isPrivate

class UnnecessaryPassThroughClassRule(config: Config) : Rule(config) {

override val issue = Issue(
id = "UnnecessaryPassThroughClass",
severity = Severity.Maintainability,
description = "Unnecessary pass-through classes increase complexity " +
"and boilerplate code without adding value.",
debt = Debt.TEN_MINS
)

override fun visitClass(klass: KtClass) {
super.visitClass(klass)
if (klass.isInterface() ||
klass.isEnum() ||
klass.isAnnotation() ||
klass.isData()
) return

val functions = klass.body?.functions?.filterNot { it.isPrivate() } ?: return
if (functions.isEmpty()) return

val passThroughClass = functions.all(::isPassThroughFunction)
if (passThroughClass) {
report(CodeSmell(issue, Entity.from(klass), failureMessage(klass)))
}
}

private fun isPassThroughFunction(function: KtNamedFunction): Boolean {
val callExpression = when (val body = function.bodyExpression) {
is KtBlockExpression -> {
if (body.statements.size == 1) {
extractCallExpression(body.statements.first())
} else {
null
}
}

is KtCallExpression,
is KtReturnExpression,
is KtDotQualifiedExpression -> extractCallExpression(body)

else -> null
}

return callExpression?.let { callExp ->
val callArguments = callExp.valueArguments.mapNotNull { it.getArgumentExpression()?.text }
val functionParameters = function.valueParameters.mapNotNull { it.name }
callArguments == functionParameters
} ?: false
}

private fun extractCallExpression(expression: KtExpression): KtCallExpression? = when (expression) {
is KtCallExpression -> expression
is KtReturnExpression -> expression.returnedExpression?.let(::extractCallExpression)
is KtDotQualifiedExpression -> expression.selectorExpression?.let(::extractCallExpression)
else -> null
}


private fun failureMessage(klass: KtClass): String = buildString {
append("The class '${klass.name}' appears to be an unnecessary pass-through class. ")
append("It only increase complexity and boilerplate code without adding any value. ")
append("Consider removing the class or adding meaningful logic.")
}
}
2 changes: 2 additions & 0 deletions src/main/resources/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ IvyExplicit:
active: true
NoImplicitFunctionReturnType:
active: true
UnnecessaryPassThroughClass:
active: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
package com.github.ivy.explicit.rule

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import io.kotest.matchers.collections.shouldHaveSize
import io.kotest.matchers.shouldBe
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test

@KotlinCoreEnvironmentTest
internal class UnnecessaryPassThroughClassRuleTest(private val env: KotlinCoreEnvironment) {

private lateinit var rule: UnnecessaryPassThroughClassRule

@BeforeEach
fun setup() {
rule = UnnecessaryPassThroughClassRule(Config.empty)
}


@Test
fun `reports an unnecessary pass-through class - case 1`() {
// given
val code = """
class A(val b: B) {
fun x() {
return b.x()
}
fun y(p1: Int, p2: Double): Double {
return b.y(p1,p2)
}
fun z(text: String) = b.z(text)
}
"""

// when
val findings = rule.compileAndLintWithContext(env, code)

// then
findings shouldHaveSize 1
val message = findings.first().message
message shouldBe """
The class 'A' appears to be an unnecessary pass-through class. It only increase complexity and boilerplate code without adding any value. Consider removing the class or adding meaningful logic.
""".trimIndent()
}

@Test
fun `reports a pass-through class that delegates to two classes`() {
// given
val code = """
class A(val b: B, val c: C) {
fun x() {
return b.x()
}
fun y(p1: Int, p2: Double): String {
return c.y(p1,p2)
}
fun z(text: String) = b.z(text)
}
"""

// when
val findings = rule.compileAndLintWithContext(env, code)

// then
findings shouldHaveSize 1
}

@Test
fun `reports a pass-through DataSource`() {
// given
val code = """import java.util.UUID
class SomeDataSource(val dao: SomeDao): Int {
fun save(value: Entity) {
dao.save(value)
}
fun findById(id: UUID): Entity? = dao.findById(id)
fun deleteBy(id: UUID) {
dao.deleteBy(id)
}
}
"""

// when
val findings = rule.compileAndLintWithContext(env, code)

// then
findings shouldHaveSize 1
}


@Test
fun `does not report interfaces`() {
// given
val code = """
interface A(val b: B) {
fun x() {
return b.x()
}
fun y(p1: Int, p2: Double) {
return b.y(p1,p2)
}
fun z(text: String) = b.z(text)
}
"""

// when
val findings = rule.compileAndLintWithContext(env, code)

// then
findings shouldHaveSize 0
}

@Test
fun `does not report classes that have at least one function with logic`() {
// given
val code = """
class A(val b: B) {
fun x() {
return b.x()
}
fun y(text: String) = b.z(text.uppercase())
}
"""

// when
val findings = rule.compileAndLintWithContext(env, code)

// then
findings shouldHaveSize 0
}

@Test
fun `does not report a class - case 1`() {
// given
val code = """
class A(val b: B): Int {
fun x(): Boolean {
return predicate(b.x())
}
}
"""

// when
val findings = rule.compileAndLintWithContext(env, code)

// then
findings shouldHaveSize 0
}

@Test
fun `does not report a class - case 2`() {
// given
val code = """import java.util.UUID
class SomeDataSource(val dao: SomeDao): Int {
fun save(value: Entity) {
return dao.save(value)
}
fun findById(id: UUID): Entity = dao.findById(id) ?: DEFAULT
}
"""

// when
val findings = rule.compileAndLintWithContext(env, code)

// then
findings shouldHaveSize 0
}
}

0 comments on commit f67bdff

Please sign in to comment.