From 8d678411b5392d9a00d74123510aac44166e2fb6 Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Mon, 9 Sep 2019 23:13:45 +0200 Subject: [PATCH] Fix for AssertThatCollectionOrMap inspection sometimes causing an index out of bounds exception. Added an settings option for AssertThatCollectionOrMap inspection respecting the degenerated case of maps with null values. Upgrade to JUnit Jupiter 5.5.2. --- README.md | 18 ++++- build.gradle | 17 ++-- .../intellij/plugins/cajon/MethodNames.kt | 4 + .../inspections/AbstractAssertJInspection.kt | 14 ++++ ...ThatCollectionOrMapExpressionInspection.kt | 79 ++++++++++++++++--- .../MoveOutMethodCallExpressionQuickFix.kt | 4 +- .../AssertThatCollectionOrMapExpression.html | 8 ++ .../plugins/cajon/AbstractCajonTest.kt | 15 +++- ...CollectionOrMapExpressionInspectionTest.kt | 52 +++++++++++- .../AssertThatGuavaOptionalInspectionTest.kt | 8 +- .../ImplicitAssertionInspectionTest.kt | 6 +- .../JUnitAssertToAssertJInspectionTest.kt | 4 +- ...ctionMapExpressionWithNullValuesAfter.java | 63 +++++++++++++++ 13 files changed, 258 insertions(+), 34 deletions(-) create mode 100644 src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionWithNullValuesAfter.java diff --git a/README.md b/README.md index 24f91df..d0494b8 100644 --- a/README.md +++ b/README.md @@ -247,6 +247,16 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the from: assertThat(map.get(key)).isNull(); to: assertThat(map).doesNotContainKey(key); ``` + + The last transformation is the default, but may not be 100% equivalent depending whether the map + is a degenerated case with ```null``` values, where ```map.get(key)``` returns ```null```, + but ```containsKey(key)``` is ```true```. + For that special case (which usually is the result of a bad design decision!) + the quickfix should rather generate ```assertThat(map).containsEntry(key, null)```. + Therefore, the behavior can be configured in the settings for this inspection to either + create the default case (```doesNotContainKey```), the degenerated case (```containsEntry```), + choosing between both fixes (does not work well for batch processing), or ignore this edge case + altogether (just to be sure to not break any code). - AssertThatEnumerableIsEmpty @@ -554,10 +564,16 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo ## Changelog +#### V1.5 (unreleased) +- Fix for AssertThatCollectionOrMap inspection sometimes causing an index out of bounds exception. +- 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. + #### V1.4 (25-Aug-19) - 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. +- Extended AssertThatCollectionOrMap inspection for several ```assertThat(map.get())``` cases as suggested by Georgij G. #### V1.3 (03-Aug-19) - New JoinVarArgsContains inspection that will detect multiple ```.contains()```, ```.doesNotContain()```, diff --git a/build.gradle b/build.gradle index 6e2f747..2230399 100644 --- a/build.gradle +++ b/build.gradle @@ -7,7 +7,7 @@ plugins { } group 'de.platon42' -version '1.4' +version '1.5' repositories { mavenCentral() @@ -22,8 +22,8 @@ dependencies { implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8" testCompile "org.assertj:assertj-core:3.13.2" testCompile "org.assertj:assertj-guava:3.2.1" - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.5.1' - testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.5.1' + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.5.2' + testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.5.2' testImplementation "org.jetbrains.kotlin:kotlin-test" // testImplementation "org.jetbrains.kotlin:kotlin-test-junit" } @@ -35,7 +35,7 @@ compileTestKotlin { kotlinOptions.jvmTarget = "1.8" } intellij { - version '2019.2.1' + version '2019.2.2' // pluginName 'Concise AssertJ Optimizing Nitpicker (Cajon)' updateSinceUntilBuild false plugins = ['java'] @@ -43,11 +43,12 @@ intellij { patchPluginXml { changeNotes """ -

V1.4 (25-Aug-19)

+

V1.5 (unreleased)

Full changelog available at Github project site.

""" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/MethodNames.kt b/src/main/java/de/platon42/intellij/plugins/cajon/MethodNames.kt index cd8af5d..98dcbe3 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/MethodNames.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/MethodNames.kt @@ -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" 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 30a619c..17f9ed7 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 @@ -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 + ) { + 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, 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 4053af6..c7996b9 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 @@ -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, 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 6028339..c8d3fe5 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 @@ -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) diff --git a/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html b/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html index f0b1a0b..635e6e6 100644 --- a/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html +++ b/src/main/resources/inspectionDescriptions/AssertThatCollectionOrMapExpression.html @@ -6,5 +6,13 @@ assertThat(map.get(key)).isEqualTo/isNotEqualTo(value) into assertThat(map).cont
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(). +
+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). \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt index fe0b0b4..5401d82 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt @@ -1,5 +1,6 @@ package de.platon42.intellij.plugins.cajon +import com.intellij.codeInsight.intention.IntentionAction import com.intellij.pom.java.LanguageLevel import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture import de.platon42.intellij.jupiter.AddLocalJarToModule @@ -21,10 +22,20 @@ import java.lang.reflect.Method abstract class AbstractCajonTest { protected fun executeQuickFixes(myFixture: JavaCodeInsightTestFixture, regex: Regex, expectedFixes: Int) { + val quickfixes = getQuickFixes(myFixture, regex, expectedFixes) + assertThat(quickfixes.groupBy { it.familyName }).hasSize(1) + quickfixes.forEach(myFixture::launchAction) + } + + protected fun executeQuickFixesNoFamilyNameCheck(myFixture: JavaCodeInsightTestFixture, regex: Regex, expectedFixes: Int) { + val quickfixes = getQuickFixes(myFixture, regex, expectedFixes) + quickfixes.forEach(myFixture::launchAction) + } + + protected fun getQuickFixes(myFixture: JavaCodeInsightTestFixture, regex: Regex, expectedFixes: Int): List { val quickfixes = myFixture.getAllQuickFixes().filter { it.text.matches(regex) } assertThat(quickfixes).`as`("Fixes matched by $regex: ${myFixture.getAllQuickFixes().map { it.text }}").hasSize(expectedFixes) - quickfixes.forEach { it.familyName } - quickfixes.forEach(myFixture::launchAction) + return quickfixes } class CutOffFixtureDisplayNameGenerator : DisplayNameGenerator.ReplaceUnderscores() { 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 b455ddc..7b4fada 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 @@ -6,10 +6,10 @@ import de.platon42.intellij.jupiter.TestDataSubPath import de.platon42.intellij.plugins.cajon.AbstractCajonTest import org.junit.jupiter.api.Test +@TestDataSubPath("inspections/CollectionMapExpression") internal class AssertThatCollectionOrMapExpressionInspectionTest : AbstractCajonTest() { @Test - @TestDataSubPath("inspections/CollectionMapExpression") internal fun assertThat_with_certain_Collection_and_Map_methods(@MyFixture myFixture: JavaCodeInsightTestFixture) { myFixture.enableInspections(AssertThatCollectionOrMapExpressionInspection::class.java) myFixture.configureByFile("CollectionMapExpressionBefore.java") @@ -28,4 +28,54 @@ internal class AssertThatCollectionOrMapExpressionInspectionTest : AbstractCajon executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().doesNotContainKey() instead"), 4) myFixture.checkResultByFile("CollectionMapExpressionAfter.java") } + + @Test + internal fun assertThat_with_certain_Collection_and_Map_methods_with_Null_values(@MyFixture myFixture: JavaCodeInsightTestFixture) { + val inspection = AssertThatCollectionOrMapExpressionInspection() + inspection.behaviorForMapValueEqualsNull = 2 + myFixture.enableInspections(inspection) + myFixture.configureByFile("CollectionMapExpressionBefore.java") + executeQuickFixes(myFixture, Regex.fromLiteral("Remove isEmpty() of actual expression and use assertThat().isEmpty() instead"), 4) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove contains() of actual expression and use assertThat().contains() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove containsAll() of actual expression and use assertThat().containsAll() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove containsKey() of actual expression and use assertThat().containsKey() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove containsValue() of actual expression and use assertThat().containsValue() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove isEmpty() of actual expression and use assertThat().isNotEmpty() instead"), 5) + 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"), 6) + 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) + getQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().doesNotContainKey() instead"), 0) + myFixture.checkResultByFile("CollectionMapExpressionWithNullValuesAfter.java") + } + + @Test + internal fun assertThat_with_certain_Collection_and_Map_methods_with_no_quickfixes_for_get_equals_null(@MyFixture myFixture: JavaCodeInsightTestFixture) { + val inspection = AssertThatCollectionOrMapExpressionInspection() + inspection.behaviorForMapValueEqualsNull = 0 + + myFixture.enableInspections(inspection) + myFixture.configureByFile("CollectionMapExpressionBefore.java") + 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 + + myFixture.enableInspections(inspection) + myFixture.configureByFile("CollectionMapExpressionBefore.java") + 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().doesNotContainKey() instead (regular map)"), 4) + getQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().containsEntry(key, null) instead (degenerated map)"), 4) + 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) + } } \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspectionTest.kt index c807283..3b65e35 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspectionTest.kt @@ -31,8 +31,8 @@ internal class AssertThatGuavaOptionalInspectionTest : AbstractCajonTest() { internal fun adds_missing_Guava_import_any_order(@MyFixture myFixture: JavaCodeInsightTestFixture) { myFixture.enableInspections(AssertThatGuavaOptionalInspection::class.java) myFixture.configureByFile("WithoutPriorGuavaImportBefore.java") - executeQuickFixes(myFixture, Regex(".*eplace .* with .*"), 4) - executeQuickFixes(myFixture, Regex("Remove .*"), 3) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex(".*eplace .* with .*"), 4) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Remove .*"), 3) myFixture.checkResultByFile("WithoutPriorGuavaImportAfter.java") } @@ -41,8 +41,8 @@ internal class AssertThatGuavaOptionalInspectionTest : AbstractCajonTest() { myFixture.enableInspections(AssertThatGuavaOptionalInspection::class.java) myFixture.configureByFile("WithoutPriorGuavaImportBefore.java") executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with Guava assertThat().isAbsent()"), 1) - executeQuickFixes(myFixture, Regex(".*eplace .* with .*"), 3) - executeQuickFixes(myFixture, Regex("Remove .*"), 3) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex(".*eplace .* with .*"), 3) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Remove .*"), 3) myFixture.checkResultByFile("WithoutPriorGuavaImportAfter.java") } } \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/ImplicitAssertionInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/ImplicitAssertionInspectionTest.kt index d94da65..3eef2f7 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/ImplicitAssertionInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/ImplicitAssertionInspectionTest.kt @@ -16,9 +16,9 @@ internal class ImplicitAssertionInspectionTest : AbstractCajonTest() { internal fun implicit_assertions_can_be_removed(@MyFixture myFixture: JavaCodeInsightTestFixture) { myFixture.enableInspections(ImplicitAssertionInspection::class.java) myFixture.configureByFile("ImplicitAssertionBefore.java") - executeQuickFixes(myFixture, Regex("Delete implicit isNotNull\\(\\) covered by .*"), 101) - executeQuickFixes(myFixture, Regex("Delete implicit isNotEmpty\\(\\) covered by .*"), 17) - executeQuickFixes(myFixture, Regex("Delete implicit isPresent\\(\\) covered by .*"), 8) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Delete implicit isNotNull\\(\\) covered by .*"), 101) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Delete implicit isNotEmpty\\(\\) covered by .*"), 17) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Delete implicit isPresent\\(\\) covered by .*"), 8) myFixture.checkResultByFile("ImplicitAssertionAfter.java") } } \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/JUnitAssertToAssertJInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/JUnitAssertToAssertJInspectionTest.kt index f2924f9..bbd85d0 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/JUnitAssertToAssertJInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/JUnitAssertToAssertJInspectionTest.kt @@ -17,8 +17,8 @@ internal class JUnitAssertToAssertJInspectionTest : AbstractCajonTest() { internal fun junit_Assertions_can_be_converted_into_AssertJ(@MyFixture myFixture: JavaCodeInsightTestFixture) { myFixture.enableInspections(JUnitAssertToAssertJInspection::class.java) myFixture.configureByFile("JUnitAssertToAssertJInspectionBefore.java") - executeQuickFixes(myFixture, Regex("Convert assert.*\\(\\) to assertThat\\(\\).*"), 48) - executeQuickFixes(myFixture, Regex("Convert assume.*\\(\\) to assumeThat\\(\\).*"), 7) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Convert assert.*\\(\\) to assertThat\\(\\).*"), 48) + executeQuickFixesNoFamilyNameCheck(myFixture, Regex("Convert assume.*\\(\\) to assumeThat\\(\\).*"), 7) myFixture.checkResultByFile("JUnitAssertToAssertJInspectionAfter.java") } } \ No newline at end of file diff --git a/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionWithNullValuesAfter.java b/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionWithNullValuesAfter.java new file mode 100644 index 0000000..69d6d17 --- /dev/null +++ b/src/test/resources/inspections/CollectionMapExpression/CollectionMapExpressionWithNullValuesAfter.java @@ -0,0 +1,63 @@ +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +import java.util.*; + +public class CollectionMapExpression { + + private void collectionMapExpression() { + List stringList = new ArrayList<>(); + List anotherList = new ArrayList<>(); + Map keyValueMap = new HashMap<>(); + + assertThat(stringList).as("foo").isEmpty(); + assertThat(stringList).isEmpty(); + assertThat(stringList).contains("foo"); + assertThat(stringList).contains("foo"); + assertThat(stringList).containsAll(anotherList); + assertThat(stringList).containsAll(anotherList); + + assertThat(stringList).as("foo").isNotEmpty(); + assertThat(stringList).isNotEmpty(); + assertThat(stringList).doesNotContain("foo"); + assertThat(stringList).doesNotContain("foo"); + assertThat(stringList.containsAll(anotherList)).isEqualTo(false); + assertThat(stringList.containsAll(anotherList)).isFalse(); + + assertThat(keyValueMap).as("foo").isEmpty(); + assertThat(keyValueMap).isEmpty(); + assertThat(keyValueMap).containsKey("foo"); + assertThat(keyValueMap).containsKey("foo"); + assertThat(keyValueMap).containsValue(2); + assertThat(keyValueMap).containsValue(2); + + assertThat(keyValueMap).as("foo").isNotEmpty(); + assertThat(keyValueMap).isNotEmpty(); + assertThat(keyValueMap).doesNotContainKey("foo"); + assertThat(keyValueMap).doesNotContainKey("foo"); + assertThat(keyValueMap).doesNotContainValue(2); + assertThat(keyValueMap).doesNotContainValue(2); + + assertThat(keyValueMap).containsEntry("foo", 2); + assertThat(keyValueMap).doesNotContainEntry("foo", 3); + assertThat(keyValueMap).containsEntry("foo", null); + assertThat(keyValueMap).containsEntry("foo", null); + assertThat(keyValueMap).containsKey("foo"); + assertThat(keyValueMap).containsKey("foo"); + + Map stringStringMap = new HashMap<>(); + assertThat(stringStringMap).containsEntry("foo", "bar"); + assertThat(stringStringMap).doesNotContainEntry("foo", "bar"); + assertThat(stringStringMap).containsEntry("foo", null); + assertThat(stringStringMap).containsEntry("foo", null); + 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(); + + org.junit.Assert.assertThat(stringList, null); + fail("oh no!"); + } +}