Repository: detekt/detekt (kotlin)

Issue:
Allow custom HelpUri in sarif output for custom rules
## Expected Behavior

When I add custom detekt rules the HelpUri in sarif output should link to my documentation for my rules.

## Current Behavior

When i add custom rules the HelpUri links to `detekt.dev` example below

```json
{
              "helpUri": "https://detekt.dev/agoda-kraft.html#ignoredreturnvaluerule",
              "id": "detekt.agoda-kraft.IgnoredReturnValueRule",
              "name": "IgnoredReturnValueRule",
              "shortDescription": {
                "text": "This rule reports when a function call's return value is ignored."
              }
            }
```

But [recently](https://github.com/detekt/detekt/pull/7686) this was removed entirely

## Context

In our internal tooling we comment on MRs, in line wiht the code change (like a code reviewer would but a bot) when changed code has a linting error or warning, helps the code reviewer do a final check, especially with warnings that might sneak through. When doing so we use meta data from the output to link the Engineer to documentation explain why the code smell is bad with good documentation on compliant and non-compliant code.

The goals is to turn static code analysis into an education tools, and less of a thing that complains and blocks me.

We are recently adding Koltin into our list of support languages and in experimenting with custom detekt rule sim happy it support sarif format so this is in the schema, but unable to customise it

![image](https://github.com/user-attachments/assets/6a90df2d-0709-4148-8553-922125aa1a1d)

## Suggested implementation

An optional Parameter on the `Issue` perhaps? that overrides the current behaviour, also if its a custom rule, i dont think that the helpUri should output at all right?

Pull request:
Closes #7717

With this API we let the third party authors to define the urls pointing to the doc of their rules. Now for those rules we generate urls that link to a `404` in our web (for example: https://detekt.dev/docs/rules/compose#UnusedModifier)

I like how the API looks: a constructor property so the users can inject it. But I'm not sure if it is useful/easy to use. I'm adding code inside `core` to handle the DefaultRuleSets and our first party rule sets (`formatting`, `ruleauthors` and `libraries`). I would like to keep the `core` agnostic of this but I think that it is an OK trade-off.

Any feedback is more than welcome here.

Patch:
diff --git a/detekt-api/api/detekt-api.api b/detekt-api/api/detekt-api.api
index aa3531d54497..1cc460290f23 100644
--- a/detekt-api/api/detekt-api.api
+++ b/detekt-api/api/detekt-api.api
@@ -277,13 +277,15 @@ public abstract interface class io/gitlab/arturbosch/detekt/api/RequiresFullAnal
 
 public class io/gitlab/arturbosch/detekt/api/Rule : io/gitlab/arturbosch/detekt/api/DetektVisitor {
 	protected field compilerResources Lio/gitlab/arturbosch/detekt/api/CompilerResources;
-	public fun <init> (Lio/gitlab/arturbosch/detekt/api/Config;Ljava/lang/String;)V
+	public fun <init> (Lio/gitlab/arturbosch/detekt/api/Config;Ljava/lang/String;Ljava/net/URI;)V
+	public synthetic fun <init> (Lio/gitlab/arturbosch/detekt/api/Config;Ljava/lang/String;Ljava/net/URI;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun getAutoCorrect ()Z
 	public final fun getBindingContext (Lio/gitlab/arturbosch/detekt/api/RequiresFullAnalysis;)Lorg/jetbrains/kotlin/resolve/BindingContext;
 	protected final fun getCompilerResources ()Lio/gitlab/arturbosch/detekt/api/CompilerResources;
 	public final fun getConfig ()Lio/gitlab/arturbosch/detekt/api/Config;
 	public final fun getDescription ()Ljava/lang/String;
 	public fun getRuleName ()Lio/gitlab/arturbosch/detekt/api/Rule$Name;
+	public final fun getUrl ()Ljava/net/URI;
 	protected fun postVisit (Lorg/jetbrains/kotlin/psi/KtFile;)V
 	protected fun preVisit (Lorg/jetbrains/kotlin/psi/KtFile;)V
 	public final fun report (Lio/gitlab/arturbosch/detekt/api/Finding;)V
diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Rule.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Rule.kt
index 46cf5dcb0d95..cd77f584d816 100644
--- a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Rule.kt
+++ b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/Rule.kt
@@ -4,6 +4,7 @@ import dev.drewhamilton.poko.Poko
 import io.gitlab.arturbosch.detekt.api.internal.validateIdentifier
 import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.resolve.BindingContext
+import java.net.URI
 
 /**
  * A rule defines how one specific code structure should look like. If code is found
@@ -13,10 +14,13 @@ import org.jetbrains.kotlin.resolve.BindingContext
  * A rule is implemented using the visitor pattern and should be started using the visit(KtFile)
  * function. If calculations must be done before or after the visiting process, here are
  * two predefined (preVisit/postVisit) functions which can be overridden to setup/teardown additional data.
+ *
+ * @property url An url pointing to the documentation of this rule
  */
 open class Rule(
     val config: Config,
     val description: String,
+    val url: URI? = null,
 ) : DetektVisitor() {
 
     /**
diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptor.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptor.kt
index 079ada101a48..104c619c7cfa 100644
--- a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptor.kt
+++ b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptor.kt
@@ -7,6 +7,7 @@ import io.gitlab.arturbosch.detekt.api.RuleInstance
 import io.gitlab.arturbosch.detekt.api.RuleSet
 import io.gitlab.arturbosch.detekt.api.RuleSetProvider
 import io.gitlab.arturbosch.detekt.api.Severity
+import io.gitlab.arturbosch.detekt.api.internal.DefaultRuleSetProvider
 import io.gitlab.arturbosch.detekt.core.util.isActiveOrDefault
 import java.net.URI
 
@@ -24,12 +25,21 @@ internal fun getRules(
 ): List<RuleDescriptor> = ruleSetProviders
     .flatMap { ruleSetProvider ->
         val ruleSetConfig = config.subConfig(ruleSetProvider.ruleSetId.value)
-        ruleSetProvider.instance().getRules(ruleSetConfig, fullAnalysis, log)
+        val urlGenerator: (Rule) -> URI? =
+            if (ruleSetProvider is DefaultRuleSetProvider ||
+                ruleSetProvider.ruleSetId.value in externalFirstPartyRuleSets
+            ) {
+                { rule -> rule.url ?: generateDefaultUrl(ruleSetProvider.ruleSetId, rule.ruleName) }
+            } else {
+                { rule -> rule.url }
+            }
+        ruleSetProvider.instance().getRules(ruleSetConfig, fullAnalysis, urlGenerator, log)
     }
 
 private fun RuleSet.getRules(
     config: Config,
     fullAnalysis: Boolean,
+    urlGenerator: (Rule) -> URI?,
     log: (() -> String) -> Unit,
 ): Sequence<RuleDescriptor> = config.subConfigKeys()
     .asSequence()
@@ -49,7 +59,7 @@ private fun RuleSet.getRules(
                 ruleInstance = RuleInstance(
                     id = ruleId,
                     ruleSetId = id,
-                    url = generateUrl(id, rule.ruleName),
+                    url = urlGenerator(rule),
                     description = rule.description,
                     severity = ruleConfig.computeSeverity(),
                     active = active && executable,
@@ -58,9 +68,15 @@ private fun RuleSet.getRules(
         }
     }
 
-private fun generateUrl(ruleSetId: RuleSet.Id, ruleName: Rule.Name) =
+private fun generateDefaultUrl(ruleSetId: RuleSet.Id, ruleName: Rule.Name) =
     URI("https://detekt.dev/docs/rules/${ruleSetId.value.lowercase()}#${ruleName.value.lowercase()}")
 
+private val externalFirstPartyRuleSets = setOf(
+    "formatting",
+    "ruleauthors",
+    "libraries",
+)
+
 /**
  * Compute severity in the priority order:
  * - Severity of the rule


Test patch:
diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptorKtTest.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptorKtTest.kt
index 0b522752fdda..f21d1d1bb7e2 100644
--- a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptorKtTest.kt
+++ b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/RuleDescriptorKtTest.kt
@@ -12,6 +12,7 @@ import io.gitlab.arturbosch.detekt.api.Severity.Error
 import io.gitlab.arturbosch.detekt.api.Severity.Info
 import io.gitlab.arturbosch.detekt.api.Severity.Warning
 import io.gitlab.arturbosch.detekt.api.config
+import io.gitlab.arturbosch.detekt.api.internal.DefaultRuleSetProvider
 import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatThrownBy
@@ -35,7 +36,7 @@ class RuleDescriptorKtTest {
     fun emptyConfigReturnsNoRule(fullAnalysis: Boolean) {
         val rules = getRules(
             fullAnalysis,
-            listOf(TestRuleSetProvider()),
+            listOf(TestDefaultRuleSetProvider()),
             Config.empty,
             log,
         )
@@ -47,7 +48,7 @@ class RuleDescriptorKtTest {
     fun returns4RulesAndIgnoreUnknownRule() {
         val rules = getRules(
             true,
-            listOf(TestRuleSetProvider()),
+            listOf(TestDefaultRuleSetProvider()),
             yamlConfigFromContent(
                 """
                     custom:
@@ -80,7 +81,7 @@ class RuleDescriptorKtTest {
     fun doesntCrashWhenConfigHasWrongType() {
         val rules = getRules(
             true,
-            listOf(TestRuleSetProvider()),
+            listOf(TestDefaultRuleSetProvider()),
             yamlConfigFromContent(
                 """
                     custom:
@@ -114,7 +115,7 @@ class RuleDescriptorKtTest {
     fun `when fullAnalysis is disabled the rules that require full analysis are inactive`() {
         val rules = getRules(
             false,
-            listOf(TestRuleSetProvider()),
+            listOf(TestDefaultRuleSetProvider()),
             yamlConfigFromContent(
                 """
                     custom:
@@ -148,7 +149,7 @@ class RuleDescriptorKtTest {
     fun `when fullAnalysis is disabled but the rule is disabled we log nothing`() {
         val rules = getRules(
             false,
-            listOf(TestRuleSetProvider()),
+            listOf(TestDefaultRuleSetProvider()),
             yamlConfigFromContent(
                 """
                     custom:
@@ -170,7 +171,7 @@ class RuleDescriptorKtTest {
     fun whenRuleSetIsInactiveReturnsAllRuleAreDisabled() {
         val rules = getRules(
             false,
-            listOf(TestRuleSetProvider()),
+            listOf(TestDefaultRuleSetProvider()),
             yamlConfigFromContent(
                 """
                     custom:
@@ -199,6 +200,38 @@ class RuleDescriptorKtTest {
         assertThat(stringBuilder.toString()).isEmpty()
     }
 
+    @Nested
+    inner class Url {
+
+        @Test
+        fun whenRuleSetIsInactiveReturnsAllRuleAreDisabled() {
+            val rules = getRules(
+                false,
+                listOf(TestCustomRuleSetProvider()),
+                yamlConfigFromContent(
+                    """
+                        custom:
+                          OneRule:
+                            active: true
+                          OneRule/foo:
+                            active: true
+                          AnotherRule:
+                            active: true
+                    """.trimIndent()
+                ),
+                log,
+            )
+
+            assertThat(rules)
+                .satisfiesExactlyInAnyOrder(
+                    RuleDescriptionMatcher(::OneRule, configActive, url = null),
+                    RuleDescriptionMatcher(::OneRule, configActive, id = "OneRule/foo", url = null),
+                    RuleDescriptionMatcher(::AnotherRule, configActive, url = "https://example.org/")
+                )
+            assertThat(stringBuilder.toString()).isEmpty()
+        }
+    }
+
     @Nested
     inner class SeverityTest {
 
@@ -207,7 +240,7 @@ class RuleDescriptorKtTest {
         fun ignoreCase(candidate: String) {
             val rules = getRules(
                 false,
-                listOf(TestRuleSetProvider()),
+                listOf(TestDefaultRuleSetProvider()),
                 yamlConfigFromContent(
                     """
                         custom:
@@ -238,7 +271,7 @@ class RuleDescriptorKtTest {
         fun supportsAll(severity: Severity) {
             val rules = getRules(
                 false,
-                listOf(TestRuleSetProvider()),
+                listOf(TestDefaultRuleSetProvider()),
                 yamlConfigFromContent(
                     """
                         custom:
@@ -269,7 +302,7 @@ class RuleDescriptorKtTest {
             assertThatThrownBy {
                 getRules(
                     false,
-                    listOf(TestRuleSetProvider()),
+                    listOf(TestDefaultRuleSetProvider()),
                     yamlConfigFromContent(
                         """
                             custom:
@@ -292,7 +325,7 @@ class RuleDescriptorKtTest {
         fun severityOnRuleSet() {
             val rules = getRules(
                 false,
-                listOf(TestRuleSetProvider()),
+                listOf(TestDefaultRuleSetProvider()),
                 yamlConfigFromContent(
                     """
                         custom:
@@ -335,11 +368,12 @@ private class RuleDescriptionMatcher(
     private val active: Boolean = true,
     private val id: String = ruleProvider(Config.empty).javaClass.simpleName,
     private val severity: Severity = Error,
+    private val url: String? = "default",
 ) : ThrowingConsumer<RuleDescriptor> {
     override fun acceptThrows(ruleDescriptor: RuleDescriptor?) {
         assertThat(ruleDescriptor?.ruleProvider).isEqualTo(ruleProvider)
         assertThat(ruleDescriptor?.config?.toMap()).isEqualTo(config)
-        assertThat(ruleDescriptor?.ruleInstance).isEqualTo(createRuleInstance(id, active, severity))
+        assertThat(ruleDescriptor?.ruleInstance).isEqualTo(createRuleInstance(id, active, url, severity))
     }
 }
 
@@ -352,16 +386,29 @@ private fun Config.toMap(): Map<String, Any?> = buildMap {
 private val configActive = mapOf("active" to true)
 private val configInactive = mapOf("active" to false)
 
-private fun createRuleInstance(id: String, active: Boolean, severity: Severity) = RuleInstance(
+private fun createRuleInstance(id: String, active: Boolean, url: String?, severity: Severity) = RuleInstance(
     id,
     RuleSet.Id("custom"),
-    URI("https://detekt.dev/docs/rules/custom#${id.substringBefore("/").lowercase()}"),
+    if (url == "default") {
+        if (id.startsWith("AnotherRule")) {
+            URI("https://example.org/")
+        } else {
+            URI("https://detekt.dev/docs/rules/custom#${id.substringBefore("/").lowercase()}")
+        }
+    } else {
+        url?.let(::URI)
+    },
     "${id.substringBefore("/")}Description",
     severity = severity,
     active = active
 )
 
-private class TestRuleSetProvider : RuleSetProvider {
+private class TestDefaultRuleSetProvider : DefaultRuleSetProvider {
+    override val ruleSetId = RuleSet.Id("custom")
+    override fun instance() = RuleSet(ruleSetId, listOf(::OneRule, ::AnotherRule, ::RequiresFullAnalysisRule))
+}
+
+private class TestCustomRuleSetProvider : RuleSetProvider {
     override val ruleSetId = RuleSet.Id("custom")
     override fun instance() = RuleSet(ruleSetId, listOf(::OneRule, ::AnotherRule, ::RequiresFullAnalysisRule))
 }
@@ -372,7 +419,7 @@ private class OneRule(config: Config) : Rule(config, "OneRuleDescription") {
     private val aConfiguration by config(1)
 }
 
-private class AnotherRule(config: Config) : Rule(config, "AnotherRuleDescription")
+private class AnotherRule(config: Config) : Rule(config, "AnotherRuleDescription", URI("https://example.org/"))
 
 private class RequiresFullAnalysisRule(
     config: Config,