Add rule deprecation mechanism and deprecate DuplicateCaseInWhenExpre…

…ssion, MissingWhenCase, RedundantElseInWhen (#5309) * Add rule deprecation mechanism * Update detekt-core/src/main/resources/deprecation.properties Co-authored-by: Nicola Corti <corti.nico@gmail.com> * Update detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/Rule.kt Co-authored-by: Nicola Corti <corti.nico@gmail.com> * Fix and reword * Suppress deprecation warnings in PotentialBugProvider.kt * Suppress deprecation warnings in PotentialBugProvider.kt * Suppress deprecation warnings in PotentialBugProvider.kt * Suppress deprecation warnings * Revert docs, exclude rule, mark deprecated with ~~ - Revert docs for 1.21.0 - Exclude ElseCaseInsteadOfExhaustiveWhen rule - Mark deprecated with Strikethrough font. Co-authored-by: Nicola Corti <corti.nico@gmail.com>
detekt · Oct 4, 2022 · 8784c78 · 8784c78
1 parent 24dce51
commit 8784c78
Show file tree

Hide file tree

Showing 19 changed files with 81 additions and 20 deletions.
diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml
@@ -419,8 +419,6 @@ potential-bugs:
       - 'java.util.HashSet'
       - 'java.util.LinkedHashMap'
       - 'java.util.HashMap'
-  DuplicateCaseInWhenExpression:
-    active: true
   ElseCaseInsteadOfExhaustiveWhen:
     active: false
   EqualsAlwaysReturnsTrueOrFalse:
@@ -466,15 +464,10 @@ potential-bugs:
   MissingPackageDeclaration:
     active: false
     excludes: ['**/*.kts']
-  MissingWhenCase:
-    active: true
-    allowElseExpression: true
   NullCheckOnMutableProperty:
     active: false
   NullableToStringCall:
     active: false
-  RedundantElseInWhen:
-    active: true
   UnconditionalJumpStatementInLoop:
     active: false
   UnnecessaryNotNullOperator:

diff --git a/detekt-core/src/main/resources/deprecation.properties b/detekt-core/src/main/resources/deprecation.properties
@@ -1,7 +1,10 @@
 complexity>LongParameterList>threshold=Use `functionThreshold` and `constructorThreshold` instead
 empty-blocks>EmptyFunctionBlock>ignoreOverriddenFunctions=Use `ignoreOverridden` instead
+potential-bugs>DuplicateCaseInWhenExpression=Rule deprecated as compiler performs this check by default
 potential-bugs>IgnoredReturnValue>restrictToAnnotatedMethods=Use `restrictToConfig` instead
 potential-bugs>LateinitUsage>excludeAnnotatedProperties=Use `ignoreAnnotated` instead
+potential-bugs>MissingWhenCase=Rule deprecated as compiler performs this check by default
+potential-bugs>RedundantElseInWhen=Rule deprecated as compiler performs this check by default
 naming>FunctionParameterNaming>ignoreOverriddenFunctions=Use `ignoreOverridden` instead
 naming>MemberNameEqualsClassName>ignoreOverriddenFunction=Use `ignoreOverridden` instead
 style>FunctionOnlyReturningConstant>excludeAnnotatedFunction=Use `ignoreAnnotated` instead

diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/Rule.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/Rule.kt
@@ -13,5 +13,8 @@ data class Rule(
     val configuration: List<Configuration> = emptyList(),
     val autoCorrect: Boolean = false,
     var inMultiRule: String? = null,
-    val requiresTypeResolution: Boolean = false
-)
+    val requiresTypeResolution: Boolean = false,
+    val deprecationMessage: String? = null
+) {
+    fun isDeprecated() = deprecationMessage != null
+}
diff --git a/.../main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleSetProviderCollector.kt b/.../main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleSetProviderCollector.kt
@@ -5,6 +5,7 @@ import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
 import io.gitlab.arturbosch.detekt.api.internal.DefaultRuleSetProvider
 import io.gitlab.arturbosch.detekt.generator.collection.exception.InvalidDocumentationException
 import io.gitlab.arturbosch.detekt.rules.isOverride
+import org.jetbrains.kotlin.psi.KtAnnotatedExpression
 import org.jetbrains.kotlin.psi.KtCallExpression
 import org.jetbrains.kotlin.psi.KtClass
 import org.jetbrains.kotlin.psi.KtFile
@@ -128,6 +129,7 @@ class RuleSetProviderVisitor : DetektVisitor() {
             val ruleArgumentNames = (ruleListExpression as? KtCallExpression)
                 ?.valueArguments
                 ?.mapNotNull { it.getArgumentExpression() }
+                ?.map { if (it is KtAnnotatedExpression) it.lastChild as KtCallExpression else it }
                 ?.mapNotNull { it.referenceExpression()?.text }
                 .orEmpty()
 

diff --git a/...generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleVisitor.kt b/...generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleVisitor.kt
@@ -35,6 +35,7 @@ internal class RuleVisitor : DetektVisitor() {
     private var parent = ""
     private val configurationCollector = ConfigurationCollector()
     private val classesMap = mutableMapOf<String, Boolean>()
+    private var deprecationMessage: String? = null
 
     fun getRule(): Rule {
         if (documentationCollector.description.isEmpty()) {
@@ -55,6 +56,7 @@ internal class RuleVisitor : DetektVisitor() {
             parent = parent,
             configuration = configurationByAnnotation,
             autoCorrect = autoCorrect,
+            deprecationMessage = deprecationMessage,
             requiresTypeResolution = requiresTypeResolution
         )
     }
@@ -105,6 +107,7 @@ internal class RuleVisitor : DetektVisitor() {
 
         autoCorrect = classOrObject.isAnnotatedWith(AutoCorrectable::class)
         requiresTypeResolution = classOrObject.isAnnotatedWith(RequiresTypeResolution::class)
+        deprecationMessage = classOrObject.firstAnnotationParameterOrNull(Deprecated::class)
 
         documentationCollector.setClass(classOrObject)
     }

diff --git a/...erator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinter.kt b/...erator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinter.kt
@@ -10,6 +10,9 @@ object DeprecatedPrinter : DocumentationPrinter<List<RuleSetPage>> {
         val builder = StringBuilder()
         item.forEach { ruleSet ->
             ruleSet.rules.forEach { rule ->
+                if (rule.isDeprecated()) {
+                    builder.appendLine(writeRuleProperty(ruleSet, rule))
+                }
                 rule.configuration.forEach { configuration ->
                     if (configuration.isDeprecated()) {
                         builder.appendLine(writeProperty(ruleSet, rule, configuration))
@@ -25,3 +28,8 @@ private fun writeProperty(ruleSet: RuleSetPage, rule: Rule, configuration: Confi
     @Suppress("UnsafeCallOnNullableType")
     return "${ruleSet.ruleSet.name}>${rule.name}>${configuration.name}=${configuration.deprecated!!}"
 }
+
+private fun writeRuleProperty(ruleSet: RuleSetPage, rule: Rule): String {
+    @Suppress("UnsafeCallOnNullableType")
+    return "${ruleSet.ruleSet.name}>${rule.name}=${rule.deprecationMessage!!}"
+}
diff --git a/...kt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RulePrinter.kt b/...kt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/RulePrinter.kt
@@ -3,6 +3,7 @@ package io.gitlab.arturbosch.detekt.generator.printer
 import io.github.detekt.utils.MarkdownContent
 import io.github.detekt.utils.bold
 import io.github.detekt.utils.codeBlock
+import io.github.detekt.utils.crossOut
 import io.github.detekt.utils.h3
 import io.github.detekt.utils.h4
 import io.github.detekt.utils.markdown
@@ -14,7 +15,12 @@ internal object RulePrinter : DocumentationPrinter<Rule> {
 
     override fun print(item: Rule): String {
         return markdown {
-            h3 { item.name }
+            if (item.isDeprecated()) {
+                h3 { crossOut { item.name } }
+                paragraph { escapeHtml(item.deprecationMessage.orEmpty()) }
+            } else {
+                h3 { item.name }
+            }
 
             if (item.description.isNotEmpty()) {
                 paragraph { escapeHtml(item.description) }

diff --git a/...otlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/RuleSetConfigPrinter.kt b/...otlin/io/gitlab/arturbosch/detekt/generator/printer/defaultconfig/RuleSetConfigPrinter.kt
@@ -30,6 +30,8 @@ internal fun YamlNode.printRuleSet(ruleSet: RuleSetProvider, rules: List<Rule>)
 }
 
 internal fun YamlNode.printRule(rule: Rule) {
+    if (rule.isDeprecated()) return
+
     node(rule.name) {
         keyValue { Config.ACTIVE_KEY to "${rule.defaultActivationStatus.active}" }
         if (rule.autoCorrect) {

diff --git a/...kt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/config/ConfigAssert.kt b/...kt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/config/ConfigAssert.kt
@@ -77,6 +77,9 @@ class ConfigAssert(
     private fun getRuleClassesInPackage(): List<Class<out Rule>> {
         return Reflections(packageName)
             .getSubTypesOf(Rule::class.java)
-            .filter { !Modifier.isAbstract(it.modifiers) }
+            .filter { rule ->
+                !Modifier.isAbstract(rule.modifiers) &&
+                    rule.annotations.none { it.annotationClass == Deprecated::class }
+            }
     }
 }
diff --git a/...or/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinterSpec.kt b/...or/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinterSpec.kt
@@ -12,7 +12,8 @@ class DeprecatedPrinterSpec {
         val expectedMarkdownString = """
             style>MagicNumber>conf2=use conf1 instead
             style>MagicNumber>conf4=use conf3 instead
-            
+            style>DuplicateCaseInWhenExpression=is deprecated
+
         """.trimIndent()
         assertThat(markdownString).isEqualTo(expectedMarkdownString)
     }

diff --git a/...enerator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/util/RuleSetPageCreator.kt b/...enerator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/util/RuleSetPageCreator.kt
@@ -102,5 +102,17 @@ internal fun createRules(): List<Rule> {
         autoCorrect = true,
         requiresTypeResolution = true
     )
-    return listOf(rule1, rule2, rule3)
+    val rule4 = Rule(
+        name = "DuplicateCaseInWhenExpression",
+        description = "Duplicated `case` statements in a `when` expression detected.",
+        nonCompliantCodeExample = "fun stuff(): Unit {}",
+        compliantCodeExample = "fun stuff() {}",
+        defaultActivationStatus = Active(since = "1.16.0"),
+        severity = "",
+        debt = "5m",
+        aliases = null,
+        parent = "",
+        deprecationMessage = "is deprecated"
+    )
+    return listOf(rule1, rule2, rule3, rule4)
 }
diff --git a/detekt-generator/src/test/resources/RuleSet.md b/detekt-generator/src/test/resources/RuleSet.md
@@ -75,3 +75,25 @@ fun stuff(): Unit {}
 ```kotlin
 fun stuff() {}
 ```
+
+### ~~DuplicateCaseInWhenExpression~~
+
+is deprecated
+
+Duplicated `case` statements in a `when` expression detected.
+
+**Active by default**: Yes - Since v1.16.0
+
+**Debt**: 5m
+
+#### Noncompliant Code:
+
+```kotlin
+fun stuff(): Unit {}
+```
+
+#### Compliant Code:
+
+```kotlin
+fun stuff() {}
+```
diff --git a/...e/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/DuplicateCaseInWhenExpression.kt b/...e/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/DuplicateCaseInWhenExpression.kt
@@ -32,6 +32,7 @@ import org.jetbrains.kotlin.psi.KtWhenExpression
  * </compliant>
  */
 @ActiveByDefault(since = "1.0.0")
+@Deprecated("Rule deprecated as compiler performs this check by default")
 class DuplicateCaseInWhenExpression(config: Config) : Rule(config) {
 
     override val issue = Issue(

diff --git a/...ules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCase.kt b/...ules-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCase.kt
@@ -68,6 +68,7 @@ import org.jetbrains.kotlin.resolve.calls.util.getType
  */
 @ActiveByDefault(since = "1.2.0")
 @RequiresTypeResolution
+@Deprecated("Rule deprecated as compiler performs this check by default")
 class MissingWhenCase(config: Config = Config.empty) : Rule(config) {
 
     override val issue: Issue = Issue(

diff --git a/...errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt b/...errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PotentialBugProvider.kt
@@ -20,7 +20,7 @@ class PotentialBugProvider : DefaultRuleSetProvider {
             Deprecation(config),
             DontDowncastCollectionTypes(config),
             DoubleMutabilityForCollection(config),
-            DuplicateCaseInWhenExpression(config),
+            @Suppress("DEPRECATION") DuplicateCaseInWhenExpression(config),
             ElseCaseInsteadOfExhaustiveWhen(config),
             EqualsAlwaysReturnsTrueOrFalse(config),
             EqualsWithHashCodeExist(config),
@@ -34,9 +34,9 @@ class PotentialBugProvider : DefaultRuleSetProvider {
             LateinitUsage(config),
             MapGetWithNotNullAssertionOperator(config),
             MissingPackageDeclaration(config),
-            MissingWhenCase(config),
+            @Suppress("DEPRECATION") MissingWhenCase(config),
             NullCheckOnMutableProperty(config),
-            RedundantElseInWhen(config),
+            @Suppress("DEPRECATION") RedundantElseInWhen(config),
             UnconditionalJumpStatementInLoop(config),
             UnnecessaryNotNullOperator(config),
             UnnecessarySafeCall(config),

diff --git a/...-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/RedundantElseInWhen.kt b/...-errorprone/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/RedundantElseInWhen.kt
@@ -59,6 +59,7 @@ import org.jetbrains.kotlin.psi.KtWhenExpression
  */
 @RequiresTypeResolution
 @ActiveByDefault(since = "1.2.0")
+@Deprecated("Rule deprecated as compiler performs this check by default")
 class RedundantElseInWhen(config: Config = Config.empty) : Rule(config) {
 
     override val issue: Issue = Issue(

diff --git a/...c/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/DuplicateCaseInWhenExpressionSpec.kt b/...c/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/DuplicateCaseInWhenExpressionSpec.kt
@@ -6,7 +6,7 @@ import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 
 class DuplicateCaseInWhenExpressionSpec {
-    private val subject = DuplicateCaseInWhenExpression(Config.empty)
+    private val subject = @Suppress("DEPRECATION") DuplicateCaseInWhenExpression(Config.empty)
 
     @Test
     fun `reports duplicated label in when`() {

diff --git a/...-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCaseSpec.kt b/...-errorprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MissingWhenCaseSpec.kt
@@ -14,7 +14,7 @@ class MissingWhenCaseSpec(private val env: KotlinCoreEnvironment) {
 
     @Nested
     inner class `MissingWhenCase rule` {
-        private val subject = MissingWhenCase()
+        private val subject = @Suppress("DEPRECATION") MissingWhenCase()
 
         @Nested
         inner class `enum` {
@@ -309,7 +309,7 @@ class MissingWhenCaseSpec(private val env: KotlinCoreEnvironment) {
 
     @Nested
     inner class `MissingWhenCase rule when else expression is not considered` {
-        private val subject = MissingWhenCase(
+        private val subject = @Suppress("DEPRECATION") MissingWhenCase(
             TestConfig(mapOf("allowElseExpression" to false))
         )
 

diff --git a/...orprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/RedundantElseInWhenSpec.kt b/...orprone/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/RedundantElseInWhenSpec.kt
@@ -10,7 +10,7 @@ import org.junit.jupiter.api.Test
 
 @KotlinCoreEnvironmentTest
 class RedundantElseInWhenSpec(private val env: KotlinCoreEnvironment) {
-    private val subject = RedundantElseInWhen()
+    private val subject = @Suppress("DEPRECATION") RedundantElseInWhen()
 
     @Nested
     inner class `enum` {