Fix for multiple JUnit Conversions in batch mode with and without delta creating an exception.

Added new AssertThatObjectExpression inspection for toString() and hashCode() and moved equals() from AssertThatBinaryExpression there.
2019-06-09 21:35:23 +02:00
parent 6795622202
commit 5c455c3ca9
19 changed files with 235 additions and 53 deletions
@@ -5,15 +5,14 @@ import com.intellij.codeInspection.ProblemsHolder
 import com.intellij.psi.*
 import com.intellij.psi.util.TypeConversionUtil
 import de.platon42.intellij.plugins.cajon.*
-import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
 import de.platon42.intellij.plugins.cajon.quickfixes.SplitBinaryExpressionMethodCallQuickFix

 class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {

    companion object {
        private const val DISPLAY_NAME = "Asserting a binary expression"
-        private const val SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE = "Split %s expression out of assertThat()"
-        private const val MORE_MEANINGFUL_MESSAGE_TEMPLATE = "Moving %s expression out of assertThat() would be more meaningful"
+        private const val SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE = "Split binary expression out of assertThat()"
+        private const val MORE_MEANINGFUL_MESSAGE_TEMPLATE = "Moving binary expression out of assertThat() would be more meaningful"
    }

    override fun getDisplayName() = DISPLAY_NAME
@@ -33,16 +32,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {
                val expectedCallExpression = statement.findOutmostMethodCall() ?: return
                val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return

-                val assertThatArgument = staticMethodCall.firstArg
-                if (assertThatArgument is PsiMethodCallExpression && OBJECT_EQUALS.test(assertThatArgument)) {
-                    val replacementMethod = expectedResult.map(MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO)
-                    registerSplitMethod(holder, expectedCallExpression, "${MethodNames.EQUALS}()", replacementMethod) { desc, method ->
-                        MoveOutMethodCallExpressionQuickFix(desc, method)
-                    }
-                    return
-                }
-
-                val binaryExpression = assertThatArgument as? PsiBinaryExpression ?: return
+                val binaryExpression = staticMethodCall.firstArg as? PsiBinaryExpression ?: return

                val leftType = binaryExpression.lOperand.type ?: return
                val rightType = binaryExpression.rOperand?.type ?: return
@@ -53,7 +43,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {
                    return
                } else if (isLeftNull || isRightNull) {
                    val replacementMethod = expectedResult.map(MethodNames.IS_NULL, MethodNames.IS_NOT_NULL)
-                    registerSplitMethod(holder, expectedCallExpression, "binary", replacementMethod) { desc, method ->
+                    registerSplitMethod(holder, expectedCallExpression, replacementMethod) { desc, method ->
                        SplitBinaryExpressionMethodCallQuickFix(desc, method, pickRightOperand = isLeftNull, noExpectedExpression = true)
                    }
                    return
@@ -75,7 +65,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {
                    (isPrimitive || isNumericType).map(TOKEN_TO_ASSERTJ_FOR_PRIMITIVE_MAP, TOKEN_TO_ASSERTJ_FOR_OBJECT_MAPPINGS)
                val replacementMethod = mappingToUse[tokenType] ?: return

-                registerSplitMethod(holder, expectedCallExpression, "binary", replacementMethod) { desc, method ->
+                registerSplitMethod(holder, expectedCallExpression, replacementMethod) { desc, method ->
                    SplitBinaryExpressionMethodCallQuickFix(desc, method, pickRightOperand = swapExpectedAndActual)
                }
            }
@@ -85,13 +75,10 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {
    private fun registerSplitMethod(
        holder: ProblemsHolder,
        expression: PsiMethodCallExpression,
-        type: String,
        replacementMethod: String,
        quickFixSupplier: (String, String) -> LocalQuickFix
    ) {
-        val description = SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE.format(type)
-        val message = MORE_MEANINGFUL_MESSAGE_TEMPLATE.format(type)
-        val quickfix = quickFixSupplier(description, replacementMethod)
-        holder.registerProblem(expression, message, quickfix)
+        val quickfix = quickFixSupplier(SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE, replacementMethod)
+        holder.registerProblem(expression, MORE_MEANINGFUL_MESSAGE_TEMPLATE, quickfix)
    }
 }
@@ -34,11 +34,6 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
            Mapping(
                CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "containsValue").parameterCount(1),
                MethodNames.CONTAINS_VALUE, MethodNames.DOES_NOT_CONTAIN_VALUE
-            ),
-            // TODO not quite a collection or map expression, maybe move this out to new inspection together with equals and hashcode.
-            Mapping(
-                CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "toString").parameterCount(0),
-                MethodNames.HAS_TO_STRING, null
            )
        )
    }
@@ -0,0 +1,59 @@
+package de.platon42.intellij.plugins.cajon.inspections
+
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.*
+import com.siyeh.ig.callMatcher.CallMatcher
+import de.platon42.intellij.plugins.cajon.*
+import de.platon42.intellij.plugins.cajon.quickfixes.HasHashCodeQuickFix
+import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
+import de.platon42.intellij.plugins.cajon.quickfixes.RemoveActualOutmostMethodCallQuickFix
+
+class AssertThatObjectExpressionInspection : AbstractAssertJInspection() {
+
+    companion object {
+        private const val DISPLAY_NAME = "Asserting equals(), toString(), or hashCode()"
+        private const val HASHCODE_MESSAGE_TEMPLATE = "Replacing hashCode() expressions by hasSameHashCode() would be more concise"
+
+        private val OBJECT_TO_STRING = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "toString").parameterCount(0)
+        private val OBJECT_HASHCODE = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "hashCode").parameterCount(0)
+    }
+
+    override fun getDisplayName() = DISPLAY_NAME
+
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
+        return object : JavaElementVisitor() {
+            override fun visitExpressionStatement(statement: PsiExpressionStatement) {
+                super.visitExpressionStatement(statement)
+                if (!statement.hasAssertThat()) {
+                    return
+                }
+                val staticMethodCall = statement.findStaticMethodCall() ?: return
+
+                val assertThatArgument = staticMethodCall.firstArg as? PsiMethodCallExpression ?: return
+                val expectedCallExpression = statement.findOutmostMethodCall() ?: return
+                when {
+                    OBJECT_EQUALS.test(assertThatArgument) -> {
+                        val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return
+                        val replacementMethod = expectedResult.map(MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO)
+                        registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
+                            MoveOutMethodCallExpressionQuickFix(desc, method)
+                        }
+                    }
+                    OBJECT_TO_STRING.test(assertThatArgument) -> {
+                        staticMethodCall.findFluentCallTo(IS_EQUAL_TO_OBJECT) ?: return
+                        registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.HAS_TO_STRING) { desc, method ->
+                            RemoveActualOutmostMethodCallQuickFix(desc, method)
+                        }
+                    }
+                    OBJECT_HASHCODE.test(assertThatArgument) -> {
+                        val isEqualTo = staticMethodCall.findFluentCallTo(IS_EQUAL_TO_INT) ?: return
+                        val expectedExpression = isEqualTo.firstArg as? PsiMethodCallExpression ?: return
+                        if (OBJECT_HASHCODE.test(expectedExpression)) {
+                            holder.registerProblem(expectedCallExpression, HASHCODE_MESSAGE_TEMPLATE, HasHashCodeQuickFix())
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
@@ -115,8 +115,7 @@ class AssertThatSizeInspection : AbstractAssertJInspection() {
                if (!expression.hasAssertThat()) {
                    return
                }
-                val isHasSize = HAS_SIZE.test(expression)
-                if (!(isHasSize)) {
+                if (!HAS_SIZE.test(expression)) {
                    return
                }
                val actualExpression = expression.firstArg
@@ -0,0 +1,37 @@
+package de.platon42.intellij.plugins.cajon.quickfixes
+
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiMethodCallExpression
+import de.platon42.intellij.plugins.cajon.*
+
+class HasHashCodeQuickFix :
+    AbstractCommonQuickFix(HASHCODE_DESCRIPTION) {
+
+    companion object {
+        private const val HASHCODE_DESCRIPTION = "Replace calls to hashCode() with hasSameHashCodeAs()"
+    }
+
+    override fun getFamilyName(): String {
+        return HASHCODE_DESCRIPTION
+    }
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val outmostCallExpression = descriptor.startElement as? PsiMethodCallExpression ?: return
+        val assertThatMethodCall = outmostCallExpression.findStaticMethodCall() ?: return
+
+        val assertExpression = assertThatMethodCall.firstArg as? PsiMethodCallExpression ?: return
+
+        val methodsToFix = assertThatMethodCall.gatherAssertionCalls()
+
+        assertExpression.replace(assertExpression.qualifierExpression)
+
+        methodsToFix
+            .forEach {
+                val innerHashCodeObject = (it.firstArg as PsiMethodCallExpression).qualifierExpression
+                val expectedExpression = createExpectedMethodCall(it, "hasSameHashCodeAs", innerHashCodeObject)
+                expectedExpression.replaceQualifierFromMethodCall(it)
+                it.replace(expectedExpression)
+            }
+    }
+}
@@ -9,7 +9,7 @@ import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.GUAVA_ASSE
 class ReplaceJUnitDeltaAssertMethodCallQuickFix(description: String, private val replacementMethod: String) : AbstractCommonQuickFix(description) {

    companion object {
-        private const val CONVERT_DESCRIPTION = "Convert JUnit assertions to assertJ"
+        private const val CONVERT_DESCRIPTION = "Convert JUnit assertions with delta to assertJ"
    }

    override fun getFamilyName(): String {