diff --git a/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/TooManyFunctions.kt b/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/TooManyFunctions.kt index d0ed5b44ad1..e528b08ea0f 100644 --- a/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/TooManyFunctions.kt +++ b/detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/TooManyFunctions.kt @@ -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'" diff --git a/detekt-rules-libraries/src/main/kotlin/io/gitlab/arturbosch/detekt/libraries/ForbiddenPublicDataClass.kt b/detekt-rules-libraries/src/main/kotlin/io/gitlab/arturbosch/detekt/libraries/ForbiddenPublicDataClass.kt index 925cf5886bf..c4aaaca3d1f 100644 --- a/detekt-rules-libraries/src/main/kotlin/io/gitlab/arturbosch/detekt/libraries/ForbiddenPublicDataClass.kt +++ b/detekt-rules-libraries/src/main/kotlin/io/gitlab/arturbosch/detekt/libraries/ForbiddenPublicDataClass.kt @@ -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) } diff --git a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/EnumNaming.kt b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/EnumNaming.kt index d2a1211f262..1c57f7d4d32 100644 --- a/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/EnumNaming.kt +++ b/detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/EnumNaming.kt @@ -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" ) ) diff --git a/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt index 707fa016e45..f246ab261af 100644 --- a/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt +++ b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt @@ -17,6 +17,7 @@ class RuleAuthorsProvider : RuleSetProvider { ruleSetId, listOf( ViolatesTypeResolutionRequirements(config), + UseEntityAtName(config), ) ) } diff --git a/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/UseEntityAtName.kt b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/UseEntityAtName.kt new file mode 100644 index 00000000000..b670f663649 --- /dev/null +++ b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/UseEntityAtName.kt @@ -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" + } +} diff --git a/detekt-rules-ruleauthors/src/main/resources/config/config.yml b/detekt-rules-ruleauthors/src/main/resources/config/config.yml index f56c2c0a4a3..daa747a5855 100644 --- a/detekt-rules-ruleauthors/src/main/resources/config/config.yml +++ b/detekt-rules-ruleauthors/src/main/resources/config/config.yml @@ -2,3 +2,5 @@ ruleauthors: active: true ViolatesTypeResolutionRequirements: active: true + UseEntityAtName: + active: true diff --git a/detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/UseEntityAtNameSpec.kt b/detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/UseEntityAtNameSpec.kt new file mode 100644 index 00000000000..06c6e88c8e2 --- /dev/null +++ b/detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/UseEntityAtNameSpec.kt @@ -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()?.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()) 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() + } +} diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseDataClass.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseDataClass.kt index 365cd0989e3..c75d4d6ec9c 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseDataClass.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseDataClass.kt @@ -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." )