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.

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

Make ForbiddenMethodCall to support property getters/setters and method references #5078

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Expand Up @@ -11,12 +11,20 @@ import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.config
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import org.jetbrains.kotlin.descriptors.CallableDescriptor
import org.jetbrains.kotlin.descriptors.PropertyDescriptor
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCallableReferenceExpression
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtPostfixExpression
import org.jetbrains.kotlin.psi.KtPrefixExpression
import org.jetbrains.kotlin.psi.psiUtil.isDotSelector
import org.jetbrains.kotlin.psi2ir.unwrappedGetMethod
import org.jetbrains.kotlin.psi2ir.unwrappedSetMethod
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.util.getCalleeExpressionIfAny
import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall

/**
Expand All @@ -28,6 +36,7 @@ import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
* import java.lang.System
* fun main() {
* System.gc()
* System::gc
* }
* </noncompliant>
*
Expand Down Expand Up @@ -69,6 +78,13 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) {
check(expression.operationReference)
}

override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) {
super.visitDotQualifiedExpression(expression)
if (expression.getCalleeExpressionIfAny()?.isDotSelector() == true) {
check(expression)
}
}

override fun visitPrefixExpression(expression: KtPrefixExpression) {
super.visitPrefixExpression(expression)
check(expression.operationReference)
Expand All @@ -79,11 +95,21 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) {
check(expression.operationReference)
}

override fun visitCallableReferenceExpression(expression: KtCallableReferenceExpression) {
super.visitCallableReferenceExpression(expression)
check(expression.callableReference)
}

private fun check(expression: KtExpression) {
if (bindingContext == BindingContext.EMPTY) return

val descriptors = expression.getResolvedCall(bindingContext)?.resultingDescriptor?.let {
listOf(it) + it.overriddenDescriptors
val foundDescriptors = if (it is PropertyDescriptor) {
listOfNotNull(it.unwrappedGetMethod, it.unwrappedSetMethod)
} else {
listOf(it)
}
foundDescriptors + foundDescriptors.flatMap(CallableDescriptor::getOverriddenDescriptors)
} ?: return

for (descriptor in descriptors) {
Expand Down
Expand Up @@ -465,7 +465,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
}

@Test
fun `should not report property call`() {
fun `should report property getters call`() {
val code = """
import java.util.Calendar

Expand All @@ -475,11 +475,47 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
}
"""
val findings =
ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.firstDayOfWeek"))).compileAndLintWithContext(
ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.getFirstDayOfWeek"))).compileAndLintWithContext(
env,
code
)
assertThat(findings).isEmpty()
assertThat(findings).hasSize(1)
}
}

@Test
fun `should report property setters call`() {
val code = """
import java.util.Calendar

fun main() {
val calendar = Calendar.getInstance()
calendar.firstDayOfWeek = 1
}
"""
val findings =
ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.setFirstDayOfWeek"))).compileAndLintWithContext(
env,
code
)
assertThat(findings).hasSize(1)
}

@Test
fun `should report reference call`() {
val code = """
import java.util.Calendar

fun main() {
val calendar = Calendar.getInstance()
calendar.let(calendar::getFirstDayOfWeek)
}
"""
val findings =
ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.getFirstDayOfWeek"))).compileAndLintWithContext(
env,
code
)
assertThat(findings).hasSize(1)
}
}