Skip to content

Commit

Permalink
Testing and rule improvement for EmptyElseBlock (#4349)
Browse files Browse the repository at this point in the history
* Added tests for empty else-block; Consolidated logic for empty if- and else-block rules

* Added KDoc for EmptyIfElseBlock.kt

* Removed EmptyIfElseBlock and replaced with a shared utility function

* Fixed Detekt issue
  • Loading branch information
severn-everett committed Dec 2, 2021
1 parent e4e2b5b commit ed829aa
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 12 deletions.
@@ -1,6 +1,8 @@
package io.gitlab.arturbosch.detekt.rules.empty

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
import org.jetbrains.kotlin.psi.KtIfExpression

Expand All @@ -9,9 +11,16 @@ import org.jetbrains.kotlin.psi.KtIfExpression
*/
@ActiveByDefault(since = "1.0.0")
class EmptyElseBlock(config: Config) : EmptyRule(config) {

override fun visitIfExpression(expression: KtIfExpression) {
super.visitIfExpression(expression)
expression.`else`?.addFindingIfBlockExprIsEmpty()
expression.`else`?.addFindingIfBlockExprIsEmpty() ?: checkThenBodyForLoneSemicolon(expression) {
report(
CodeSmell(
issue,
Entity.from(it),
"This else block is empty and can be removed."
)
)
}
}
}
Expand Up @@ -4,25 +4,23 @@ import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.lexer.KtSingleValueToken
import org.jetbrains.kotlin.psi.KtIfExpression

/**
* Reports empty `if` blocks. Empty blocks of code serve no purpose and should be removed.
*/
@ActiveByDefault(since = "1.0.0")
class EmptyIfBlock(config: Config) : EmptyRule(config) {

override fun visitIfExpression(expression: KtIfExpression) {
super.visitIfExpression(expression)
expression.then?.addFindingIfBlockExprIsEmpty() ?: checkThenBodyForLoneSemicolon(expression)
}

private fun checkThenBodyForLoneSemicolon(expression: KtIfExpression) {
val valueOfNextSibling = (expression.nextSibling as? LeafPsiElement)?.elementType as? KtSingleValueToken
if (valueOfNextSibling?.value?.trim() == ";") {
report(CodeSmell(issue, Entity.from(expression), "This if block is empty and can be removed."))
expression.then?.addFindingIfBlockExprIsEmpty() ?: checkThenBodyForLoneSemicolon(expression) {
report(
CodeSmell(
issue,
Entity.from(it),
"This if block is empty and can be removed."
)
)
}
}
}
Expand Up @@ -8,8 +8,11 @@ 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.rules.hasCommentInside
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.lexer.KtSingleValueToken
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtIfExpression

/**
* Rule to detect empty blocks of code.
Expand All @@ -36,6 +39,13 @@ abstract class EmptyRule(
checkBlockExpr(true)
}

protected fun checkThenBodyForLoneSemicolon(expression: KtIfExpression, reportBlock: (KtIfExpression) -> Unit) {
val valueOfNextSibling = (expression.nextSibling as? LeafPsiElement)?.elementType as? KtSingleValueToken
if (valueOfNextSibling?.value?.trim() == ";") {
reportBlock(expression)
}
}

private fun KtExpression.checkBlockExpr(skipIfCommented: Boolean = false) {
if (this !is KtBlockExpression) return
val hasComment = hasCommentInside()
Expand Down
@@ -0,0 +1,105 @@
package io.gitlab.arturbosch.detekt.rules.empty

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.test.compileAndLint
import org.assertj.core.api.Assertions
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe

class EmptyElseBlockSpec : Spek({

val subject by memoized { EmptyElseBlock(Config.empty) }

describe("EmptyElseBlock rule") {
it("reports empty else block") {
val code = """
fun f() {
val i = 0
if (i == 0) {
println(i)
} else {
}
}
"""
Assertions.assertThat(subject.compileAndLint(code)).hasSize(1)
}

it("reports empty else blocks with trailing semicolon") {
val code = """
fun f() {
val i = 0
if (i == 0) {
println(i)
} else ;
}
"""
Assertions.assertThat(subject.compileAndLint(code)).hasSize(1)
}
it("reports empty else with trailing semicolon on new line") {
val code = """
fun f() {
var i = 0
if (i == 0) {
println(i)
} else
;
i++
}
"""
Assertions.assertThat(subject.compileAndLint(code)).hasSize(1)
}

it("reports empty else with trailing semicolon and braces") {
val code = """
fun f() {
var i = 0
if (i == 0) {
println()
} else; {
}
i++
}
"""
Assertions.assertThat(subject.compileAndLint(code)).hasSize(1)
}

it("does not report nonempty else with braces") {
val code = """
fun f() {
var i = 0
if (i == 0) {
println(i)
} else {
i++
}
}
"""
Assertions.assertThat(subject.compileAndLint(code)).isEmpty()
}

it("does not report nonempty else without braces") {
val code = """
fun f() {
var i = 0
if (i == 0) {
println(i)
} else i++
}
"""
Assertions.assertThat(subject.compileAndLint(code)).isEmpty()
}

it("does not report nonempty else without braces but semicolon") {
val code = """
fun f() {
var i = 0
if (i == 0) {
println(i)
} else i++;
}
"""
Assertions.assertThat(subject.compileAndLint(code)).isEmpty()
}
}
})

0 comments on commit ed829aa

Please sign in to comment.