Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ForbiddenMethodCall - Handle sequence of overridden methods #6478

Merged
merged 3 commits into from
Sep 12, 2023
Merged

ForbiddenMethodCall - Handle sequence of overridden methods #6478

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d57beda
Handle sequence of overridden methods
atulgpt Sep 11, 2023
6ee176d
Apply suggestions from code review
atulgpt Sep 12, 2023
c1d8560
Create set inside let instead of using `toSet()`
atulgpt Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 15 additions & 9 deletions ...ules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could avoid this call to toSet if we create sets directly instead of lists inside the let.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the code


for (descriptor in descriptors) {
methods.find { it.value.match(descriptor) }?.let { forbidden ->
Expand Down
56 changes: 56 additions & 0 deletions ...-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {
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<String> {
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) {
atulgpt marked this conversation as resolved.
Show resolved Hide resolved
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 = """
Expand Down