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 index dd69de36fe..35ae0d2ccb 100644 --- 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 @@ -5,17 +5,20 @@ 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 org.cqfn.diktat.ruleset.utils.logicalInfixMethods - +import com.bpodgursky.jbool_expressions.And import com.bpodgursky.jbool_expressions.Expression +import com.bpodgursky.jbool_expressions.NExpression +import com.bpodgursky.jbool_expressions.Or import com.bpodgursky.jbool_expressions.options.ExprOptions import com.bpodgursky.jbool_expressions.parsers.ExprParser import com.bpodgursky.jbool_expressions.parsers.TokenMapper +import com.bpodgursky.jbool_expressions.rules.DeMorgan +import com.bpodgursky.jbool_expressions.rules.Rule +import com.bpodgursky.jbool_expressions.rules.RuleList import com.bpodgursky.jbool_expressions.rules.RulesHelper import com.pinterest.ktlint.core.ast.ElementType.BINARY_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.CONDITION -import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.PARENTHESIZED import com.pinterest.ktlint.core.ast.ElementType.PREFIX_EXPRESSION import com.pinterest.ktlint.core.ast.isLeaf @@ -26,7 +29,7 @@ import org.jetbrains.kotlin.psi.KtParenthesizedExpression import org.jetbrains.kotlin.psi.KtPrefixExpression import org.jetbrains.kotlin.psi.psiUtil.parents -import java.lang.RuntimeException +typealias ExpressionCreator = (List?>) -> Expression /** * Rule that checks if the boolean expression can be simplified. @@ -63,11 +66,7 @@ class BooleanExpressionsRule(configRules: List) : DiktatRule( throw exc } } - val distributiveLawString = checkDistributiveLaw(expr, expressionsReplacement, node) - val simplifiedExpression = distributiveLawString?.let { - ExprParser.parse(distributiveLawString) - } - ?: RulesHelper.applySet(expr, RulesHelper.demorganRules(), ExprOptions.noCaching()) + val simplifiedExpression = RulesHelper.applySet(expr, allRules(), ExprOptions.noCaching()) if (expr != simplifiedExpression) { COMPLEX_BOOLEAN_EXPRESSION.warnAndFix(configRules, emitWarn, isFixMode, node.text, node.startOffset, node) { fixBooleanExpression(node, simplifiedExpression, expressionsReplacement) @@ -174,103 +173,6 @@ class BooleanExpressionsRule(configRules: List) : DiktatRule( KotlinParser().createNode(expressionsReplacement.restoreFullExpression(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, - expressionsReplacement: ExpressionsReplacement, - node: ASTNode - ): String? { - // checking that expression can be considered as distributive law - val commonDistributiveOperand = getCommonDistributiveOperand(node, expr.toString(), expressionsReplacement)?.toString() ?: return null - val correctSymbolsSequence = expressionsReplacement.getTokens().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, - expressionsReplacement: ExpressionsReplacement - ): 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 (expressionsReplacement.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 - } - } - private fun KtBinaryExpression.isXorExpression() = operationReference.text == "xor" /** @@ -350,22 +252,91 @@ class BooleanExpressionsRule(configRules: List) : DiktatRule( return resultExpression } - /** - * Returns collection of token are used to construct full expression in jbool format. - * - * @return collection of token are used to construct full expression in jbool format - */ - fun getTokens(): Collection = expressionToToken.values - private fun getLetter(letters: HashMap, key: String) = letters .computeIfAbsent(key) { ('A'.code + letters.size).toChar().toString() } } + /** + * Rule 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) + */ + @Suppress("UnsafeCallOnNullableType") + private class DistributiveLaw : Rule, K>() { + override fun applyInternal(input: NExpression, options: ExprOptions): Expression { + val exprFactory = options.exprFactory!! + val orExpressionCreator: ExpressionCreator = { expressions -> exprFactory.or(expressions.toTypedArray()) } + val andExpressionCreator: ExpressionCreator = { expressions -> exprFactory.and(expressions.toTypedArray()) } + return when (input) { + is And -> applyInternal(input, orExpressionCreator, andExpressionCreator) + is Or -> applyInternal(input, andExpressionCreator, orExpressionCreator) + else -> throw UnsupportedOperationException("Not supported input expression: ${input.exprType}") + } + } + + private fun applyInternal( + input: NExpression, + upperExpressionCreator: ExpressionCreator, + innerExpressionCreator: ExpressionCreator + ): Expression { + // we can be here only after `isApply` -- common exists + val commonExpression = findCommonExpression(input.children)!! + return upperExpressionCreator( + listOf(commonExpression, + innerExpressionCreator( + input.expressions.map { excludeChild(it, upperExpressionCreator, commonExpression) } + ))) + } + + private fun excludeChild( + expression: Expression, + expressionCreator: ExpressionCreator, + childToExclude: Expression + ): Expression { + val leftChildren = expression.children.filterNot { it.equals(childToExclude) } + return if (leftChildren.size == 1) { + leftChildren.first() + } else { + expressionCreator(leftChildren) + } + } + + /** + * Checks the input expression + */ + override fun isApply(inputNullable: Expression?): Boolean = inputNullable?.let { input -> + when (input) { + is And -> isApplicable, Or>(input) + is Or -> isApplicable, And>(input) + else -> false + } + } ?: false + + private inline fun , reified C : NExpression> isApplicable(input: E): Boolean { + val children = input.children ?: return false + if (children.size < 2 || children.any { it !is C }) { + return false + } + return findCommonExpression(children) != null + } + + private fun findCommonExpression(children: List>): Expression? = children.drop(1) + .fold(children[0].children) { commons, child -> + commons.filter { childResult -> + child.children.any { it.equals(childResult) } + } + }.firstOrNull() + } + companion object { - const val DISTRIBUTIVE_LAW_MIN_EXPRESSIONS = 3 - const val DISTRIBUTIVE_LAW_MIN_OPERATIONS = 3 - const val NAME_ID = "acm-boolean-expressions-rule" + const val NAME_ID = "boolean-expressions-rule" + + private fun allRules(): RuleList { + val rules: MutableList> = ArrayList(RulesHelper.simplifyRules().rules) + rules.add(DeMorgan()) + rules.add(DistributiveLaw()) + return RuleList(rules) + } } } 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 index 37244e76d6..297a8d8d58 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawExpected.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawExpected.kt @@ -21,4 +21,19 @@ fun some() { if (a > 5 && (b > 6 || c > 7 || d > 8)) { } + + // long case + if (a > 5 && ((b > 6 && z > 3) || (c > 7 && y > 4) || (d > 8 && w > 5))) { + + } + + // long case #2.1 + if (b > 6 && a > 5 && (z > 3 || c > 7 || w > 5)) { + + } + + // long case #2.2 + if (b > 6 || a > 5 || (z > 3 && c > 7 && w > 5)) { + + } } 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 index ce07c8e3cf..cf2a8e3343 100644 --- a/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawTest.kt +++ b/diktat-rules/src/test/resources/test/paragraph3/boolean_expressions/DistributiveLawTest.kt @@ -21,4 +21,19 @@ fun some() { if ((b > 6 && a > 5) || (c > 7 && a > 5) || (a > 5 && d > 8)) { } + + // long case + if ((b > 6 && a > 5 && z > 3) || (c > 7 && a > 5 && y > 4) || (a > 5 && d > 8 && w > 5)) { + + } + + // long case #2.1 + if ((b > 6 && a > 5 && z > 3) || (c > 7 && a > 5 && b > 6) || (a > 5 && b > 6 && w > 5)) { + + } + + // long case #2.2 + if ((b > 6 || a > 5 || z > 3) && (c > 7 || a > 5 || b > 6) && (a > 5 || b > 6 || w > 5)) { + + } }