From 855fb03f7c703d691b909ac42ff2dd0be0c11a55 Mon Sep 17 00:00:00 2001
From: chrisly42 <chrisly@platon42.de>
Date: Tue, 24 Sep 2019 22:16:11 +0200
Subject: [PATCH] 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).

---
 README.md                                     |  2 ++
 build.gradle                                  |  2 ++
 ...ThatCollectionOrMapExpressionInspection.kt | 17 ++++++++----
 .../AssertThatGuavaOptionalInspection.kt      |  1 +
 ...CollectionOrMapExpressionInspectionTest.kt | 27 +++++++++++++++++--
 5 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/README.md b/README.md
index d0494b8..d5ee242 100644
--- a/README.md
+++ b/README.md
@@ -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.
diff --git a/build.gradle b/build.gradle
index 2230399..4b00d69 100644
--- a/build.gradle
+++ b/build.gradle
@@ -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.
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 c7996b9..155feee 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
@@ -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 }
diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspection.kt
index 1b383fc..66ba8ca 100644
--- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspection.kt
+++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspection.kt
@@ -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 ->
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 7b4fada..e0a0ddd 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
@@ -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")