diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClass.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClass.kt index 577b4cce41f..b8a13aadb9a 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClass.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClass.kt @@ -13,6 +13,7 @@ import org.jetbrains.kotlin.backend.common.pop import org.jetbrains.kotlin.descriptors.ClassifierDescriptor import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.psi.KtClass +import org.jetbrains.kotlin.psi.KtExpressionWithLabel import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.KtReferenceExpression import org.jetbrains.kotlin.psi.psiUtil.containingClass @@ -85,6 +86,11 @@ class UnnecessaryInnerClass(config: Config = Config.empty) : Rule(config) { checkForOuterUsage(expression) } + override fun visitExpressionWithLabel(expression: KtExpressionWithLabel) { + super.visitExpressionWithLabel(expression) + checkForOuterUsage(expression) + } + // Replace this "constructor().apply{}" pattern with buildList() when the Kotlin // API version is upgraded to 1.6 private fun findParentClasses(ktClass: KtClass): List = ArrayList().apply { @@ -111,8 +117,24 @@ class UnnecessaryInnerClass(config: Config = Config.empty) : Rule(config) { } } - private fun findResolvedContainingClassId(reference: KtReferenceExpression): ClassId? { - return bindingContext[BindingContext.REFERENCE_TARGET, reference] + private fun checkForOuterUsage(expressionToResolve: KtExpressionWithLabel) { + val currentClass = classChain.peek() ?: return + val parentClasses = candidateClassToParentClasses[currentClass] ?: return + + val resolvedContainingClassName = expressionToResolve.getLabelName() + /* + * 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.name == resolvedContainingClassName } + if (index >= 0) { + candidateClassToParentClasses.remove(currentClass) + parentClasses.subList(0, index).forEach { candidateClassToParentClasses.remove(it) } + } + } + + private fun findResolvedContainingClassId(expression: KtReferenceExpression): ClassId? { + return bindingContext[BindingContext.REFERENCE_TARGET, expression] ?.containingDeclaration ?.safeAs() ?.classId diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClassSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClassSpec.kt index a20068ddcf4..de71fb8be23 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClassSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryInnerClassSpec.kt @@ -356,6 +356,23 @@ class UnnecessaryInnerClassSpec(val env: KotlinCoreEnvironment) { assertThat(subject.lintWithContext(env, code)).isEmpty() } + + @Test + fun `when the innermost class refers the outermost class via a labeled expression`() { + val code = """ + class A { + inner class B { + inner class C { + fun outer(): A { + return this@A + } + } + } + } + """.trimIndent() + + assertThat(subject.lintWithContext(env, code)).isEmpty() + } } @Test @@ -382,4 +399,34 @@ class UnnecessaryInnerClassSpec(val env: KotlinCoreEnvironment) { assertThat(subject.lintWithContext(env, code)).isEmpty() } + + @Test + fun `does not report labeled expressions to outer class`() { + val code = """ + class A { + inner class B { + fun outer(): A { + return this@A + } + } + } + """.trimIndent() + + assertThat(subject.lintWithContext(env, code)).isEmpty() + } + + @Test + fun `reports irrelevant labeled expressions`() { + val code = """ + class A { + inner class B { + fun inner() { + return Unit.apply { this@inner } + } + } + } + """.trimIndent() + + assertThat(subject.lintWithContext(env, code)).hasSize(1) + } }