diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index 742626495b3..3fd7bf592ee 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -149,6 +149,10 @@ potential-bugs: active: true style: + BracesOnIfStatements: + active: true + singleLine: 'consistent' + multiLine: 'consistent' CanBeNonNullable: active: true CascadingCallWrapping: diff --git a/detekt-core/src/main/resources/default-detekt-config.yml b/detekt-core/src/main/resources/default-detekt-config.yml index 4874ec93ed6..559029aa147 100644 --- a/detekt-core/src/main/resources/default-detekt-config.yml +++ b/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: diff --git a/detekt-core/src/main/resources/deprecation.properties b/detekt-core/src/main/resources/deprecation.properties index 5175f2ca810..318c23293ec 100644 --- a/detekt-core/src/main/resources/deprecation.properties +++ b/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 diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinter.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinter.kt index d42c0ce5bea..e51ac0695de 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/printer/DeprecatedPrinter.kt +++ b/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() } diff --git a/detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunWithCoroutineScopeReceiverSpec.kt b/detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunWithCoroutineScopeReceiverSpec.kt index f8e1d1529b3..79852ae4068 100644 --- a/detekt-rules-coroutines/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/coroutines/SuspendFunWithCoroutineScopeReceiverSpec.kt +++ b/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 { diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/BracesOnIfStatements.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/BracesOnIfStatements.kt new file mode 100644 index 00000000000..bf8f44159e9 --- /dev/null +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/BracesOnIfStatements.kt @@ -0,0 +1,270 @@ +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.com.intellij.psi.PsiElement +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 will 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. + * + * Single-line if-statement has no line break (\n): + * ```kotlin + * if (a) b else c + * ``` + * Multi-line if-statement has at least one line break (\n): + * ```kotlin + * if (a) b + * else c + * ``` + * + * + * // 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 + * } + * + * // singleLine = 'necessary' + * if (a) { b } else { c; d } + * + * // multiLine = 'necessary' + * if (a) { + * b + * c + * } else if (d) { + * e + * } else { + * f + * } + * + * + * + * // 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 + * + * // singleLine = 'necessary' + * if (a) b else { c; d } + * + * // multiLine = 'necessary' + * if (a) { + * b + * c + * } else if (d) + * e + * else + * f + * + */ +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) + + val parent = expression.parentIfCandidate() + // Ignore `else` branches, they're handled by the initial `if`'s visit. + // But let us process `then` branches and conditions, because they might be `if` themselves. + if (parent is KtIfExpression && parent.`else` === expression) return + + val branches: List = walk(expression) + validate(branches, policy(expression)) + } + + private fun walk(expression: KtExpression): List { + val list = mutableListOf() + 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, policy: BracePolicy) { + val violators = when (policy) { + BracePolicy.Always -> { + list.filter { !hasBraces(it) } + } + + BracePolicy.Necessary -> { + list.filter { !isMultiStatement(it) && hasBraces(it) } + } + + BracePolicy.Never -> { + list.filter { hasBraces(it) } + } + + BracePolicy.Consistent -> { + val braces = list.count { hasBraces(it) } + val noBraces = list.count { !hasBraces(it) } + if (braces != 0 && noBraces != 0) { + list.take(1) + } else { + emptyList() + } + } + } + violators.forEach { report(it, policy) } + } + + private fun report(violator: KtExpression, policy: BracePolicy) { + val iff = violator.parentIfCandidate() 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)) + } + + /** + * Returns a potential parent of the expression, that could be a [KtIfExpression]. + * There's a double-indirection needed because the `then` and `else` branches + * are represented as a [org.jetbrains.kotlin.psi.KtContainerNodeForControlStructureBody]. + * Also, the condition inside the `if` is in an intermediate [org.jetbrains.kotlin.psi.KtContainerNode]. + * ``` + * if (parent) + * / | \ + * cond then else (parent) + * | | | + * expr expr expr + * ``` + * @see org.jetbrains.kotlin.KtNodeTypes.CONDITION + * @see org.jetbrains.kotlin.KtNodeTypes.THEN + * @see org.jetbrains.kotlin.KtNodeTypes.ELSE + */ + private fun KtExpression.parentIfCandidate(): PsiElement? = + this.parent.parent + + private fun isMultiStatement(expression: KtExpression): Boolean = + 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("|")}") + } + } +} diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt index 889df2144e2..6f10aea530d 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt +++ b/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), diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatements.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatements.kt deleted file mode 100644 index c189217d140..00000000000 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatements.kt +++ /dev/null @@ -1,65 +0,0 @@ -package io.gitlab.arturbosch.detekt.rules.style.optional - -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 org.jetbrains.kotlin.com.intellij.psi.PsiElement -import org.jetbrains.kotlin.psi.KtBlockExpression -import org.jetbrains.kotlin.psi.KtExpression -import org.jetbrains.kotlin.psi.KtIfExpression -import org.jetbrains.kotlin.psi.KtWhenExpression -import org.jetbrains.kotlin.psi.psiUtil.siblings - -/** - * This rule detects multi-line `if` statements which do not have braces. - * Adding braces would improve readability and avoid possible errors. - * - * - * val i = 1 - * if (i > 0) - * println(i) - * - * - * - * val x = if (condition) 5 else 4 - * - */ -class MandatoryBracesIfStatements(config: Config = Config.empty) : Rule(config) { - - override val issue = Issue( - "MandatoryBracesIfStatements", - Severity.Style, - "Multi-line if statement was found that does not have braces. " + - "These braces should be added to improve readability.", - Debt.FIVE_MINS - ) - - override fun visitIfExpression(expression: KtIfExpression) { - super.visitIfExpression(expression) - - val thenExpression = expression.then ?: return - if (thenExpression !is KtBlockExpression && hasNewLineAfter(expression.rightParenthesis)) { - report(CodeSmell(issue, Entity.from(expression.ifKeyword), issue.description)) - } - - val elseExpression = expression.`else` ?: return - if (mustBeOnSameLine(elseExpression) && hasNewLineAfter(expression.elseKeyword)) { - report(CodeSmell(issue, Entity.from(expression.elseKeyword ?: elseExpression), issue.description)) - } - } - - private fun hasNewLineAfter(element: PsiElement?): Boolean { - if (element == null) return false - return element - .siblings(forward = true, withItself = false) - .takeWhile { it.text != "else" } - .any { it.textContains('\n') } - } - - private fun mustBeOnSameLine(expression: KtExpression): Boolean = - expression !is KtIfExpression && expression !is KtBlockExpression && expression !is KtWhenExpression -} diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/BracesOnIfStatementsSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/BracesOnIfStatementsSpec.kt new file mode 100644 index 00000000000..f1e9d9a06c2 --- /dev/null +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/BracesOnIfStatementsSpec.kt @@ -0,0 +1,2206 @@ +package io.gitlab.arturbosch.detekt.rules.style + +import io.gitlab.arturbosch.detekt.api.TextLocation +import io.gitlab.arturbosch.detekt.rules.style.BracesOnIfStatements.BracePolicy +import io.gitlab.arturbosch.detekt.rules.style.BracesOnIfStatementsSpec.Companion.NOT_RELEVANT +import io.gitlab.arturbosch.detekt.rules.style.BracesOnIfStatementsSpec.Companion.test +import io.gitlab.arturbosch.detekt.test.TestConfig +import io.gitlab.arturbosch.detekt.test.assertThat +import io.gitlab.arturbosch.detekt.test.compileAndLint +import io.gitlab.arturbosch.detekt.test.lint +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatCode +import org.junit.jupiter.api.DynamicContainer.dynamicContainer +import org.junit.jupiter.api.DynamicNode +import org.junit.jupiter.api.DynamicTest.dynamicTest +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestFactory + +/** + * Note: this class makes extensive use of dynamic tests and containers, few tips for maintenance: + * * Numbers are made relative to the code snippet rather than the whole code passed to Kotlin compiler. [test]. + * * To debug a specific test case, remove all other test cases from the same method. + * * Test coverage is added for all possible configuration options, see [NOT_RELEVANT]. + */ +@Suppress( + "ClassName", + "LongMethod", + "CommentOverPrivateProperty" +) +class BracesOnIfStatementsSpec { + + @Test + fun `validate behavior of occurrence function`() { + val code = "fun f() { if (true) else if (true) if (true) true }" + assertThat("if"(1)(code)).isEqualTo(10 to 12) + assertThat("if"(2)(code)).isEqualTo(25 to 27) + assertThat("if"(3)(code)).isEqualTo(35 to 37) + assertThat("else"(1)(code)).isEqualTo(20 to 24) + assertThatCode { "if"(4)(code) }.isInstanceOf(IllegalArgumentException::class.java) + assertThatCode { "if"(0)(code) }.isInstanceOf(IllegalArgumentException::class.java) + assertThatCode { "else"(2)(code) }.isInstanceOf(IllegalArgumentException::class.java) + } + + @Nested + inner class singleLine { + + @Nested + inner class `=always` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(BracePolicy.Always.config, NOT_RELEVANT, code, *locations) + + @TestFactory + fun `missing braces are flagged`() = listOf( + flag("if (true) println()", "if"(1)), + flag("if (true) println() else println()", "if"(1), "else"(1)), + flag( + "if (true) println() else if (true) println()", + "if"(1), + "if"(2) + ), + flag( + "if (true) println() else if (true) println() else println()", + "if"(1), + "if"(2), + "else"(2) + ), + flag( + "if (true) println() else if (true) println() else if (true) println()", + "if"(1), + "if"(2), + "if"(3) + ), + flag( + "if (true) println() else if (true) println() else if (true) println() else println()", + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `existing braces are accepted`() = listOf( + flag("if (true) { println() }", *NOTHING), + flag("if (true) { println() } else { println() }", *NOTHING), + flag("if (true) { println() } else if (true) { println() }", *NOTHING), + flag("if (true) { println() } else if (true) { println() } else { println() }", *NOTHING), + flag("if (true) { println() } else if (true) { println() } else if (true) { println() }", *NOTHING), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() } else { println() }", + *NOTHING + ), + ) + + @TestFactory + fun `partially missing braces are flagged (first branch)`() = listOf( + flag("if (true) { println() } else println()", "else"(1)), + flag("if (true) { println() } else if (true) println()", "if"(2)), + flag( + "if (true) { println() } else if (true) println() else println()", + "if"(2), + "else"(2) + ), + flag( + "if (true) { println() } else if (true) println() else if (true) println()", + "if"(2), + "if"(3) + ), + flag( + "if (true) { println() } else if (true) println() else if (true) println() else println()", + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `partially missing braces are flagged (last branch)`() = listOf( + flag("if (true) println() else { println() }", "if"(1)), + flag("if (true) println() else if (true) { println() }", "if"(1)), + flag( + "if (true) println() else if (true) println() else { println() }", + "if"(1), + "if"(2) + ), + flag( + "if (true) println() else if (true) println() else if (true) { println() }", + "if"(1), + "if"(2) + ), + flag( + "if (true) println() else if (true) println() else if (true) println() else { println() }", + "if"(1), + "if"(2), + "if"(3) + ), + ) + + @TestFactory + fun `partially missing braces are flagged (middle branches)`() = listOf( + flag( + "if (true) println() else if (true) { println() } else println()", + "if"(1), + "else"(2) + ), + flag( + "if (true) println() else if (true) { println() } else if (true) println()", + "if"(1), + "if"(3) + ), + flag( + "if (true) println() else if (true) { println() } else if (true) { println() } else println()", + "if"(1), + "else"(3) + ), + ) + } + + @Nested + inner class `=never` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(BracePolicy.Never.config, NOT_RELEVANT, code, *locations) + + @TestFactory fun `no braces are accepted`() = listOf( + flag("if (true) println()", *NOTHING), + flag("if (true) println() else println()", *NOTHING), + flag("if (true) println() else if (true) println()", *NOTHING), + flag("if (true) println() else if (true) println() else println()", *NOTHING), + flag("if (true) println() else if (true) println() else if (true) println()", *NOTHING), + flag("if (true) println() else if (true) println() else if (true) println() else println()", *NOTHING), + ) + + @TestFactory + fun `existing braces are flagged`() = listOf( + flag("if (true) { println() }", "if"(1)), + flag("if (true) { println() } else { println() }", "if"(1), "else"(1)), + flag("if (true) { println() } else if (true) { println() }", "if"(1), "if"(2)), + flag( + "if (true) { println() } else if (true) { println() } else { println() }", + "if"(1), + "if"(2), + "else"(2) + ), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() }", + "if"(1), + "if"(2), + "if"(3) + ), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() } else { println() }", + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (first branch)`() = listOf( + flag("if (true) { println() }", "if"(1)), + flag("if (true) { println() } else println()", "if"(1)), + flag("if (true) { println() } else if (true) println()", "if"(1)), + flag("if (true) { println() } else if (true) println() else println()", "if"(1)), + flag("if (true) { println() } else if (true) println() else if (true) println()", "if"(1)), + flag( + "if (true) { println() } else if (true) println() else if (true) println() else println()", + "if"(1) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (last branch)`() = listOf( + flag("if (true) println() else { println() }", "else"(1)), + flag("if (true) println() else if (true) { println() }", "if"(2)), + flag("if (true) println() else if (true) println() else { println() }", "else"(2)), + flag("if (true) println() else if (true) println() else if (true) { println() }", "if"(3)), + flag( + "if (true) println() else if (true) println() else if (true) println() else { println() }", + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (middle branches)`() = listOf( + flag("if (true) println() else if (true) { println() } else println()", "if"(2)), + flag("if (true) println() else if (true) { println() } else if (true) println()", "if"(2)), + flag( + "if (true) println() else if (true) { println() } else if (true) { println() } else println()", + "if"(2), + "if"(3) + ), + ) + + @TestFactory fun `fully extra braces are flagged`() = listOf( + flag("if (true) { println() }", "if"(1)), + flag("if (true) { println() } else { println() }", "if"(1), "else"(1)), + flag("if (true) { println() } else if (true) { println() }", "if"(1), "if"(2)), + flag( + "if (true) { println() } else if (true) { println() } else { println() }", + "if"(1), + "if"(2), + "else"(2) + ), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() }", + "if"(1), + "if"(2), + "if"(3) + ), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() } else { println() }", + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + } + + @Nested + inner class `=necessary` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(BracePolicy.Necessary.config, NOT_RELEVANT, code, *locations) + + @TestFactory + fun `no braces are accepted`() = listOf( + flag("if (true) println()", *NOTHING), + flag("if (true) println() else println()", *NOTHING), + flag("if (true) println() else if (true) println()", *NOTHING), + flag("if (true) println() else if (true) println() else println()", *NOTHING), + flag("if (true) println() else if (true) println() else if (true) println()", *NOTHING), + flag("if (true) println() else if (true) println() else if (true) println() else println()", *NOTHING), + ) + + @TestFactory + fun `existing braces are flagged`() = listOf( + flag("if (true) { println() }", "if"(1)), + flag("if (true) { println() } else { println() }", "if"(1), "else"(1)), + flag("if (true) { println() } else if (true) { println() }", "if"(1), "if"(2)), + flag( + "if (true) { println() } else if (true) { println() } else { println() }", + "if"(1), + "if"(2), + "else"(2) + ), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() }", + "if"(1), + "if"(2), + "if"(3) + ), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() } else { println() }", + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (first branch)`() = listOf( + flag("if (true) { println() }", "if"(1)), + flag("if (true) { println() } else println()", "if"(1)), + flag("if (true) { println() } else if (true) println()", "if"(1)), + flag("if (true) { println() } else if (true) println() else println()", "if"(1)), + flag("if (true) { println() } else if (true) println() else if (true) println()", "if"(1)), + flag( + "if (true) { println() } else if (true) println() else if (true) println() else println()", + "if"(1) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (last branch)`() = listOf( + flag("if (true) println() else { println() }", "else"(1)), + flag("if (true) println() else if (true) { println() }", "if"(2)), + flag("if (true) println() else if (true) println() else { println() }", "else"(2)), + flag("if (true) println() else if (true) println() else if (true) { println() }", "if"(3)), + flag( + "if (true) println() else if (true) println() else if (true) println() else { println() }", + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (middle branches)`() = listOf( + flag("if (true) println() else if (true) { println() } else println()", "if"(2)), + flag("if (true) println() else if (true) { println() } else if (true) println()", "if"(2)), + flag( + "if (true) println() else if (true) { println() } else if (true) { println() } else println()", + "if"(2), + "if"(3), + ), + ) + + @TestFactory + fun `existing braces are not flagged when necessary`() = listOf( + flag("if (true) { println(); println() }", *NOTHING), + flag("if (true) println() else { println(); println() }", *NOTHING), + flag("if (true) println() else if (true) { println(); println() }", *NOTHING), + flag("if (true) println() else if (true) println() else { println(); println() }", *NOTHING), + ) + + @TestFactory + fun `extra braces are flagged when not necessary (first)`() = listOf( + flag("if (true) { println(); println() } else { println() }", "else"(1)), + flag("if (true) { println(); println() } else if (true) { println() }", "if"(2)), + flag("if (true) { println(); println() } else if (true) println() else { println() }", "else"(2)), + flag("if (true) { println(); println() } else if (true) { println() } else println()", "if"(2)), + flag( + "if (true) { println(); println() } else if (true) { println() } else { println() }", + "if"(2), + "else"(2) + ), + ) + + @TestFactory + fun `extra braces are flagged when not necessary (last)`() = listOf( + flag("if (true) { println() } else { println(); println() }", "if"(1)), + flag("if (true) { println() } else if (true) { println(); println() }", "if"(1)), + flag("if (true) { println() } else if (true) println() else { println(); println() }", "if"(1)), + flag("if (true) { println() } else if (true) println() else { println(); println() }", "if"(1)), + flag("if (true) println() else if (true) { println() } else { println(); println() }", "if"(2)), + flag( + "if (true) { println() } else if (true) { println() } else { println(); println() }", + "if"(1), + "if"(2) + ), + ) + } + + @Nested + inner class `=consistent` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(BracePolicy.Consistent.config, NOT_RELEVANT, code, *locations) + + @TestFactory + fun `no braces are accepted`() = listOf( + flag("if (true) println()", *NOTHING), + flag("if (true) println() else println()", *NOTHING), + flag("if (true) println() else if (true) println()", *NOTHING), + flag("if (true) println() else if (true) println() else println()", *NOTHING), + flag("if (true) println() else if (true) println() else if (true) println()", *NOTHING), + flag("if (true) println() else if (true) println() else if (true) println() else println()", *NOTHING), + ) + + @TestFactory + fun `existing braces are flagged`() = listOf( + flag("if (true) { println() }", *NOTHING), + flag("if (true) { println() } else { println() }", *NOTHING), + flag("if (true) { println() } else if (true) { println() }", *NOTHING), + flag("if (true) { println() } else if (true) { println() } else { println() }", *NOTHING), + flag("if (true) { println() } else if (true) { println() } else if (true) { println() }", *NOTHING), + flag( + "if (true) { println() } else if (true) { println() } else if (true) { println() } else { println() }", + *NOTHING + ), + ) + + @TestFactory + fun `partial braces are flagged (first branch)`() = listOf( + flag("if (true) { println() }", *NOTHING), + flag("if (true) { println() } else println()", "if"(1)), + flag("if (true) { println() } else if (true) println()", "if"(1)), + flag("if (true) { println() } else if (true) println() else println()", "if"(1)), + flag("if (true) { println() } else if (true) println() else if (true) println()", "if"(1)), + flag( + "if (true) { println() } else if (true) println() else if (true) println() else println()", + "if"(1) + ), + ) + + @TestFactory + fun `partial braces are flagged (last branch)`() = listOf( + flag("if (true) println() else { println() }", "if"(1)), + flag("if (true) println() else if (true) { println() }", "if"(1)), + flag("if (true) println() else if (true) println() else { println() }", "if"(1)), + flag("if (true) println() else if (true) println() else if (true) { println() }", "if"(1)), + flag( + "if (true) println() else if (true) println() else if (true) println() else { println() }", + "if"(1), + ), + ) + + @TestFactory + fun `partial braces are flagged (middle branches)`() = listOf( + flag("if (true) println() else if (true) { println() } else println()", "if"(1)), + flag("if (true) println() else if (true) { println() } else if (true) println()", "if"(1)), + flag( + "if (true) println() else if (true) { println() } else if (true) { println() } else println()", + "if"(1) + ), + ) + } + } + + @Nested + inner class multiLine { + + @Nested + inner class `=always` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(NOT_RELEVANT, BracePolicy.Always.config, code, *locations) + + @TestFactory + fun `missing braces are flagged`() = listOf( + flag( + """ + if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else + println() + """.trimIndent(), + "if"(1), + "else"(1) + ), + flag( + """ + if (true) + println() + else if (true) + println() + """.trimIndent(), + "if"(1), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + "if"(1), + "if"(2), + "else"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `existing braces are accepted`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + *NOTHING + ), + ) + + @TestFactory + fun `partially missing braces are flagged (first branch)`() = listOf( + flag( + """ + if (true) { + println() + } else + println() + """.trimIndent(), + "else"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else + println() + """.trimIndent(), + "if"(2), + "else"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + """.trimIndent(), + "if"(2), + "if"(3) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `partially missing braces are flagged (last branch)`() = listOf( + flag( + """ + if (true) + println() + else { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(1), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3) + ), + ) + + @TestFactory + fun `partially missing braces are flagged (middle branches)`() = listOf( + flag( + """ + if (true) + println() + else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(1), + "else"(2) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(1), + "if"(3) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(1), + "else"(3) + ), + ) + } + + @Nested + inner class `=never` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(NOT_RELEVANT, BracePolicy.Never.config, code, *locations) + + @TestFactory fun `no braces are accepted`() = listOf( + flag( + """ + if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + ) + + @TestFactory + fun `existing braces are flagged`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "else"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + "if"(1), + "if"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "else"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (first branch)`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + "if"(1) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (last branch)`() = listOf( + flag( + """ + if (true) + println() + else { + println() + } + """.trimIndent(), + "else"(1) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "else"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(3) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (middle branches)`() = listOf( + flag( + """ + if (true) + println() + else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) { + println() + } else println() + """.trimIndent(), + "if"(2), + "if"(3) + ), + ) + + @TestFactory fun `fully extra braces are flagged`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "else"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + "if"(1), + "if"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "else"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + } + + @Nested + inner class `=necessary` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(NOT_RELEVANT, BracePolicy.Necessary.config, code, *locations) + + @TestFactory + fun `no braces are accepted`() = listOf( + flag( + """ + if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + ) + + @TestFactory + fun `existing braces are flagged`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "else"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + "if"(1), + "if"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "else"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(1), + "if"(2), + "if"(3), + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (first branch)`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + "if"(1) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (last branch)`() = listOf( + flag( + """ + if (true) + println() + else { + println() + } + """.trimIndent(), + "else"(1) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "else"(2) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(3) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "else"(3) + ), + ) + + @TestFactory + fun `partially extra braces are flagged (middle branches)`() = listOf( + flag( + """ + if (true) + println() + else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(2), + "if"(3) + ), + ) + + @TestFactory + fun `existing braces are not flagged when necessary`() = listOf( + flag( + """ + if (true) { + println() + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else { + println() + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) { + println() + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else { + println() + println() + } + """.trimIndent(), + *NOTHING + ), + ) + + @TestFactory + fun `extra braces are flagged when not necessary (first)`() = listOf( + flag( + """ + if (true) { + println() + println() + } else { + println() + } + """.trimIndent(), + "else"(1) + ), + flag( + """ + if (true) { + println() + println() + } else if (true) { + println() + } + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) { + println() + println() + } else if (true) + println() + else { + println() + } + """.trimIndent(), + "else"(2) + ), + flag( + """ + if (true) { + println() + println() + } else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) { + println() + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + "if"(2), + "else"(2) + ), + ) + + @TestFactory + fun `extra braces are flagged when not necessary (last)`() = listOf( + flag( + """ + if (true) { + println() + } else { + println() + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else { + println() + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else { + println() + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else { + println() + println() + } + """.trimIndent(), + "if"(2) + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else { + println() + println() + } + """.trimIndent(), + "if"(1), + "if"(2) + ), + ) + } + + @Nested + inner class `=consistent` { + + private fun flag(code: String, vararg locations: (String) -> Pair) = + testCombinations(NOT_RELEVANT, BracePolicy.Consistent.config, code, *locations) + + @TestFactory + fun `no braces are accepted`() = listOf( + flag( + """ + if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + *NOTHING + ), + ) + + @TestFactory + fun `existing braces are flagged`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else if (true) { + println() + } else if (true) { + println() + } else { + println() + } + """.trimIndent(), + *NOTHING + ), + ) + + @TestFactory + fun `partial braces are flagged (first branch)`() = listOf( + flag( + """ + if (true) { + println() + } + """.trimIndent(), + *NOTHING + ), + flag( + """ + if (true) { + println() + } else + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) { + println() + } else if (true) + println() + else if (true) + println() + else + println() + """.trimIndent(), + "if"(1) + ), + ) + + @TestFactory + fun `partial braces are flagged (last branch)`() = listOf( + flag( + """ + if (true) + println() + else { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) { + println() + } + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) + println() + else if (true) + println() + else { + println() + } + """.trimIndent(), + "if"(1), + ), + ) + + @TestFactory + fun `partial braces are flagged (middle branches)`() = listOf( + flag( + """ + if (true) + println() + else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) + println() + """.trimIndent(), + "if"(1) + ), + flag( + """ + if (true) + println() + else if (true) { + println() + } else if (true) { + println() + } else + println() + """.trimIndent(), + "if"(1) + ), + ) + } + } + + @Nested + inner class nested { + + @TestFactory + fun `nested ifs are flagged for consistency`() = testCombinations( + singleLine = NOT_RELEVANT, + multiLine = BracePolicy.Consistent.config, + code = """ + if (true) { + if (true) { + println() + } else println() + } else println() + """.trimIndent(), + locations = arrayOf( + "if"(1), + "if"(2), + ), + ) + + @TestFactory + fun `only multiline ifs are are flagged (complex)`() = testCombinations( + singleLine = BracePolicy.Never.config, + multiLine = BracePolicy.Always.config, + code = """ + if (if (true) true else false) + if (true) true else false + else + println(if (true) true else false) + """.trimIndent(), + locations = arrayOf( + "if"(1), + "else"(3), + ), + ) + + @TestFactory + fun `only multiline ifs are are flagged (simple)`() = testCombinations( + singleLine = BracePolicy.Never.config, + multiLine = BracePolicy.Always.config, + code = """ + if (true) + if (true) true else false + """.trimIndent(), + locations = arrayOf( + "if"(1), + ), + ) + + @TestFactory + fun `nested ifs are flagged for always`() = testCombinations( + singleLine = BracePolicy.Always.config, + multiLine = BracePolicy.Always.config, + code = """ + if (if (true) true else false) + if (true) println() else println() + else + println(if (true) true else false) + """.trimIndent(), + locations = arrayOf( + "if"(1), + "if"(2), + "else"(1), + "if"(3), + "else"(2), + "else"(3), + "if"(4), + "else"(4), + ), + ) + + @TestFactory + fun `nested ifs inside condition are flagged for always`() = testCombinations( + singleLine = BracePolicy.Always.config, + multiLine = NOT_RELEVANT, + code = """ + if (if (if (true) true else false) true else false) println() + """.trimIndent(), + locations = arrayOf( + "if"(1), + "if"(2), + "if"(3), + "else"(1), + "else"(2), + ), + ) + + @TestFactory + fun `nested ifs inside then are flagged for always`() = testCombinations( + singleLine = BracePolicy.Always.config, + multiLine = NOT_RELEVANT, + code = """ + if (true) if (true) if (true) println() else println() else println() else println() + """.trimIndent(), + locations = arrayOf( + "if"(1), + "if"(2), + "if"(3), + "else"(1), + "else"(2), + "else"(3), + ), + ) + } + + @TestFactory + fun `whens are not flagged`() = testCombinations( + singleLine = NOT_RELEVANT, + multiLine = NOT_RELEVANT, + code = """ + when (true) { + true -> println() + else -> println() + } + """.trimIndent(), + locations = NOTHING + ) + + companion object { + + /** + * Not relevant means, that it should be covered, but for that specific test case the value doesn't matter. + * This needs to be covered still, to make sure it never becomes relevant. + * In this rule, configuration options should be separate for each config, not coupled with each other. + */ + private const val NOT_RELEVANT: String = "*" + + /** + * Nothing is expected to be flagged as a finding. + */ + private val NOTHING: Array<(String) -> Pair> = emptyArray() + + private fun createSubject(singleLine: String, multiLine: String): BracesOnIfStatements { + val config = TestConfig( + mapOf( + "singleLine" to singleLine, + "multiLine" to multiLine + ) + ) + return BracesOnIfStatements(config) + } + + private fun options(option: String): List = + if (option == NOT_RELEVANT) { + BracePolicy.values().map { it.config } + } else { + listOf(option) + } + + private fun testCombinations( + singleLine: String, + multiLine: String, + code: String, + vararg locations: (String) -> Pair + ): DynamicNode { + val codeLocation = locations.map { it(code) }.toTypedArray() + // Separately compile the code because otherwise all the combinations would compile them again and again. + val compileTest = dynamicTest("Compiles: $code") { + BracesOnIfStatements().compileAndLint(code) + } + val validationTests = createBraceTests(singleLine, multiLine) { rule -> + rule.test(code, *codeLocation) + } + val locationString = if (NOTHING.contentEquals(codeLocation)) { + "nothing" + } else { + codeLocation.map { TextLocation(it.first, it.second) }.toString() + } + return dynamicContainer("flags $locationString in `$code`", validationTests + compileTest) + } + + private fun BracesOnIfStatements.test(code: String, vararg locations: Pair) { + // This creates a 10 character prefix (signature/9, space/1) for every code example. + // Note: not compileAndLint for performance reasons, compilation is in a separate test. + val findings = lint("fun f() { $code }") + // Offset text locations by the above prefix, it results in 0-indexed locations. + val offset = 10 + assertThat(findings) + .hasTextLocations( + *(locations.map { it.first + offset to it.second + offset }).toTypedArray() + ) + } + + /** + * Generates a list of tests for the given brace policy combinations. + * The expectations in the test will be the same for all combinations. + * + * @see options for how the arguments are interpreted. + */ + private fun createBraceTests( + singleLine: String, + multiLine: String, + test: (BracesOnIfStatements) -> Unit + ): List { + val singleOptions = options(singleLine) + val multiOptions = options(multiLine) + require(singleOptions.isNotEmpty() && multiOptions.isNotEmpty()) { + "No options to test: $singleLine -> $singleOptions, $multiLine -> $multiOptions" + } + return singleOptions.flatMap { singleLineOption -> + multiOptions.map { multiLineOption -> + val trace = Exception("Async stack trace of TestFactory") + dynamicTest("singleLine=$singleLineOption, multiLine=$multiLineOption") { + try { + // Note: if you jumped here from a failed test's stack trace, + // select the "Async stack trace of TestFactory" cause's + // last relevant stack line to jump to the actual test code. + test(createSubject(singleLineOption, multiLineOption)) + } catch (e: Throwable) { + generateSequence(e) { it.cause }.last().initCause(trace) + throw e + } + } + } + } + } + + operator fun String.invoke(ordinal: Int): (String) -> Pair = + { code -> + fun String.next(string: String, start: Int): Int? = indexOf(string, start).takeIf { it != -1 } + + val indices = generateSequence(code.next(this, 0)) { startIndex -> + code.next(this, startIndex + 1) + } + val index = requireNotNull(indices.elementAtOrNull(ordinal - 1)) { + "There's no $ordinal. occurrence of '$this' in '$code'" + } + index to index + this.length + } + } +} diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatementsSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatementsSpec.kt deleted file mode 100644 index 8b47f72a371..00000000000 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/MandatoryBracesIfStatementsSpec.kt +++ /dev/null @@ -1,217 +0,0 @@ -package io.gitlab.arturbosch.detekt.rules.style.optional - -import io.gitlab.arturbosch.detekt.api.Config -import io.gitlab.arturbosch.detekt.test.assertThat -import io.gitlab.arturbosch.detekt.test.compileAndLint -import org.junit.jupiter.api.Nested -import org.junit.jupiter.api.Test - -class MandatoryBracesIfStatementsSpec { - val subject = MandatoryBracesIfStatements(Config.empty) - - @Nested - inner class `if statements which should have braces` { - - @Test - fun `reports a simple if`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) - println() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(1) - assertThat(findings).hasTextLocations(14 to 16) - } - - @Test - fun `reports a simple if with a single statement in multiple lines`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) 50 - .toString() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(1) - } - - @Test - fun `reports if-else with a single statement in multiple lines`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) 50 - .toString() else 50 - .toString() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(2) - assertThat(findings).hasTextLocations(11 to 13, 44 to 48) - } - - @Test - fun `reports if-else`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) - println() - else - println() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(2) - assertThat(findings).hasTextLocations(14 to 16, 46 to 50) - } - - @Test - fun `reports if-else with else-if`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) - println() - else if (false) - println() - else - println() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(3) - assertThat(findings).hasTextLocations(14 to 16, 51 to 53, 84 to 88) - } - - @Test - fun `reports if with braces but else without`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) { - println() - } else - println() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(1) - assertThat(findings).hasTextLocations(50 to 54) - } - - @Test - fun `reports else with braces but if without`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) - println() - else { - println() - } - } - """.trimIndent() - ) - - assertThat(findings).hasSize(1) - assertThat(findings).hasTextLocations(14 to 16) - } - - @Test - fun `reports else in new line`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) println() - else println() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(1) - assertThat(findings).hasTextLocations(14 to 16) - } - - @Test - fun `reports only else body on new line`() { - val findings = subject.compileAndLint( - """ - fun f() { - if (true) println() else - println() - } - """.trimIndent() - ) - - assertThat(findings).hasSize(1) - assertThat(findings).hasTextLocations(34 to 38) - } - } - - @Nested - inner class `if statements with braces` { - - @Test - fun `does not report if statements with braces`() { - val code = """ - fun f() { - if (true) { - println() - } - if (true) - { - println() - } - if (true) { println() } - } - """.trimIndent() - assertThat(subject.compileAndLint(code)).isEmpty() - } - } - - @Nested - inner class `single-line if statements which don't need braces` { - - @Test - fun `does not report single-line if statements`() { - val code = """ - fun f() { - if (true) println() - if (true) println() else println() - if (true) println() else if (false) println() else println() - } - """.trimIndent() - assertThat(subject.compileAndLint(code)).isEmpty() - } - } - - @Nested - inner class `multi-line when following an else statement without requiring braces` { - - @Test - fun `does not report multi-line when`() { - val code = """ - fun f(i: Int) { - if (true) { - println() - } else when(i) { - 1 -> println(1) - else -> println() - } - } - """.trimIndent() - assertThat(subject.compileAndLint(code)).isEmpty() - } - } -}