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

Add option to add a reason to ForbiddenMethodCall #4910

Merged
merged 5 commits into from Jul 23, 2022
Merged

Add option to add a reason to ForbiddenMethodCall #4910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a9e8e45
Improve tests
BraisGabin Jun 4, 2022
032efe2
Improve issue message
BraisGabin Jun 4, 2022
ae028c0
Removes that test support with comma-separated strings
BraisGabin Jun 4, 2022
eb9a7c0
Add support for reasons in ForbiddenMethodCall
BraisGabin Jun 4, 2022
ae4f246
Update detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt…
BraisGabin Jul 20, 2022
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
6 changes: 4 additions & 2 deletions detekt-core/src/main/resources/default-detekt-config.yml
View file
Open in desktop
Expand Up @@ -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: ['**']
Expand Down
2 changes: 1 addition & 1 deletion ...kt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImport.kt
View file
Open in desktop
Expand Up @@ -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 =
Expand Down
24 changes: 17 additions & 7 deletions ...ules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCall.kt
View file
Open in desktop
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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?)
}
8 changes: 4 additions & 4 deletions ...ules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenImportSpec.kt
View file
Open in desktop
Expand Up @@ -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.",
)
}

Expand Down Expand Up @@ -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.")
}
}
51 changes: 21 additions & 30 deletions ...-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenMethodCallSpec.kt
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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()
}

Expand Down Expand Up @@ -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 = """
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
)
Expand All @@ -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
)
Expand All @@ -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
)
Expand All @@ -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
)
Expand Down