New inspection AssertThatBinaryExpressionIsTrueOrFalse that will find and fix common binary expressions and equals() statements (more than 150 combinations) inside assertThat().

2019-04-07 11:29:07 +02:00
parent 6a44237ca0
commit 5812e72227
12 changed files with 712 additions and 15 deletions
@@ -53,11 +53,18 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
        @NonNls
        const val IS_NOT_ZERO_METHOD = "isNotZero"
        @NonNls
+        const val IS_TRUE_METHOD = "isTrue"
+        @NonNls
+        const val IS_FALSE_METHOD = "isFalse"
+        @NonNls
        const val HAS_SIZE_METHOD = "hasSize"

        val ASSERT_THAT_INT = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, ASSERT_THAT_METHOD)
            .parameterTypes("int")!!

+        val ASSERT_THAT_BOOLEAN = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, ASSERT_THAT_METHOD)
+            .parameterTypes("boolean")!!
+
        val IS_EQUAL_TO_OBJECT = CallMatcher.instanceCall(ABSTRACT_ASSERT_CLASSNAME, IS_EQUAL_TO_METHOD)
            .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!!
        val IS_NOT_EQUAL_TO_OBJECT = CallMatcher.instanceCall(ABSTRACT_ASSERT_CLASSNAME, IS_NOT_EQUAL_TO_METHOD)
@@ -87,8 +94,15 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
        val IS_NOT_ZERO = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, IS_NOT_ZERO_METHOD)
            .parameterCount(0)!!

+        val IS_TRUE = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, IS_TRUE_METHOD)
+            .parameterCount(0)!!
+        val IS_FALSE = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, IS_FALSE_METHOD)
+            .parameterCount(0)!!
+
        val COLLECTION_SIZE = CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "size")
            .parameterCount(0)!!
+        val OBJECT_EQUALS = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "equals")
+            .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!!
    }

    override fun getGroupDisplayName(): String {
@@ -0,0 +1,144 @@
+package de.platon42.intellij.plugins.cajon.inspections
+
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.*
+import com.intellij.psi.tree.IElementType
+import com.intellij.psi.util.PsiTreeUtil
+import com.intellij.psi.util.TypeConversionUtil
+import de.platon42.intellij.plugins.cajon.quickfixes.SplitBinaryExpressionMethodCallQuickFix
+import de.platon42.intellij.plugins.cajon.quickfixes.SplitEqualsExpressionMethodCallQuickFix
+
+class AssertThatBinaryExpressionIsTrueOrFalseInspection : AbstractAssertJInspection() {
+
+    companion object {
+        private const val DISPLAY_NAME = "Asserting a binary expression"
+        private const val SPLIT_BINARY_EXPRESSION_DESCRIPTION = "Split binary expression out of assertThat()"
+        private const val SPLIT_EQUALS_EXPRESSION_DESCRIPTION = "Split equals() expression out of assertThat()"
+        private const val BINARY_MORE_MEANINGFUL_MESSAGE = "Moving binary expression out of assertThat() would be more meaningful"
+        private const val EQUALS_MORE_MEANINGFUL_MESSAGE = "Moving equals() expression out of assertThat() would be more meaningful"
+
+        private val PRIMITIVE_MAPPINGS = listOf(
+            Mapping(JavaTokenType.EQEQ, "isEqualTo()", "isNotEqualTo()"),
+            Mapping(JavaTokenType.NE, "isNotEqualTo()", "isEqualTo()"),
+            Mapping(JavaTokenType.GT, "isGreaterThan()", "isLessThanOrEqualTo()"),
+            Mapping(JavaTokenType.GE, "isGreaterThanOrEqualTo()", "isLessThan()"),
+            Mapping(JavaTokenType.LT, "isLessThan()", "isGreaterThanOrEqualTo()"),
+            Mapping(JavaTokenType.LE, "isLessThanOrEqualTo()", "isGreaterThan()")
+        )
+
+        private val PRIMITIVE_MAPPINGS_SWAPPED = listOf(
+            Mapping(JavaTokenType.EQEQ, "isEqualTo()", "isNotEqualTo()"),
+            Mapping(JavaTokenType.NE, "isNotEqualTo()", "isEqualTo()"),
+            Mapping(JavaTokenType.GT, "isLessThan()", "isGreaterThanOrEqualTo()"),
+            Mapping(JavaTokenType.GE, "isLessThanOrEqualTo()", "isGreaterThan()"),
+            Mapping(JavaTokenType.LT, "isGreaterThan()", "isLessThanOrEqualTo()"),
+            Mapping(JavaTokenType.LE, "isGreaterThanOrEqualTo()", "isLessThan()")
+        )
+
+        private val OBJECT_MAPPINGS = listOf(
+            Mapping(JavaTokenType.EQEQ, "isSameAs()", "isNotSameAs()"),
+            Mapping(JavaTokenType.NE, "isNotSameAs()", "isSameAs()")
+        )
+    }
+
+    override fun getDisplayName() = DISPLAY_NAME
+
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
+        return object : JavaElementVisitor() {
+            override fun visitMethodCallExpression(expression: PsiMethodCallExpression) {
+                super.visitMethodCallExpression(expression)
+                if (!ASSERT_THAT_BOOLEAN.test(expression)) {
+                    return
+                }
+
+                val statement = PsiTreeUtil.getParentOfType(expression, PsiStatement::class.java) ?: return
+                val expectedCallExpression = PsiTreeUtil.findChildOfType(statement, PsiMethodCallExpression::class.java) ?: return
+                val isInverted = getExpectedResult(expectedCallExpression) ?: return
+
+                val assertThatArgument = expression.argumentList.expressions[0] ?: return
+                if (assertThatArgument is PsiMethodCallExpression && OBJECT_EQUALS.test(assertThatArgument)) {
+                    val replacementMethod = if (isInverted) "isNotEqualTo()" else "isEqualTo()"
+                    holder.registerProblem(
+                        expression,
+                        EQUALS_MORE_MEANINGFUL_MESSAGE,
+                        SplitEqualsExpressionMethodCallQuickFix(SPLIT_EQUALS_EXPRESSION_DESCRIPTION, replacementMethod)
+                    )
+                    return
+                }
+
+                val binaryExpression = assertThatArgument as? PsiBinaryExpression ?: return
+
+                val leftType = binaryExpression.lOperand.type ?: return
+                val rightType = binaryExpression.rOperand?.type ?: return
+
+                val isLeftNull = TypeConversionUtil.isNullType(leftType)
+                val isRightNull = TypeConversionUtil.isNullType(rightType)
+                if (isLeftNull && isRightNull) {
+                    return
+                } else if (isLeftNull || isRightNull) {
+                    registerSplitBinaryExpressionMethod(
+                        holder,
+                        expression,
+                        if (isInverted) "isNotNull()" else "isNull()",
+                        pickRightOperand = isLeftNull,
+                        noExpectedExpression = true
+                    )
+                    return
+                }
+
+                val isPrimitive = TypeConversionUtil.isPrimitiveAndNotNull(leftType) && TypeConversionUtil.isPrimitiveAndNotNull(rightType)
+                val isNumericType = TypeConversionUtil.isNumericType(leftType) && TypeConversionUtil.isNumericType(rightType)
+                val constantEvaluationHelper = JavaPsiFacade.getInstance(expression.project).constantEvaluationHelper
+                val swapExpectedAndActual = constantEvaluationHelper.computeConstantExpression(binaryExpression.lOperand) != null
+
+                val tokenType = binaryExpression.operationSign.tokenType
+                val mappingToUse =
+                    if (isPrimitive || isNumericType) {
+                        if (swapExpectedAndActual) PRIMITIVE_MAPPINGS_SWAPPED else PRIMITIVE_MAPPINGS
+                    } else {
+                        OBJECT_MAPPINGS
+                    }
+                val mapping = mappingToUse.find { it.tokenType == tokenType } ?: return
+                val replacementMethod = if (isInverted) mapping.replacementInverted else mapping.replacement
+
+                registerSplitBinaryExpressionMethod(holder, expression, replacementMethod, pickRightOperand = swapExpectedAndActual)
+            }
+
+            private fun getExpectedResult(expectedCallExpression: PsiMethodCallExpression): Boolean? {
+                val isTrue = IS_TRUE.test(expectedCallExpression)
+                val isFalse = IS_FALSE.test(expectedCallExpression)
+                if (isTrue || isFalse) {
+                    return isFalse
+                } else {
+                    val isEqualTo = IS_EQUAL_TO_BOOLEAN.test(expectedCallExpression)
+                    val isNotEqualTo = IS_NOT_EQUAL_TO_BOOLEAN.test(expectedCallExpression)
+                    if (isEqualTo || isNotEqualTo) {
+                        val constValue = calculateConstantParameterValue(expectedCallExpression, 0) as? Boolean ?: return null
+                        return isEqualTo xor constValue
+                    }
+                }
+                return null
+            }
+
+            private fun registerSplitBinaryExpressionMethod(
+                holder: ProblemsHolder,
+                expression: PsiMethodCallExpression,
+                replacementMethod: String,
+                pickRightOperand: Boolean = false,
+                noExpectedExpression: Boolean = false
+            ) {
+                holder.registerProblem(
+                    expression,
+                    BINARY_MORE_MEANINGFUL_MESSAGE,
+                    SplitBinaryExpressionMethodCallQuickFix(SPLIT_BINARY_EXPRESSION_DESCRIPTION, replacementMethod, pickRightOperand, noExpectedExpression)
+                )
+            }
+        }
+    }
+
+    private class Mapping(
+        val tokenType: IElementType,
+        val replacement: String,
+        val replacementInverted: String
+    )
+}
@@ -0,0 +1,36 @@
+package de.platon42.intellij.plugins.cajon.quickfixes
+
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.openapi.project.Project
+import com.intellij.psi.JavaPsiFacade
+import com.intellij.psi.PsiBinaryExpression
+import com.intellij.psi.PsiMethodCallExpression
+import com.intellij.psi.PsiStatement
+import com.intellij.psi.util.PsiTreeUtil
+
+class SplitBinaryExpressionMethodCallQuickFix(
+    description: String,
+    private val replacementMethod: String,
+    private val pickRightOperand: Boolean,
+    private val noExpectedExpression: Boolean
+) : AbstractCommonQuickFix(description) {
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val element = descriptor.startElement
+        val factory = JavaPsiFacade.getElementFactory(element.project)
+        val methodCallExpression = element as? PsiMethodCallExpression ?: return
+        val binaryExpression = methodCallExpression.argumentList.expressions[0] as? PsiBinaryExpression ?: return
+        val expectedArgument = (if (pickRightOperand) binaryExpression.lOperand else binaryExpression.rOperand)?.copy() ?: return
+        binaryExpression.replace(if (pickRightOperand) binaryExpression.rOperand!! else binaryExpression.lOperand)
+
+        val statement = PsiTreeUtil.getParentOfType(element, PsiStatement::class.java) ?: return
+        val oldExpectedExpression = PsiTreeUtil.findChildOfType(statement, PsiMethodCallExpression::class.java) ?: return
+        val expectedExpression =
+            factory.createExpressionFromText("a.${if (noExpectedExpression) replacementMethod else replacementMethod.replace("()", "(e)")}", element) as PsiMethodCallExpression
+        if (!noExpectedExpression) {
+            expectedExpression.argumentList.expressions[0].replace(expectedArgument)
+        }
+        expectedExpression.methodExpression.qualifierExpression!!.replace(oldExpectedExpression.methodExpression.qualifierExpression!!)
+        oldExpectedExpression.replace(expectedExpression)
+    }
+}
@@ -0,0 +1,28 @@
+package de.platon42.intellij.plugins.cajon.quickfixes
+
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.openapi.project.Project
+import com.intellij.psi.JavaPsiFacade
+import com.intellij.psi.PsiMethodCallExpression
+import com.intellij.psi.PsiStatement
+import com.intellij.psi.util.PsiTreeUtil
+
+class SplitEqualsExpressionMethodCallQuickFix(description: String, private val replacementMethod: String) : AbstractCommonQuickFix(description) {
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val element = descriptor.startElement
+        val factory = JavaPsiFacade.getElementFactory(element.project)
+        val methodCallExpression = element as? PsiMethodCallExpression ?: return
+        val equalsMethodCall = methodCallExpression.argumentList.expressions[0] as? PsiMethodCallExpression ?: return
+        val expectedArgument = equalsMethodCall.argumentList.expressions[0].copy()
+        equalsMethodCall.replace(equalsMethodCall.methodExpression.qualifierExpression!!)
+
+        val statement = PsiTreeUtil.getParentOfType(element, PsiStatement::class.java) ?: return
+        val oldExpectedExpression = PsiTreeUtil.findChildOfType(statement, PsiMethodCallExpression::class.java) ?: return
+        val expectedExpression =
+            factory.createExpressionFromText("a.${replacementMethod.replace("()", "(e)")}", element) as PsiMethodCallExpression
+        expectedExpression.argumentList.expressions[0].replace(expectedArgument)
+        expectedExpression.methodExpression.qualifierExpression!!.replace(oldExpectedExpression.methodExpression.qualifierExpression!!)
+        oldExpectedExpression.replace(expectedExpression)
+    }
+}
@@ -21,21 +21,20 @@
    <extensions defaultExtensionNs="com.intellij">
        <localInspection groupPath="Java" shortName="AssertThatObjectIsNullOrNotNull" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatObjectIsNullOrNotNullInspection"/>
-        <localInspection groupPath="Java" shortName="AssertThatBooleanIsTrueOrFalse" enabledByDefault="true"
-                         level="WARNING"
+        <localInspection groupPath="Java" shortName="AssertThatBooleanIsTrueOrFalse" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatBooleanIsTrueOrFalseInspection"/>
        <localInspection groupPath="Java" shortName="AssertThatStringIsEmpty" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatStringIsEmptyInspection"/>
-        <localInspection groupPath="Java" shortName="AssertThatEnumerableIsEmpty" enabledByDefault="true"
-                         level="WARNING"
+        <localInspection groupPath="Java" shortName="AssertThatEnumerableIsEmpty" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatEnumerableIsEmptyInspection"/>

-        <localInspection groupPath="Java" shortName="AssertThatSize" enabledByDefault="true"
-                         level="WARNING"
+        <localInspection groupPath="Java" shortName="AssertThatSize" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatSizeInspection"/>

-        <localInspection groupPath="Java" shortName="JUnitAssertToAssertJ" enabledByDefault="true"
-                         level="WARNING"
+        <localInspection groupPath="Java" shortName="AssertThatBinaryExpressionIsTrueOrFalse" enabledByDefault="true" level="WARNING"
+                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatBinaryExpressionIsTrueOrFalseInspection"/>
+
+        <localInspection groupPath="Java" shortName="JUnitAssertToAssertJ" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.JUnitAssertToAssertJInspection"/>
    </extensions>

@@ -0,0 +1,11 @@
+<html>
+<body>
+Turns a binary expression in the kind of assertThat(actual &lt;operator&gt; expected) with .isTrue()/isFalse()/isEqualTo()/isNotEqualTo()
+into assertThat(actual).is&lt;operator&gt;(expected).
+<!-- tooltip end -->
+There are over 150 combinations that are found with this inspections.
+It also detects assertThat(actual.equals(expected)) assertions.
+Also works with constant expressions on the expected side.
+Swaps actual and expected when actual is a constant expression (correctly transforming the used operator).
+</body>
+</html>