From f67bdffed840747e514f4cbe4251989c3d36652b Mon Sep 17 00:00:00 2001 From: iliyangermanov Date: Thu, 8 Feb 2024 23:26:59 +0200 Subject: [PATCH] Add "UnnecessaryPassThroughClass" rule --- .../explicit/IvyExplicitRuleSetProvider.kt | 6 +- .../rule/UnnecessaryPassThroughClassRule.kt | 71 +++++++ src/main/resources/config/config.yml | 2 + .../UnnecessaryPassThroughClassRuleTest.kt | 183 ++++++++++++++++++ 4 files changed, 258 insertions(+), 4 deletions(-) create mode 100644 src/main/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRule.kt create mode 100644 src/test/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRuleTest.kt diff --git a/src/main/kotlin/com/github/ivy/explicit/IvyExplicitRuleSetProvider.kt b/src/main/kotlin/com/github/ivy/explicit/IvyExplicitRuleSetProvider.kt index a1b677a..5f10fdf 100644 --- a/src/main/kotlin/com/github/ivy/explicit/IvyExplicitRuleSetProvider.kt +++ b/src/main/kotlin/com/github/ivy/explicit/IvyExplicitRuleSetProvider.kt @@ -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 @@ -19,6 +16,7 @@ class IvyExplicitRuleSetProvider : RuleSetProvider { DataClassDefaultValuesRule(config), DataClassTypedIDsRule(config), NoImplicitFunctionReturnTypeRule(config), + UnnecessaryPassThroughClassRule(config), ), ) } diff --git a/src/main/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRule.kt b/src/main/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRule.kt new file mode 100644 index 0000000..38ea3e4 --- /dev/null +++ b/src/main/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRule.kt @@ -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.") + } +} diff --git a/src/main/resources/config/config.yml b/src/main/resources/config/config.yml index 570a115..bdcb85f 100644 --- a/src/main/resources/config/config.yml +++ b/src/main/resources/config/config.yml @@ -7,3 +7,5 @@ IvyExplicit: active: true NoImplicitFunctionReturnType: active: true + UnnecessaryPassThroughClass: + active: true diff --git a/src/test/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRuleTest.kt b/src/test/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRuleTest.kt new file mode 100644 index 0000000..1b5ad14 --- /dev/null +++ b/src/test/kotlin/com/github/ivy/explicit/rule/UnnecessaryPassThroughClassRuleTest.kt @@ -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 + } +}