From d57beda67d311df3bcaa17b23663d130be6db26c Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Tue, 12 Sep 2023 01:28:56 +0530 Subject: [PATCH 1/3] Handle sequence of overridden methods --- .../detekt/rules/style/ForbiddenMethodCall.kt | 24 +++++--- .../rules/style/ForbiddenMethodCallSpec.kt | 56 +++++++++++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt index 2859e1576da..b54a2898eeb 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt @@ -12,7 +12,6 @@ import io.gitlab.arturbosch.detekt.api.config import io.gitlab.arturbosch.detekt.api.internal.Configuration import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution import io.gitlab.arturbosch.detekt.api.valuesWithReason -import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.SyntheticPropertyDescriptor @@ -26,6 +25,7 @@ import org.jetbrains.kotlin.psi.KtPrefixExpression import org.jetbrains.kotlin.psi.psiUtil.isDotSelector import org.jetbrains.kotlin.resolve.calls.util.getCalleeExpressionIfAny import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall +import org.jetbrains.kotlin.resolve.descriptorUtil.overriddenTreeUniqueAsSequence /** * Reports all method or constructor invocations that are forbidden. @@ -113,14 +113,20 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) { } private fun check(expression: KtExpression) { - val descriptors = expression.getResolvedCall(bindingContext)?.resultingDescriptor?.let { - val foundDescriptors = if (it is PropertyDescriptor) { - listOfNotNull(it.unwrappedGetMethod, it.unwrappedSetMethod) - } else { - listOf(it) - } - foundDescriptors + foundDescriptors.flatMap(CallableDescriptor::getOverriddenDescriptors) - } ?: return + val descriptors = + expression.getResolvedCall(bindingContext)?.resultingDescriptor?.let { callableDescriptor -> + val foundDescriptors = if (callableDescriptor is PropertyDescriptor) { + listOfNotNull( + callableDescriptor.unwrappedGetMethod, + callableDescriptor.unwrappedSetMethod + ) + } else { + listOf(callableDescriptor) + } + foundDescriptors.flatMap { + it.overriddenTreeUniqueAsSequence(true).toList() + } + }?.toSet() ?: return for (descriptor in descriptors) { methods.find { it.value.match(descriptor) }?.let { forbidden -> diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt index b2f64a47235..4a94468c15f 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt @@ -433,6 +433,62 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) { assertThat(findings).hasSize(1) } + @Test + fun `should report class grandparent interface default method is not allowed`() { + val code = """ + package org.example.com + open class SimpleComparator : Comparator { + override fun compare(p0: String?, p1: String?): Int { + return 0 + } + } + + class ComplexComparator : SimpleComparator() { + override fun compare(p0: String?, p1: String?): Int { + return 0 + } + } + + fun foo(bar1: ComplexComparator) { + bar1.reversed() + val bar2 = ComplexComparator() + bar2.reversed() + } + """.trimIndent() + val findings = ForbiddenMethodCall( + TestConfig(METHODS to listOf("java.util.Comparator.reversed")) + ).compileAndLintWithContext(env, code) + assertThat(findings).hasSize(2) + } + + @Test + fun `should report class when grandparent default interface is not allowed extending other class`() { + val code = """ + package org.example.com + open class Parent + open class SimpleComparator : Parent(), Comparator { + override fun compare(p0: String?, p1: String?): Int { + return 0 + } + } + + class ComplexComparator : SimpleComparator() { + override fun compare(p0: String?, p1: String?): Int { + return 0 + } + } + + fun foo(i: ComplexComparator) { + val bar = ComplexComparator() + bar.reversed() + } + """.trimIndent() + val findings = ForbiddenMethodCall( + TestConfig(METHODS to listOf("java.util.Comparator.reversed")) + ).compileAndLintWithContext(env, code) + assertThat(findings).hasSize(1) + } + @Test fun `should report functions with lambda params`() { val code = """ From 6ee176d379d7c04bc0e1beea77260e8a4a626c0b Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Tue, 12 Sep 2023 15:24:44 +0530 Subject: [PATCH 2/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Brais Gabín --- .../arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt index 4a94468c15f..431fecd1742 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt @@ -478,7 +478,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) { } } - fun foo(i: ComplexComparator) { + fun foo() { val bar = ComplexComparator() bar.reversed() } From c1d8560cb45aac7eebac3cf117e8c089fed69fff Mon Sep 17 00:00:00 2001 From: Atul Gupta Date: Tue, 12 Sep 2023 15:34:50 +0530 Subject: [PATCH 3/3] Create set inside let instead of using `toSet()` --- .../detekt/rules/style/ForbiddenMethodCall.kt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt index b54a2898eeb..2f36f176703 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt @@ -12,6 +12,7 @@ import io.gitlab.arturbosch.detekt.api.config import io.gitlab.arturbosch.detekt.api.internal.Configuration import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution import io.gitlab.arturbosch.detekt.api.valuesWithReason +import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.SyntheticPropertyDescriptor @@ -113,20 +114,20 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) { } private fun check(expression: KtExpression) { - val descriptors = + val descriptors: Set = expression.getResolvedCall(bindingContext)?.resultingDescriptor?.let { callableDescriptor -> val foundDescriptors = if (callableDescriptor is PropertyDescriptor) { - listOfNotNull( + setOfNotNull( callableDescriptor.unwrappedGetMethod, callableDescriptor.unwrappedSetMethod ) } else { - listOf(callableDescriptor) + setOf(callableDescriptor) } - foundDescriptors.flatMap { - it.overriddenTreeUniqueAsSequence(true).toList() + foundDescriptors.flatMapTo(mutableSetOf()) { + it.overriddenTreeUniqueAsSequence(true).toSet() } - }?.toSet() ?: return + } ?: return for (descriptor in descriptors) { methods.find { it.value.match(descriptor) }?.let { forbidden ->