From a9e8e45f47e5c5f4aa7e86462f9de844847b1368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sat, 4 Jun 2022 12:54:47 +0200 Subject: [PATCH 1/5] Improve tests --- .../arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt | 4 ++++ 1 file changed, 4 insertions(+) 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 754419f7773..ba03ac4c046 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 @@ -28,6 +28,8 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) { SourceLocation(2, 5), SourceLocation(3, 5) ) + assertThat(findings[0]).hasMessage("The method kotlin.io.print has been forbidden in the Detekt config.") + assertThat(findings[1]).hasMessage("The method kotlin.io.println has been forbidden in the Detekt config.") } @Test @@ -202,6 +204,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 From 032efe2e53986d21957f4a298eca0bb2dd337543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sat, 4 Jun 2022 12:55:38 +0200 Subject: [PATCH 2/5] Improve issue message --- .../arturbosch/detekt/rules/style/ForbiddenMethodCall.kt | 2 +- .../detekt/rules/style/ForbiddenMethodCallSpec.kt | 6 +++--- 2 files changed, 4 insertions(+), 4 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 f3cea1f1b66..626a0db18ae 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 @@ -114,7 +114,7 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) { for (descriptor in descriptors) { methods.find { it.match(descriptor) }?.let { functionMatcher -> - val message = "The method $functionMatcher has been forbidden in the Detekt config." + val message = "The method `$functionMatcher` has been forbidden in the Detekt config." report(CodeSmell(issue, Entity.from(expression), message)) } } 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 ba03ac4c046..64f9a4061cc 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 @@ -28,8 +28,8 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) { SourceLocation(2, 5), SourceLocation(3, 5) ) - assertThat(findings[0]).hasMessage("The method kotlin.io.print has been forbidden in the Detekt config.") - assertThat(findings[1]).hasMessage("The method kotlin.io.println has been forbidden in the Detekt config.") + assertThat(findings[0]).hasMessage("The method `kotlin.io.print` has been forbidden in the Detekt config.") + assertThat(findings[1]).hasMessage("The method `kotlin.io.println` has been forbidden in the Detekt config.") } @Test @@ -205,7 +205,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) { 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.") + .hasMessage("The method `java.time.LocalDate.now()` has been forbidden in the Detekt config.") } @Test From ae028c092c9105af9e8f46addf999f0ca8cb956c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sat, 4 Jun 2022 13:11:25 +0200 Subject: [PATCH 3/5] Removes that test support with comma-separated strings --- .../rules/style/ForbiddenMethodCallSpec.kt | 28 ++++--------------- 1 file changed, 5 insertions(+), 23 deletions(-) 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 64f9a4061cc..5b87b69a12e 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 @@ -40,11 +40,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() } @@ -114,22 +112,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 = """ @@ -461,7 +443,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 ) @@ -479,7 +461,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 ) From eb9a7c0d147fcfae7eec90c5ed18dcce0c45260c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Sat, 4 Jun 2022 13:20:02 +0200 Subject: [PATCH 4/5] Add support for reasons in ForbiddenMethodCall --- .../main/resources/default-detekt-config.yml | 6 +++-- .../detekt/rules/style/ForbiddenMethodCall.kt | 24 +++++++++++++------ .../rules/style/ForbiddenMethodCallSpec.kt | 23 +++++++++++------- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 7574e161edb..0463407d328 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/detekt-core/src/main/resources/default-detekt-config.yml @@ -531,8 +531,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/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 626a0db18ae..db9da2e971e 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 @@ -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 by config( - listOf( - "kotlin.io.print", - "kotlin.io.println", + private val methods: List 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/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 5b87b69a12e..e0b456d798e 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 @@ -23,13 +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[0]).hasMessage("The method `kotlin.io.print` has been forbidden in the Detekt config.") - assertThat(findings[1]).hasMessage("The method `kotlin.io.println` has been forbidden in the Detekt config.") + + 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 @@ -480,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 ) @@ -498,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 ) From ae4f2463f7118a9fa52acc12be5e57361175eb17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Wed, 20 Jul 2022 22:52:08 +0200 Subject: [PATCH 5/5] 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> --- .../arturbosch/detekt/rules/style/ForbiddenImport.kt | 2 +- .../arturbosch/detekt/rules/style/ForbiddenMethodCall.kt | 2 +- .../arturbosch/detekt/rules/style/ForbiddenImportSpec.kt | 8 ++++---- .../detekt/rules/style/ForbiddenMethodCallSpec.kt | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImport.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImport.kt index c8ed4d005f4..ed6a1b474c8 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImport.kt +++ b/detekt-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/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 db9da2e971e..a49cfb05eb2 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 @@ -121,7 +121,7 @@ class ForbiddenMethodCall(config: Config = Config.empty) : Rule(config) { 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." + "The method `${forbidden.value}` has been forbidden in the detekt config." } report(CodeSmell(issue, Entity.from(expression), message)) } diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImportSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImportSpec.kt index 3e8e4e842cc..18370f8a807 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImportSpec.kt +++ b/detekt-rules-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/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 e0b456d798e..c5485f58ea1 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 @@ -192,7 +192,7 @@ class ForbiddenMethodCallSpec(val env: KotlinCoreEnvironment) { 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.") + .hasMessage("The method `java.time.LocalDate.now()` has been forbidden in the detekt config.") } @Test