Add option to add a reason to ForbiddenMethodCall (#4910)

* Improve tests * Improve issue message * Removes that test support with comma-separated strings * Add support for reasons in ForbiddenMethodCall * Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com> Co-authored-by: schalkms <30376729+schalkms@users.noreply.github.com>
detekt · Jul 23, 2022 · 855065b · 855065b
1 parent 13d823c
commit 855065b
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 44 deletions.
diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml
@@ -534,8 +534,10 @@ style:
   ForbiddenMethodCall:
     active: false
     methods:
-      - 'kotlin.io.print'
-      - 'kotlin.io.println'
+      - reason: 'print does not allow you to configure the output stream. Use a logger instead.'
+        value: 'kotlin.io.print'
+      - reason: 'println does not allow you to configure the output stream. Use a logger instead.'
+        value: 'kotlin.io.println'
   ForbiddenPublicDataClass:
     active: true
     excludes: ['**']

diff --git a/...kt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImport.kt b/...kt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImport.kt
@@ -62,7 +62,7 @@ class ForbiddenImport(config: Config = Config.empty) : Rule(config) {
     }
 
     private fun defaultReason(forbiddenImport: String): String {
-        return "The import `$forbiddenImport` has been forbidden in the Detekt config."
+        return "The import `$forbiddenImport` has been forbidden in the detekt config."
     }
 
     private fun containsForbiddenPattern(import: String): Boolean =

diff --git a/...ules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt b/...ules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt
@@ -1,6 +1,7 @@
 package io.gitlab.arturbosch.detekt.rules.style
 
 import io.github.detekt.tooling.api.FunctionMatcher
+import io.github.detekt.tooling.api.FunctionMatcher.Companion.fromFunctionSignature
 import io.gitlab.arturbosch.detekt.api.CodeSmell
 import io.gitlab.arturbosch.detekt.api.Config
 import io.gitlab.arturbosch.detekt.api.Debt
@@ -11,6 +12,7 @@ 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 io.gitlab.arturbosch.detekt.api.valuesWithReason
 import org.jetbrains.kotlin.descriptors.CallableDescriptor
 import org.jetbrains.kotlin.descriptors.PropertyDescriptor
 import org.jetbrains.kotlin.psi.KtBinaryExpression
@@ -61,12 +63,14 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) {
             "`fun String.hello(a: Int)` you should add the receiver parameter as the first parameter like this: " +
             "`hello(kotlin.String, kotlin.Int)`"
     )
-    private val methods: List<FunctionMatcher> by config(
-        listOf(
-            "kotlin.io.print",
-            "kotlin.io.println",
+    private val methods: List<Forbidden> by config(
+        valuesWithReason(
+            "kotlin.io.print" to "print does not allow you to configure the output stream. Use a logger instead.",
+            "kotlin.io.println" to "println does not allow you to configure the output stream. Use a logger instead.",
         )
-    ) { it.map(FunctionMatcher::fromFunctionSignature) }
+    ) { list ->
+        list.map { Forbidden(fromFunctionSignature(it.value), it.reason) }
+    }
 
     override fun visitCallExpression(expression: KtCallExpression) {
         super.visitCallExpression(expression)
@@ -113,10 +117,16 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) {
         } ?: return
 
         for (descriptor in descriptors) {
-            methods.find { it.match(descriptor) }?.let { functionMatcher ->
-                val message = "The method $functionMatcher has been forbidden in the Detekt config."
+            methods.find { it.value.match(descriptor) }?.let { forbidden ->
+                val message = if (forbidden.reason != null) {
+                    "The method `${forbidden.value}` has been forbidden: ${forbidden.reason}"
+                } else {
+                    "The method `${forbidden.value}` has been forbidden in the detekt config."
+                }
                 report(CodeSmell(issue, Entity.from(expression), message))
             }
         }
     }
+
+    private data class Forbidden(val value: FunctionMatcher, val reason: String?)
 }
diff --git a/...ules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImportSpec.kt b/...ules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImportSpec.kt
@@ -48,8 +48,8 @@ class ForbiddenImportSpec {
         assertThat(findings)
             .extracting("message")
             .containsExactlyInAnyOrder(
-                "The import `kotlin.jvm.JvmField` has been forbidden in the Detekt config.",
-                "The import `kotlin.SinceKotlin` has been forbidden in the Detekt config.",
+                "The import `kotlin.jvm.JvmField` has been forbidden in the detekt config.",
+                "The import `kotlin.SinceKotlin` has been forbidden in the detekt config.",
             )
     }
 
@@ -133,8 +133,8 @@ class ForbiddenImportSpec {
             ForbiddenImport(TestConfig(mapOf(FORBIDDEN_PATTERNS to "net.*R|com.*expiremental"))).lint(code)
         assertThat(findings).hasSize(2)
         assertThat(findings[0].message)
-            .isEqualTo("The import `net.example.R.dimen` has been forbidden in the Detekt config.")
+            .isEqualTo("The import `net.example.R.dimen` has been forbidden in the detekt config.")
         assertThat(findings[1].message)
-            .isEqualTo("The import `net.example.R.dimension` has been forbidden in the Detekt config.")
+            .isEqualTo("The import `net.example.R.dimension` has been forbidden in the detekt config.")
     }
 }
diff --git a/...-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt b/...-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt
@@ -23,11 +23,18 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
         }
         """
         val findings = ForbiddenMethodCall(TestConfig()).compileAndLintWithContext(env, code)
-        assertThat(findings).hasSize(2)
-        assertThat(findings).hasSourceLocations(
-            SourceLocation(2, 5),
-            SourceLocation(3, 5)
-        )
+
+        assertThat(findings)
+            .hasSize(2)
+            .hasSourceLocations(
+                SourceLocation(2, 5),
+                SourceLocation(3, 5),
+            )
+            .extracting("message")
+            .containsExactly(
+                "The method `kotlin.io.print` has been forbidden: print does not allow you to configure the output stream. Use a logger instead.",
+                "The method `kotlin.io.println` has been forbidden: println does not allow you to configure the output stream. Use a logger instead.",
+            )
     }
 
     @Test
@@ -38,11 +45,9 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
             System.out.println("hello")
         }
         """
-        val findings =
-            ForbiddenMethodCall(TestConfig(mapOf(METHODS to "  "))).compileAndLintWithContext(
-                env,
-                code
-            )
+        val findings = ForbiddenMethodCall(
+            TestConfig(mapOf(METHODS to listOf("  ")))
+        ).compileAndLintWithContext(env, code)
         assertThat(findings).isEmpty()
     }
 
@@ -112,22 +117,6 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
         assertThat(findings).hasTextLocations(48 to 64, 76 to 80)
     }
 
-    @Test
-    fun `should report multiple different methods config with sting`() {
-        val code = """
-        import java.lang.System
-        fun main() {
-        System.out.println("hello")
-            System.gc()
-        }
-        """
-        val findings = ForbiddenMethodCall(
-            TestConfig(mapOf(METHODS to "java.io.PrintStream.println, java.lang.System.gc"))
-        ).compileAndLintWithContext(env, code)
-        assertThat(findings).hasSize(2)
-        assertThat(findings).hasTextLocations(48 to 64, 76 to 80)
-    }
-
     @Test
     fun `should report equals operator`() {
         val code = """
@@ -202,6 +191,8 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
         ).compileAndLintWithContext(env, code)
         assertThat(findings).hasSize(1)
         assertThat(findings).hasSourceLocation(5, 26)
+        assertThat(findings[0])
+            .hasMessage("The method `java.time.LocalDate.now()` has been forbidden in the detekt config.")
     }
 
     @Test
@@ -457,7 +448,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
                 }
             """
             val findings =
-                ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.getFirstDayOfWeek"))).compileAndLintWithContext(
+                ForbiddenMethodCall(TestConfig(mapOf(METHODS to listOf("java.util.Calendar.getFirstDayOfWeek")))).compileAndLintWithContext(
                     env,
                     code
                 )
@@ -475,7 +466,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
                 }
             """
             val findings =
-                ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.getFirstDayOfWeek"))).compileAndLintWithContext(
+                ForbiddenMethodCall(TestConfig(mapOf(METHODS to listOf("java.util.Calendar.getFirstDayOfWeek")))).compileAndLintWithContext(
                     env,
                     code
                 )
@@ -494,7 +485,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
                 }
             """
         val findings =
-            ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.setFirstDayOfWeek"))).compileAndLintWithContext(
+            ForbiddenMethodCall(TestConfig(mapOf(METHODS to listOf("java.util.Calendar.setFirstDayOfWeek")))).compileAndLintWithContext(
                 env,
                 code
             )
@@ -512,7 +503,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) {
                 }
             """
         val findings =
-            ForbiddenMethodCall(TestConfig(mapOf(METHODS to "java.util.Calendar.compareTo"))).compileAndLintWithContext(
+            ForbiddenMethodCall(TestConfig(mapOf(METHODS to listOf("java.util.Calendar.compareTo")))).compileAndLintWithContext(
                 env,
                 code
             )