Repository: detekt/detekt (kotlin)

Issue:
Allow ForbiddenSuppress to disallow suppressing ForbiddenSuppress
According to the [docs](https://detekt.dev/docs/next/rules/style/#forbiddensuppress) and [PR](https://github.com/detekt/detekt/pull/4899), `ForbiddenSuppress` cannot disallow supression of itself. 

## Expected Behavior

This should be allowed, because otherwise there is no way to strictly enforce a rule.

## Current Behavior

There is no way to strictly enforce a rule.

## Context

If `ForbiddenSuppress` cannot disallow supressing itself, then as the PR says this rule does not really forbid any suppression, it just discourages it.

I do not know if this limitation is by design, because of a technical limitation, or both.

If it is only by design, I would argue that the developer should be allowed to strictly enforce a rule in their project by forbidding suppression of it completely. Its a decision for the developer, not for this library to make.

If it is a technical limitation, then I would propose that a workaround is implemented. One idea for a workaround, if required, is that this could be a special option for the detekt engine itself, and not considered a regular rule. That way, the suppresion mechaism doesn't take precedence.

One idea for how it could look in the yaml is this: 

```yaml
build:
    forbidSuppressingForbiddenSuppression: true
```

I personally find the tongue twister fun, but it could of course be replaced by something simpler such as `forbidSuppressionsStrictly`.

The point is that if there is a technical limitation, a special configuration option could be added that is not technically part of any rule, but is an option on detekt itself. This special option would report issues just like a normal detekt rule, but it would not itself be suppressible. It would still be easy to disable by setting `forbidSuppressingForbiddenSuppression: false`, but this would be up to the developer. While it might sound radical to bake a issue-reporter into detekt itself without being in the form of a rule, I think this would be the single case where it might be justified.

Pull request:
Fixes https://github.com/detekt/detekt/issues/7038

Of the [5 options](https://github.com/detekt/detekt/issues/7038#issuecomment-2027003939) listed by @BraisGabin, this is an implementation of option 4. In the issue, there were basically two votes for option 3 and two votes for option 4. If I added my vote, I would have said any of options 1-4 would work (just not 5, since it doesn't fix this issue). I gave option 4 a shot because it seemed the easiest for me.

This PR gets the job done and resolves the issue, but there are some questionable aspects of the implementation. I would ask about these in particular:

- `isForbiddenSuppress` is possibly not robust in that it checks the class based on the class name. This is because I needed to implement the logic in `core`, which can't do an `is` type check for `ForbiddenSuppress`. One alternative I though of would be to add an additional `suppressable` property to the `Rule` class, but I though that would be bad because this would allow other rules to become unsurpassable outside of user configuration. Given that the only alternative is bad, I think that checking based on classname might ok. At least it is reinforced by the tests in `ForbiddenSuppressSpec`. If the classname or package of `ForbiddenSuppress` was ever refactored, those tests should fail.

- The documentation edit is now a bit wordy and can be confusing. Once we finalize the implementation, maybe it can be re-worded in a simpler way.

- Suppressors now do not work at all on ForbiddenSuppress. This matches the original goal, however the annotation and function suppresser configuration properties are still present in the configuration spec and are valid yaml. Not sure the best way to handle this, but maybe the verification of the yaml could do a special check to ensure that these properties are not defined for ForbiddenSuppress in the yaml file. Alternatively, maybe we could somehow remove the suppresser properties completely for the ForbiddenSuppress configuration, but I am not sure how.

- I tried to run `detekt` and `test` tasks before submitting this PR, but got failures in both that were unrelated to this PR. I read the CONTRIBUTING guide and the first step says to run `./gradlew publishToMavenLocal`, but I don't want to do this because that might affect other projects on my machine. So I'm unsure if that is why my `detekt` and `test` tasks are failing in other ways.

Patch:
diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/Suppressions.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/Suppressions.kt
index dfa2cc2e6c15..b9b456849e7c 100644
--- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/Suppressions.kt
+++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/suppressors/Suppressions.kt
@@ -1,12 +1,17 @@
 package io.gitlab.arturbosch.detekt.core.suppressors
 
+import io.gitlab.arturbosch.detekt.api.Rule
 import io.gitlab.arturbosch.detekt.api.RuleSet
+import io.gitlab.arturbosch.detekt.core.extractRuleName
 import org.jetbrains.kotlin.psi.KtAnnotated
 import org.jetbrains.kotlin.psi.KtAnnotationEntry
 import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
 import kotlin.text.RegexOption.IGNORE_CASE
 
+internal fun isForbiddenSuppressById(id: String) = extractRuleName(id) == FORBIDDEN_SUPPRESS_NAME
+private val FORBIDDEN_SUPPRESS_NAME = Rule.Name("ForbiddenSuppress")
+
 /**
  * Checks if this psi element is suppressed by @Suppress or @SuppressWarnings annotations.
  * If this element cannot have annotations, the first annotative parent is searched.
@@ -18,6 +23,8 @@ fun KtElement.isSuppressedBy(id: String, aliases: Set<String>, ruleSetId: RuleSe
     }
     acceptedSuppressionIds.addAll(aliases)
 
+    if (isForbiddenSuppressById(id)) return false
+
     return allAnnotationEntries()
         .filter { it.typeReference?.text in suppressionAnnotations }
         .flatMap { it.valueArguments }
diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt
index 64ecd9011fa8..1454387dec2d 100644
--- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt
+++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppress.kt
@@ -18,7 +18,7 @@ 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.
  *
- * This rule is not capable of reporting suppression of itself, as that's a language feature with precedence.
+ * This rule is special in that it itself cannot be suppressed. This ensures that all rules can be enforced strictly with this rule. This rule is not capable of reporting suppression of itself, as that's a language feature with precedence. However, attempting to suppress this rule by any means will have no effect besides producing a warning.
  *
  * <noncompliant>
  * package foo
@@ -74,7 +74,13 @@ class ForbiddenSuppress(config: Config) : Rule(
 
     private fun List<KtValueArgument>.filterForbiddenRules(): List<String> = mapNotNull { argument ->
         val text = argument.findDescendantOfType<KtLiteralStringTemplateEntry>()?.text
-        if (rules.contains(text)) text else null
+        if (text == "ForbiddenSuppress") {
+            null
+        } else if (rules.contains(text)) {
+            text
+        } else {
+            null
+        }
     }
 
     private companion object {


Test patch:
diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppressSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppressSpec.kt
index 9734feaceaec..e624bf6ca29a 100644
--- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppressSpec.kt
+++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/ForbiddenSuppressSpec.kt
@@ -176,7 +176,7 @@ internal class ForbiddenSuppressSpec {
         }
 
         @Test
-        fun `does not catch suppression of any forbidden rule when one of them`() {
+        fun `cannot be suppressed`() {
             val code = """
                 package config
 
@@ -184,7 +184,7 @@ internal class ForbiddenSuppressSpec {
                 class Foo
             """.trimIndent()
             val findings = subject.compileAndLint(code)
-            assertThat(findings).isEmpty()
+            assertThat(findings).hasSize(1)
         }
     }