New Rule: BracesOnIfStatements with configurable parameters

config/detekt/detekt.yml

@@ -149,6 +149,10 @@ potential-bugs:
     active: true
 
 style:
+  BracesOnIfStatements:
+    active: true
+    singleLine: 'consistent'
+    multiLine: 'consistent'
   CanBeNonNullable:
     active: true
   CascadingCallWrapping:

detekt-core/src/main/resources/default-detekt-config.yml

@@ -500,6 +500,10 @@ style:
   active: true
   AlsoCouldBeApply:
     active: false
+  BracesOnIfStatements:
+    active: false
+    singleLine: 'never'
+    multiLine: 'always'
   CanBeNonNullable:
     active: false
   CascadingCallWrapping:
@@ -598,8 +602,6 @@ style:
     ignoreEnums: false
     ignoreRanges: false
     ignoreExtensionFunctions: true
-  MandatoryBracesIfStatements:
-    active: false
   MandatoryBracesLoops:
     active: false
   MaxChainedCallsOnSameLine:

detekt-core/src/main/resources/deprecation.properties

@@ -17,4 +17,5 @@ formatting>TrailingComma=Rule is split between `TrailingCommaOnCallSite` and `Tr
 style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin
 style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin
 style>LibraryEntitiesShouldNotBePublic=Rule migrated to `libraries` ruleset plugin
+style>MandatoryBracesIfStatements=Use `BracesOnIfStatements` with `always` configuration instead
 complexity>ComplexMethod=Rule is renamed to `CyclomaticComplexMethod` to distinguish between Cyclomatic Complexity and Cognitive Complexity

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinter.kt

@@ -41,6 +41,7 @@ internal fun writeMigratedRules(): String {
         style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin
         style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin
         style>LibraryEntitiesShouldNotBePublic=Rule migrated to `libraries` ruleset plugin
+        style>MandatoryBracesIfStatements=Use `BracesOnIfStatements` with `always` configuration instead
         complexity>ComplexMethod=Rule is renamed to `CyclomaticComplexMethod` to distinguish between Cyclomatic Complexity and Cognitive Complexity
     """.trimIndent()
 }

detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunWithCoroutineScopeReceiverSpec.kt

@@ -100,6 +100,7 @@ class SuspendFunWithCoroutineScopeReceiverSpec(val env: KotlinCoreEnvironment) {
             val code = """
                 import kotlinx.coroutines.coroutineScope
                 import kotlinx.coroutines.delay
+                import kotlinx.coroutines.launch
                 import kotlin.time.Duration.Companion.seconds
 
                 suspend fun foo() = coroutineScope {
@@ -117,6 +118,7 @@ class SuspendFunWithCoroutineScopeReceiverSpec(val env: KotlinCoreEnvironment) {
                 import kotlinx.coroutines.delay
                 import kotlinx.coroutines.launch
                 import kotlinx.coroutines.CoroutineScope
+                import kotlinx.coroutines.coroutineScope
 
                 suspend fun Long.foo() = coroutineScope {
                     launch {

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/BracesOnIfStatements.kt

@@ -0,0 +1,222 @@
+package io.gitlab.arturbosch.detekt.rules.style
+
+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.Issue
+import io.gitlab.arturbosch.detekt.api.Rule
+import io.gitlab.arturbosch.detekt.api.Severity
+import io.gitlab.arturbosch.detekt.api.config
+import io.gitlab.arturbosch.detekt.api.internal.Configuration
+import org.jetbrains.kotlin.psi.KtBlockExpression
+import org.jetbrains.kotlin.psi.KtExpression
+import org.jetbrains.kotlin.psi.KtIfExpression
+
+/**
+ * This rule detects `if` statements which do not comply with the specified rules.
+ * Keeping braces consistent would improve readability and avoid possible errors.
+ *
+ * The available options are:
+ *  * `always`: forces braces on all `if` and `else` branches in the whole codebase.
+ *  * `consistent`: ensures that braces are consistent within each `if`-`else if`-`else` chain.
+ *    If there's a brace on one of the branches, all branches should have it.
+ *  * `necessary`: forces no braces on any `if` and `else` branches in the whole codebase
+ *    except where necessary for multi-statement branches.
+ *  * `never`: forces no braces on any `if` and `else` branches in the whole codebase.
+ *
+ * SingleLine if-statement has no line break (\n):
+ * ```
+ * if (a) b else c
+ * ```
+ * MultiLine if-statement has at least one line break (\n):
+ * ```
+ * if (a) b
+ * else c
+ * ```
+ *
+ * <noncompliant>
+ *
+ * // singleLine = 'never'
+ * if (a) { b } else { c }
+ *
+ * if (a) { b } else c
+ *
+ * if (a) b else { c; d }
+ *
+ * // multiLine = 'never'
+ * if (a) {
+ *    b
+ * } else {
+ *    c
+ * }
+ *
+ * // singleLine = 'always'
+ * if (a) b else c
+ *
+ * if (a) { b } else c
+ *
+ * // multiLine = 'always'
+ * if (a) {
+ *    b
+ * } else
+ *    c
+ *
+ * // singleLine = 'consistent'
+ * if (a) b else { c }
+ * if (a) b else if (c) d else { e }
+ *
+ * // multiLine = 'consistent'
+ * if (a)
+ *    b
+ * else {
+ *    c
+ * }
+ * </noncompliant>
+ *
+ * <compliant>
+ * // singleLine = 'never'
+ * if (a) b else c
+ *
+ * // multiLine = 'never'
+ * if (a)
+ *    b
+ * else
+ *    c
+ *
+ * // singleLine = 'always'
+ * if (a) { b } else { c }
+ *
+ * if (a) { b } else if (c) { d }
+ *
+ * // multiLine = 'always'
+ * if (a) {
+ *    b
+ * } else {
+ *    c
+ * }
+ *
+ * if (a) {
+ *    b
+ * } else if (c) {
+ *    d
+ * }
+ *
+ * // singleLine = 'consistent'
+ * if (a) { b } else { c }
+ *
+ * if (a) b else c
+ *
+ * if (a) { b } else { c; d }
+ *
+ * // multiLine = 'consistent'
+ * if (a) {
+ *    b
+ * } else {
+ *    c
+ * }
+ *
+ * if (a) b
+ * else c
+ * </compliant>
+ */
+class BracesOnIfStatements(config: Config = Config.empty) : Rule(config) {
+
+    override val issue = Issue(
+        javaClass.simpleName,
+        Severity.Style,
+        "Braces do not comply with the specified policy",
+        Debt.FIVE_MINS
+    )
+
+    @Configuration("single-line braces policy")
+    private val singleLine: BracePolicy by config("never") { BracePolicy.getValue(it) }
+
+    @Configuration("multi-line braces policy")
+    private val multiLine: BracePolicy by config("always") { BracePolicy.getValue(it) }
+
+    override fun visitIfExpression(expression: KtIfExpression) {
+        super.visitIfExpression(expression)
+        if (expression.parent.parent is KtIfExpression) return
+
+        val branches: List<KtExpression> = walk(expression)
+        validate(branches, policy(expression))
+    }
+
+    private fun walk(expression: KtExpression): List<KtExpression> {
+        val list = mutableListOf<KtExpression>()
+        var current: KtExpression? = expression
+        while (current is KtIfExpression) {
+            current.then?.let { list.add(it) }
+            // Don't add `if` because it's an `else if` which we treat as one unit.
+            current.`else`?.takeIf { it !is KtIfExpression }?.let { list.add(it) }
+            current = current.`else`
+        }
+        return list
+    }
+
+    private fun validate(list: List<KtExpression>, policy: BracePolicy) {
+        when (policy) {
+            BracePolicy.Always -> {
+                list.filter { !hasBraces(it) }.report(policy)
+            }
+
+            BracePolicy.Necessary -> {
+                list.filter { !isMultiStatement(it) && hasBraces(it) }.report(policy)
+            }
+
+            BracePolicy.Never -> {
+                list.filter { hasBraces(it) }.report(policy)
+            }
+
+            BracePolicy.Consistent -> {
+                val braces = list.count { hasBraces(it) }
+                val noBraces = list.count { !hasBraces(it) }
+                if (braces != 0 && noBraces != 0) {
+                    list.take(1).report(policy)
+                }
+            }
+        }
+    }
+
+    private fun List<KtExpression>.report(policy: BracePolicy) {
+        this.forEach { report(it, policy) }
+    }
+
+    private fun report(violator: KtExpression, policy: BracePolicy) {
+        val iff = violator.parent.parent as KtIfExpression
+        val reported = when {
+            iff.then === violator -> iff.ifKeyword
+            iff.`else` === violator -> iff.elseKeyword
+            else -> error("Violating element (${violator.text}) is not part of this if (${iff.text})")
+        }
+        val message = when (policy) {
+            BracePolicy.Always -> "Missing braces on this branch, add them."
+            BracePolicy.Consistent -> "Inconsistent braces, make sure all branches either have or don't have braces."
+            BracePolicy.Necessary -> "Extra braces exist on this branch, remove them (ignore multi-statement)."
+            BracePolicy.Never -> "Extra braces exist on this branch, remove them."
+        }
+        report(CodeSmell(issue, Entity.from(reported ?: violator), message))
+    }
+
+    private fun isMultiStatement(expression: KtExpression) =
+        expression is KtBlockExpression && expression.statements.size > 1
+
+    private fun policy(expression: KtExpression): BracePolicy =
+        if (expression.textContains('\n')) multiLine else singleLine
+
+    private fun hasBraces(expression: KtExpression): Boolean = expression is KtBlockExpression
+
+    enum class BracePolicy(val config: String) {
+        Always("always"),
+        Consistent("consistent"),
+        Necessary("necessary"),
+        Never("never");
+
+        companion object {
+            fun getValue(arg: String): BracePolicy =
+                values().singleOrNull { it.config == arg }
+                    ?: error("Unknown value $arg, allowed values are: ${values().joinToString("|")}")
+        }
+    }
+}

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt

@@ -4,7 +4,6 @@ import io.gitlab.arturbosch.detekt.api.Config
 import io.gitlab.arturbosch.detekt.api.RuleSet
 import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
 import io.gitlab.arturbosch.detekt.api.internal.DefaultRuleSetProvider
-import io.gitlab.arturbosch.detekt.rules.style.optional.MandatoryBracesIfStatements
 import io.gitlab.arturbosch.detekt.rules.style.optional.MandatoryBracesLoops
 import io.gitlab.arturbosch.detekt.rules.style.optional.OptionalUnit
 import io.gitlab.arturbosch.detekt.rules.style.optional.PreferToOverPairSyntax
@@ -76,7 +75,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
             UnnecessaryLet(config),
             MayBeConst(config),
             PreferToOverPairSyntax(config),
-            MandatoryBracesIfStatements(config),
+            BracesOnIfStatements(config),
             MandatoryBracesLoops(config),
             NullableBooleanCheck(config),
             VarCouldBeVal(config),