diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/NullChecksRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/NullChecksRule.kt index 799ef0afa9..61c8bb9996 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/NullChecksRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/NullChecksRule.kt @@ -9,14 +9,17 @@ import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.CONDITION +import com.pinterest.ktlint.core.ast.ElementType.ELSE import com.pinterest.ktlint.core.ast.ElementType.IF import com.pinterest.ktlint.core.ast.ElementType.IF_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.NULL import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.THEN import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT_LIST import com.pinterest.ktlint.core.ast.parent import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType import org.jetbrains.kotlin.psi.KtBinaryExpression import org.jetbrains.kotlin.psi.KtIfExpression @@ -72,13 +75,13 @@ class NullChecksRule(private val configRules: List) : Rule("null-ch ElementType.EQEQ, ElementType.EQEQEQ -> warnAndFixOnNullCheck(condition, true, "use '.let/.also/?:/e.t.c' instead of ${condition.text}") { - // todo implement fixer + fixNullInIfCondition(node, condition, true) } // `!==` and `!==` comparison can be fixed with `.let/also` operators ElementType.EXCLEQ, ElementType.EXCLEQEQEQ -> warnAndFixOnNullCheck(condition, true, "use '.let/.also/?:/e.t.c' instead of ${condition.text}") { - // todo implement fixer + fixNullInIfCondition(node, condition, false) } else -> return } @@ -86,7 +89,46 @@ class NullChecksRule(private val configRules: List) : Rule("null-ch } } - @Suppress("COMMENT_WHITE_SPACE") + @Suppress("UnsafeCallOnNullableType") + private fun fixNullInIfCondition(condition: ASTNode, + binaryExpression: KtBinaryExpression, + isEqualToNull: Boolean) { + val variableName = binaryExpression.left!!.text + val thenCodeLines = condition.extractLinesFromBlock(THEN) + val elseCodeLines = condition.extractLinesFromBlock(ELSE) + val text = if (isEqualToNull) { + if (elseCodeLines.isNullOrEmpty()) { + "$variableName ?: run {\n${thenCodeLines?.joinToString(separator = "\n")}\n}" + } else { + """ + |$variableName?.let { + |${elseCodeLines.joinToString(separator = "\n")} + |} + |?: run { + |${thenCodeLines?.joinToString(separator = "\n")} + |} + """.trimMargin() + } + } else { + if (elseCodeLines.isNullOrEmpty()) { + "$variableName?.let {\n${thenCodeLines?.joinToString(separator = "\n")}\n}" + } else { + """ + |$variableName?.let { + |${thenCodeLines?.joinToString(separator = "\n")} + |} + |?: run { + |${elseCodeLines.joinToString(separator = "\n")} + |} + """.trimMargin() + } + } + val tree = KotlinParser().createNode(text) + condition.treeParent.treeParent.addChild(tree, condition.treeParent) + condition.treeParent.treeParent.removeChild(condition.treeParent) + } + + @Suppress("COMMENT_WHITE_SPACE", "UnsafeCallOnNullableType") private fun nullCheckInOtherStatements(binaryExprNode: ASTNode) { val condition = (binaryExprNode.psi as KtBinaryExpression) if (isNullCheckBinaryExpression(condition)) { @@ -104,10 +146,13 @@ class NullChecksRule(private val configRules: List) : Rule("null-ch if (listOf(ElementType.EXCLEQ, ElementType.EXCLEQEQEQ).contains(condition.operationToken)) { warnAndFixOnNullCheck( condition, - false, + true, "use 'requireNotNull' instead of require(${condition.text})" ) { - // todo implement fixer + val variableName = (binaryExprNode.psi as KtBinaryExpression).left!!.text + val newMethod = KotlinParser().createNode("requireNotNull($variableName)") + grandParent.treeParent.treeParent.addChild(newMethod, grandParent.treeParent) + grandParent.treeParent.treeParent.removeChild(grandParent.treeParent) } } } @@ -115,6 +160,17 @@ class NullChecksRule(private val configRules: List) : Rule("null-ch } } + @Suppress("WRONG_INDENTATION") + private fun ASTNode.extractLinesFromBlock(type: IElementType): List? = + treeParent + .getFirstChildWithType(type) + ?.text + ?.trim('{', '}') + ?.split("\n") + ?.filter { it.isNotBlank() } + ?.map { it.trim() } + ?.toList() + @Suppress("UnsafeCallOnNullableType") private fun isNullCheckBinaryExpression(condition: KtBinaryExpression): Boolean = // check that binary expression has `null` as right or left operand diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NullChecksRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NullChecksRuleFixTest.kt new file mode 100644 index 0000000000..6e8fccd617 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NullChecksRuleFixTest.kt @@ -0,0 +1,22 @@ +package org.cqfn.diktat.ruleset.chapter4 + +import org.cqfn.diktat.ruleset.rules.NullChecksRule +import org.cqfn.diktat.util.FixTestBase + +import generated.WarningNames +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class NullChecksRuleFixTest : FixTestBase("test/paragraph4/null_checks", ::NullChecksRule) { + @Test + @Tag(WarningNames.AVOID_NULL_CHECKS) + fun `should fix if conditions`() { + fixAndCompare("IfConditionNullCheckExpected.kt", "IfConditionNullCheckTest.kt") + } + + @Test + @Tag(WarningNames.AVOID_NULL_CHECKS) + fun `should fix require function`() { + fixAndCompare("RequireFunctionExpected.kt", "RequireFunctionTest.kt") + } +} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NullChecksRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NullChecksRuleWarnTest.kt index fe322bd2a8..4aab9afc8a 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NullChecksRuleWarnTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter4/NullChecksRuleWarnTest.kt @@ -41,6 +41,9 @@ class NullChecksRuleWarnTest : LintTestBase(::NullChecksRule) { | println("null") | return | } + | myVar ?: kotlin.run { + | println("null") + | } | } """.trimMargin(), LintError(3, 11, ruleId, Warnings.AVOID_NULL_CHECKS.warnText() + @@ -183,7 +186,7 @@ class NullChecksRuleWarnTest : LintTestBase(::NullChecksRule) { | } """.trimMargin(), LintError(2, 14, ruleId, Warnings.AVOID_NULL_CHECKS.warnText() + - " use 'requireNotNull' instead of require(myVar != null)", false), + " use 'requireNotNull' instead of require(myVar != null)", true), ) } } diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt index e99af228b7..6b8cf166bf 100644 --- a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/smoke/DiktatSmokeTest.kt @@ -91,6 +91,12 @@ class DiktatSmokeTest : FixTestBase("test/smoke/src/main/kotlin", fixAndCompare("DefaultPackageExpected.kt", "DefaultPackageTest.kt") } + @Test + @Tag("DiktatRuleSetProvider") + fun `smoke test #7`() { + fixAndCompare("Example7Expected.kt", "Example7Test.kt") + } + @Test @Tag("DiktatRuleSetProvider") fun `smoke test #6`() { diff --git a/diktat-rules/src/test/resources/test/paragraph4/null_checks/IfConditionNullCheckExpected.kt b/diktat-rules/src/test/resources/test/paragraph4/null_checks/IfConditionNullCheckExpected.kt new file mode 100644 index 0000000000..e994196c76 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph4/null_checks/IfConditionNullCheckExpected.kt @@ -0,0 +1,33 @@ +package test.paragraph4.null_checks + +fun test() { + val some: Int? = null + some ?: run { +println("some") +bar() +} + + some?.let { +println("some") +bar() +} + + if (some == null && true) { + print("asd") + } + + some?.let { +print("qwe") +} +?: run { +print("asd") +} + + some?.let { +print("qqq") +} +?: run { +print("www") +} +} + diff --git a/diktat-rules/src/test/resources/test/paragraph4/null_checks/IfConditionNullCheckTest.kt b/diktat-rules/src/test/resources/test/paragraph4/null_checks/IfConditionNullCheckTest.kt new file mode 100644 index 0000000000..5e7d750744 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph4/null_checks/IfConditionNullCheckTest.kt @@ -0,0 +1,31 @@ +package test.paragraph4.null_checks + +fun test() { + val some: Int? = null + if (some == null) { + println("some") + bar() + } + + if (some != null) { + println("some") + bar() + } + + if (some == null && true) { + print("asd") + } + + if (some == null) { + print("asd") + } else { + print("qwe") + } + + if (some != null) { + print("qqq") + } else { + print("www") + } +} + diff --git a/diktat-rules/src/test/resources/test/paragraph4/null_checks/RequireFunctionExpected.kt b/diktat-rules/src/test/resources/test/paragraph4/null_checks/RequireFunctionExpected.kt new file mode 100644 index 0000000000..d6e86de574 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph4/null_checks/RequireFunctionExpected.kt @@ -0,0 +1,8 @@ +package test.paragraph4.null_checks + +fun test() { + val some: Int? = null + + requireNotNull(some) +} + diff --git a/diktat-rules/src/test/resources/test/paragraph4/null_checks/RequireFunctionTest.kt b/diktat-rules/src/test/resources/test/paragraph4/null_checks/RequireFunctionTest.kt new file mode 100644 index 0000000000..3391a1ccf5 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph4/null_checks/RequireFunctionTest.kt @@ -0,0 +1,8 @@ +package test.paragraph4.null_checks + +fun test() { + val some: Int? = null + + require(some != null) +} + diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example7Expected.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example7Expected.kt new file mode 100644 index 0000000000..9d2b5deebc --- /dev/null +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example7Expected.kt @@ -0,0 +1,23 @@ +package org.cqfn.diktat + +fun foo() { + val prop: Int = 0 + + prop ?: run { + println("prop is null") + bar() + } + + prop?.let { + baz() + gaz() + } + + prop?.let { + doAnotherSmth() + } + ?: run { + doSmth() + } +} + diff --git a/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example7Test.kt b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example7Test.kt new file mode 100644 index 0000000000..cd2f5d98df --- /dev/null +++ b/diktat-rules/src/test/resources/test/smoke/src/main/kotlin/Example7Test.kt @@ -0,0 +1,22 @@ +package org.cqfn.diktat + +fun foo() { + val prop: Int? = null + + if (prop == null) { + println("prop is null") + bar() + } + + if (prop != null) { + baz() + gaz() + } + + if (prop == null) { + doSmth() + } else { + doAnotherSmth() + } +} +