From 7b68c23fda3addd3ca14877bd71d3e64274d2d3f Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Thu, 16 Jun 2022 16:44:22 +0300 Subject: [PATCH] Support DistributiveLaw as jbool.Rule ### What's done: - implemented DistributiveLaw extends com.bpodgursky.jbool_expressions.rules.Rule.Rule --- .../rules/chapter3/BooleanExpressionsRule.kt | 210 ++++++++---------- .../DistributiveLawExpected.kt | 5 + .../DistributiveLawTest.kt | 5 + 3 files changed, 106 insertions(+), 114 deletions(-) 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..03ca6614a6 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 @@ -7,11 +7,16 @@ 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 @@ -26,8 +31,6 @@ import org.jetbrains.kotlin.psi.KtParenthesizedExpression import org.jetbrains.kotlin.psi.KtPrefixExpression import org.jetbrains.kotlin.psi.psiUtil.parents -import java.lang.RuntimeException - /** * 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,102 @@ class BooleanExpressionsRule(configRules: List) : DiktatRule( return resultExpression } + 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) + * + */ + private class DistributiveLaw : Rule, K>() { + + private val andExpressionCreator: ExpressionCreator = { exprList -> + And.of(exprList.toList()) + } + + private val orExpressionCreator: ExpressionCreator = { exprList -> + Or.of(exprList.toList()) + } + + override fun applyInternal(input: NExpression?, options: ExprOptions?): Expression { + requireNotNull(input) { + "input expression is null" + } + 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 { + val commonExpression = requireNotNull(findCommonExpression(input.children)) { + "Common expression is not found for $input" + } + 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) + } + } + /** - * 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 + override fun isApply(inputNullable: Expression?): Boolean { + return inputNullable?.let { input -> + when (input) { + is And -> isApplicable, Or>(input) + is Or -> isApplicable, And>(input) + else -> false + } + } ?: false + } - private fun getLetter(letters: HashMap, key: String) = letters - .computeIfAbsent(key) { - ('A'.code + letters.size).toChar().toString() + private inline fun , reified C: NExpression> isApplicable(input: E): Boolean { + val children = input.children ?: return false + + if (children.any { it !is C }) { + return false } + return findCommonExpression(children) != null + } + + private fun findCommonExpression(children: List>): Expression? { + return 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) + } } } + +typealias ExpressionCreator = (List?>) -> Expression 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..2c43f5c8ec 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,9 @@ 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))) { + + } } 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..5538583549 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,9 @@ 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)) { + + } }