Extended AssertThatCollectionOrMap inspection for several assertThat(map.get()) cases as suggested by Stefan H.

This commit is contained in:
Chris Hodges 2019-08-23 19:58:11 +02:00
parent 315b93d90d
commit 05dc4905ca
10 changed files with 156 additions and 27 deletions

View File

@ -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()```,

View File

@ -47,6 +47,7 @@ patchPluginXml {
<ul>
<li>Minor fix for highlighting of JoinVarArgsContains inspection.
<li>Extended AssertThatSize inspection to Maps, too.
<li>Extended AssertThatCollectionOrMap inspection for several assertThat(map.get()) cases as suggested by Stefan H.
</ul>
<p>Full changelog available at <a href="https://github.com/chrisly42/cajon-plugin#changelog">Github project site</a>.</p>
"""

View File

@ -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"

View File

@ -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)

View File

@ -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)
}
}
}
}

View File

@ -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)
}
}
}
}

View File

@ -1,8 +1,10 @@
<html>
<body>
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).
<!-- tooltip end -->
<br>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().
</body>
</html>

View File

@ -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")
}
}

View File

@ -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<String, String> 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();

View File

@ -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<String, String> 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();