diff --git a/README.md b/README.md index cc8784c..f1ed5e7 100644 --- a/README.md +++ b/README.md @@ -232,6 +232,22 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the ``` Analogously with ```isFalse()``` (except for ```containsAll()```). + Additional transformations for maps: + + ``` + from: assertThat(map.get(key)).isEqualTo(value); + to: assertThat(map).containsEntry(key, value); + + from: assertThat(map.get(key)).isNotEqualTo(value); + to: assertThat(map).doesNotContainEntry(key, value); + + from: assertThat(map.get(key)).isNotNull(); + to: assertThat(map).containsKey(key); + + from: assertThat(map.get(key)).isNull(); + to: assertThat(map).doesNotContainKey(key); + ``` + - AssertThatEnumerableIsEmpty Uses ```isEmpty()``` for ```hasSize(0)``` iterable assertions instead. @@ -540,6 +556,7 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo #### V1.4 (unreleased) - Minor fix for highlighting of JoinVarArgsContains inspection. - Extended AssertThatSize inspection to Maps, too. +- Extended AssertThatCollectionOrMap inspection for several ```assertThat(map.get())``` cases as suggested by Stefan H. #### V1.3 (03-Aug-19) - New JoinVarArgsContains inspection that will detect multiple ```.contains()```, ```.doesNotContain()```, diff --git a/build.gradle b/build.gradle index dbee7a8..c3ce8f4 100644 --- a/build.gradle +++ b/build.gradle @@ -47,6 +47,7 @@ patchPluginXml {

Full changelog available at Github project site.

""" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt b/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt index 978c4fb..3c6c86b 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt @@ -30,6 +30,12 @@ class AssertJClassNames { @NonNls const val ABSTRACT_INTEGER_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractIntegerAssert" @NonNls + const val ABSTRACT_LONG_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractLongAssert" + @NonNls + const val ABSTRACT_FLOAT_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractFloatAssert" + @NonNls + const val ABSTRACT_DOUBLE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractDoubleAssert" + @NonNls const val ABSTRACT_COMPARABLE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractComparableAssert" @NonNls const val ABSTRACT_STRING_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractStringAssert" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractAssertJInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractAssertJInspection.kt index 6517d35..30a619c 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractAssertJInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractAssertJInspection.kt @@ -9,10 +9,12 @@ import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.tree.IElementType import com.intellij.psi.util.PsiTypesUtil import com.siyeh.ig.callMatcher.CallMatcher -import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_ASSERT_CLASSNAME import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_BOOLEAN_ASSERT_CLASSNAME import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_COMPARABLE_ASSERT_CLASSNAME +import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_DOUBLE_ASSERT_CLASSNAME +import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_FLOAT_ASSERT_CLASSNAME import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_INTEGER_ASSERT_CLASSNAME +import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_LONG_ASSERT_CLASSNAME import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_STRING_ASSERT_CLASSNAME import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ASSERTIONS_CLASSNAME import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ASSERT_INTERFACE @@ -84,13 +86,30 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() { .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!! val IS_EQUAL_TO_STRING = CallMatcher.instanceCall(ABSTRACT_STRING_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO) .parameterTypes(CommonClassNames.JAVA_LANG_STRING)!! - val IS_NOT_EQUAL_TO_OBJECT = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_NOT_EQUAL_TO) - .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!! + val IS_EQUAL_TO_INT = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO) + .parameterTypes("int")!! + val IS_EQUAL_TO_LONG = CallMatcher.instanceCall(ABSTRACT_LONG_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO) + .parameterTypes("long")!! + val IS_EQUAL_TO_FLOAT = CallMatcher.instanceCall(ABSTRACT_FLOAT_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO) + .parameterTypes("float")!! + val IS_EQUAL_TO_DOUBLE = CallMatcher.instanceCall(ABSTRACT_DOUBLE_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO) + .parameterTypes("double")!! val IS_EQUAL_TO_BOOLEAN = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO) .parameterTypes("boolean")!! - val IS_NOT_EQUAL_TO_BOOLEAN = - CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO) - .parameterTypes("boolean")!! + + val IS_NOT_EQUAL_TO_OBJECT = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_NOT_EQUAL_TO) + .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!! + val IS_NOT_EQUAL_TO_BOOLEAN = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO) + .parameterTypes("boolean")!! + val IS_NOT_EQUAL_TO_INT = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO) + .parameterTypes("int")!! + val IS_NOT_EQUAL_TO_LONG = CallMatcher.instanceCall(ABSTRACT_LONG_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO) + .parameterTypes("long")!! + val IS_NOT_EQUAL_TO_FLOAT = CallMatcher.instanceCall(ABSTRACT_FLOAT_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO) + .parameterTypes("float")!! + val IS_NOT_EQUAL_TO_DOUBLE = CallMatcher.instanceCall(ABSTRACT_DOUBLE_ASSERT_CLASSNAME, MethodNames.IS_NOT_EQUAL_TO) + .parameterTypes("double")!! + val IS_SAME_AS_OBJECT = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_SAME_AS) .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!! val IS_NOT_SAME_AS_OBJECT = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_NOT_SAME_AS) @@ -106,8 +125,6 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() { val HAS_SIZE = CallMatcher.instanceCall(ENUMERABLE_ASSERT_INTERFACE, MethodNames.HAS_SIZE) .parameterTypes("int")!! - val IS_EQUAL_TO_INT = CallMatcher.instanceCall(ABSTRACT_ASSERT_CLASSNAME, MethodNames.IS_EQUAL_TO) - .parameterTypes("int")!! val IS_GREATER_THAN_INT = CallMatcher.instanceCall(ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_GREATER_THAN) .parameterTypes("int")!! val IS_GREATER_THAN_OR_EQUAL_TO_INT = CallMatcher.instanceCall(ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_GREATER_THAN_OR_EQUAL_TO) diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspection.kt index 6bf13e6..4053af6 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspection.kt @@ -11,6 +11,26 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection( companion object { private const val DISPLAY_NAME = "Asserting a collection or map specific expression" + private val MAP_GET_MATCHER = CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "get").parameterCount(1) + + private val ANY_IS_EQUAL_TO_MATCHER = CallMatcher.anyOf( + IS_EQUAL_TO_OBJECT, + IS_EQUAL_TO_STRING, + IS_EQUAL_TO_INT, + IS_EQUAL_TO_LONG, + IS_EQUAL_TO_FLOAT, + IS_EQUAL_TO_DOUBLE, + IS_EQUAL_TO_BOOLEAN + ) + private val ANY_IS_NOT_EQUAL_TO_MATCHER = CallMatcher.anyOf( + IS_NOT_EQUAL_TO_OBJECT, + IS_NOT_EQUAL_TO_INT, + IS_NOT_EQUAL_TO_LONG, + IS_NOT_EQUAL_TO_FLOAT, + IS_NOT_EQUAL_TO_DOUBLE, + IS_NOT_EQUAL_TO_BOOLEAN + ) + private val MAPPINGS = listOf( Mapping( CallMatcher.anyOf( @@ -46,17 +66,37 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection( super.visitExpressionStatement(statement) if (!statement.hasAssertThat()) return val staticMethodCall = statement.findStaticMethodCall() ?: return - if (!ASSERT_THAT_BOOLEAN.test(staticMethodCall)) return val assertThatArgument = staticMethodCall.firstArg as? PsiMethodCallExpression ?: return - val mapping = MAPPINGS.firstOrNull { it.callMatcher.test(assertThatArgument) } ?: return - val expectedCallExpression = statement.findOutmostMethodCall() ?: return - val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return + if (MAP_GET_MATCHER.test(assertThatArgument)) { + val nullOrNotNull = expectedCallExpression.getAllTheSameNullNotNullConstants() + if (nullOrNotNull != null) { + val replacementMethod = nullOrNotNull.map("containsKey", "doesNotContainKey") + registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method -> + MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true) + } + } else { + if (ANY_IS_EQUAL_TO_MATCHER.test(expectedCallExpression)) { + registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, "containsEntry") { desc, method -> + MoveOutMethodCallExpressionQuickFix(desc, method, keepExpectedAsSecondArgument = true) + } + } else if (ANY_IS_NOT_EQUAL_TO_MATCHER.test(expectedCallExpression)) { + registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, "doesNotContainEntry") { desc, method -> + MoveOutMethodCallExpressionQuickFix(desc, method, keepExpectedAsSecondArgument = true) + } + } + } + } else { + if (!ASSERT_THAT_BOOLEAN.test(staticMethodCall)) return + val mapping = MAPPINGS.firstOrNull { it.callMatcher.test(assertThatArgument) } ?: return - val replacementMethod = if (expectedResult) mapping.replacementForTrue else mapping.replacementForFalse ?: return - registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method -> - MoveOutMethodCallExpressionQuickFix(desc, method) + val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return + + val replacementMethod = if (expectedResult) mapping.replacementForTrue else mapping.replacementForFalse ?: return + registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method -> + MoveOutMethodCallExpressionQuickFix(desc, method) + } } } } diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/MoveOutMethodCallExpressionQuickFix.kt b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/MoveOutMethodCallExpressionQuickFix.kt index debfdca..6028339 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/MoveOutMethodCallExpressionQuickFix.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/MoveOutMethodCallExpressionQuickFix.kt @@ -9,7 +9,8 @@ class MoveOutMethodCallExpressionQuickFix( description: String, private val replacementMethod: String, private val useNullNonNull: Boolean = false, - private val noExpectedExpression: Boolean = false + private val noExpectedExpression: Boolean = false, + private val keepExpectedAsSecondArgument: Boolean = false ) : AbstractCommonQuickFix(description) { @@ -27,17 +28,28 @@ class MoveOutMethodCallExpressionQuickFix( val assertExpression = assertThatMethodCall.firstArg as? PsiMethodCallExpression ?: return val assertExpressionArg = if (noExpectedExpression) null else assertExpression.getArgOrNull(0)?.copy() - val methodsToFix = assertThatMethodCall.collectMethodCallsUpToStatement() - .filter { (if (useNullNonNull) it.getExpectedNullNonNullResult() else it.getExpectedBooleanResult()) != null } - .toList() + if (keepExpectedAsSecondArgument) { + assertExpressionArg ?: return + val secondArg = outmostCallExpression.getArgOrNull(0)?.copy() ?: return - assertExpression.replace(assertExpression.qualifierExpression) + assertExpression.replace(assertExpression.qualifierExpression) - methodsToFix - .forEach { - val expectedExpression = createExpectedMethodCall(it, replacementMethod, *listOfNotNull(assertExpressionArg).toTypedArray()) - expectedExpression.replaceQualifierFromMethodCall(it) - it.replace(expectedExpression) - } + val expectedExpression = createExpectedMethodCall(outmostCallExpression, replacementMethod, assertExpressionArg, secondArg) + expectedExpression.replaceQualifierFromMethodCall(outmostCallExpression) + outmostCallExpression.replace(expectedExpression) + } else { + val methodsToFix = assertThatMethodCall.collectMethodCallsUpToStatement() + .filter { (if (useNullNonNull) it.getExpectedNullNonNullResult() else it.getExpectedBooleanResult()) != null } + .toList() + + assertExpression.replace(assertExpression.qualifierExpression) + + methodsToFix + .forEach { + val expectedExpression = createExpectedMethodCall(it, replacementMethod, *listOfNotNull(assertExpressionArg).toTypedArray()) + expectedExpression.replaceQualifierFromMethodCall(it) + it.replace(expectedExpression) + } + } } } \ No newline at end of file diff --git a/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html b/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html index 546567b..f0b1a0b 100644 --- a/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html +++ b/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html @@ -1,8 +1,10 @@ -Turns assertThat(collectionOrMap.someMethod(arg)).isTrue/isFalse() into assertThat(collectionOrMap).someMethod(arg). +Turns assertThat(collectionOrMap.someMethod(arg)).isTrue/isFalse() into assertThat(collectionOrMap).someMethod(arg) and +assertThat(map.get(key)).isEqualTo/isNotEqualTo(value) into assertThat(map).containsEntry(key, value).
someMethod() can be isEmpty(), contains(), and containsAll() for collections and isEmpty(), containsKey(), and containsValue() for maps. +get() may be transformed into containsKey(), doesNotContainKey(), containsEntry() or doesNotContainEntry(). \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspectionTest.kt index 7953087..b455ddc 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspectionTest.kt @@ -22,6 +22,10 @@ internal class AssertThatCollectionOrMapExpressionInspectionTest : AbstractCajon executeQuickFixes(myFixture, Regex.fromLiteral("Remove contains() of actual expression and use assertThat().doesNotContain() instead"), 2) executeQuickFixes(myFixture, Regex.fromLiteral("Remove containsKey() of actual expression and use assertThat().doesNotContainKey() instead"), 2) executeQuickFixes(myFixture, Regex.fromLiteral("Remove containsValue() of actual expression and use assertThat().doesNotContainValue() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().containsEntry() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().doesNotContainEntry() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().containsKey() instead"), 4) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().doesNotContainKey() instead"), 4) myFixture.checkResultByFile("CollectionMapExpressionAfter.java") } } \ No newline at end of file diff --git a/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionAfter.java b/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionAfter.java index 91f3cb5..b4c780f 100644 --- a/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionAfter.java +++ b/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionAfter.java @@ -38,6 +38,21 @@ public class CollectionMapExpression { assertThat(keyValueMap).doesNotContainValue(2); assertThat(keyValueMap).doesNotContainValue(2); + assertThat(keyValueMap).containsEntry("foo", 2); + assertThat(keyValueMap).doesNotContainEntry("foo", 3); + assertThat(keyValueMap).doesNotContainKey("foo"); + assertThat(keyValueMap).doesNotContainKey("foo"); + assertThat(keyValueMap).containsKey("foo"); + assertThat(keyValueMap).containsKey("foo"); + + Map stringStringMap = new HashMap<>(); + assertThat(stringStringMap).containsEntry("foo", "bar"); + assertThat(stringStringMap).doesNotContainEntry("foo", "bar"); + assertThat(stringStringMap).doesNotContainKey("foo"); + assertThat(stringStringMap).doesNotContainKey("foo"); + assertThat(stringStringMap).containsKey("foo"); + assertThat(stringStringMap).containsKey("foo"); + assertThat(stringList).as("foo").isNotEmpty().as("bar").isNotEmpty(); assertThat(stringList.isEmpty()).as("foo").isEqualTo(false).as("bar").isTrue(); assertThat(stringList.isEmpty()).as("foo").satisfies(it -> it.booleanValue()).as("bar").isFalse(); diff --git a/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionBefore.java b/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionBefore.java index cfc5230..1a725bc 100644 --- a/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionBefore.java +++ b/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionBefore.java @@ -38,6 +38,21 @@ public class CollectionMapExpression { assertThat(keyValueMap.containsValue(2)).isEqualTo(false); assertThat(keyValueMap.containsValue(2)).isFalse(); + assertThat(keyValueMap.get("foo")).isEqualTo(2); + assertThat(keyValueMap.get("foo")).isNotEqualTo(3); + assertThat(keyValueMap.get("foo")).isEqualTo(null); + assertThat(keyValueMap.get("foo")).isNull(); + assertThat(keyValueMap.get("foo")).isNotEqualTo(null); + assertThat(keyValueMap.get("foo")).isNotNull(); + + Map stringStringMap = new HashMap<>(); + assertThat(stringStringMap.get("foo")).isEqualTo("bar"); + assertThat(stringStringMap.get("foo")).isNotEqualTo("bar"); + assertThat(stringStringMap.get("foo")).isEqualTo(null); + assertThat(stringStringMap.get("foo")).isNull(); + assertThat(stringStringMap.get("foo")).isNotEqualTo(null); + assertThat(stringStringMap.get("foo")).isNotNull(); + assertThat(stringList.isEmpty()).as("foo").isEqualTo(false).as("bar").isFalse(); assertThat(stringList.isEmpty()).as("foo").isEqualTo(false).as("bar").isTrue(); assertThat(stringList.isEmpty()).as("foo").satisfies(it -> it.booleanValue()).as("bar").isFalse();