Added new TwistedAssertion inspection that will warn about assertions with the actual expression being a constant indicating swapped use of actual and expected expressions.

Added new BogusAssertion inspection that showing typical copy and paste errors where actual and expected expressions are the same.
2020-02-14 13:36:04 +01:00
parent 58298fabc6
commit 62f59b0fe2
18 changed files with 702 additions and 13 deletions
@@ -44,6 +44,22 @@ class AssertJClassNames {
        @NonNls
        const val ABSTRACT_MAP_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractMapAssert"
        @NonNls
+        const val ABSTRACT_BOOLEAN_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractBooleanArrayAssert"
+        @NonNls
+        const val ABSTRACT_BYTE_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractByteArrayAssert"
+        @NonNls
+        const val ABSTRACT_SHORT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractShortArrayAssert"
+        @NonNls
+        const val ABSTRACT_INT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractIntArrayAssert"
+        @NonNls
+        const val ABSTRACT_LONG_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractLongArrayAssert"
+        @NonNls
+        const val ABSTRACT_FLOAT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractFloatArrayAssert"
+        @NonNls
+        const val ABSTRACT_DOUBLE_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractDoubleArrayAssert"
+        @NonNls
+        const val ABSTRACT_CHAR_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractCharArrayAssert"
+        @NonNls
        const val ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractObjectArrayAssert"
        @NonNls
        const val ABSTRACT_ITERABLE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractIterableAssert"
@@ -43,7 +43,7 @@ fun PsiElement.findStaticMethodCall(): PsiMethodCallExpression? {
 fun PsiElement.gatherAssertionCalls(): List<PsiMethodCallExpression> {
    val assertThatMethodCall = findStaticMethodCall() ?: return emptyList()
    return assertThatMethodCall.collectMethodCallsUpToStatement()
-        .filterNot { NOT_ACTUAL_ASSERTIONS.test(it) }
+        .filterNot(NOT_ACTUAL_ASSERTIONS::test)
        .toList()
 }

@@ -0,0 +1,138 @@
+package de.platon42.intellij.plugins.cajon.inspections
+
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.JavaElementVisitor
+import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.PsiExpressionStatement
+import com.intellij.psi.PsiMethodCallExpression
+import com.intellij.psi.util.PsiTreeUtil
+import com.siyeh.ig.callMatcher.CallMatcher
+import com.siyeh.ig.psiutils.EquivalenceChecker
+import de.platon42.intellij.plugins.cajon.*
+
+class BogusAssertionInspection : AbstractAssertJInspection() {
+
+    companion object {
+        private const val DISPLAY_NAME = "Bogus assertion due to same actual and expected expressions"
+        private const val ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE = "Actual expression in assertThat() is the same as expected"
+
+        private val SAME_OBJECT =
+            CallMatcher.instanceCall(
+                AssertJClassNames.ASSERT_INTERFACE,
+                MethodNames.IS_EQUAL_TO,
+                MethodNames.IS_SAME_AS,
+                "hasSameClassAs",
+                "hasSameHashCodeAs"
+            ).parameterCount(1)
+
+        private val ARRAY_METHODS = arrayOf(
+            "hasSameSizeAs",
+            MethodNames.CONTAINS,
+            "containsAnyOf",
+            "containsExactly",
+            "containsExactlyInAnyOrder",
+            "containsOnly",
+            "containsSequence",
+            "containsSubsequence",
+            "startsWith",
+            "endsWith"
+        )
+
+        private val SAME_BOOLEAN_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_BOOLEAN_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_BYTE_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_BYTE_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_SHORT_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_SHORT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_INT_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_INT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_LONG_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_LONG_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_FLOAT_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_FLOAT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_DOUBLE_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_DOUBLE_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_CHAR_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_CHAR_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+        private val SAME_OBJECT_ARRAY_CONTENTS =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME, *ARRAY_METHODS).parameterCount(1)
+
+        private val SAME_ENUMERABLE_CONTENTS =
+            CallMatcher.instanceCall(
+                AssertJClassNames.ENUMERABLE_ASSERT_INTERFACE,
+                MethodNames.HAS_SAME_SIZE_AS
+            ).parameterCount(1)
+
+        private val SAME_ITERABLE_CONTENTS =
+            CallMatcher.instanceCall(
+                AssertJClassNames.ABSTRACT_ITERABLE_ASSERT_CLASSNAME,
+                "hasSameElementsAs",
+                MethodNames.CONTAINS_ALL,
+                "containsAnyElementsOf",
+                "containsOnlyElementsOf",
+                "containsExactlyElementsOf",
+                "containsSequence",
+                "containsSubsequence"
+            ).parameterCount(1)
+
+        private val SAME_MAP_CONTENTS =
+            CallMatcher.instanceCall(
+                AssertJClassNames.ABSTRACT_MAP_ASSERT_CLASSNAME,
+                "containsAllEntriesOf",
+                "containsExactlyEntriesOf",
+                "containsExactlyInAnyOrderEntriesOf",
+                MethodNames.HAS_SAME_SIZE_AS
+            ).parameterCount(1)
+
+        private val SAME_CHAR_SEQUENCE_CONTENTS =
+            CallMatcher.instanceCall(
+                AssertJClassNames.ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME,
+                MethodNames.IS_EQUAL_TO,
+                MethodNames.IS_EQUAL_TO_IC,
+                MethodNames.STARTS_WITH,
+                MethodNames.ENDS_WITH,
+                "containsSequence",
+                "containsSubsequence"
+            ).parameterCount(1)
+
+        private val SAME_ACTUAL_AND_EXPECTED_MATCHERS = CallMatcher.anyOf(
+            SAME_OBJECT,
+            SAME_ENUMERABLE_CONTENTS,
+            SAME_ITERABLE_CONTENTS,
+            SAME_MAP_CONTENTS,
+            SAME_CHAR_SEQUENCE_CONTENTS,
+
+            SAME_BOOLEAN_ARRAY_CONTENTS,
+            SAME_BYTE_ARRAY_CONTENTS,
+            SAME_SHORT_ARRAY_CONTENTS,
+            SAME_INT_ARRAY_CONTENTS,
+            SAME_LONG_ARRAY_CONTENTS,
+            SAME_FLOAT_ARRAY_CONTENTS,
+            SAME_DOUBLE_ARRAY_CONTENTS,
+            SAME_CHAR_ARRAY_CONTENTS,
+            SAME_OBJECT_ARRAY_CONTENTS
+        )
+    }
+
+    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 assertThatCall = PsiTreeUtil.findChildrenOfType(statement, PsiMethodCallExpression::class.java).find { ALL_ASSERT_THAT_MATCHERS.test(it) } ?: return
+                val actualExpression = assertThatCall.firstArg
+                val allCalls = assertThatCall.collectMethodCallsUpToStatement().toList()
+                // Note: replace with TrackingEquivalenceChecker() for IDEA >= 2019.1
+                val equivalenceChecker = EquivalenceChecker.getCanonicalPsiEquivalence()!!
+                val isSameExpression = allCalls
+                    .filter(SAME_ACTUAL_AND_EXPECTED_MATCHERS::test)
+                    .any { equivalenceChecker.expressionsAreEquivalent(actualExpression, it.firstArg) }
+                if (isSameExpression) {
+                    holder.registerProblem(statement, ACTUAL_IS_EQUAL_TO_EXPECTED_MESSAGE)
+                }
+            }
+        }
+    }
+}
@@ -26,7 +26,7 @@ class JoinVarArgsContainsInspection : AbstractAssertJInspection() {
    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
        return object : JavaElementVisitor() {
            override fun visitExpressionStatement(statement: PsiExpressionStatement) {
-                super.visitStatement(statement)
+                super.visitExpressionStatement(statement)
                if (!statement.hasAssertThat()) return
                val assertThatCall = PsiTreeUtil.findChildrenOfType(statement, PsiMethodCallExpression::class.java).find { ALL_ASSERT_THAT_MATCHERS.test(it) } ?: return

@@ -35,7 +35,7 @@ class JoinVarArgsContainsInspection : AbstractAssertJInspection() {
                if (allCalls.find(COMPLEX_CALLS_THAT_MAKES_STUFF_TRICKY::test) != null) return

                val onlyAssertionCalls = allCalls
-                    .filterNot { NOT_ACTUAL_ASSERTIONS.test(it) }
+                    .filterNot(NOT_ACTUAL_ASSERTIONS::test)
                    .toList()

                for (methodMatcher in MATCHERS) {
@@ -0,0 +1,91 @@
+package de.platon42.intellij.plugins.cajon.inspections
+
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.psi.JavaElementVisitor
+import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.PsiExpressionStatement
+import com.intellij.psi.PsiMethodCallExpression
+import com.intellij.psi.util.PsiTreeUtil
+import com.siyeh.ig.callMatcher.CallMatcher
+import de.platon42.intellij.plugins.cajon.*
+import de.platon42.intellij.plugins.cajon.quickfixes.SwapActualAndExpectedExpressionMethodCallQuickFix
+
+class TwistedAssertionInspection : AbstractAssertJInspection() {
+
+    companion object {
+        private const val DISPLAY_NAME = "Twisted or suspicious actual and expected expressions"
+        private const val TWISTED_ACTUAL_AND_EXPECTED_MESSAGE = "Twisted actual and expected expressions in assertion"
+        private const val SWAP_ACTUAL_AND_EXPECTED_DESCRIPTION = "Swap actual and expected expressions in assertion"
+        private const val SWAP_ACTUAL_AND_EXPECTED_AND_REPLACE_DESCRIPTION_TEMPLATE = "Replace %s() by %s() and swap actual and expected expressions"
+        private const val ACTUAL_IS_A_CONSTANT_MESSAGE = "Actual expression in assertThat() is a constant"
+
+        private val GENERIC_IS_EQUAL_TO = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_EQUAL_TO).parameterCount(1)
+        private val GENERIC_IS_NOT_EQUAL_TO = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_NOT_EQUAL_TO).parameterCount(1)
+        private val GENERIC_IS_SAME_AS = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_SAME_AS).parameterCount(1)
+        private val GENERIC_IS_NOT_SAME_AS = CallMatcher.instanceCall(AssertJClassNames.ASSERT_INTERFACE, MethodNames.IS_NOT_SAME_AS).parameterCount(1)
+        private val GENERIC_IS_GREATER_THAN = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_GREATER_THAN).parameterCount(1)
+        private val GENERIC_IS_GREATER_THAN_OR_EQUAL_TO =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_GREATER_THAN_OR_EQUAL_TO).parameterCount(1)
+        private val GENERIC_IS_LESS_THAN = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_LESS_THAN).parameterCount(1)
+        private val GENERIC_IS_LESS_THAN_OR_EQUAL_TO =
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_LESS_THAN_OR_EQUAL_TO).parameterCount(1)
+
+        private val STRING_IS_EQUAL_TO_IC = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO_IC).parameterCount(1)
+
+        private val CALL_MATCHER_TO_REPLACEMENT_MAP = mapOf(
+            GENERIC_IS_EQUAL_TO to MethodNames.IS_EQUAL_TO,
+            GENERIC_IS_NOT_EQUAL_TO to MethodNames.IS_NOT_EQUAL_TO,
+            GENERIC_IS_SAME_AS to MethodNames.IS_SAME_AS,
+            GENERIC_IS_NOT_SAME_AS to MethodNames.IS_NOT_SAME_AS,
+            GENERIC_IS_GREATER_THAN to MethodNames.IS_LESS_THAN_OR_EQUAL_TO,
+            GENERIC_IS_GREATER_THAN_OR_EQUAL_TO to MethodNames.IS_LESS_THAN,
+            GENERIC_IS_LESS_THAN to MethodNames.IS_GREATER_THAN_OR_EQUAL_TO,
+            GENERIC_IS_LESS_THAN_OR_EQUAL_TO to MethodNames.IS_GREATER_THAN,
+
+            STRING_IS_EQUAL_TO_IC to MethodNames.IS_EQUAL_TO_IC,
+            CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO_IC).parameterCount(1)
+                    to MethodNames.IS_NOT_EQUAL_TO_IC
+        )
+    }
+
+    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 assertThatCall = PsiTreeUtil.findChildrenOfType(statement, PsiMethodCallExpression::class.java).find { ALL_ASSERT_THAT_MATCHERS.test(it) } ?: return
+                val actualExpression = assertThatCall.firstArg
+                actualExpression.calculateConstantValue() ?: return
+                val allCalls = assertThatCall.collectMethodCallsUpToStatement().toList()
+                val tooComplex = allCalls.find(USING_COMPARATOR::test) != null
+                if (!tooComplex) {
+                    val onlyAssertionCalls = allCalls
+                        .filterNot(NOT_ACTUAL_ASSERTIONS::test)
+                        .toList()
+                    if (onlyAssertionCalls.size == 1) {
+                        val originalMethodCall = onlyAssertionCalls.first()
+                        val matchedMethod = CALL_MATCHER_TO_REPLACEMENT_MAP.asSequence().firstOrNull { it.key.test(originalMethodCall) }
+                        if (matchedMethod != null) {
+                            val originalMethodName = getOriginalMethodName(originalMethodCall)
+                            val replacementMethod = matchedMethod.value
+                            val description = if (originalMethodName == replacementMethod) {
+                                SWAP_ACTUAL_AND_EXPECTED_DESCRIPTION
+                            } else {
+                                SWAP_ACTUAL_AND_EXPECTED_AND_REPLACE_DESCRIPTION_TEMPLATE.format(originalMethodName, replacementMethod)
+                            }
+                            holder.registerProblem(
+                                statement,
+                                TWISTED_ACTUAL_AND_EXPECTED_MESSAGE,
+                                SwapActualAndExpectedExpressionMethodCallQuickFix(description, replacementMethod)
+                            )
+                            return
+                        }
+                    }
+                }
+                holder.registerProblem(statement, ACTUAL_IS_A_CONSTANT_MESSAGE)
+            }
+        }
+    }
+}
@@ -0,0 +1,34 @@
+package de.platon42.intellij.plugins.cajon.quickfixes
+
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.openapi.project.Project
+import de.platon42.intellij.plugins.cajon.*
+
+class SwapActualAndExpectedExpressionMethodCallQuickFix(
+    description: String,
+    private val replacementMethod: String
+) : AbstractCommonQuickFix(description) {
+
+    companion object {
+        private const val SPLIT_EXPRESSION_DESCRIPTION = "Swap actual and expected expressions of assertions"
+    }
+
+    override fun getFamilyName(): String {
+        return SPLIT_EXPRESSION_DESCRIPTION
+    }
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val assertThatMethodCall = descriptor.startElement.findStaticMethodCall() ?: return
+
+        val methodToFix = assertThatMethodCall.collectMethodCallsUpToStatement()
+            .filterNot(NOT_ACTUAL_ASSERTIONS::test)
+            .first()
+
+        val oldActualExpression = assertThatMethodCall.firstArg.copy()!!
+        assertThatMethodCall.firstArg.replace(methodToFix.firstArg)
+
+        val expectedExpression = createExpectedMethodCall(methodToFix, replacementMethod, oldActualExpression)
+        expectedExpression.replaceQualifierFromMethodCall(methodToFix)
+        methodToFix.replace(expectedExpression)
+    }
+}
@@ -5,10 +5,11 @@

    <description><![CDATA[
    Cajon is an IntelliJ IDEA Plugin for shortening and optimizing AssertJ assertions.
-    It adds several inspections and quick fixes to fully use the fluent assertion methods
+    It adds several <b>inspections and quick fixes</b> to fully use the fluent assertion methods
    and thus makes the intention clear and concise, also generating better messages on test failures.
-    It can also be used to convert JUnit 4 assertions and assumptions to AssertJ.
-    It supports referencing inside extracting()-methods with strings, adding refactoring safety.
+    It can also be used to <b>convert JUnit 4 assertions and assumptions to AssertJ</b>.
+    It supports <b>referencing inside extracting</b>()-methods with strings, adding refactoring safety.
+    <b>Bogus or twisted assertions</b> are also reported.
    <p>
    <a href="https://github.com/chrisly42/cajon-plugin/blob/master/README.md">Full documentation here...</a>
    ]]></description>
@@ -67,6 +68,11 @@
        <localInspection groupPath="Java" shortName="ImplicitAssertion" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.ImplicitAssertionInspection"/>

+        <localInspection groupPath="Java" shortName="TwistedAssertion" enabledByDefault="true" level="WARNING"
+                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.TwistedAssertionInspection"/>
+        <localInspection groupPath="Java" shortName="BogusAssertion" enabledByDefault="true" level="WARNING"
+                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.BogusAssertionInspection"/>
+
        <localInspection groupPath="Java" shortName="JUnitAssertToAssertJ" enabledByDefault="true" level="WARNING"
                         implementationClass="de.platon42.intellij.plugins.cajon.inspections.JUnitAssertToAssertJInspection"/>
    </extensions>
@@ -0,0 +1,6 @@
+<html>
+<body>
+Finds typical copy and paste errors where the assertion will never fail, such as assertThat(foo).isEqualTo(foo), because actual
+and expected expressions are the same.
+</body>
+</html>
@@ -0,0 +1,7 @@
+<html>
+<body>
+Finds assertion method calls that have the expected and actual expressions twisted, such as assertThat(5).isEqualTo(foo).
+<!-- tooltip end -->
+For some obvious cases, a quickfix to swap the actual and expected expressions is provided.
+</body>
+</html>