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

Fix Conflicting TOO_MANY_BLANK_LINES and BRACES_BLOCK_STRUCTURE_ERROR with empty block #797

Merged
merged 5 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.cqfn.diktat.ruleset.constants.Warnings.BRACES_BLOCK_STRUCTURE_ERROR
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.*

import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.BLOCK
import com.pinterest.ktlint.core.ast.ElementType.BODY
import com.pinterest.ktlint.core.ast.ElementType.CATCH
Expand Down Expand Up @@ -233,6 +234,11 @@ class BlockStructureBraces(configRules: List<RulesConfig>) : DiktatRule(
return
}
val space = node.findChildByType(RBRACE)!!.treePrev
node.findParentNodeWithSpecificType(ElementType.LAMBDA_ARGUMENT)?.let {
if (space.text.isEmpty()) {
return
Cheshiriks marked this conversation as resolved.
Show resolved Hide resolved
}
}
if (checkBraceNode(space)) {
BRACES_BLOCK_STRUCTURE_ERROR.warnAndFix(configRules, emitWarn, isFixMode, "no newline before closing brace",
(space.treeNext ?: node.findChildByType(RBRACE))!!.startOffset, node) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,9 @@ import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.common.config.rules.getRuleConfig
import org.cqfn.diktat.ruleset.constants.Warnings.EMPTY_BLOCK_STRUCTURE_ERROR
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.findLBrace
import org.cqfn.diktat.ruleset.utils.findLeafWithSpecificType
import org.cqfn.diktat.ruleset.utils.findParentNodeWithSpecificType
import org.cqfn.diktat.ruleset.utils.hasParent
import org.cqfn.diktat.ruleset.utils.isBlockEmpty
import org.cqfn.diktat.ruleset.utils.isOverridden
import org.cqfn.diktat.ruleset.utils.isPascalCase
import org.cqfn.diktat.ruleset.utils.*

import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_LITERAL
import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
Expand Down Expand Up @@ -41,7 +36,10 @@ class EmptyBlock(configRules: List<RulesConfig>) : DiktatRule(
checkEmptyBlock(newNode, configuration)
}

@Suppress("UnsafeCallOnNullableType")
private fun isNewLine(node: ASTNode) =
node.findChildByType(WHITE_SPACE)?.text?.contains("\n") ?: true
Cheshiriks marked this conversation as resolved.
Show resolved Hide resolved

@Suppress("UnsafeCallOnNullableType", "TOO_LONG_FUNCTION")
private fun checkEmptyBlock(node: ASTNode, configuration: EmptyBlockStyleConfiguration) {
if (node.treeParent.isOverridden() || isAnonymousSamClass(node)) {
return
Expand All @@ -51,6 +49,19 @@ class EmptyBlock(configRules: List<RulesConfig>) : DiktatRule(
EMPTY_BLOCK_STRUCTURE_ERROR.warn(configRules, emitWarn, isFixMode, "empty blocks are forbidden unless it is function with override keyword",
node.startOffset, node)
} else {
node.findParentNodeWithSpecificType(ElementType.LAMBDA_ARGUMENT)?.let {
// Lambda body is always has a BLOCK -> run { } - (LBRACE, WHITE_SPACE, BLOCK "", RBRACE)
if (isNewLine(node)) {
val freeText = "do not put newlines in empty lambda"
EMPTY_BLOCK_STRUCTURE_ERROR.warnAndFix(configRules, emitWarn, isFixMode, freeText, node.startOffset, node) {
val whiteSpaceNode = node.findChildByType(WHITE_SPACE)
whiteSpaceNode?.let {
node.replaceChild(whiteSpaceNode, PsiWhiteSpaceImpl(" "))
}
}
}
return
}
val space = node.findChildByType(RBRACE)!!.treePrev
if (configuration.emptyBlockNewline && !space.text.contains("\n")) {
EMPTY_BLOCK_STRUCTURE_ERROR.warnAndFix(configRules, emitWarn, isFixMode, "different style for empty block",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.cqfn.diktat.ruleset.rules.chapter3.files
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.ruleset.constants.Warnings.TOO_MANY_BLANK_LINES
import org.cqfn.diktat.ruleset.rules.DiktatRule
import org.cqfn.diktat.ruleset.utils.findParentNodeWithSpecificType
import org.cqfn.diktat.ruleset.utils.getFirstChildWithType
import org.cqfn.diktat.ruleset.utils.leaveExactlyNumNewLines
import org.cqfn.diktat.ruleset.utils.leaveOnlyOneNewLine
Expand All @@ -12,6 +13,7 @@ import com.pinterest.ktlint.core.ast.ElementType.BLOCK
import com.pinterest.ktlint.core.ast.ElementType.CLASS_BODY
import com.pinterest.ktlint.core.ast.ElementType.FILE
import com.pinterest.ktlint.core.ast.ElementType.FUNCTION_LITERAL
import com.pinterest.ktlint.core.ast.ElementType.LAMBDA_ARGUMENT
import com.pinterest.ktlint.core.ast.ElementType.LBRACE
import com.pinterest.ktlint.core.ast.ElementType.RBRACE
import com.pinterest.ktlint.core.ast.ElementType.SCRIPT
Expand Down Expand Up @@ -44,6 +46,13 @@ class BlankLinesRule(configRules: List<RulesConfig>) : DiktatRule(
it.elementType == BLOCK && it.treeParent?.elementType != SCRIPT ||
it.elementType == CLASS_BODY || it.elementType == FUNCTION_LITERAL
}) {
node.findParentNodeWithSpecificType(LAMBDA_ARGUMENT)?.let {
// Lambda body is always has a BLOCK -> run { } - (LBRACE, WHITE_SPACE, BLOCK "", RBRACE)
if (node.treeNext.text.isEmpty()) {
return
}
}

if ((node.treeNext.elementType == RBRACE) xor (node.treePrev.elementType == LBRACE)) {
// if both are present, this is not beginning or end
// if both are null, then this block is empty and is handled in another rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,20 @@ class BlockStructureBracesWarnTest : LintTestBase(::BlockStructureBraces) {
)
}

@Test
@Tag(WarningNames.BRACES_BLOCK_STRUCTURE_ERROR)
fun `check lambda with empty block`() {
lintMethod(
"""
|fun foo() {
| run {
|
| }
|}
""".trimMargin()
)
}

@Test
@Tag(WarningNames.BRACES_BLOCK_STRUCTURE_ERROR)
fun `check empty block in else expression`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,39 +96,39 @@ class EmptyBlockWarnTest : LintTestBase(::EmptyBlock) {
}

@Test
fun `check if-else expression without block`() {
fun `empty lambda`() {
lintMethod(
"""
|fun foo() {
| if (node.treeParent != null) return else println(true)
| run { }
|}
""".trimMargin()
""".trimMargin(),
rulesConfigList = rulesConfigListEmptyBlockExist
)
}

@Test
fun `check for expresion and while without block`() {
fun `check if-else expression without block`() {
lintMethod(
"""
|fun foo() {
| for(x in 0..10) println(x)
| val x = 10
| while (x > 0)
| --x
| if (node.treeParent != null) return else println(true)
|}
""".trimMargin()
)
}

@Test
fun `check empty lambda`() {
fun `check for expresion and while without block`() {
lintMethod(
"""
|fun foo() {
| val y = listOf<Int>().map {}
| for(x in 0..10) println(x)
| val x = 10
| while (x > 0)
| --x
|}
""".trimMargin(),
LintError(2, 30, ruleId, "${EMPTY_BLOCK_STRUCTURE_ERROR.warnText()} empty blocks are forbidden unless it is function with override keyword", false)
""".trimMargin()
)
}

Expand All @@ -138,14 +138,28 @@ class EmptyBlockWarnTest : LintTestBase(::EmptyBlock) {
lintMethod(
"""
|fun foo() {
| val y = listOf<Int>().map {}
| val y = listOf<Int>().map {
|
| }
|}
""".trimMargin(),
LintError(2, 30, ruleId, "${EMPTY_BLOCK_STRUCTURE_ERROR.warnText()} different style for empty block", true),
LintError(2, 30, ruleId, "${EMPTY_BLOCK_STRUCTURE_ERROR.warnText()} do not put newlines in empty lambda", true),
rulesConfigList = rulesConfigListEmptyBlockExist
)
}

@Test
fun `check empty lambda`() {
lintMethod(
"""
|fun foo() {
| val y = listOf<Int>().map { }
|}
""".trimMargin(),
LintError(2, 30, ruleId, "${EMPTY_BLOCK_STRUCTURE_ERROR.warnText()} empty blocks are forbidden unless it is function with override keyword", false)
)
}

@Test
fun `should not trigger on anonymous SAM classes #1`() {
lintMethod(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ class BlankLinesWarnTest : LintTestBase(::BlankLinesRule) {
)
}

@Test
@Tag(WarningNames.TOO_MANY_BLANK_LINES)
fun `check lambda with empty block`() {
lintMethod(
"""
|fun foo() {
| run {
|
| }
|}
""".trimMargin()
)
}

@Test
@Tag(WarningNames.TOO_MANY_BLANK_LINES)
fun `should prohibit usage of two or more consecutive blank lines`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ fun foo () {
fun goo () {
var x = 10
if (x == 10) return else println(10)
val y = listOf<Int>().map {
}
val y = listOf<Int>().map { }
for(x in 0..10) println(x)
while (x > 0)
--x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ fun foo () {
fun goo () {
var x = 10
if (x == 10) return else println(10)
val y = listOf<Int>().map {}
val y = listOf<Int>().map {

}
for(x in 0..10) println(x)
while (x > 0)
--x
Expand Down