chrisly42/cajon-plugin
1
0
Fork 0
Code Issues 4 Pull Requests 1 Packages Projects Releases Wiki Activity

Fix for AssertThatCollectionOrMap inspection sometimes causing an index out of bounds exception.

Browse Source 
Added an settings option for AssertThatCollectionOrMap inspection respecting the degenerated case of maps with null values.
Upgrade to JUnit Jupiter 5.5.2.
This commit is contained in:
chrisly42
2019-09-09 23:13:45 +02:00
parent 341d1877df
commit 8d678411b5
13 changed files with 258 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/de/platon42/intellij/plugins/cajon/MethodNames.kt
+4
View File
@@ -102,6 +102,10 @@ class MethodNames {
@NonNls
const val DOES_NOT_CONTAIN_VALUE = "doesNotContainValue"
@NonNls
const val CONTAINS_ENTRY = "containsEntry"
@NonNls
const val DOES_NOT_CONTAIN_ENTRY = "doesNotContainEntry"
@NonNls
const val IS_EQUAL_TO_IC = "isEqualToIgnoringCase"
@NonNls
const val IS_NOT_EQUAL_TO_IC = "isNotEqualToIgnoringCase"
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractAssertJInspection.kt
+14
View File
@@ -227,6 +227,20 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
holder.registerProblem(expression, message, quickfix)
}
protected fun registerMoveOutMethod(
holder: ProblemsHolder,
expression: PsiMethodCallExpression,
oldActualExpression: PsiMethodCallExpression,
replacementMethod: String,
quickFixSupplier: (String) -> List<LocalQuickFix>
) {
val originalMethod = getOriginalMethodName(oldActualExpression) ?: return
val description = MOVE_ACTUAL_EXPRESSION_DESCRIPTION_TEMPLATE.format(originalMethod, replacementMethod)
val message = MOVING_OUT_MESSAGE_TEMPLATE.format(originalMethod)
val quickfixes = quickFixSupplier(description)
holder.registerProblem(expression, message, *quickfixes.toTypedArray())
}
protected fun registerReplaceMethod(
holder: ProblemsHolder,
expression: PsiMethodCallExpression,
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspection.kt
+67 -12
View File
@@ -1,15 +1,22 @@
package de.platon42.intellij.plugins.cajon.inspections
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.openapi.ui.ComboBox
import com.intellij.psi.*
import com.intellij.util.ui.FormBuilder
import com.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
import java.awt.BorderLayout
import javax.swing.JComponent
import javax.swing.JPanel
class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection() {
companion object {
private const val DISPLAY_NAME = "Asserting a collection or map specific expression"
private const val DEFAULT_MAP_VALUES_NEVER_NULL = 1
private val MAP_GET_MATCHER = CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "get").parameterCount(1)
@@ -34,25 +41,25 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
private val MAPPINGS = listOf(
Mapping(
CallMatcher.anyOf(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "isEmpty").parameterCount(0),
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "isEmpty").parameterCount(0)
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, MethodNames.IS_EMPTY).parameterCount(0),
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, MethodNames.IS_EMPTY).parameterCount(0)
),
MethodNames.IS_EMPTY, MethodNames.IS_NOT_EMPTY
),
Mapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "contains").parameterCount(1),
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, MethodNames.CONTAINS).parameterCount(1),
MethodNames.CONTAINS, MethodNames.DOES_NOT_CONTAIN
),
Mapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, "containsAll").parameterCount(1),
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, MethodNames.CONTAINS_ALL).parameterCount(1),
MethodNames.CONTAINS_ALL, null
),
Mapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "containsKey").parameterCount(1),
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, MethodNames.CONTAINS_KEY).parameterCount(1),
MethodNames.CONTAINS_KEY, MethodNames.DOES_NOT_CONTAIN_KEY
),
Mapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "containsValue").parameterCount(1),
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, MethodNames.CONTAINS_VALUE).parameterCount(1),
MethodNames.CONTAINS_VALUE, MethodNames.DOES_NOT_CONTAIN_VALUE
)
)
@@ -60,6 +67,9 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
override fun getDisplayName() = DISPLAY_NAME
@JvmField
var behaviorForMapValueEqualsNull: Int = DEFAULT_MAP_VALUES_NEVER_NULL
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
return object : JavaElementVisitor() {
override fun visitExpressionStatement(statement: PsiExpressionStatement) {
@@ -67,22 +77,53 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
if (!statement.hasAssertThat()) return
val staticMethodCall = statement.findStaticMethodCall() ?: return
val assertThatArgument = staticMethodCall.firstArg as? PsiMethodCallExpression ?: return
val assertThatArgument = staticMethodCall.getArgOrNull(0) as? PsiMethodCallExpression ?: return
val expectedCallExpression = statement.findOutmostMethodCall() ?: 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 ->
if (nullOrNotNull == true) {
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.CONTAINS_KEY) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true)
}
} else if (nullOrNotNull == false) {
when (behaviorForMapValueEqualsNull) {
1 -> // as doesNotContainKey(key)
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.DOES_NOT_CONTAIN_KEY) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true)
}
2 -> // as containsEntry(key, null)
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.CONTAINS_ENTRY) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, keepExpectedAsSecondArgument = true, useNullNonNull = true)
}
3 -> // both
registerMoveOutMethod(
holder,
expectedCallExpression,
assertThatArgument,
MethodNames.DOES_NOT_CONTAIN_KEY + "/" + MethodNames.CONTAINS_ENTRY
) { desc ->
listOf(
MoveOutMethodCallExpressionQuickFix(
"Remove get() of actual expression and use assertThat().doesNotContainKey() instead (regular map)",
MethodNames.DOES_NOT_CONTAIN_KEY,
useNullNonNull = true
),
MoveOutMethodCallExpressionQuickFix(
"Remove get() of actual expression and use assertThat().containsEntry(key, null) instead (degenerated map)",
MethodNames.CONTAINS_ENTRY,
keepExpectedAsSecondArgument = true,
useNullNonNull = true
)
)
}
}
} else {
if (ANY_IS_EQUAL_TO_MATCHER.test(expectedCallExpression)) {
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, "containsEntry") { desc, method ->
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.CONTAINS_ENTRY) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, keepExpectedAsSecondArgument = true)
}
} else if (ANY_IS_NOT_EQUAL_TO_MATCHER.test(expectedCallExpression)) {
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, "doesNotContainEntry") { desc, method ->
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.DOES_NOT_CONTAIN_ENTRY) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, keepExpectedAsSecondArgument = true)
}
}
@@ -102,6 +143,20 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
}
}
override fun createOptionsPanel(): JComponent {
val comboBox = ComboBox(
arrayOf("ignore", "as doesNotContainKey(key)", "as containsEntry(key, null)", "both choices")
)
comboBox.selectedIndex = behaviorForMapValueEqualsNull
comboBox.addActionListener { behaviorForMapValueEqualsNull = comboBox.selectedIndex }
val panel = JPanel(BorderLayout())
panel.add(
FormBuilder.createFormBuilder().addLabeledComponent("Fix get() on maps expecting null values:", comboBox).panel,
BorderLayout.NORTH
)
return panel
}
private class Mapping(
val callMatcher: CallMatcher,
val replacementForTrue: String,
src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/MoveOutMethodCallExpressionQuickFix.kt
+3 -1
View File
@@ -2,6 +2,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.PsiMethodCallExpression
import de.platon42.intellij.plugins.cajon.*
@@ -30,7 +31,8 @@ class MoveOutMethodCallExpressionQuickFix(
if (keepExpectedAsSecondArgument) {
assertExpressionArg ?: return
val secondArg = outmostCallExpression.getArgOrNull(0)?.copy() ?: return
val secondArg =
if (useNullNonNull) JavaPsiFacade.getElementFactory(project).createExpressionFromText("null", null) else outmostCallExpression.getArgOrNull(0)?.copy() ?: return
assertExpression.replace(assertExpression.qualifierExpression)
src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html
+8
View File
@@ -6,5 +6,13 @@ assertThat(map.get(key)).isEqualTo/isNotEqualTo(value) into assertThat(map).cont
<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().
<br>
If you are using degenerated maps in your project that may contain null values (i.e.
map.contains(key) == true AND map.get(key) == null
is valid for some entries in your map), the default behavior of the quickfix
assertThat(map.get(key)).isNull() turning into assertThat(map).doesNotContainKey(key)
is not an equivalent transformation. The settings below can change this behavior to instead transform it into
assertThat(map).containsEntry(key, null) for those cases, create both quickfix choices or simply ignore this
case altogether (if in doubt).
</body>
</html>