New ruleauthors rule for Entity.from(x.nameIdentifier ?: x) -> Entity…

….atName(x) (#5444)

* Add new rule UseNamedLocation

* Implement rule UseNamedLocation

* Ignore multi-arg overload

* Highlight method name instead of whole expression.

* Test for message, and give a nice message.

* Fix findings in Detekt codebase:

detekt-rules-libraries\src\main\kotlin\io\gitlab\arturbosch\detekt\libraries\ForbiddenPublicDataClass.kt:60:48: Recommended to use Entity.atName(klass) instead. [UseNamedLocation]
detekt-rules-naming\src\main\kotlin\io\gitlab\arturbosch\detekt\rules\naming\EnumNaming.kt:37:28: Recommended to use Entity.atName(enumEntry) instead. [UseNamedLocation]
detekt-rules-complexity\src\main\kotlin\io\gitlab\arturbosch\detekt\rules\complexity\TooManyFunctions.kt:144:28: Recommended to use Entity.atName(declaration) instead. [UseNamedLocation]
detekt-rules-style\src\main\kotlin\io\gitlab\arturbosch\detekt\rules\style\UseDataClass.kt:106:32: Recommended to use Entity.atName(klass) instead. [UseNamedLocation]

* Remove unnecessary test complexity (no type resolution)

* Remove dead code and support more expressions

* Rename to UseEntityAtName

* Fix nullability issues

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'"

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)
         }

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"
                 )
             )

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),
         )
     )
 }

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"
+    }
+}

detekt-rules-ruleauthors/src/main/resources/config/config.yml

@@ -2,3 +2,5 @@ ruleauthors:
   active: true
   ViolatesTypeResolutionRequirements:
     active: true
+  UseEntityAtName:
+    active: true

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<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()
+    }
+}

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."
                     )