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 simplification of boolean expression #1523

Merged
merged 8 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -5,6 +5,7 @@ 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.logicalInfixMethodMapping
import org.cqfn.diktat.ruleset.utils.logicalInfixMethods
import com.bpodgursky.jbool_expressions.Expression
import com.bpodgursky.jbool_expressions.options.ExprOptions
Expand Down Expand Up @@ -177,6 +178,7 @@ class BooleanExpressionsRule(configRules: List<RulesConfig>) : DiktatRule(
*/
internal inner class ExpressionsReplacement {
private val expressionToToken: HashMap<String, String> = LinkedHashMap()
private val tokenToExpression: HashMap<String, String> = HashMap()
private val tokenToOrderedToken: HashMap<String, String> = HashMap()

/**
Expand Down Expand Up @@ -206,8 +208,14 @@ class BooleanExpressionsRule(configRules: List<RulesConfig>) : DiktatRule(
fun addExpression(expressionAstNode: ASTNode) {
val expressionText = expressionAstNode.textWithoutComments()
// support case when `boolean_expression` matches to `!boolean_expression`
val expression = if (expressionText.startsWith('!')) expressionText.substring(1) else expressionText
getLetter(expressionToToken, expression)
val (expression, negativeExpression) = if (expressionText.startsWith('!')) {
expressionText.substring(1) to expressionText
} else {
expressionText to getNegativeExpression(expressionAstNode, expressionText)
}
val letter = getLetter(expressionToToken, expression)
tokenToExpression["!$letter"] = negativeExpression
tokenToExpression[letter] = expression
}

/**
Expand All @@ -216,13 +224,12 @@ class BooleanExpressionsRule(configRules: List<RulesConfig>) : DiktatRule(
* @param fullExpression full boolean expression in kotlin
* @return full expression in jbool format
*/
@Suppress("UnsafeCallOnNullableType")
fun replaceExpressions(fullExpression: String): String {
var resultExpression = fullExpression
expressionToToken.keys
.sortedByDescending { it.length }
.forEach { refExpr ->
resultExpression = resultExpression.replace(refExpr, expressionToToken[refExpr]!!)
resultExpression = resultExpression.replace(refExpr, expressionToToken.getValue(refExpr))
}
return resultExpression
}
Expand All @@ -241,17 +248,27 @@ class BooleanExpressionsRule(configRules: List<RulesConfig>) : DiktatRule(
}
resultExpression = resultExpression.format(args = tokenToOrderedToken.keys.toTypedArray())
// restore expression
expressionToToken.values.forEachIndexed { index, value ->
tokenToExpression.keys.forEachIndexed { index, value ->
resultExpression = resultExpression.replace(value, "%${index + 1}\$s")
}
resultExpression = resultExpression.format(args = expressionToToken.keys.toTypedArray())
resultExpression = resultExpression.format(args = tokenToExpression.values.toTypedArray())
return resultExpression
}

private fun getLetter(letters: HashMap<String, String>, key: String) = letters
.computeIfAbsent(key) {
('A'.code + letters.size).toChar().toString()
}

private fun getNegativeExpression(expressionAstNode: ASTNode, expression: String): String =
if (expressionAstNode.elementType == BINARY_EXPRESSION) {
val operation = (expressionAstNode.psi as KtBinaryExpression).operationReference.text
logicalInfixMethodMapping[operation]?.let {
expression.replace(operation, it)
} ?: "!($expression)"
} else {
"!$expression"
}
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,24 @@ internal const val EMPTY_BLOCK_TEXT = "{}"
*/
internal val standardMethods = listOf("main", "equals", "hashCode", "toString", "clone", "finalize")

/**
* Mapping (value is negative infix) of infix methods that return Boolean
*/
internal val logicalInfixMethodMapping = mapOf(
"==" to "!=",
"!=" to "==",
">" to "<=",
"<" to ">=",
">=" to "<",
"<=" to ">",
"in" to "!in",
"!in" to "in",
)

/**
* List of infix methods that return Boolean
*/
internal val logicalInfixMethods = setOf("==", "!=", ">", "<", ">=", "<=", "in", "!in", "xor")
internal val logicalInfixMethods = logicalInfixMethodMapping.keys + "xor"

/**
* List of element types present in empty code block `{ }`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,10 @@ class BooleanExpressionsRuleFixTest : FixTestBase("test/paragraph3/boolean_expre
fun `check handling of negative expression`() {
fixAndCompare("NegativeExpressionExpected.kt", "NegativeExpressionTest.kt")
}

@Test
@Tag(WarningNames.COMPLEX_BOOLEAN_EXPRESSION)
fun `check expression simplification`() {
fixAndCompare("ExpressionSimplificationExpected.kt", "ExpressionSimplificationTest.kt")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package test.paragraph3.boolean_expressions

fun F.foo1() {
if (this.valueParameters[i].getFunctionName() != other.valueParameters[i].getFunctionName() || this.valueParameters[i].getFunctionType() == other.valueParameters[i].getFunctionType()
) {
return false
}
}

fun F.foo2() {
if (this.valueParameters[i].getFunctionName() <= other.valueParameters[i].getFunctionName() || this.valueParameters[i].getFunctionType() >= other.valueParameters[i].getFunctionType()
) {
return false
}
}

fun F.foo3() {
if (!(this.valueParameters[i].getFunctionName() xor other.valueParameters[i].getFunctionName()) || !(this.valueParameters[i].getFunctionType() xor other.valueParameters[i].getFunctionType())
) {
return false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package test.paragraph3.boolean_expressions

fun F.foo1() {
if (!(this.valueParameters[i].getFunctionName() == other.valueParameters[i].getFunctionName() &&
this.valueParameters[i].getFunctionType() != other.valueParameters[i].getFunctionType())
) {
return false
}
}

fun F.foo2() {
if (!(this.valueParameters[i].getFunctionName() > other.valueParameters[i].getFunctionName() &&
this.valueParameters[i].getFunctionType() < other.valueParameters[i].getFunctionType())
) {
return false
}
}

fun F.foo3() {
if (!(this.valueParameters[i].getFunctionName() xor other.valueParameters[i].getFunctionName() &&
this.valueParameters[i].getFunctionType() xor other.valueParameters[i].getFunctionType())
) {
return false
}
}