Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rule deprecation mechanism and deprecate DuplicateCaseInWhenExpression, MissingWhenCase, RedundantElseInWhen #5309

Merged
merged 9 commits into from Oct 4, 2022
7 changes: 0 additions & 7 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -419,8 +419,6 @@ potential-bugs:
- 'java.util.HashSet'
- 'java.util.LinkedHashMap'
- 'java.util.HashMap'
DuplicateCaseInWhenExpression:
active: true
ElseCaseInsteadOfExhaustiveWhen:
active: false
VitalyVPinchuk marked this conversation as resolved.
Show resolved Hide resolved
EqualsAlwaysReturnsTrueOrFalse:
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions 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
Expand Down
Expand Up @@ -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
}
Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down
Expand Up @@ -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()) {
Expand All @@ -55,6 +56,7 @@ internal class RuleVisitor : DetektVisitor() {
parent = parent,
configuration = configurationByAnnotation,
autoCorrect = autoCorrect,
deprecationMessage = deprecationMessage,
requiresTypeResolution = requiresTypeResolution
)
}
Expand Down Expand Up @@ -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)
}
Expand Down
Expand Up @@ -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))
Expand All @@ -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!!}"
}
Expand Up @@ -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
Expand All @@ -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) }
Expand Down
Expand Up @@ -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) {
Expand Down
Expand Up @@ -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 }
}
}
}
Expand Up @@ -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)
}
Expand Down
Expand Up @@ -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)
}
22 changes: 22 additions & 0 deletions detekt-generator/src/test/resources/RuleSet.md
Expand Up @@ -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() {}
```
VitalyVPinchuk marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -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(
Expand Down
Expand Up @@ -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(
Expand Down
Expand Up @@ -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),
Expand All @@ -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),
Expand Down
Expand Up @@ -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(
Expand Down
Expand Up @@ -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`() {
Expand Down
Expand Up @@ -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` {
Expand Down Expand Up @@ -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))
)

Expand Down
Expand Up @@ -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` {
Expand Down