Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New ruleauthors rule for Entity.from(x.nameIdentifier ?: x) -> Entity.atName(x) #5444

Merged
merged 11 commits into from Oct 25, 2022
Expand Up @@ -141,7 +141,7 @@ class TooManyFunctions(config: Config = Config.empty) : Rule(config) {
report(
ThresholdedCodeSmell(
issue,
Entity.from(declaration.nameIdentifier ?: declaration),
Entity.atName(declaration),
Metric("SIZE", amount, thresholdInObjects),
"Object '${declaration.name}' with '$amount' functions detected. " +
"Defined threshold inside objects is set to '$thresholdInObjects'"
Expand Down
Expand Up @@ -57,7 +57,7 @@ class ForbiddenPublicDataClass(config: Config = Config.empty) : Rule(config) {
}
if (isPublicOrProtected) {
if (klass.isData()) {
report(CodeSmell(issue, Entity.from(klass.nameIdentifier ?: klass), ""))
report(CodeSmell(issue, Entity.atName(klass), ""))
}
super.visitClass(klass)
}
Expand Down
Expand Up @@ -34,7 +34,7 @@ class EnumNaming(config: Config = Config.empty) : Rule(config) {
report(
CodeSmell(
issue,
Entity.from(enumEntry.nameIdentifier ?: enumEntry),
Entity.atName(enumEntry),
message = "Enum entry names should match the pattern: $enumEntryPattern"
)
)
Expand Down
Expand Up @@ -17,6 +17,7 @@ class RuleAuthorsProvider : RuleSetProvider {
ruleSetId,
listOf(
ViolatesTypeResolutionRequirements(config),
UseEntityAtName(config),
)
)
}
@@ -0,0 +1,83 @@
package io.gitlab.arturbosch.detekt.authors

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtNamedDeclaration
import org.jetbrains.kotlin.psi.KtPostfixExpression
import org.jetbrains.kotlin.psi.KtQualifiedExpression
import org.jetbrains.kotlin.psi.psiUtil.getCallNameExpression
import org.jetbrains.kotlin.psi.psiUtil.getReceiverExpression

/**
* If a rule [report]s issues using [Entity.from] with [KtNamedDeclaration.getNameIdentifier],
* then it can be replaced with [Entity.atName] for more semantic code and better baseline support.
*/
@ActiveByDefault("1.22.0")
class UseEntityAtName(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
"UseEntityAtName",
Severity.Defect,
"Prefer Entity.atName to Entity.from(....nameIdentifier).",
Debt.FIVE_MINS
)

override fun visitCallExpression(expression: KtCallExpression) {
super.visitCallExpression(expression)

if (isEntityFromCall(expression) && expression.valueArguments.size == 1) {
val arg = expression.valueArguments.single()
val target = findNameIdentifierReceiver(arg.getArgumentExpression())
if (target != null) {
report(
CodeSmell(
issue,
Entity.from(expression.getCallNameExpression() ?: expression),
"Recommended to use Entity.atName(${target.text}) instead."
)
)
}
}
}

/**
* @param expression Ideally this method will never get a `null`,
* because a value argument without a value, or a postfix without a receiver,
* or a binary expression without a left side, should never really happen.
* Making [expression] nullable is just a safety measure to be more lenient on code in the wild.
*/
private fun findNameIdentifierReceiver(expression: KtExpression?): KtExpression? =
when {
expression is KtQualifiedExpression ->
if (expression.selectorExpression?.text == "nameIdentifier") {
expression.receiverExpression
} else {
null
}

expression is KtPostfixExpression && expression.operationToken == KtTokens.EXCLEXCL ->
findNameIdentifierReceiver(expression.baseExpression)

expression is KtBinaryExpression && expression.operationToken == KtTokens.ELVIS ->
findNameIdentifierReceiver(expression.left)

else ->
null
}

private fun isEntityFromCall(expression: KtCallExpression): Boolean {
val callNameExpression = expression.getCallNameExpression()
return callNameExpression?.text == "from" &&
callNameExpression.getReceiverExpression()?.text == "Entity"
}
}
2 changes: 2 additions & 0 deletions detekt-rules-ruleauthors/src/main/resources/config/config.yml
Expand Up @@ -2,3 +2,5 @@ ruleauthors:
active: true
ViolatesTypeResolutionRequirements:
active: true
UseEntityAtName:
active: true
@@ -0,0 +1,149 @@
package io.gitlab.arturbosch.detekt.authors

import io.gitlab.arturbosch.detekt.test.assertThat
import io.gitlab.arturbosch.detekt.test.compileAndLint
import org.junit.jupiter.api.Test

internal class UseEntityAtNameSpec {

private val rule = UseEntityAtName()

@Test
fun `should not report calls when there's no name involved`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.com.intellij.psi.PsiElement

fun Rule.f(element: PsiElement) {
report(CodeSmell(issue, Entity.from(element), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).isEmpty()
}

@Test
fun `should not report calls when atName is already used`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.psi.KtNamedDeclaration

fun Rule.f(element: KtNamedDeclaration) {
report(CodeSmell(issue, Entity.atName(element), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).isEmpty()
}

@Test
fun `should report calls where nameIdentifier is used directly with bang`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner

fun Rule.f(element: PsiNameIdentifierOwner) {
report(CodeSmell(issue, Entity.from(element.nameIdentifier!!), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).hasSize(1).hasTextLocations("from")
assertThat(findings.single()).hasMessage("Recommended to use Entity.atName(element) instead.")
}

@Test
fun `should report calls where nameIdentifier is used directly with double-bang`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner

fun Rule.f(element: PsiNameIdentifierOwner) {
report(CodeSmell(issue, Entity.from(element.nameIdentifier!!!!), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).hasSize(1).hasTextLocations("from")
assertThat(findings.single()).hasMessage("Recommended to use Entity.atName(element) instead.")
}

@Test
fun `should report calls where nameIdentifier is used with elvis with same fallback`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner

fun Rule.f(element: PsiNameIdentifierOwner) {
report(CodeSmell(issue, Entity.from(element.nameIdentifier ?: element), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).hasSize(1).hasTextLocations("from")
assertThat(findings.single()).hasMessage("Recommended to use Entity.atName(element) instead.")
}

@Test
fun `should report calls where nameIdentifier is used with elvis with complex fallback`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.com.intellij.psi.PsiExpression
import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType

fun Rule.f(element: PsiExpression) {
report(CodeSmell(issue, Entity.from(element.getStrictParentOfType<KtClass>()?.nameIdentifier ?: element), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).hasSize(1).hasTextLocations("from")
assertThat(findings.single())
.hasMessage("Recommended to use Entity.atName(element.getStrictParentOfType<KtClass>()) instead.")
}

@Test
fun `should report calls where nameIdentifier is used with elvis with other fallback`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner

fun Rule.f(element: PsiNameIdentifierOwner, element2: PsiElement) {
report(CodeSmell(issue, Entity.from(element.nameIdentifier ?: element2), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).hasSize(1).hasTextLocations("from")
assertThat(findings.single()).hasMessage("Recommended to use Entity.atName(element) instead.")
}

@Test
fun `should not report calls where from is used with multiple parameters`() {
val code = """
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Rule
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner

fun Rule.f(element: PsiNameIdentifierOwner, element2: PsiElement) {
report(CodeSmell(issue, Entity.from(element.nameIdentifier ?: element2, 0), "message"))
}
""".trimIndent()
val findings = rule.compileAndLint(code)
assertThat(findings).isEmpty()
}
}
Expand Up @@ -103,7 +103,7 @@ class UseDataClass(config: Config = Config.empty) : Rule(config) {
report(
CodeSmell(
issue,
Entity.from(klass.nameIdentifier ?: klass),
Entity.atName(klass),
"The class ${klass.nameAsSafeName} defines no " +
"functionality and only holds data. Consider converting it to a data class."
)
Expand Down