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),
UseNamedLocation(config),
)
)
}
@@ -0,0 +1,85 @@
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.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtNamedDeclaration
import org.jetbrains.kotlin.psi.KtPostfixExpression
import org.jetbrains.kotlin.psi.psiUtil.getCallNameExpression
import org.jetbrains.kotlin.psi.psiUtil.getReceiverExpression
import org.jetbrains.kotlin.psi.psiUtil.referenceExpression

/**
* If a rule reports 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 UseNamedLocation(config: Config = Config.empty) : Rule(config) {
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved

override val issue = Issue(
"UseNamedLocation",
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().getArgumentExpression()!!
val target = findNameIdentifierReceiver(arg)
if (target != null) {
report(
CodeSmell(
issue,
Entity.from(expression.getCallNameExpression()!!),
"Recommended to use Entity.atName(${target.text}) instead."
)
)
}
}
}

private fun findNameIdentifierReceiver(expression: KtExpression): KtExpression? =
when {
expression is KtDotQualifiedExpression ->
if (expression.selectorExpression?.text == "nameIdentifier") {
expression.receiverExpression
} else {
null
}

expression is KtCallExpression ->
if (expression.getCallNameExpression()?.text == "nameIdentifier") {
expression.referenceExpression()
} else {
null
}

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

expression is KtBinaryExpression && expression.operationToken == KtTokens.ELVIS ->
findNameIdentifierReceiver(expression.left!!)
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved

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
UseNamedLocation:
active: true
@@ -0,0 +1,128 @@
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 UseNamedLocationSpec {

private val rule = UseNamedLocation()

@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 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