Adding support for full method signatures in ForbiddenMethodCall (#3505)

detekt-psi-utils/api/detekt-psi-utils.api

@@ -104,6 +104,7 @@ public final class io/gitlab/arturbosch/detekt/rules/KtValueArgumentKt {
 }
 
 public final class io/gitlab/arturbosch/detekt/rules/MethodSignatureKt {
+	public static final fun extractMethodNameAndParams (Ljava/lang/String;)Lkotlin/Pair;
 	public static final fun hasCorrectEqualsParameter (Lorg/jetbrains/kotlin/psi/KtFunction;)Z
 	public static final fun isEqualsFunction (Lorg/jetbrains/kotlin/psi/KtFunction;)Z
 	public static final fun isHashCodeFunction (Lorg/jetbrains/kotlin/psi/KtFunction;)Z

detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/MethodSignature.kt

@@ -16,6 +16,20 @@ fun KtFunction.hasCorrectEqualsParameter() =
 
 fun KtNamedFunction.isMainFunction() = hasMainSignature() && (this.isTopLevel || isMainInsideObject())
 
+fun extractMethodNameAndParams(methodSignature: String): Pair<String, List<String>?> {
+    val tokens = methodSignature.split("(", ")")
+        .map { it.trim() }
+
+    val methodName = tokens.first().replace("`", "")
+    val params = if (tokens.size > 1) {
+        tokens[1].split(",").map { it.trim() }.filter { it.isNotBlank() }
+    } else {
+        null
+    }
+
+    return methodName to params
+}
+
 private fun KtNamedFunction.hasMainSignature() =
     this.name == "main" && this.isPublicNotOverridden() && this.hasMainParameter()
 

detekt-psi-utils/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/MethodSignatureSpec.kt

@@ -0,0 +1,57 @@
+package io.gitlab.arturbosch.detekt.rules
+
+import org.assertj.core.api.Assertions.assertThat
+import org.spekframework.spek2.Spek
+import org.spekframework.spek2.style.specification.describe
+
+class MethodSignatureSpec : Spek({
+    describe("extractMethodNameAndParams") {
+        listOf(
+            TestCase(
+                testDescription = "should return method name and null params list in case of simplifies signature",
+                methodSignature = "java.time.LocalDate.now",
+                expectedMethodName = "java.time.LocalDate.now",
+                expectedParams = null
+            ),
+            TestCase(
+                testDescription = "should return method name and empty params list for full signature parameterless method",
+                methodSignature = "java.time.LocalDate.now()",
+                expectedMethodName = "java.time.LocalDate.now",
+                expectedParams = emptyList()
+            ),
+            TestCase(
+                testDescription = "should return method name and params list for full signature method with single param",
+                methodSignature = "java.time.LocalDate.now(java.time.Clock)",
+                expectedMethodName = "java.time.LocalDate.now",
+                expectedParams = listOf("java.time.Clock")
+            ),
+            TestCase(
+                testDescription = "should return method name and params list for full signature method with multiple params",
+                methodSignature = "java.time.LocalDate.of(kotlin.Int, kotlin.Int, kotlin.Int)",
+                expectedMethodName = "java.time.LocalDate.of",
+                expectedParams = listOf("kotlin.Int", "kotlin.Int", "kotlin.Int")
+            ),
+            TestCase(
+                testDescription = "should return method name and params list for full signature method with multiple params " +
+                        "where method name has spaces and special characters",
+                methodSignature = "io.gitlab.arturbosch.detekt.SomeClass.`some , method`(kotlin.String)",
+                expectedMethodName = "io.gitlab.arturbosch.detekt.SomeClass.some , method",
+                expectedParams = listOf("kotlin.String")
+            )
+        ).forEach { testCase ->
+            it(testCase.testDescription) {
+                val (methodName, params) = extractMethodNameAndParams(testCase.methodSignature)
+
+                assertThat(methodName).isEqualTo(testCase.expectedMethodName)
+                assertThat(params).isEqualTo(testCase.expectedParams)
+            }
+        }
+    }
+})
+
+private class TestCase(
+    val testDescription: String,
+    val methodSignature: String,
+    val expectedMethodName: String,
+    val expectedParams: List<String>?
+)

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

@@ -8,6 +8,8 @@ import io.gitlab.arturbosch.detekt.api.Issue
 import io.gitlab.arturbosch.detekt.api.Rule
 import io.gitlab.arturbosch.detekt.api.Severity
 import io.gitlab.arturbosch.detekt.api.internal.valueOrDefaultCommaSeparated
+import io.gitlab.arturbosch.detekt.rules.extractMethodNameAndParams
+import org.jetbrains.kotlin.js.descriptorUtils.getJetTypeFqName
 import org.jetbrains.kotlin.psi.KtBinaryExpression
 import org.jetbrains.kotlin.psi.KtCallExpression
 import org.jetbrains.kotlin.psi.KtExpression
@@ -29,7 +31,10 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
  * }
  * </noncompliant>
  *
- * @configuration methods - Comma separated list of fully qualified method signatures which are forbidden
+ * @configuration methods - Comma separated list of fully qualified method signatures which are forbidden.
+ * Methods can be defined without full signature (i.e. `java.time.LocalDate.now`) which will report calls of all methods
+ * with this name or with full signature (i.e. `java.time.LocalDate(java.time.Clock)`) which would report only call
+ * with this concrete signature.
  *  (default: `['kotlin.io.println', 'kotlin.io.print']`)
  *
  * @requiresTypeResolution
@@ -45,6 +50,7 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) {
     )
 
     private val forbiddenMethods = valueOrDefaultCommaSeparated(METHODS, DEFAULT_METHODS)
+        .map { extractMethodNameAndParams(it) }
 
     override fun visitCallExpression(expression: KtCallExpression) {
         super.visitCallExpression(expression)
@@ -70,14 +76,29 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) {
         if (bindingContext == BindingContext.EMPTY) return
 
         val resolvedCall = expression.getResolvedCall(bindingContext) ?: return
-        val fqName = resolvedCall.resultingDescriptor.fqNameOrNull()?.asString()
+        val methodName = resolvedCall.resultingDescriptor.fqNameOrNull()?.asString()
+        val encounteredParamTypes = resolvedCall.resultingDescriptor.valueParameters
+            .map { it.type.getJetTypeFqName(false) }
 
-        if (fqName != null && fqName in forbiddenMethods) {
-            report(
-                CodeSmell(
-                    issue, Entity.from(expression), "The method $fqName has been forbidden in the Detekt config."
-                )
-            )
+        if (methodName != null) {
+            forbiddenMethods
+                .filter { methodName == it.first }
+                .forEach {
+                    val expectedParamTypes = it.second
+                    val noParamsProvided = expectedParamTypes == null
+                    val paramsMatch = expectedParamTypes == encounteredParamTypes
+
+                    if (noParamsProvided || paramsMatch) {
+                        report(
+                            CodeSmell(
+                                issue,
+                                Entity.from(expression),
+                                "The method ${it.first}(${expectedParamTypes?.joinToString() ?: ""}) " +
+                                        "has been forbidden in the Detekt config."
+                            )
+                        )
+                    }
+                }
         }
     }
 

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

@@ -95,7 +95,14 @@ class ForbiddenMethodCallSpec : Spek({
             }
             """
             val findings = ForbiddenMethodCall(
-                TestConfig(mapOf(ForbiddenMethodCall.METHODS to listOf("java.io.PrintStream.println", "java.lang.System.gc")))
+                TestConfig(
+                    mapOf(
+                        ForbiddenMethodCall.METHODS to listOf(
+                            "java.io.PrintStream.println",
+                            "java.lang.System.gc"
+                        )
+                    )
+                )
             ).compileAndLintWithContext(env, code)
             assertThat(findings).hasSize(2)
             assertThat(findings).hasTextLocations(48 to 64, 76 to 80)
@@ -119,7 +126,7 @@ class ForbiddenMethodCallSpec : Spek({
         it("should report equals operator") {
             val code = """
                 fun main() {
-                    java.math.BigDecimal(5.5) == java.math.BigDecimal(5.5) 
+                    java.math.BigDecimal(5.5) == java.math.BigDecimal(5.5)
                 }
             """
             val findings = ForbiddenMethodCall(
@@ -153,5 +160,118 @@ class ForbiddenMethodCallSpec : Spek({
             ).compileAndLintWithContext(env, code)
             assertThat(findings).hasSize(1)
         }
+
+        it("should report both methods when using just method name without full signature") {
+            val code = """
+                import java.time.Clock
+                import java.time.LocalDate
+                fun test() {
+                    val clock = Clock.systemUTC()
+                    val date = LocalDate.now()
+                    val date2 = LocalDate.now(clock)
+                }
+            """
+            val findings = ForbiddenMethodCall(
+                TestConfig(mapOf(ForbiddenMethodCall.METHODS to listOf("java.time.LocalDate.now")))
+            ).compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(2)
+        }
+
+        it("should report parameterless method when full signature matches") {
+            val code = """
+                import java.time.Clock
+                import java.time.LocalDate
+                fun test() {
+                    val clock = Clock.systemUTC()
+                    val date = LocalDate.now()
+                    val date2 = LocalDate.now(clock)
+                }
+            """
+            val findings = ForbiddenMethodCall(
+                TestConfig(mapOf(ForbiddenMethodCall.METHODS to listOf("java.time.LocalDate.now()")))
+            ).compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(5, 26)
+        }
+
+        it("should report method with param when full signature matches") {
+            val code = """
+                import java.time.Clock
+                import java.time.LocalDate
+                fun test() {
+                    val clock = Clock.systemUTC()
+                    val date = LocalDate.now()
+                    val date2 = LocalDate.now(clock)
+                }
+            """
+            val findings = ForbiddenMethodCall(
+                TestConfig(mapOf(ForbiddenMethodCall.METHODS to listOf("java.time.LocalDate.now(java.time.Clock)")))
+            ).compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(6, 27)
+        }
+
+        it("should report method with multiple params when full signature matches") {
+            val code = """
+                import java.time.LocalDate
+                fun test() {
+                    val date = LocalDate.of(2020, 1, 1)
+                }
+            """
+            val findings = ForbiddenMethodCall(
+                TestConfig(mapOf(ForbiddenMethodCall.METHODS to listOf("java.time.LocalDate.of(kotlin.Int, kotlin.Int, kotlin.Int)")))
+            ).compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(3, 26)
+        }
+
+        it("should report method with multiple params when full signature matches with additional spacing") {
+            val code = """
+                import java.time.LocalDate
+                fun test() {
+                    val date = LocalDate.of(2020, 1, 1)
+                }
+            """
+            val findings = ForbiddenMethodCall(
+                TestConfig(mapOf(ForbiddenMethodCall.METHODS to listOf("java.time.LocalDate.of(kotlin.Int,kotlin.Int,kotlin.Int)")))
+            ).compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(3, 26)
+        }
+
+        it("should report method with multiple params when method has spaces and commas") {
+            val code = """
+                package io.gitlab.arturbosch.detekt.rules.style
+                
+                fun `some, test`() = "String"
+
+                fun test() {
+                    val s = `some, test`()
+                }
+            """
+            val findings = ForbiddenMethodCall(
+                TestConfig(mapOf(ForbiddenMethodCall.METHODS to listOf("io.gitlab.arturbosch.detekt.rules.style.`some, test`()")))
+            ).compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(6, 13)
+        }
+
+        it("should report method with default params") {
+            val code = """
+                package io.gitlab.arturbosch.detekt.rules.style
+                
+                fun defaultParamsMethod(s:String, i:Int = 0) = s + i
+
+                fun test() {
+                    val s = defaultParamsMethod("test")
+                }
+            """
+            val findings = ForbiddenMethodCall(
+                TestConfig(mapOf(ForbiddenMethodCall.METHODS to
+                        listOf("io.gitlab.arturbosch.detekt.rules.style.defaultParamsMethod(kotlin.String,kotlin.Int)")))
+            ).compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings).hasSourceLocation(6, 13)
+        }
     }
 })