diff --git a/diktat-analysis.yml b/diktat-analysis.yml index ddab5fee1c..fd509c66d0 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -502,4 +502,7 @@ enabled: true # Check if kts script contains other functions except run code - name: RUN_IN_SCRIPT + enabled: true +# Check if boolean expression can be simplified +- name: COMPLEX_BOOLEAN_EXPRESSION enabled: true \ No newline at end of file diff --git a/diktat-rules/pom.xml b/diktat-rules/pom.xml index 9a452e1fa7..dc3462d6b4 100644 --- a/diktat-rules/pom.xml +++ b/diktat-rules/pom.xml @@ -73,6 +73,11 @@ mockito-all test + + + com.bpodgursky + jbool_expressions + diff --git a/diktat-rules/src/main/kotlin/generated/WarningNames.kt b/diktat-rules/src/main/kotlin/generated/WarningNames.kt index 762f6198b2..77791e1ea6 100644 --- a/diktat-rules/src/main/kotlin/generated/WarningNames.kt +++ b/diktat-rules/src/main/kotlin/generated/WarningNames.kt @@ -155,6 +155,8 @@ public object WarningNames { public const val COMPLEX_EXPRESSION: String = "COMPLEX_EXPRESSION" + public const val COMPLEX_BOOLEAN_EXPRESSION: String = "COMPLEX_BOOLEAN_EXPRESSION" + public const val STRING_CONCATENATION: String = "STRING_CONCATENATION" public const val TOO_MANY_BLANK_LINES: String = "TOO_MANY_BLANK_LINES" diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index 44bbc518fb..a447d06d27 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -114,6 +114,7 @@ enum class Warnings( WRONG_NEWLINES(true, "3.6.2", "incorrect line breaking"), TRAILING_COMMA(true, "3.6.2", "use trailing comma"), COMPLEX_EXPRESSION(false, "3.6.3", "complex dot qualified expression should be replaced with variable"), + COMPLEX_BOOLEAN_EXPRESSION(true, "3.6.4", "too complex boolean expression, that can be simplified"), // FixMe: autofixing will be added for this rule STRING_CONCATENATION(true, "3.15.1", "strings should not be concatenated using plus operator - use string templates instead if the statement fits one line"), diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt index 9322a7e9fe..36f649a277 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt @@ -16,6 +16,7 @@ import org.cqfn.diktat.ruleset.rules.chapter2.kdoc.KdocFormatting import org.cqfn.diktat.ruleset.rules.chapter2.kdoc.KdocMethods import org.cqfn.diktat.ruleset.rules.chapter3.AnnotationNewLineRule import org.cqfn.diktat.ruleset.rules.chapter3.BlockStructureBraces +import org.cqfn.diktat.ruleset.rules.chapter3.BooleanExpressionsRule import org.cqfn.diktat.ruleset.rules.chapter3.BracesInConditionalsAndLoopsRule import org.cqfn.diktat.ruleset.rules.chapter3.ClassLikeStructuresOrderRule import org.cqfn.diktat.ruleset.rules.chapter3.CollapseIfStatementsRule @@ -208,6 +209,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS ::AvoidNestedFunctionsRule, ::ExtensionFunctionsSameNameRule, ::LambdaLengthRule, + ::BooleanExpressionsRule, // formatting: moving blocks, adding line breaks, indentations etc. ::BlockStructureBraces, ::ConsecutiveSpacesRule, diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/BooleanExpressionsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/BooleanExpressionsRule.kt new file mode 100644 index 0000000000..069d1d17bf --- /dev/null +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter3/BooleanExpressionsRule.kt @@ -0,0 +1,203 @@ +package org.cqfn.diktat.ruleset.rules.chapter3 + +import org.cqfn.diktat.common.config.rules.RulesConfig +import org.cqfn.diktat.ruleset.constants.Warnings.COMPLEX_BOOLEAN_EXPRESSION +import org.cqfn.diktat.ruleset.rules.DiktatRule +import org.cqfn.diktat.ruleset.utils.KotlinParser +import org.cqfn.diktat.ruleset.utils.findAllNodesWithCondition +import org.cqfn.diktat.ruleset.utils.findLeafWithSpecificType + +import com.bpodgursky.jbool_expressions.Expression +import com.bpodgursky.jbool_expressions.parsers.ExprParser +import com.bpodgursky.jbool_expressions.rules.RuleSet +import com.pinterest.ktlint.core.ast.ElementType +import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION +import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE +import com.pinterest.ktlint.core.ast.ElementType.PARENTHESIZED +import org.jetbrains.kotlin.com.intellij.lang.ASTNode + +import java.lang.RuntimeException + +/** + * Rule that warns if the boolean expression can be simplified. + */ +class BooleanExpressionsRule(configRules: List) : DiktatRule( + "boolean-expressions-rule", + configRules, + listOf(COMPLEX_BOOLEAN_EXPRESSION)) { + override fun logic(node: ASTNode) { + if (node.elementType == ElementType.CONDITION) { + checkBooleanExpression(node) + } + } + + @Suppress("TooGenericExceptionCaught") + private fun checkBooleanExpression(node: ASTNode) { + // This map is used to assign a variable name for every elementary boolean expression. + val mapOfExpressionToChar: HashMap = HashMap() + val correctedExpression = formatBooleanExpressionAsString(node, mapOfExpressionToChar) + // If there are method calls in conditions + val expr: Expression = try { + ExprParser.parse(correctedExpression) + } catch (exc: RuntimeException) { + if (exc.message?.startsWith("Unrecognized!") == true) { + // this comes up if there is an unparsable expression. For example a.and(b) + return + } else { + throw exc + } + } + val distributiveLawString = checkDistributiveLaw(expr, mapOfExpressionToChar, node) + val simplifiedExpression = distributiveLawString?.let { + ExprParser.parse(distributiveLawString) + } + ?: RuleSet.simplify(expr) + if (expr != simplifiedExpression) { + COMPLEX_BOOLEAN_EXPRESSION.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) { + fixBooleanExpression(node, simplifiedExpression, mapOfExpressionToChar) + } + } + } + + /** + * @param node + * @param mapOfExpressionToChar + * @return corrected string + */ + internal fun formatBooleanExpressionAsString(node: ASTNode, mapOfExpressionToChar: HashMap): String { + // `A` character in ASCII + var characterAsciiCode = 'A'.code + node + .findAllNodesWithCondition({ it.elementType == BINARY_EXPRESSION }) + .filterNot { it.text.contains("&&") || it.text.contains("||") } + .forEach { expression -> + mapOfExpressionToChar.computeIfAbsent(expression.text) { + characterAsciiCode++.toChar() + } + } + // Library is using & as && and | as ||. + var correctedExpression = "(${node + .text + .replace("&&", "&") + .replace("||", "|")})" + mapOfExpressionToChar.forEach { (refExpr, char) -> + correctedExpression = correctedExpression.replace(refExpr, char.toString()) + } + return correctedExpression + } + + private fun fixBooleanExpression( + node: ASTNode, + simplifiedExpr: Expression, + mapOfExpressionToChar: HashMap) { + var correctKotlinBooleanExpression = simplifiedExpr + .toString() + .replace("&", "&&") + .replace("|", "||") + .drop(1) // dropping first ( + .dropLast(1) // dropping last ) + mapOfExpressionToChar.forEach { (key, value) -> + correctKotlinBooleanExpression = correctKotlinBooleanExpression.replace(value.toString(), key) + } + node.replaceChild(node.firstChildNode, KotlinParser().createNode(correctKotlinBooleanExpression)) + } + + /** + * Checks if boolean expression can be simplified with distributive law. + * + * @return String? null if it cannot be simplified. Simplified string otherwise. + */ + private fun checkDistributiveLaw( + expr: Expression, + mapOfExpressionToChar: HashMap, + node: ASTNode): String? { + // checking that expression can be considered as distributive law + val commonDistributiveOperand = getCommonDistributiveOperand(node, expr.toString(), mapOfExpressionToChar) ?: return null + val correctSymbolsSequence = mapOfExpressionToChar.values.toMutableList() + correctSymbolsSequence.remove(commonDistributiveOperand) + correctSymbolsSequence.add(0, commonDistributiveOperand) + val expressionsLogicalOperator = expr.toString().first { it == '&' || it == '|' } + // we return expression depending on second operator + return returnNeededDistributiveExpression(expressionsLogicalOperator, correctSymbolsSequence) + } + + /** + * Returns correct result string in distributive law + */ + private fun returnNeededDistributiveExpression(firstLogicalOperator: Char, symbols: List): String { + val secondSymbol = if (firstLogicalOperator == '&') '|' else '&' // this is used to alter symbols + val resultString = StringBuilder() + symbols.forEachIndexed { index, symbol -> + if (index == 0) { + resultString.append("$symbol $firstLogicalOperator (") + } else { + resultString.append("$symbol $secondSymbol ") + } + } + // remove last space and last operate + return StringBuilder(resultString.dropLast(2)).append(")").toString() + } + + /** + * Method that checks that the expression can be simplified by distributive law. + * Distributive law - A && B || A && C -> A && (B || C) or (A || B) && (A || C) -> A || (B && C) + * + * @return common operand for distributed law + */ + private fun getCommonDistributiveOperand( + node: ASTNode, + expression: String, + mapOfExpressionToChar: HashMap): Char? { + val operationSequence = expression.filter { it == '&' || it == '|' } + val numberOfOperationReferences = operationSequence.length + // There should be three operands and three operation references in order to consider the expression + // Moreover the operation references between operands should alternate. + if (mapOfExpressionToChar.size < DISTRIBUTIVE_LAW_MIN_EXPRESSIONS || + numberOfOperationReferences < DISTRIBUTIVE_LAW_MIN_OPERATIONS || + !isSequenceAlternate(operationSequence)) { + return null + } + return if (operationSequence.first() == '&') { + getCommonOperand(expression, '|', '&') + } else { + // this is done for excluding A || B && A || C without parenthesis. + val parenthesizedExpressions = node.findAllNodesWithCondition({ it.elementType == PARENTHESIZED }) + parenthesizedExpressions.forEach { + it.findLeafWithSpecificType(OPERATION_REFERENCE) ?: run { + return null + } + } + getCommonOperand(expression, '&', '|') + } + } + + private fun isSequenceAlternate(seq: String) = seq.zipWithNext().all { it.first != it.second } + + /** + * This method returns common operand in distributive law. + * We need common operand for special case, when the first expression is not common. + * For example: (some != null && a) || (a && c) || (a && d). When the expressions are mapped to `char`s, `some != null` points to `A` character + */ + private fun getCommonOperand( + expression: String, + firstSplitDelimiter: Char, + secondSplitDelimiter: Char): Char? { + val expressions = expression.split(firstSplitDelimiter) + val listOfPairs: MutableList> = mutableListOf() + expressions.forEach { expr -> + listOfPairs.add(expr.filterNot { it == ' ' || it == '(' || it == ')' }.split(secondSplitDelimiter)) + } + val firstOperands = listOfPairs.first() + listOfPairs.removeFirst() + return when { + listOfPairs.all { it.contains(firstOperands.first()) } -> firstOperands.first().first() + listOfPairs.all { it.contains(firstOperands.last()) } -> firstOperands.last().first() + else -> null + } + } + + companion object { + const val DISTRIBUTIVE_LAW_MIN_EXPRESSIONS = 3 + const val DISTRIBUTIVE_LAW_MIN_OPERATIONS = 3 + } +} diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt index d9f77e422c..9511f73af2 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/utils/AstNodeUtils.kt @@ -33,7 +33,6 @@ import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.OVERRIDE_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.PRIVATE_KEYWORD -import com.pinterest.ktlint.core.ast.ElementType.PROPERTY import com.pinterest.ktlint.core.ast.ElementType.PROTECTED_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.PUBLIC_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml index 17583be793..4d85d15adf 100644 --- a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml +++ b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml @@ -505,4 +505,7 @@ enabled: true # Check if kts script contains other functions except run code - name: RUN_IN_SCRIPT + enabled: true +# Check if boolean expression can be simplified +- name: COMPLEX_BOOLEAN_EXPRESSION enabled: true \ No newline at end of file diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml index d53410af99..8e7e65a3f4 100644 --- a/diktat-rules/src/main/resources/diktat-analysis.yml +++ b/diktat-rules/src/main/resources/diktat-analysis.yml @@ -499,4 +499,7 @@ enabled: true # Check if kts script contains other functions except run code - name: RUN_IN_SCRIPT + enabled: true +# Check if boolean expression can be simplified +- name: COMPLEX_BOOLEAN_EXPRESSION enabled: true \ No newline at end of file diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/BooleanExpressionsRuleFixTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/BooleanExpressionsRuleFixTest.kt new file mode 100644 index 0000000000..c7d2357d87 --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/BooleanExpressionsRuleFixTest.kt @@ -0,0 +1,22 @@ +package org.cqfn.diktat.ruleset.chapter3 + +import org.cqfn.diktat.ruleset.rules.chapter3.BooleanExpressionsRule +import org.cqfn.diktat.util.FixTestBase + +import generated.WarningNames +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class BooleanExpressionsRuleFixTest : FixTestBase("test/paragraph3/boolean_expressions", ::BooleanExpressionsRule) { + @Test + @Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION) + fun fixBooleanExpressions() { + fixAndCompare("BooleanExpressionsExpected.kt", "BooleanExpressionsTest.kt") + } + + @Test + @Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION) + fun `check distributive law fixing`() { + fixAndCompare("DistributiveLawExpected.kt", "DistributiveLawTest.kt") + } +} diff --git a/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/BooleanExpressionsRuleWarnTest.kt b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/BooleanExpressionsRuleWarnTest.kt new file mode 100644 index 0000000000..f2eb00cb9b --- /dev/null +++ b/diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter3/BooleanExpressionsRuleWarnTest.kt @@ -0,0 +1,170 @@ +package org.cqfn.diktat.ruleset.chapter3 + +import org.cqfn.diktat.ruleset.constants.Warnings +import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID +import org.cqfn.diktat.ruleset.rules.chapter3.BooleanExpressionsRule +import org.cqfn.diktat.ruleset.utils.KotlinParser +import org.cqfn.diktat.util.LintTestBase + +import com.pinterest.ktlint.core.LintError +import generated.WarningNames +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Tag +import org.junit.jupiter.api.Test + +class BooleanExpressionsRuleWarnTest : LintTestBase(::BooleanExpressionsRule) { + private val ruleId = "$DIKTAT_RULE_SET_ID:boolean-expressions-rule" + + @Test + @Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION) + fun `check boolean expression`() { + lintMethod( + """ + |fun foo() { + | if (some != null && some != null && some == null) { + | goo() + | } + | + | if (some != null && some == 7) { + | + | } + | + | if (a > 3 && b > 3 && a > 3) { + | + | } + | + | if (a && a && b > 4) { + | + | } + |} + """.trimMargin(), + LintError(2, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} some != null && some != null && some == null", true), + LintError(10, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} a > 3 && b > 3 && a > 3", true), + LintError(14, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} a && a && b > 4", true) + ) + } + + @Test + @Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION) + fun `check laws#1`() { + lintMethod( + """ + |fun foo() { + | if (some != null && (some != null || a > 5)) { + | + | } + | + | if (a > 5 || (a > 5 && b > 6)) { + | + | } + | + | if (!!(a > 5 && q > 6)) { + | + | } + | + | if (a > 5 && false) { + | + | } + | + | if (a > 5 || (!(a > 5) && b > 5)) { + | + | } + |} + """.trimMargin(), + LintError(2, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} some != null && (some != null || a > 5)", true), + LintError(6, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} a > 5 || (a > 5 && b > 6)", true), + LintError(10, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} !!(a > 5 && q > 6)", true), + LintError(14, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} a > 5 && false", true), + LintError(18, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} a > 5 || (!(a > 5) && b > 5)", true) + ) + } + + @Test + @Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION) + fun `check distributive laws`() { + lintMethod( + """ + |fun foo() { + | if ((a > 5 || b > 5) && (a > 5 || c > 5)) { + | + | } + | + | if (a > 5 && b > 5 || a > 5 && c > 5) { + | + | } + |} + """.trimMargin(), + LintError(2, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} (a > 5 || b > 5) && (a > 5 || c > 5)", true), + LintError(6, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} a > 5 && b > 5 || a > 5 && c > 5", true) + ) + } + + @Test + @Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION) + fun `check distributive laws #2`() { + lintMethod( + """ + |fun foo() { + | if ((a > 5 || b > 5) && (a > 5 || c > 5) && (a > 5 || d > 5)) { + | + | } + | + | if (a > 5 && b > 5 || a > 5 && c > 5 || a > 5 || d > 5) { + | + | } + |} + """.trimMargin(), + LintError(2, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} (a > 5 || b > 5) && (a > 5 || c > 5) && (a > 5 || d > 5)", true), + LintError(6, 9, ruleId, "${Warnings.COMPLEX_BOOLEAN_EXPRESSION.warnText()} a > 5 && b > 5 || a > 5 && c > 5 || a > 5 || d > 5", true) + ) + } + + @Test + @Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION) + fun `should not trigger on method calls`() { + lintMethod( + """ + |fun foo() { + | if (a.and(b)) { + | + | } + |} + """.trimMargin() + ) + } + + @Test + fun `test makeCorrectExpressionString method #1`() { + val firstCondition = KotlinParser().createNode("a > 5 && b < 6") + val returnedString = BooleanExpressionsRule(emptyList()).formatBooleanExpressionAsString(firstCondition, HashMap()) + Assertions.assertEquals(returnedString, "(A & B)") + } + + @Test + fun `test makeCorrectExpressionString method #2`() { + val firstCondition = KotlinParser().createNode("a > 5 && b < 6 && c > 7 || a > 5") + val returnedString = BooleanExpressionsRule(emptyList()).formatBooleanExpressionAsString(firstCondition, HashMap()) + Assertions.assertEquals(returnedString, "(A & B & C | A)") + } + + @Test + fun `test makeCorrectExpressionString method #3`() { + val firstCondition = KotlinParser().createNode("a > 5 && b < 6 && (c > 3 || b < 6) && a > 5") + val returnedString = BooleanExpressionsRule(emptyList()).formatBooleanExpressionAsString(firstCondition, HashMap()) + Assertions.assertEquals(returnedString, "(A & B & (C | B) & A)") + } + + @Test + fun `test makeCorrectExpressionString method #4`() { + val firstCondition = KotlinParser().createNode("a > 5 && b < 6 && (c > 3 || b < 6) && a > 666") + val returnedString = BooleanExpressionsRule(emptyList()).formatBooleanExpressionAsString(firstCondition, HashMap()) + Assertions.assertEquals(returnedString, "(A & B & (C | B) & D)") + } + + @Test + fun `test makeCorrectExpressionString method #5`() { + val firstCondition = KotlinParser().createNode("a.and(b)") + val returnedString = BooleanExpressionsRule(emptyList()).formatBooleanExpressionAsString(firstCondition, HashMap()) + Assertions.assertEquals(returnedString, "(a.and(b))") + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/BooleanExpressionsExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/BooleanExpressionsExpected.kt new file mode 100644 index 0000000000..b0f8b12e5e --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/BooleanExpressionsExpected.kt @@ -0,0 +1,15 @@ +package test.paragraph3.boolean_expressions + +fun foo() { + if (some != null && some == 6) { + + } + + if ((some != null || c > 2) && (a > 4 || b > 3)) { + + } + + if (a > 3 && b > 3) { + + } +} \ No newline at end of file diff --git a/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/BooleanExpressionsTest.kt b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/BooleanExpressionsTest.kt new file mode 100644 index 0000000000..3b92634694 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/BooleanExpressionsTest.kt @@ -0,0 +1,15 @@ +package test.paragraph3.boolean_expressions + +fun foo() { + if (some != null && some != null && some == 6) { + + } + + if ((some != null || c > 2) && (a > 4 || b > 3)) { + + } + + if (a > 3 && b > 3 && a > 3) { + + } +} \ No newline at end of file diff --git a/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawExpected.kt b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawExpected.kt new file mode 100644 index 0000000000..37244e76d6 --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawExpected.kt @@ -0,0 +1,24 @@ +package test.paragraph3.boolean_expressions + +fun some() { + if (a > 5 && (b > 6 || c > 7)) { + + } + + if (a > 5 || (b > 6 && c > 7)) { + + } + + if (a > 5 || (b > 6 && c > 7 && d > 8)) { + + } + + if (a > 5 && (b > 6 || c > 7 || d > 8)) { + + } + + // Special case + if (a > 5 && (b > 6 || c > 7 || d > 8)) { + + } +} diff --git a/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawTest.kt b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawTest.kt new file mode 100644 index 0000000000..ce07c8e3cf --- /dev/null +++ b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawTest.kt @@ -0,0 +1,24 @@ +package test.paragraph3.boolean_expressions + +fun some() { + if (a > 5 && b > 6 || c > 7 && a > 5) { + + } + + if ((a > 5 || b > 6) && (c > 7 || a > 5)) { + + } + + if ((a > 5 || b > 6) && (c > 7 || a > 5) && (a > 5 || d > 8)) { + + } + + if ((a > 5 && b > 6) || (c > 7 && a > 5) || (a > 5 && d > 8)) { + + } + + // Special case + if ((b > 6 && a > 5) || (c > 7 && a > 5) || (a > 5 && d > 8)) { + + } +} diff --git a/info/available-rules.md b/info/available-rules.md index a218bcadf5..66a1411355 100644 --- a/info/available-rules.md +++ b/info/available-rules.md @@ -87,6 +87,7 @@ | 3 | 3.15.2 | STRING_TEMPLATE_CURLY_BRACES | Check: warns if there is redundant curly braces in string template
Fix: deletes curly braces | yes | no | + | | 3 | 3.15.2 | STRING_TEMPLATE_QUOTES | Check: warns if there are redundant quotes in string template
Fix: deletes quotes and $ symbol | yes | no | + | | 3 | 3.16.1 | COLLAPSE_IF_STATEMENTS | Check: warns if there are redundant nested if-statements, which could be collapsed into a single one by concatenating their conditions | yes | no | - | +| 3 | 3.16.2 | COMPLEX_BOOLEAN_EXPRESSION | Check: warns if boolean expression is complex and can be simplified.
Fix: replaces boolean expression with the simpler one | yes | no | + | | 4 | 4.2.1 | SMART_CAST_NEEDED | Check: warns if casting can be omitted
Fix: Deletes casting | yes | no | - | | | 4 | 4.3.1 | NULLABLE_PROPERTY_TYPE | Check: warns if immutable property is initialized with null or if immutable property can have non-nullable type instead of nullable
Fix: suggests initial value instead of null or changes immutable property type | yes | no | - | | 4 | 4.2.2 | TYPE_ALIAS | Check: if type reference of property is longer than expected | yes | typeReferenceLength | -| diff --git a/info/guide/guide-TOC.md b/info/guide/guide-TOC.md index a7932210ec..3926539583 100644 --- a/info/guide/guide-TOC.md +++ b/info/guide/guide-TOC.md @@ -65,6 +65,9 @@ I [Preface](#c0) * [3.15 Strings](#c3.15) * [3.15.1 Concatenation of Strings](#r3.15.1) * [3.15.2 String template format](#r3.15.2) + * [3.16 Conditional statements](#c3.16) + * [3.16.1 Collapsing redundant nested if-statements](#r3.16.1) + * [3.16.2 Too complex conditions](#r3.16.2) [4. Variables and types](#c4) * [4.1 Variables](#c4.1) @@ -114,4 +117,6 @@ I [Preface](#c0) * [6.3 Interfaces](#c6.3) * [6.4 Objects](#c6.4) * [6.4.1 Instead of using utility classes/objects, use extensions](#r6.4.1) - * [6.4.2 Objects should be used for Stateless Interfaces](#r6.4.2) + * [6.4.2 Objects should be used for Stateless Interfaces](#r6.4.2) +* [6.5 Kts Files](#c6.5) + * [6.5.1 kts files should wrap logic into top-level scope](#r6.5.1) diff --git a/info/guide/guide-chapter-3.md b/info/guide/guide-chapter-3.md index ed471365d3..95099f8152 100644 --- a/info/guide/guide-chapter-3.md +++ b/info/guide/guide-chapter-3.md @@ -932,3 +932,19 @@ if (cond1 && (cond2 || cond3)) { doSomething() } ``` +#### 3.16.2 Too complex conditions +Too complex conditions should be simplified according to boolean algebra rules, if it is possible + +**Valid example** +```kotlin +if (cond1 && cond2) { + foo() +} +``` + +**Invalid example** +```kotlin +if (cond1 && cond2 && cond1) { + foo() +} +``` \ No newline at end of file diff --git a/info/guide/guide-chapter-6.md b/info/guide/guide-chapter-6.md index 397cc506ae..7a4b2312ce 100644 --- a/info/guide/guide-chapter-6.md +++ b/info/guide/guide-chapter-6.md @@ -468,7 +468,8 @@ object O: I { override fun foo() {} } ``` - +### 6.5 Kts Files +This section describes general rules for `.kts` files #### 6.5.1 kts files should wrap logic into top-level scope It is still recommended wrapping logic inside functions and avoid using top-level statements for function calls or wrapping blocks of code in top-level scope functions like `run`. diff --git a/pom.xml b/pom.xml index ad16e1d224..27418a758b 100644 --- a/pom.xml +++ b/pom.xml @@ -56,6 +56,7 @@ 1.4.32 0.8.7 3.6.1 + 1.23 @@ -158,6 +159,12 @@ 1.10.19 test + + + com.bpodgursky + jbool_expressions + ${jbool.version} +