commit 7d4c0ad900e447e3e781c4f38f1c8a69c3fea51f
parent 43307fa6bc0b490b20026299e81f7132c81d2959
Author: Segun Famisa <sfamisa@mozilla.com>
Date: Sat, 25 Oct 2025 10:39:35 +0000
Bug 1991023 - Remove ForbiddenSuppressionRule in favor of detekt's ForbiddenSuppress r=android-reviewers,boek
We previously added a `ForbiddenSuppressionRule` but afterwards, we found out that there was a built-in equivalent called `ForbiddenSuppress`, so we would like to remove the custom one, to avoid duplication and any potential confusion on which rule to use.
Differential Revision: https://phabricator.services.mozilla.com/D266299
Diffstat:
5 files changed, 0 insertions(+), 440 deletions(-)
diff --git a/mobile/android/android-components/components/tooling/detekt/src/main/kotlin/mozilla/components/tooling/detekt/MozillaRuleSetProvider.kt b/mobile/android/android-components/components/tooling/detekt/src/main/kotlin/mozilla/components/tooling/detekt/MozillaRuleSetProvider.kt
@@ -7,7 +7,6 @@ package mozilla.components.tooling.detekt
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
-import mozilla.components.tooling.detekt.health.ForbiddenSuppressionRule
/**
* Set of custom mozilla rules to be loaded in detekt utility.
@@ -20,7 +19,6 @@ class MozillaRuleSetProvider : RuleSetProvider {
ruleSetId,
listOf(
ProjectLicenseRule(config),
- ForbiddenSuppressionRule(config),
),
)
}
diff --git a/mobile/android/android-components/components/tooling/detekt/src/main/kotlin/mozilla/components/tooling/detekt/health/ForbiddenSuppressionRule.kt b/mobile/android/android-components/components/tooling/detekt/src/main/kotlin/mozilla/components/tooling/detekt/health/ForbiddenSuppressionRule.kt
@@ -1,148 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package mozilla.components.tooling.detekt.health
-
-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.Finding
-import io.gitlab.arturbosch.detekt.api.Issue
-import io.gitlab.arturbosch.detekt.api.Location
-import io.gitlab.arturbosch.detekt.api.Rule
-import io.gitlab.arturbosch.detekt.api.RuleSetId
-import io.gitlab.arturbosch.detekt.api.Severity
-import io.gitlab.arturbosch.detekt.api.config
-import org.jetbrains.kotlin.psi.KtAnnotationEntry
-import org.jetbrains.kotlin.psi.KtCallExpression
-import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression
-import org.jetbrains.kotlin.psi.KtExpression
-import org.jetbrains.kotlin.psi.KtStringTemplateExpression
-
-/**
- * Detekt rule to report whether a forbidden rule has been suppressed.
- *
- * The goal of this rule is to have some rules that we never want to suppress. This is a bit recursive
- * because we also do not want this rule to be suppressed.
- *
- * Usage:
- *
- * Add these lines to your detekt config yml file:
- *
- * ```yaml
- * mozilla-rules:
- * ForbiddenSuppression:
- * active: true
- * # includeSelf means that a violation will be reported if ForbiddenSuppression is suppressed
- * # default value is true.
- * includeSelf: true
- * forbiddenSuppressions:
- * - 'ForbiddenRule1'
- * - 'ForbiddenRule2'
- * ```
- */
-class ForbiddenSuppressionRule(config: Config = Config.empty) : Rule(config) {
-
- /**
- * We are manually keeping track of the findings because we want to prevent this rule from being
- * suppressed, and detekt avoids reporting suppressed findings.
- */
- private val _findings: MutableList<Finding> = mutableListOf()
- override val findings: List<Finding>
- get() = _findings
-
- /**
- * Config indicating whether or not to include this rule as part of the forbidden suppressions
- *
- * Default value is true.
- */
- private val includeSelf: Boolean by config(defaultValue = true)
-
- /**
- * Config indicating the forbidden suppression. This is applied as a list in the configuration
- * but treated as a set internally for quick checks of violations.
- */
- private val forbiddenSuppressions: Set<String> by config(emptyList<String>()) {
- buildSet {
- addAll(it)
- if (includeSelf) {
- add(issue.id)
- }
- }
- }
-
- override val issue: Issue
- get() = Issue(
- id = "ForbiddenSuppression",
- severity = Severity.CodeSmell,
- description = "Forbidden suppression has been detected.",
- debt = Debt.TWENTY_MINS,
- )
-
- override fun visitAnnotationEntry(annotationEntry: KtAnnotationEntry) {
- super.visitAnnotationEntry(annotationEntry)
-
- val annotation = annotationEntry.shortName?.identifier ?: return
-
- val supportedAnnotations = setOf("Suppress", "SuppressWarnings")
- if (annotation !in supportedAnnotations) return
-
- val annotationArgs = annotationEntry.valueArguments.mapNotNull { arg ->
- arg.getArgumentExpression()?.getArguments()
- }.flatten().toSet()
-
- val violations = annotationArgs.intersect(forbiddenSuppressions)
-
- if (violations.isNotEmpty()) {
- val location = Location.from(annotationEntry)
- report(
- finding = CodeSmell(
- issue = issue,
- entity = Entity.from(
- annotationEntry,
- location,
- ),
- message = buildString {
- append("Suppressing ${violations.joinToString(",") { it.trim() }} ")
- append("has been forbidden. Please consider fixing the issue(s) instead of suppressing it")
- },
- ),
- )
- }
- }
-
- override fun report(finding: Finding, aliases: Set<String>, ruleSetId: RuleSetId?) {
- _findings.add(finding)
- }
-
- override fun clearFindings() {
- _findings.clear()
- super.clearFindings()
- }
-
- private fun KtExpression.getArguments(): List<String>? {
- return when (this) {
- is KtStringTemplateExpression -> {
- this.entries.mapNotNull { it.text }.joinToString("")
- .removeSurrounding("\"")
- .split(",")
- .map { it.trim() }
- .filter { it.isNotEmpty() }
- }
-
- is KtCollectionLiteralExpression -> {
- this.innerExpressions.mapNotNull { it.getArguments() }.flatten()
- }
-
- is KtCallExpression -> {
- valueArguments.mapNotNull { callArgument ->
- callArgument.getArgumentExpression()?.getArguments()
- }.flatten()
- }
-
- else -> null
- }
- }
-}
diff --git a/mobile/android/android-components/components/tooling/detekt/src/test/kotlin/mozilla/components/tooling/detekt/health/ForbiddenSuppressionRuleTest.kt b/mobile/android/android-components/components/tooling/detekt/src/test/kotlin/mozilla/components/tooling/detekt/health/ForbiddenSuppressionRuleTest.kt
@@ -1,282 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-package mozilla.components.tooling.detekt.health
-
-import io.gitlab.arturbosch.detekt.test.TestConfig
-import io.gitlab.arturbosch.detekt.test.compileAndLint
-import io.gitlab.arturbosch.detekt.test.lint
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertTrue
-import org.junit.Test
-
-class ForbiddenSuppressionRuleTest {
-
- @Test
- fun `single forbidden suppression is reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf("VariableNaming"),
- ),
- )
-
- val code = """
- class Test {
- @Suppress("VariableNaming")
- fun suppressedFunction() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(1, findings.size)
- }
-
- @Test
- fun `forbidden suppression with argument name is reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf("ForbiddenRule1"),
- ),
- )
-
- val code = """
- class Test {
- @Suppress(names = ["ForbiddenRule1"])
- fun suppressedFunction() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(1, findings.size)
- }
-
- @Test
- fun `forbidden suppression on an expression is reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf("UNCHECKED_CAST"),
- ),
- )
-
- val code = """
- class Test {
- fun myFunction() {
- @Suppress("UNCHECKED_CAST")
- val result = someValue as String
- // Other code without suppression
- println("Hello")
- }
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(1, findings.size)
- }
-
- @Test
- fun `forbidden suppression at file level is reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf("ForbiddenRule"),
- ),
- )
-
- val code = """
- @file:Suppress("ForbiddenRule")
- class Test {
- fun myFunction() {
- println("Hello")
- }
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(1, findings.size)
- }
-
- @Test
- fun `unforbidden suppression is not reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf("ForbiddenRule1"),
- ),
- )
-
- val code = """
- class Test {
- @Suppress("Unforbidden")
- fun myFunction() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(0, findings.size)
- }
-
- @Test
- fun `multiple forbidden suppressions in multiple @Suppress annotations are reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf(
- "ForbiddenRule1",
- "ForbiddenRule2",
- ),
- ),
- )
-
- val code = """
- class Test {
- @Suppress("ForbiddenRule1")
- fun myFunction1() = Unit
- @Suppress("ForbiddenRule1, UnforbiddenRule")
- fun myFunction2() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(2, findings.size)
- }
-
- @Test
- fun `multiple forbidden suppressions as separate string args in the same annotation are reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf(
- "ForbiddenRule1",
- "ForbiddenRule2",
- ),
- ),
- )
-
- val code = """
- class Test {
- @Suppress("ForbiddenRule1","UnforbiddenRule","ForbiddenRule2")
- fun myFunction1() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(1, findings.size)
- assertTrue(
- "Finding message should contain ForbiddenRule1",
- findings[0].message.contains("ForbiddenRule1"),
- )
- assertTrue(
- "Finding message should contain ForbiddenRule2",
- findings[0].message.contains("ForbiddenRule2"),
- )
- }
-
- @Test
- fun `multiple forbidden suppressions in the same annotation are reported`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf(
- "ForbiddenRule1",
- "ForbiddenRule2",
- ),
- ),
- )
-
- val code = """
- class Test {
- @Suppress("ForbiddenRule1,UnforbiddenRule,ForbiddenRule2")
- fun myFunction1() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(1, findings.size)
- assertTrue(
- "Finding message should contain ForbiddenRule1",
- findings[0].message.contains("ForbiddenRule1"),
- )
- assertTrue(
- "Finding message should contain ForbiddenRule2",
- findings[0].message.contains("ForbiddenRule2"),
- )
- }
-
- @Test
- fun `suppression of ForbiddenSuppression is reported when includeSelf is not specified`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf(
- "ForbiddenRule1",
- "ForbiddenRule2",
- ),
- ),
- )
-
- val code = """
- class Test {
- @Suppress("ForbiddenSuppression", "UnforbiddenRule")
- fun myFunction1() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertEquals(
- "Expected that 1 finding is reported",
- 1,
- findings.size,
- )
- assertTrue(
- "Finding message should contain ForbiddenSuppression",
- findings[0].message.contains("ForbiddenSuppression"),
- )
- }
-
- @Test
- fun `suppression of ForbiddenSuppression is reported when includeSelf is true`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf(
- "ForbiddenRule1",
- "ForbiddenRule2",
- ),
- "includeSelf" to true,
- ),
- )
-
- val code = """
- class Test {
- @Suppress("ForbiddenSuppression", "UnforbiddenRule")
- fun myFunction1() = Unit
- }
- """.trimIndent()
-
- val findings = rule.compileAndLint(code)
- assertTrue(
- "Finding message should contain ForbiddenSuppression",
- findings[0].message.contains("ForbiddenSuppression"),
- )
- }
-
- @Test
- fun `suppression of ForbiddenSuppression is not reported if includeSelf is false`() {
- val rule = ForbiddenSuppressionRule(
- config = TestConfig(
- "forbiddenSuppressions" to listOf(
- "ForbiddenRule1",
- "ForbiddenRule2",
- ),
- "includeSelf" to false,
- ),
- )
-
- val code = """
- class Test {
- @Suppress("ForbiddenSuppression", "UnforbiddenRule")
- fun myFunction1() = Unit
- }
- """.trimIndent()
-
- val findings = rule.lint(code)
- assertTrue(
- "Expected that no finding is reported",
- findings.isEmpty(),
- )
- }
-}
diff --git a/mobile/android/android-components/config/detekt.yml b/mobile/android/android-components/config/detekt.yml
@@ -786,7 +786,3 @@ style:
ignoreLateinitVar: false
WildcardImport:
active: true
-
-mozilla-rules:
- ForbiddenSuppression:
- active: true
diff --git a/mobile/android/fenix/config/detekt.yml b/mobile/android/fenix/config/detekt.yml
@@ -806,7 +806,3 @@ mozilla-detekt-rules:
active: true
MozillaUseLazyMonitored:
active: true
-
-mozilla-rules:
- ForbiddenSuppression:
- active: true
