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

Adds ForbiddenSuppress rule #4899

Merged
merged 1 commit into from Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -526,6 +526,9 @@ style:
ignorePackages:
- '*.internal'
- '*.internal.*'
ForbiddenSuppress:
active: false
rules: []
ForbiddenVoid:
active: true
ignoreOverridden: false
Expand Down
@@ -0,0 +1,90 @@
package io.gitlab.arturbosch.detekt.rules.style

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Entity
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.config
import io.gitlab.arturbosch.detekt.api.internal.Configuration
import org.jetbrains.kotlin.psi.KtAnnotationEntry
import org.jetbrains.kotlin.psi.KtLiteralStringTemplateEntry
import org.jetbrains.kotlin.psi.KtValueArgument
import org.jetbrains.kotlin.psi.KtValueArgumentList
import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType

/**
* This rule allows to set a list of rules whose suppression is forbidden.
* This can be used to discourage the abuse of the `Suppress` and `SuppressWarnings` annotations.
* Detekt will report suppression of all forbidden rules.
* This rule is not capable of reporting suppression of itself, as that's a language feature with precedence.
*
* <noncompliant>
* package foo
*
* // When the rule "MaximumLineLength" is forbidden
* @@Suppress("MaximumLineLength", "UNCHECKED_CAST")
* class Bar
* </noncompliant>
*
* <compliant>
* package foo
*
* // When the rule "MaximumLineLength" is forbidden
* @@Suppress("UNCHECKED_CAST")
* class Bar
* </compliant>
*/
class ForbiddenSuppress(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
javaClass.simpleName,
Severity.Maintainability,
"Suppressing a rule which is forbidden in current configuration.",
Debt.TEN_MINS
)

@Configuration("Rules whose suppression is forbidden.")
private val rules: List<String> by config(emptyList())

override fun visitAnnotationEntry(annotationEntry: KtAnnotationEntry) {
if (rules.isEmpty()) return
val shortName = annotationEntry.shortName?.asString()
if (shortName == KOTLIN_SUPPRESS || shortName == JAVA_SUPPRESS) {
val nonCompliantRules = annotationEntry.children
.find { it is KtValueArgumentList }
?.children
?.filterIsInstance<KtValueArgument>()
?.filterForbiddenRules()
.orEmpty()
if (nonCompliantRules.isNotEmpty()) {
report(
CodeSmell(
issue,
Entity.from(annotationEntry),
message = "Cannot @Suppress ${nonCompliantRules.formatMessage()} " +
"due to the current configuration.",
)
)
}
}
}

private fun List<String>.formatMessage(): String = if (size > 1) {
"rules "
} else {
"rule "
} + joinToString(", ") { "\"$it\"" }

private fun List<KtValueArgument>.filterForbiddenRules(): List<String> = mapNotNull { argument ->
val text = argument.findDescendantOfType<KtLiteralStringTemplateEntry>()?.text
if (rules.contains(text)) text else null
}

private companion object {
private const val KOTLIN_SUPPRESS = "Suppress"
private const val JAVA_SUPPRESS = "SuppressWarnings"
}
}
Expand Up @@ -38,6 +38,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
ForbiddenImport(config),
ForbiddenMethodCall(config),
ForbiddenPublicDataClass(config),
ForbiddenSuppress(config),
FunctionOnlyReturningConstant(config),
SpacingBetweenPackageAndImports(config),
LoopWithTooManyJumpStatements(config),
Expand Down
@@ -0,0 +1,201 @@
package io.gitlab.arturbosch.detekt.rules.style

import io.gitlab.arturbosch.detekt.test.TestConfig
import io.gitlab.arturbosch.detekt.test.assertThat
import io.gitlab.arturbosch.detekt.test.compileAndLint
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test

internal class ForbiddenSuppressSpec {

@Nested
inner class `checking for suppressions of rule ARule` {
private val subject = ForbiddenSuppress(
TestConfig("rules" to listOf("ARule"))
)

@Test
fun `supports java suppress annotations`() {
val code = """
package config

@SuppressWarnings("ARule")
class Foo
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings).hasSourceLocation(3, 1)
assertThat(findings.first()).hasMessage(
"Cannot @Suppress rule \"ARule\" due to the current configuration."
)
}

@Test
fun `reports file-level suppression of forbidden rule`() {
val code = """
@file:Suppress("ARule")
package config
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings).hasSourceLocation(1, 1)
assertThat(findings.first()).hasMessage(
"Cannot @Suppress rule \"ARule\" due to the current configuration."
)
}

@Test
fun `reports top-level suppression of forbidden rule`() {
val code = """
package config

@Suppress("ARule")
fun foo() { }
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings).hasSourceLocation(3, 1)
assertThat(findings.first()).hasMessage(
"Cannot @Suppress rule \"ARule\" due to the current configuration."
)
}

@Test
fun `reports line-level suppression of forbidden rule`() {
val code = """
package config

fun foo() {
@Suppress("ARule")
println("bar")
}
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings).hasSourceLocation(4, 5)
assertThat(findings.first()).hasMessage(
"Cannot @Suppress rule \"ARule\" due to the current configuration."
)
}

@Test
fun `doesn't report non-forbidden rule`() {
val code = """
package config

@Suppress("UNCHECKED_CAST")
fun foo() { }
"""
val findings = subject.compileAndLint(code)
assertThat(findings).isEmpty()
}

@Test
fun `does not include non-forbidden rule in report`() {
val code = """
package config

@Suppress("UNCHECKED_CAST", "ARule")
fun foo() { }
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings.first()).hasMessage(
"Cannot @Suppress rule \"ARule\" due to the current configuration."
)
}

@Test
fun `runs and does not report suppress without rules`() {
val code = """
@file:Suppress()
package config

@Suppress
fun foo() { }
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(0)
}
}

@Nested
inner class `checking multiple forbidden rules` {
private val subject = ForbiddenSuppress(
TestConfig("rules" to listOf("ARule", "BRule"))
)

@Test
fun `reports suppression of both forbidden rules`() {
val code = """
@file:Suppress("ARule", "BRule")
package config
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings).hasSourceLocation(1, 1)
assertThat(findings.first()).hasMessage(
"Cannot @Suppress rules \"ARule\", \"BRule\" " +
"due to the current configuration."
)
}

@Test
fun `reports method-level suppression of one of two forbidden rules`() {
val code = """
package config

@Suppress("BRule")
fun foo() { }
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(1)
assertThat(findings).hasSourceLocation(3, 1)
assertThat(findings.first()).hasMessage(
"Cannot @Suppress rule \"BRule\" due to the current configuration."
)
}
}

@Nested
inner class `checking suppression of self` {
private val subject = ForbiddenSuppress(
TestConfig("rules" to listOf("ForbiddenSuppress", "ARule"))
)

@Test
fun `does not catch self-suppression`() {
val code = """
@file:Suppress("ForbiddenSuppress")
package config
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(0)
}

@Test
fun `does not catch suppression of any forbidden rule when one of them`() {
val code = """
@file:Suppress("ForbiddenSuppress", "ARule")
package config
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(0)
}
}

@Nested
inner class `checking active with no rules defined` {
private val subject = ForbiddenSuppress(TestConfig())

@Test
fun `will not report issues with no forbidden rules defined`() {
val code = """
@file:Suppress("ARule")
package config
"""
val findings = subject.compileAndLint(code)
assertThat(findings).hasSize(0)
}
}
}