One more option for AssertThatCollectionOrMap inspection for warnings without quickfix.

AssertThatGuavaOptional inspections will now avoid conversions from .get() to .contains() for array types (currently not correctly supported by AssertJ-Guava).
This commit is contained in:
Chris Hodges 2019-09-24 22:16:11 +02:00
parent 8d678411b5
commit 855fb03f7c
5 changed files with 42 additions and 7 deletions

View File

@ -566,6 +566,8 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo
#### V1.5 (unreleased)
- Fix for AssertThatCollectionOrMap inspection sometimes causing an index out of bounds exception.
- AssertThatGuavaOptional inspections will now avoid conversions from ```.get()``` to ```.contains()```
for array types (currently not correctly supported by ```contains()``` in AssertJ-Guava).
- Added an settings option for AssertThatCollectionOrMap inspection respecting the degenerated case of maps with ```null``` values.
It is now possible to change the behavior for ```map.get(key) == null```, so it can offer either ```.doesNotContainKey()``` (default)
or ```.containsEntry(key, null)```, or even both.

View File

@ -46,6 +46,8 @@ patchPluginXml {
<h4>V1.5 (unreleased)</h4>
<ul>
<li>Fix for AssertThatCollectionOrMap inspection sometimes causing an index out of bounds exception.
<li>AssertThatGuavaOptional inspections will now avoid conversions from .get() to .contains()
for array types (currently not correctly supported by AssertJ-Guava).
<li>Added an settings option for AssertThatCollectionOrMap inspection respecting the degenerated case of maps with null values.
It is now possible to change the behavior for map.get(key) == null, so it can offer either .doesNotContainKey() (default)
or .containsEntry(key, null), or even both.

View File

@ -16,7 +16,7 @@ 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 const val DEFAULT_MAP_VALUES_NEVER_NULL = 2
private val MAP_GET_MATCHER = CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "get").parameterCount(1)
@ -87,15 +87,22 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
}
} else if (nullOrNotNull == false) {
when (behaviorForMapValueEqualsNull) {
1 -> // as doesNotContainKey(key)
1 -> // warning only
registerMoveOutMethod(
holder,
expectedCallExpression,
assertThatArgument,
""
) { _ -> emptyList() }
2 -> // as doesNotContainKey(key)
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.DOES_NOT_CONTAIN_KEY) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true)
}
2 -> // as containsEntry(key, null)
3 -> // as containsEntry(key, null)
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.CONTAINS_ENTRY) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, keepExpectedAsSecondArgument = true, useNullNonNull = true)
}
3 -> // both
4 -> // both
registerMoveOutMethod(
holder,
expectedCallExpression,
@ -145,7 +152,7 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
override fun createOptionsPanel(): JComponent {
val comboBox = ComboBox(
arrayOf("ignore", "as doesNotContainKey(key)", "as containsEntry(key, null)", "both choices")
arrayOf("ignore", "warning only, no fixes", "as doesNotContainKey(key)", "as containsEntry(key, null)", "both choices")
)
comboBox.selectedIndex = behaviorForMapValueEqualsNull
comboBox.addActionListener { behaviorForMapValueEqualsNull = comboBox.selectedIndex }

View File

@ -29,6 +29,7 @@ class AssertThatGuavaOptionalInspection : AbstractAssertJInspection() {
val actualExpression = staticMethodCall.firstArg as? PsiMethodCallExpression ?: return
val outmostMethodCall = statement.findOutmostMethodCall() ?: return
if (GUAVA_OPTIONAL_GET.test(actualExpression)) {
if (actualExpression.resolveMethod()?.returnType is PsiArrayType) return
val expectedCallExpression = staticMethodCall.gatherAssertionCalls().singleOrNull() ?: return
if (CallMatcher.anyOf(IS_EQUAL_TO_OBJECT, IS_EQUAL_TO_STRING).test(expectedCallExpression)) {
registerMoveOutMethod(holder, outmostMethodCall, actualExpression, MethodNames.CONTAINS) { desc, method ->

View File

@ -4,6 +4,7 @@ import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture
import de.platon42.intellij.jupiter.MyFixture
import de.platon42.intellij.jupiter.TestDataSubPath
import de.platon42.intellij.plugins.cajon.AbstractCajonTest
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
@TestDataSubPath("inspections/CollectionMapExpression")
@ -32,7 +33,7 @@ internal class AssertThatCollectionOrMapExpressionInspectionTest : AbstractCajon
@Test
internal fun assertThat_with_certain_Collection_and_Map_methods_with_Null_values(@MyFixture myFixture: JavaCodeInsightTestFixture) {
val inspection = AssertThatCollectionOrMapExpressionInspection()
inspection.behaviorForMapValueEqualsNull = 2
inspection.behaviorForMapValueEqualsNull = 3
myFixture.enableInspections(inspection)
myFixture.configureByFile("CollectionMapExpressionBefore.java")
executeQuickFixes(myFixture, Regex.fromLiteral("Remove isEmpty() of actual expression and use assertThat().isEmpty() instead"), 4)
@ -64,10 +65,32 @@ internal class AssertThatCollectionOrMapExpressionInspectionTest : AbstractCajon
getQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().doesNotContainKey() instead"), 0)
}
@Test
internal fun assertThat_with_certain_Collection_and_Map_methods_with_only_warnings_for_get_equals_null(@MyFixture myFixture: JavaCodeInsightTestFixture) {
val inspection = AssertThatCollectionOrMapExpressionInspection()
inspection.behaviorForMapValueEqualsNull = 1
myFixture.enableInspections(inspection)
myFixture.configureByFile("CollectionMapExpressionBefore.java")
val highlights = myFixture.doHighlighting()
.asSequence()
.filter { it.description == "Moving get() expression out of assertThat() would be more concise" }
.filter {
it.quickFixActionRanges?.any { innerit -> innerit.first.action.text.contains("Inspection 'Asserting a collection or map specific expression") } ?: true
}
.toList()
assertThat(highlights).hasSize(4)
getQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().containsEntry() instead"), 2)
getQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().doesNotContainEntry() instead"), 2)
getQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().containsKey() instead"), 4)
getQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().doesNotContainKey() instead"), 0)
}
@Test
internal fun assertThat_with_certain_Collection_and_Map_methods_with_both_quickfixes_for_get_equals_null(@MyFixture myFixture: JavaCodeInsightTestFixture) {
val inspection = AssertThatCollectionOrMapExpressionInspection()
inspection.behaviorForMapValueEqualsNull = 3
inspection.behaviorForMapValueEqualsNull = 4
myFixture.enableInspections(inspection)
myFixture.configureByFile("CollectionMapExpressionBefore.java")