Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This rule is based on discussion #4890. Checks for suppression of one or more rules that have been decided to never be suppressed within a code base. For example, if we've set MaximumLineLength to a value, every file must be compliant, and if there is a special case where compliance is discouraged, we can simply exclude this file within YML configuration. This rule cannot prevent itself from being suppressed, but should serve to discourage the over use of `@Suppress` annotations.
- Loading branch information
Showing
4 changed files
with
257 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
90 changes: 90 additions & 0 deletions
90
...-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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.KtStringTemplateExpression | ||
import org.jetbrains.kotlin.psi.KtValueArgument | ||
import org.jetbrains.kotlin.psi.KtValueArgumentList | ||
|
||
/** | ||
* This rule allows to set a list of rules whose suppression is forbidden. This can be used to discourage 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, | ||
"", | ||
Debt.TEN_MINS | ||
) | ||
|
||
@Configuration("Rules whose suppression is forbidden.") | ||
private val rules: List<String> by config(emptyList()) | ||
|
||
override fun visitAnnotationEntry(annotationEntry: KtAnnotationEntry) { | ||
val shortName = annotationEntry.shortName!!.asString() | ||
if (shortName == KOTLIN_SUPPRESS || shortName == JAVA_SUPPRESS) { | ||
val nonCompliantRules = annotationEntry.children | ||
.find { it is KtValueArgumentList } | ||
?.children | ||
?.filterIsInstance<KtValueArgument>() | ||
?.findForbiddenRules() ?: emptyList() | ||
if (nonCompliantRules.isNotEmpty()) { | ||
report( | ||
CodeSmell( | ||
issue, | ||
Entity.from(annotationEntry), | ||
message = "Cannot @Suppress ${nonCompliantRules.formatMessage()}.", | ||
) | ||
) | ||
} | ||
} | ||
} | ||
|
||
private fun List<String>.formatMessage(): String = if (size > 1) { | ||
"rules " | ||
} else { | ||
"rule " | ||
} + joinToString(", ") { "\"$it\"" } | ||
|
||
private fun List<KtValueArgument>.findForbiddenRules(): List<String> = mapNotNull { argument -> | ||
val stringTemplate = argument.children | ||
.find { it is KtStringTemplateExpression } | ||
?.children | ||
?.find { it is KtLiteralStringTemplateEntry } | ||
val text = stringTemplate?.text | ||
if (rules.contains(text)) text else null | ||
} | ||
|
||
private companion object { | ||
private const val KOTLIN_SUPPRESS = "Suppress" | ||
private const val JAVA_SUPPRESS = "SuppressWarnings" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
163 changes: 163 additions & 0 deletions
163
...es-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppressSpec.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
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 | ||
import org.junit.jupiter.params.ParameterizedTest | ||
import org.junit.jupiter.params.provider.ValueSource | ||
|
||
internal class ForbiddenSuppressSpec { | ||
|
||
@Nested | ||
inner class `checking for suppressions of rule ARule` { | ||
private val subject = ForbiddenSuppress( | ||
TestConfig("rules" to listOf("ARule")) | ||
) | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = ["Suppress", "SuppressWarnings"]) | ||
fun `supports both kotlin and java suppress annotations`(annotation: String) { | ||
val code = """ | ||
package config | ||
@file:$annotation("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\".") | ||
} | ||
|
||
@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\".") | ||
} | ||
|
||
@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\".") | ||
} | ||
|
||
@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\".") | ||
} | ||
|
||
@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\".") | ||
} | ||
} | ||
|
||
@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\"." | ||
) | ||
} | ||
|
||
@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\".") | ||
} | ||
} | ||
|
||
@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) | ||
} | ||
} | ||
} |