UnnecessaryInnerClass: fix false positive with references to function type variables (#4738)

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

@@ -12,21 +12,11 @@ import org.jetbrains.kotlin.backend.common.peek
 import org.jetbrains.kotlin.backend.common.pop
 import org.jetbrains.kotlin.descriptors.ClassifierDescriptor
 import org.jetbrains.kotlin.name.ClassId
-import org.jetbrains.kotlin.psi.KtBinaryExpression
-import org.jetbrains.kotlin.psi.KtCallExpression
 import org.jetbrains.kotlin.psi.KtClass
-import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
-import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.KtFile
-import org.jetbrains.kotlin.psi.KtIfExpression
-import org.jetbrains.kotlin.psi.KtNamedFunction
-import org.jetbrains.kotlin.psi.KtParameter
-import org.jetbrains.kotlin.psi.KtProperty
 import org.jetbrains.kotlin.psi.KtReferenceExpression
-import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
 import org.jetbrains.kotlin.psi.psiUtil.containingClass
 import org.jetbrains.kotlin.resolve.BindingContext
-import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
 import org.jetbrains.kotlin.resolve.descriptorUtil.classId
 import org.jetbrains.kotlin.utils.addToStdlib.safeAs
 
@@ -90,39 +80,9 @@ class UnnecessaryInnerClass(config: Config = Config.empty) : Rule(config) {
         classChain.pop()
     }
 
-    override fun visitProperty(property: KtProperty) {
-        super.visitProperty(property)
-        checkForOuterUsage(listOfNotNull(property.initializer))
-    }
-
-    override fun visitNamedFunction(function: KtNamedFunction) {
-        super.visitNamedFunction(function)
-        checkForOuterUsage(listOfNotNull(function.initializer))
-    }
-
-    override fun visitCallExpression(expression: KtCallExpression) {
-        super.visitCallExpression(expression)
-        checkForOuterUsage(expression.collectDescendantsOfType<KtReferenceExpression>() + expression)
-    }
-
-    override fun visitParameter(parameter: KtParameter) {
-        super.visitParameter(parameter)
-        checkForOuterUsage(listOfNotNull(parameter.defaultValue))
-    }
-
-    override fun visitBinaryExpression(expression: KtBinaryExpression) {
-        super.visitBinaryExpression(expression)
-        checkForOuterUsage(listOfNotNull(expression.left, expression.right))
-    }
-
-    override fun visitIfExpression(expression: KtIfExpression) {
-        super.visitIfExpression(expression)
-        checkForOuterUsage(listOfNotNull(expression.condition as? KtReferenceExpression))
-    }
-
-    override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) {
-        super.visitDotQualifiedExpression(expression)
-        checkForOuterUsage(listOf(expression.receiverExpression))
+    override fun visitReferenceExpression(expression: KtReferenceExpression) {
+        super.visitReferenceExpression(expression)
+        checkForOuterUsage(expression)
     }
 
     // Replace this "constructor().apply{}" pattern with buildList() when the Kotlin
@@ -135,27 +95,24 @@ class UnnecessaryInnerClass(config: Config = Config.empty) : Rule(config) {
         }
     }
 
-    private fun checkForOuterUsage(expressionsToResolve: List<KtElement>) {
+    private fun checkForOuterUsage(expressionToResolve: KtReferenceExpression) {
         val currentClass = classChain.peek() ?: return
         val parentClasses = candidateClassToParentClasses[currentClass] ?: return
 
-        expressionsToResolve.forEach { ktElement ->
-            val resolvedContainingClassId = findResolvedContainingClassId(ktElement)
-            /*
-             * If class A -> inner class B -> inner class C, and class C has outer usage of A,
-             * then both B and C should stay as inner classes.
-             */
-            val index = parentClasses.indexOfFirst { it.getClassId() == resolvedContainingClassId }
-            if (index >= 0) {
-                candidateClassToParentClasses.remove(currentClass)
-                parentClasses.subList(0, index).forEach { candidateClassToParentClasses.remove(it) }
-            }
+        val resolvedContainingClassId = findResolvedContainingClassId(expressionToResolve)
+        /*
+         * If class A -> inner class B -> inner class C, and class C has outer usage of A,
+         * then both B and C should stay as inner classes.
+         */
+        val index = parentClasses.indexOfFirst { it.getClassId() == resolvedContainingClassId }
+        if (index >= 0) {
+            candidateClassToParentClasses.remove(currentClass)
+            parentClasses.subList(0, index).forEach { candidateClassToParentClasses.remove(it) }
         }
     }
 
-    private fun findResolvedContainingClassId(ktElement: KtElement): ClassId? {
-        return ktElement.getResolvedCall(bindingContext)
-            ?.resultingDescriptor
+    private fun findResolvedContainingClassId(reference: KtReferenceExpression): ClassId? {
+        return bindingContext[BindingContext.REFERENCE_TARGET, reference]
             ?.containingDeclaration
             ?.safeAs<ClassifierDescriptor>()
             ?.classId

detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClassSpec.kt

@@ -298,6 +298,23 @@ class UnnecessaryInnerClassSpec(val env: KotlinCoreEnvironment) {
 
                 assertThat(subject.lintWithContext(env, code)).isEmpty()
             }
+
+            @Test
+            fun `to call a function type variable of the member`() {
+                val code = """
+                    class A {
+                        val foo: () -> Unit = {}
+                        
+                        inner class B {
+                            fun bar() {
+                                foo()
+                            }
+                        }
+                    }
+                """.trimIndent()
+
+                assertThat(subject.lintWithContext(env, code)).isEmpty()
+            }
         }
 
         @Nested