Added several transformations to AssertThatStringExpression inspection. Specifically, uses of matches(), compareToIgnoreCase(), indexOf(), and trim().

2019-09-30 20:27:43 +02:00
parent 1983750077
commit 6dab8ad552
10 changed files with 239 additions and 10 deletions
@@ -120,6 +120,8 @@ abstract class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool()
        val IS_NOT_NULL = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_NOT_NULL)
            .parameterCount(0)!!

+        val IS_EMPTY = CallMatcher.instanceCall(ENUMERABLE_ASSERT_INTERFACE, MethodNames.IS_EMPTY)
+            .parameterCount(0)!!
        val IS_NOT_EMPTY = CallMatcher.instanceCall(ENUMERABLE_ASSERT_INTERFACE, MethodNames.IS_NOT_EMPTY)
            .parameterCount(0)!!
        val HAS_SIZE = CallMatcher.instanceCall(ENUMERABLE_ASSERT_INTERFACE, MethodNames.HAS_SIZE)
@@ -139,6 +141,14 @@ abstract class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool()
            .parameterCount(0)!!
        val IS_NOT_ZERO = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_NOT_ZERO)
            .parameterCount(0)!!
+        val IS_NEGATIVE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isNegative")
+            .parameterCount(0)!!
+        val IS_NOT_NEGATIVE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isNotNegative")
+            .parameterCount(0)!!
+        val IS_POSITIVE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isPositive")
+            .parameterCount(0)!!
+        val IS_NOT_POSITIVE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isNotPositive")
+            .parameterCount(0)!!

        val IS_TRUE = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, MethodNames.IS_TRUE)
            .parameterCount(0)!!
@@ -24,19 +24,26 @@ abstract class AbstractMoveOutInspection : AbstractAssertJInspection() {
        for (mapping in mappings.filter { it.callMatcher.test(assertThatArgument) }) {
            if (mapping.expectBoolean && ASSERT_THAT_BOOLEAN.test(staticMethodCall)) {
                val expectedBooleanResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: continue
+                if (mapping.additionalCondition?.invoke(statement, expectedCallExpression) == false) continue
                val replacementMethod = if (expectedBooleanResult) mapping.replacementForTrue else mapping.replacementForFalse ?: return
                registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
                    MoveOutMethodCallExpressionQuickFix(desc, method)
                }
            } else if (mapping.expectNullNonNull != null) {
                val expectedNullNonNullResult = expectedCallExpression.getExpectedNullNonNullResult() ?: continue
+                if (mapping.additionalCondition?.invoke(statement, expectedCallExpression) == false) continue
                val replacementMethod = if (expectedNullNonNullResult xor mapping.expectNullNonNull) mapping.replacementForTrue else mapping.replacementForFalse ?: continue
                registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
                    MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true)
                }
            } else if (mapping.expectedMatcher?.test(expectedCallExpression) == true) {
+                if (mapping.additionalCondition?.invoke(statement, expectedCallExpression) == false) continue
                registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, mapping.replacementForTrue) { desc, method ->
-                    MoveOutMethodCallExpressionQuickFix(desc, method, replaceOnlyThisMethod = mapping.expectedMatcher)
+                    MoveOutMethodCallExpressionQuickFix(
+                        desc, method,
+                        replaceOnlyThisMethod = mapping.expectedMatcher,
+                        replaceFromOriginalMethod = mapping.replaceFromOriginalMethod
+                    )
                }
            }
        }
@@ -48,6 +55,8 @@ abstract class AbstractMoveOutInspection : AbstractAssertJInspection() {
        val replacementForFalse: String? = null,
        val expectBoolean: Boolean = false,
        val expectNullNonNull: Boolean? = null,
-        val expectedMatcher: CallMatcher? = null
+        val expectedMatcher: CallMatcher? = null,
+        val replaceFromOriginalMethod: Boolean = false,
+        val additionalCondition: ((PsiExpressionStatement, PsiMethodCallExpression) -> Boolean)? = null
    )
 }
@@ -1,18 +1,25 @@
 package de.platon42.intellij.plugins.cajon.inspections

 import com.intellij.codeInspection.ProblemsHolder
-import com.intellij.psi.CommonClassNames
-import com.intellij.psi.JavaElementVisitor
-import com.intellij.psi.PsiElementVisitor
-import com.intellij.psi.PsiExpressionStatement
+import com.intellij.psi.*
 import com.siyeh.ig.callMatcher.CallMatcher
 import de.platon42.intellij.plugins.cajon.MethodNames
+import de.platon42.intellij.plugins.cajon.calculateConstantValue
+import de.platon42.intellij.plugins.cajon.firstArg

 class AssertThatStringExpressionInspection : AbstractMoveOutInspection() {

    companion object {
        private const val DISPLAY_NAME = "Asserting a string specific expression"

+        private val ARG_IS_ZERO_CONST: (PsiExpressionStatement, PsiMethodCallExpression) -> Boolean = { _, call -> call.firstArg.calculateConstantValue() == 0 }
+        private val ARG_IS_MINUS_ONE_CONST: (PsiExpressionStatement, PsiMethodCallExpression) -> Boolean = { _, call -> call.firstArg.calculateConstantValue() == -1 }
+
+        private val STRING_COMPARE_TO_IGNORE_CASE =
+            CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "compareToIgnoreCase").parameterTypes(CommonClassNames.JAVA_LANG_STRING)
+        private val STRING_INDEX_OF = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "indexOf").parameterTypes(CommonClassNames.JAVA_LANG_STRING)
+        private val STRING_TRIM = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "trim").parameterCount(0)
+
        private val MAPPINGS = listOf(
            MoveOutMapping(
                CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "isEmpty").parameterCount(0),
@@ -40,6 +47,94 @@ class AssertThatStringExpressionInspection : AbstractMoveOutInspection() {
            MoveOutMapping(
                CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "endsWith").parameterTypes(CommonClassNames.JAVA_LANG_STRING),
                MethodNames.ENDS_WITH, MethodNames.DOES_NOT_END_WITH, expectBoolean = true
+            ),
+            MoveOutMapping(
+                CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "matches").parameterTypes(CommonClassNames.JAVA_LANG_STRING),
+                "matches", "doesNotMatch", expectBoolean = true
+            ),
+
+            MoveOutMapping(
+                STRING_COMPARE_TO_IGNORE_CASE,
+                MethodNames.IS_EQUAL_TO_IC, expectedMatcher = IS_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_ZERO_CONST
+            ),
+            MoveOutMapping(
+                STRING_COMPARE_TO_IGNORE_CASE,
+                MethodNames.IS_EQUAL_TO_IC, expectedMatcher = IS_ZERO, replaceFromOriginalMethod = true
+            ),
+            MoveOutMapping(
+                STRING_COMPARE_TO_IGNORE_CASE,
+                MethodNames.IS_NOT_EQUAL_TO_IC, expectedMatcher = IS_NOT_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_ZERO_CONST
+            ),
+            MoveOutMapping(
+                STRING_COMPARE_TO_IGNORE_CASE,
+                MethodNames.IS_NOT_EQUAL_TO_IC, expectedMatcher = IS_NOT_ZERO, replaceFromOriginalMethod = true
+            ),
+
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.STARTS_WITH, expectedMatcher = IS_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_ZERO_CONST
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.STARTS_WITH, expectedMatcher = IS_ZERO, replaceFromOriginalMethod = true
+            ),
+
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.DOES_NOT_START_WITH, expectedMatcher = IS_NOT_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_ZERO_CONST
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.DOES_NOT_START_WITH, expectedMatcher = IS_NOT_ZERO, replaceFromOriginalMethod = true
+            ),
+
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.CONTAINS, expectedMatcher = IS_NOT_NEGATIVE, replaceFromOriginalMethod = true
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.CONTAINS, expectedMatcher = IS_NOT_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_MINUS_ONE_CONST
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.CONTAINS, expectedMatcher = IS_GREATER_THAN_OR_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_ZERO_CONST
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.CONTAINS, expectedMatcher = IS_GREATER_THAN_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_MINUS_ONE_CONST
+            ),
+
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.DOES_NOT_CONTAIN, expectedMatcher = IS_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_MINUS_ONE_CONST
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.DOES_NOT_CONTAIN, expectedMatcher = IS_NEGATIVE, replaceFromOriginalMethod = true
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.DOES_NOT_CONTAIN, expectedMatcher = IS_LESS_THAN_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_ZERO_CONST
+            ),
+            MoveOutMapping(
+                STRING_INDEX_OF,
+                MethodNames.DOES_NOT_CONTAIN, expectedMatcher = IS_LESS_THAN_OR_EQUAL_TO_INT, replaceFromOriginalMethod = true,
+                additionalCondition = ARG_IS_MINUS_ONE_CONST
+            ),
+
+            MoveOutMapping(
+                STRING_TRIM,
+                "isNotBlank", expectedMatcher = IS_NOT_EMPTY
            )
        )
    }
@@ -3,6 +3,7 @@ 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.PsiExpression
 import com.intellij.psi.PsiMethodCallExpression
 import com.siyeh.ig.callMatcher.CallMatcher
 import de.platon42.intellij.plugins.cajon.*
@@ -13,7 +14,8 @@ class MoveOutMethodCallExpressionQuickFix(
    private val useNullNonNull: Boolean = false,
    private val noExpectedExpression: Boolean = false,
    private val keepExpectedAsSecondArgument: Boolean = false,
-    private val replaceOnlyThisMethod: CallMatcher? = null
+    private val replaceOnlyThisMethod: CallMatcher? = null,
+    private val replaceFromOriginalMethod: Boolean = false
 ) :
    AbstractCommonQuickFix(description) {

@@ -29,7 +31,7 @@ class MoveOutMethodCallExpressionQuickFix(
        val outmostCallExpression = descriptor.startElement as? PsiMethodCallExpression ?: return
        val assertThatMethodCall = outmostCallExpression.findStaticMethodCall() ?: return
        val assertExpression = assertThatMethodCall.firstArg as? PsiMethodCallExpression ?: return
-        val assertExpressionArg = if (noExpectedExpression) null else assertExpression.getArgOrNull(0)?.copy()
+        val assertExpressionArg = if (noExpectedExpression) null else assertExpression.getArgOrNull(0)?.copy() as PsiExpression?

        when {
            replaceOnlyThisMethod != null -> {
@@ -41,7 +43,11 @@ class MoveOutMethodCallExpressionQuickFix(

                methodsToFix
                    .forEach {
-                        val expectedExpression = createExpectedMethodCall(it, replacementMethod, *it.argumentList.expressions)
+                        val expectedExpression = createExpectedMethodCall(
+                            it,
+                            replacementMethod,
+                            *if (replaceFromOriginalMethod) arrayOf(assertExpressionArg!!) else it.argumentList.expressions
+                        )
                        expectedExpression.replaceQualifierFromMethodCall(it)
                        it.replace(expectedExpression)
                    }
@@ -2,6 +2,7 @@
 <body>
 Turns assertThat(string.someMethod(arg)).isTrue/isFalse() into assertThat(string).someMethod(arg).
 <!-- tooltip end -->
-<br>someMethod() can be isEmpty(), equals(), equalsIgnoreCase(), contentEquals(), contains(), startsWith(), and endsWith().
+<br>someMethod() can be isEmpty(), equals(), equalsIgnoreCase(), contentEquals(), contains(), startsWith(), endsWith(),
+matches(), compareToIgnoreCase(), indexOf(), and trim().
 </body>
 </html>
@@ -20,6 +20,8 @@ internal class AssertThatStringExpressionInspectionTest : AbstractCajonTest() {
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove contains() of actual expression and use assertThat().contains() instead"), 4)
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove startsWith() of actual expression and use assertThat().startsWith() instead"), 2)
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove endsWith() of actual expression and use assertThat().endsWith() instead"), 2)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove matches() of actual expression and use assertThat().matches() instead"), 2)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove matches() of actual expression and use assertThat().doesNotMatch() instead"), 2)
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove isEmpty() of actual expression and use assertThat().isNotEmpty() instead"), 3)
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove equals() of actual expression and use assertThat().isNotEqualTo() instead"), 2)
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove equalsIgnoreCase() of actual expression and use assertThat().isNotEqualToIgnoringCase() instead"), 2)
@@ -27,6 +29,13 @@ internal class AssertThatStringExpressionInspectionTest : AbstractCajonTest() {
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove contains() of actual expression and use assertThat().doesNotContain() instead"), 4)
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove startsWith() of actual expression and use assertThat().doesNotStartWith() instead"), 2)
        executeQuickFixes(myFixture, Regex.fromLiteral("Remove endsWith() of actual expression and use assertThat().doesNotEndWith() instead"), 3)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareToIgnoreCase() of actual expression and use assertThat().isEqualToIgnoringCase() instead"), 2)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareToIgnoreCase() of actual expression and use assertThat().isNotEqualToIgnoringCase() instead"), 2)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove indexOf() of actual expression and use assertThat().startsWith() instead"), 2)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove indexOf() of actual expression and use assertThat().doesNotStartWith() instead"), 2)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove indexOf() of actual expression and use assertThat().contains() instead"), 4)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove indexOf() of actual expression and use assertThat().doesNotContain() instead"), 4)
+        executeQuickFixes(myFixture, Regex.fromLiteral("Remove trim() of actual expression and use assertThat().isNotBlank() instead"), 1)
        myFixture.checkResultByFile("StringExpressionAfter.java")
    }
 }
@@ -26,6 +26,8 @@ public class StringExpression {
        assertThat(string).startsWith("foo");
        assertThat(string).endsWith("foo");
        assertThat(string).endsWith("foo");
+        assertThat(string).matches("foo");
+        assertThat(string).matches("foo");

        assertThat(string).as("foo").isNotEmpty();
        assertThat(string).isNotEmpty();
@@ -46,11 +48,43 @@ public class StringExpression {
        assertThat(string).doesNotStartWith("foo");
        assertThat(string).doesNotEndWith("foo");
        assertThat(string).doesNotEndWith("foo");
+        assertThat(string).doesNotMatch("foo");
+        assertThat(string).doesNotMatch("foo");

        assertThat(string).as("foo").doesNotEndWith("foo").as("bar").doesNotEndWith("foo");
        assertThat(string.endsWith("foo")).as("foo").isEqualTo(false).as("bar").isTrue();
        assertThat(string.endsWith("foo")).as("foo").satisfies(it -> it.booleanValue()).as("bar").isFalse();

+        assertThat(string).isEqualToIgnoringCase("foo");
+        assertThat(string).isEqualToIgnoringCase("foo");
+        assertThat(string.compareToIgnoreCase("foo")).isEqualTo(1);
+        assertThat(string.compareToIgnoreCase("foo")).isLessThan(0);
+
+        assertThat(string).isNotEqualToIgnoringCase("foo");
+        assertThat(string).isNotEqualToIgnoringCase("foo");
+        assertThat(string.compareToIgnoreCase("foo")).isNotEqualTo(1);
+        assertThat(string.compareToIgnoreCase("foo")).isGreaterThan(0);
+
+        assertThat(string).startsWith("foo");
+        assertThat(string).doesNotStartWith("foo");
+        assertThat(string).startsWith("foo");
+        assertThat(string.indexOf("foo")).isEqualTo(1);
+        assertThat(string).doesNotContain("foo");
+        assertThat(string).doesNotStartWith("foo");
+        assertThat(string.indexOf("foo")).isNotEqualTo(1);
+        assertThat(string).contains("foo");
+        assertThat(string).doesNotContain("foo");
+        assertThat(string).doesNotContain("foo");
+        assertThat(string).contains("foo");
+        assertThat(string).contains("foo");
+        assertThat(string).doesNotContain("foo");
+        assertThat(string.indexOf("foo")).isPositive();
+        assertThat(string).contains("foo");
+        assertThat(string.indexOf("foo")).isNotPositive();
+
+        assertThat(string.trim()).isEmpty(); // would turn into isJavaBlank(), which is deprecated. Should be isNotNull().isBlank() then...
+        assertThat(string).isNotBlank();
+
        org.junit.Assert.assertThat(string, null);
        fail("oh no!");
    }
@@ -26,6 +26,8 @@ public class StringExpression {
        assertThat(string.startsWith("foo")).isTrue();
        assertThat(string.endsWith("foo")).isEqualTo(true);
        assertThat(string.endsWith("foo")).isTrue();
+        assertThat(string.matches("foo")).isEqualTo(true);
+        assertThat(string.matches("foo")).isTrue();

        assertThat(string.isEmpty()).as("foo").isEqualTo(false);
        assertThat(string.isEmpty()).isNotEqualTo(true);
@@ -46,11 +48,43 @@ public class StringExpression {
        assertThat(string.startsWith("foo")).isFalse();
        assertThat(string.endsWith("foo")).isEqualTo(false);
        assertThat(string.endsWith("foo")).isFalse();
+        assertThat(string.matches("foo")).isEqualTo(false);
+        assertThat(string.matches("foo")).isFalse();

        assertThat(string.endsWith("foo")).as("foo").isEqualTo(false).as("bar").isFalse();
        assertThat(string.endsWith("foo")).as("foo").isEqualTo(false).as("bar").isTrue();
        assertThat(string.endsWith("foo")).as("foo").satisfies(it -> it.booleanValue()).as("bar").isFalse();

+        assertThat(string.compareToIgnoreCase("foo")).isEqualTo(0);
+        assertThat(string.compareToIgnoreCase("foo")).isZero();
+        assertThat(string.compareToIgnoreCase("foo")).isEqualTo(1);
+        assertThat(string.compareToIgnoreCase("foo")).isLessThan(0);
+
+        assertThat(string.compareToIgnoreCase("foo")).isNotEqualTo(0);
+        assertThat(string.compareToIgnoreCase("foo")).isNotZero();
+        assertThat(string.compareToIgnoreCase("foo")).isNotEqualTo(1);
+        assertThat(string.compareToIgnoreCase("foo")).isGreaterThan(0);
+
+        assertThat(string.indexOf("foo")).isZero();
+        assertThat(string.indexOf("foo")).isNotZero();
+        assertThat(string.indexOf("foo")).isEqualTo(0);
+        assertThat(string.indexOf("foo")).isEqualTo(1);
+        assertThat(string.indexOf("foo")).isEqualTo(-1);
+        assertThat(string.indexOf("foo")).isNotEqualTo(0);
+        assertThat(string.indexOf("foo")).isNotEqualTo(1);
+        assertThat(string.indexOf("foo")).isNotEqualTo(-1);
+        assertThat(string.indexOf("foo")).isLessThan(0);
+        assertThat(string.indexOf("foo")).isLessThanOrEqualTo(-1);
+        assertThat(string.indexOf("foo")).isGreaterThan(-1);
+        assertThat(string.indexOf("foo")).isGreaterThanOrEqualTo(0);
+        assertThat(string.indexOf("foo")).isNegative();
+        assertThat(string.indexOf("foo")).isPositive();
+        assertThat(string.indexOf("foo")).isNotNegative();
+        assertThat(string.indexOf("foo")).isNotPositive();
+
+        assertThat(string.trim()).isEmpty(); // would turn into isJavaBlank(), which is deprecated. Should be isNotNull().isBlank() then...
+        assertThat(string.trim()).isNotEmpty();
+
        org.junit.Assert.assertThat(string, null);
        fail("oh no!");
    }