Implement rule to check the correct usage of @RequiresTypeResolution (

#5182)

Co-authored-by: marschwar <marschwar@users.noreply.github.com>
Co-authored-by: Nicola Corti <corti.nico@gmail.com>

config/detekt/detekt.yml

@@ -277,3 +277,8 @@ libraries:
     active: true
     excludes: ['**/*.kt']
     includes: ['**/detekt-api/src/main/**/api/*.kt']
+
+ruleauthors:
+  ViolatesTypeResolutionRequirements:
+    active: true
+    excludes: ['**/test/**', '**/*Test.kt', '**/*Spec.kt']

detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/MultiRule.kt

@@ -8,6 +8,7 @@ import org.jetbrains.kotlin.psi.KtFile
  * scanning the source code line by line to increase performance.
  */
 @Deprecated("multi rules are much more difficult to maintain and support will be removed in the future")
+@Suppress("ViolatesTypeResolutionRequirements")
 abstract class MultiRule : BaseRule() {
 
     abstract val rules: List<Rule>

detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongParameterList.kt

@@ -28,6 +28,7 @@ import org.jetbrains.kotlin.psi.KtSecondaryConstructor
 /**
  * Reports functions and constructors which have more parameters than a certain threshold.
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.0.0")
 class LongParameterList(config: Config = Config.empty) : Rule(config) {
     override val issue = Issue(

detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/ImplicitDefaultLocale.kt

@@ -42,6 +42,7 @@ import org.jetbrains.kotlin.resolve.calls.util.getType
  * str.toLowerCase(Locale.US)
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.16.0")
 class ImplicitDefaultLocale(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/LateinitUsage.kt

@@ -28,6 +28,7 @@ import org.jetbrains.kotlin.psi.psiUtil.containingClass
  * }
  * </noncompliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 class LateinitUsage(config: Config = Config.empty) : Rule(config) {
 
     override val issue = Issue(

detekt-rules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MapGetWithNotNullAssertionOperator.kt

@@ -43,6 +43,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
  * map.getOrElse("key", { "" })
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.21.0")
 class MapGetWithNotNullAssertionOperator(config: Config) : Rule(config) {
 

detekt-rules-exceptions/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/exceptions/InstanceOfCheckForException.kt

@@ -44,6 +44,7 @@ import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf
  * }
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.21.0")
 class InstanceOfCheckForException(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/MemberNameEqualsClassName.kt

@@ -52,6 +52,7 @@ import org.jetbrains.kotlin.resolve.BindingContext
  * }
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.2.0")
 class MemberNameEqualsClassName(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-performance/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/performance/SpreadOperator.kt

@@ -50,6 +50,7 @@ import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
  * }
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.0.0")
 class SpreadOperator(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-ruleauthors/build.gradle.kts

@@ -7,3 +7,7 @@ dependencies {
     testImplementation(projects.detektTest)
     testImplementation(libs.assertj)
 }
+
+tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompile> {
+    kotlinOptions.freeCompilerArgs = listOf("-Xcontext-receivers")
+}

detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt

@@ -13,6 +13,10 @@ class RuleAuthorsProvider : RuleSetProvider {
 
     override val ruleSetId: String = "ruleauthors"
 
-    @Suppress("UseEmptyCounterpart")
-    override fun instance(config: Config) = RuleSet(ruleSetId, listOf())
+    override fun instance(config: Config) = RuleSet(
+        ruleSetId,
+        listOf(
+            ViolatesTypeResolutionRequirements(config),
+        )
+    )
 }

detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/ViolatesTypeResolutionRequirements.kt

@@ -0,0 +1,96 @@
+@file:Suppress("ForbiddenComment")
+
+package io.gitlab.arturbosch.detekt.authors
+
+import io.gitlab.arturbosch.detekt.api.BaseRule
+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 io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
+import io.gitlab.arturbosch.detekt.rules.fqNameOrNull
+import org.jetbrains.kotlin.psi.KtClass
+import org.jetbrains.kotlin.psi.KtFile
+import org.jetbrains.kotlin.psi.KtNameReferenceExpression
+import org.jetbrains.kotlin.psi.KtReferenceExpression
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
+import org.jetbrains.kotlin.resolve.descriptorUtil.getAllSuperclassesWithoutAny
+import kotlin.reflect.KClass
+
+/**
+ * If a rule uses the property [BaseRule.bindingContext] should be annotated with `@RequiresTypeResolution`.
+ * And if the rule doesn't use that property it shouldn't be annotated with it.
+ */
+@ActiveByDefault("1.22.0")
+@RequiresTypeResolution
+class ViolatesTypeResolutionRequirements(config: Config = Config.empty) : Rule(config) {
+    override val issue = Issue(
+        javaClass.simpleName,
+        Severity.Defect,
+        "`@RequiresTypeResolution` should be used if and only if the property `bindingContext` is used.",
+        Debt.FIVE_MINS
+    )
+
+    private val klasses: MutableList<KtClass> = mutableListOf()
+    private var usesBindingContext: Boolean = false
+
+    override fun visitKtFile(file: KtFile) {
+        super.visitKtFile(file)
+        klasses.forEach { klass ->
+            val isAnnotatedWithRequiresTypeResolution = klass.isAnnotatedWith(RequiresTypeResolution::class)
+            if (usesBindingContext && !isAnnotatedWithRequiresTypeResolution) {
+                report(
+                    CodeSmell(
+                        issue,
+                        Entity.atName(klass),
+                        "`${klass.name}` uses `bindingContext` but is not annotated with `@RequiresTypeResolution`"
+                    )
+                )
+            } else if (!usesBindingContext && isAnnotatedWithRequiresTypeResolution) {
+                report(
+                    CodeSmell(
+                        issue,
+                        Entity.atName(klass),
+                        "`${klass.name}` is annotated with `@RequiresTypeResolution` but doesn't use `bindingContext`"
+                    )
+                )
+            }
+        }
+        klasses.clear()
+        usesBindingContext = false
+    }
+
+    override fun visitClass(klass: KtClass) {
+        super.visitClass(klass)
+
+        if (klass.extendsFrom(BaseRule::class)) {
+            klasses.add(klass)
+        }
+    }
+
+    override fun visitReferenceExpression(expression: KtReferenceExpression) {
+        super.visitReferenceExpression(expression)
+        usesBindingContext = usesBindingContext ||
+            (expression is KtNameReferenceExpression && expression.text == "bindingContext")
+    }
+}
+
+context(BaseRule) private inline fun <reified T : Any> KtClass.extendsFrom(kClass: KClass<T>): Boolean {
+    return bindingContext[BindingContext.CLASS, this]
+        ?.getAllSuperclassesWithoutAny()
+        .orEmpty()
+        .any { it.fqNameOrNull()?.toString() == checkNotNull(kClass.qualifiedName) }
+}
+
+context(BaseRule) private inline fun <reified T : Any> KtClass.isAnnotatedWith(kClass: KClass<T>): Boolean {
+    return annotationEntries
+        .asSequence()
+        .mapNotNull { it.typeReference }
+        .mapNotNull { bindingContext[BindingContext.TYPE, it] }
+        .any { it.fqNameOrNull()?.toString() == checkNotNull(kClass.qualifiedName) }
+}

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

@@ -1,2 +1,4 @@
 ruleauthors:
   active: true
+  ViolatesTypeResolutionRequirements:
+    active: true

detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/ViolatesTypeResolutionRequirementsSpec.kt

@@ -0,0 +1,115 @@
+package io.gitlab.arturbosch.detekt.authors
+
+import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
+import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
+import org.assertj.core.api.Assertions.assertThat
+import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
+import org.junit.jupiter.api.Test
+
+@KotlinCoreEnvironmentTest
+internal class ViolatesTypeResolutionRequirementsSpec(private val env: KotlinCoreEnvironment) {
+
+    private val rule = ViolatesTypeResolutionRequirements()
+
+    @Test
+    fun `should not report classes that don't extend from BaseRule`() {
+        val code = """
+            class A {
+                val issue: Int = error("bindingContext")
+            }
+        """.trimIndent()
+        val findings = rule.compileAndLintWithContext(env, code)
+        assertThat(findings).isEmpty()
+    }
+
+    @Test
+    fun `should report Rules that use bindingContext and are not annotated`() {
+        val code = """
+            import io.gitlab.arturbosch.detekt.api.Config
+            import io.gitlab.arturbosch.detekt.api.Rule
+            
+            class A(config: Config) : Rule(config) {
+                override val issue = error("I don't care")
+
+                private fun asdf() {
+                    bindingContext
+                }
+            }
+        """.trimIndent()
+        val findings = rule.compileAndLintWithContext(env, code)
+        assertThat(findings).hasSize(1)
+    }
+
+    @Test
+    fun `should not report Rules that doesn't use bindingContext and are not annotated`() {
+        val code = """
+            import io.gitlab.arturbosch.detekt.api.Config
+            import io.gitlab.arturbosch.detekt.api.Rule
+            
+            class A(config: Config) : Rule(config) {
+                override val issue = error("I don't care")
+            }
+        """.trimIndent()
+        val findings = rule.compileAndLintWithContext(env, code)
+        assertThat(findings).isEmpty()
+    }
+
+    @Test
+    fun `should not report Rules that use bindingContext and are annotated`() {
+        val code = """
+            import io.gitlab.arturbosch.detekt.api.Config
+            import io.gitlab.arturbosch.detekt.api.Rule
+            import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
+
+            @RequiresTypeResolution
+            class A(config: Config) : Rule(config) {
+                override val issue = error("I don't care")
+
+                private fun asdf() {
+                    bindingContext
+                }
+            }
+        """.trimIndent()
+        val findings = rule.compileAndLintWithContext(env, code)
+        assertThat(findings).isEmpty()
+    }
+
+    @Test
+    fun `should report Rules that don't use bindingContext and are annotated`() {
+        val code = """
+            import io.gitlab.arturbosch.detekt.api.Config
+            import io.gitlab.arturbosch.detekt.api.Rule
+            import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
+
+            @RequiresTypeResolution
+            class A(config: Config) : Rule(config) {
+                override val issue = error("I don't care")
+            }
+        """.trimIndent()
+        val findings = rule.compileAndLintWithContext(env, code)
+        assertThat(findings).hasSize(1)
+    }
+
+    @Test
+    fun `should report Rules that use bindingContext outside class and are not annotated`() {
+        val code = """
+            import io.gitlab.arturbosch.detekt.api.Config
+            import io.gitlab.arturbosch.detekt.api.Rule
+
+            class A(config: Config) : Rule(config) {
+                override val issue = error("I don't care")
+
+                private fun asdf() {
+                    extension()
+                }
+            }
+
+            inline fun Rule.extension(): Boolean {
+                bindingContext
+                return true
+            }
+        """.trimIndent()
+        val findings = rule.compileAndLintWithContext(env, code)
+        assertThat(findings).hasSize(1)
+    }
+}

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/FunctionOnlyReturningConstant.kt

@@ -35,6 +35,7 @@ import org.jetbrains.kotlin.psi.psiUtil.containingClass
  * const val constantString = "1"
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.2.0")
 class FunctionOnlyReturningConstant(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryAbstractClass.kt

@@ -44,6 +44,7 @@ import org.jetbrains.kotlin.types.typeUtil.isInterface
  * }
  * </noncompliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.2.0")
 class UnnecessaryAbstractClass(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedImports.kt

@@ -28,6 +28,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.getImportableDescriptor
  * Exempt from this rule are imports resulting from references to elements within KDoc and
  * from destructuring declarations (componentN imports).
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 class UnusedImports(config: Config) : Rule(config) {
 
     override val issue = Issue(

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivateMember.kt

@@ -56,6 +56,7 @@ private const val ARRAY_GET_METHOD_NAME = "get"
  * If these private elements are unused they should be removed. Otherwise, this dead code
  * can lead to confusion and potential bugs.
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.16.0")
 class UnusedPrivateMember(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseCheckOrError.kt

@@ -37,6 +37,7 @@ import org.jetbrains.kotlin.psi.KtThrowExpression
  * }
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.21.0")
 class UseCheckOrError(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseDataClass.kt

@@ -45,6 +45,7 @@ import org.jetbrains.kotlin.types.KotlinType
  * class A(val b: B) : I by b
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 class UseDataClass(config: Config = Config.empty) : Rule(config) {
 
     override val issue: Issue = Issue(

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseRequire.kt

@@ -29,6 +29,7 @@ import org.jetbrains.kotlin.psi.KtThrowExpression
  * require(value >= 0) { "value is $value but should be at least 0" }
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 @ActiveByDefault(since = "1.21.0")
 class UseRequire(config: Config = Config.empty) : Rule(config) {
 

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnit.kt

@@ -50,6 +50,7 @@ import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
  * override fun foo() = Unit
  * </compliant>
  */
+@Suppress("ViolatesTypeResolutionRequirements")
 class OptionalUnit(config: Config = Config.empty) : Rule(config) {
 
     override val issue = Issue(