From e3444db213365088cc4cc7c9f8e509cb559243ad Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Tue, 24 Sep 2019 22:49:14 +0200 Subject: [PATCH] Fixes to AssertThatSize inspection after extending it for Maps in previous release as not all combinations for .hasSameSizeAs() are supported. Bumped to IntelliJ IDEA 2019.2.3. --- README.md | 8 ++-- build.gradle | 6 ++- .../plugins/cajon/AssertJClassNames.kt | 2 + .../inspections/AssertThatSizeInspection.kt | 25 +++++++----- .../AssertThatSizeInspectionTest.kt | 6 +-- .../resources/inspections/Size/SizeAfter.java | 40 +++++++++---------- .../inspections/Size/SizeBefore.java | 40 +++++++++---------- 7 files changed, 69 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index d5ee242..b508f40 100644 --- a/README.md +++ b/README.md @@ -312,8 +312,8 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the from: assertThat("string".length()).isLessThan(1); to: assertThat("string").isEmpty(); - from: assertThat("string".length()).isEqualTo(map.size()) - to: assertThat("string").hasSameSizeAs(map); + from: assertThat(map.size()).isEqualTo(anotherMap.size()) + to: assertThat(map).hasSameSizeAs(anotherMap); from: assertThat("string".length()).hasSize("strong".length()) to: assertThat("string").hasSameSizeAs("strong"); @@ -564,13 +564,15 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo ## Changelog -#### V1.5 (unreleased) +#### V1.5 (24-Sep-19) - 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. +- Fixes to AssertThatSize inspection after extending it for Maps in previous release as not all + combinations for ```.hasSameSizeAs()``` are supported. #### V1.4 (25-Aug-19) - Minor fix for highlighting of JoinVarArgsContains inspection. diff --git a/build.gradle b/build.gradle index 4b00d69..8e40a03 100644 --- a/build.gradle +++ b/build.gradle @@ -35,7 +35,7 @@ compileTestKotlin { kotlinOptions.jvmTarget = "1.8" } intellij { - version '2019.2.2' + version '2019.2.3' // pluginName 'Concise AssertJ Optimizing Nitpicker (Cajon)' updateSinceUntilBuild false plugins = ['java'] @@ -43,7 +43,7 @@ intellij { patchPluginXml { changeNotes """ -

V1.5 (unreleased)

+

V1.5 (24-Sep-19)

Full changelog available at Github project site.

""" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt b/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt index 3c6c86b..1d306d0 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt @@ -42,6 +42,8 @@ class AssertJClassNames { @NonNls const val ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractCharSequenceAssert" @NonNls + const val ABSTRACT_MAP_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractMapAssert" + @NonNls const val ABSTRACT_ITERABLE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractIterableAssert" @NonNls const val ABSTRACT_OPTIONAL_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractOptionalAssert" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspection.kt index 0e97284..eea3631 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspection.kt @@ -5,6 +5,7 @@ import com.intellij.psi.* import de.platon42.intellij.plugins.cajon.* import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_ITERABLE_ASSERT_CLASSNAME +import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.ABSTRACT_MAP_ASSERT_CLASSNAME import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceHasSizeMethodCallQuickFix import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceSizeMethodCallQuickFix @@ -34,7 +35,7 @@ class AssertThatSizeInspection : AbstractAssertJInspection() { && ((psiReferenceExpression.resolve() as? PsiField)?.name == "length")) } - fun getMatch(expression: PsiMethodCallExpression, isForArrayCollectionOrMap: Boolean, isForString: Boolean): Match? { + fun getMatch(expression: PsiMethodCallExpression, isForArrayOrCollection: Boolean, isForMap: Boolean, isForString: Boolean): Match? { val isLastExpression = expression.parent is PsiStatement val constValue = expression.calculateConstantParameterValue(0) if (IS_EQUAL_TO_INT.test(expression)) { @@ -42,10 +43,11 @@ class AssertThatSizeInspection : AbstractAssertJInspection() { Match(expression, MethodNames.IS_EMPTY, noExpectedExpression = true) } else { val equalToExpression = expression.firstArg - val equalsArrayCollectionOrMapSize = isArrayLength(equalToExpression) || - isCollectionSize(equalToExpression) || isMapSize(equalToExpression) - if (isForArrayCollectionOrMap && equalsArrayCollectionOrMapSize || - isForString && (equalsArrayCollectionOrMapSize || isCharSequenceLength(equalToExpression)) + val equalsArrayOrCollectionSize = isArrayLength(equalToExpression) || + isCollectionSize(equalToExpression) + if ((isForArrayOrCollection && equalsArrayOrCollectionSize) + || (isForMap && (equalsArrayOrCollectionSize || isMapSize(equalToExpression))) + || (isForString && (equalsArrayOrCollectionSize || isCharSequenceLength(equalToExpression))) ) { Match(expression, MethodNames.HAS_SAME_SIZE_AS, expectedIsCollection = true) } else { @@ -83,12 +85,13 @@ class AssertThatSizeInspection : AbstractAssertJInspection() { if (!ASSERT_THAT_INT.test(staticMethodCall)) return val actualExpression = staticMethodCall.firstArg - val isForArrayCollectionOrMap = isArrayLength(actualExpression) || isCollectionSize(actualExpression) || isMapSize(actualExpression) + val isForArrayOrCollection = isArrayLength(actualExpression) || isCollectionSize(actualExpression) + val isForMap = isMapSize(actualExpression) val isForString = isCharSequenceLength(actualExpression) - if (!(isForArrayCollectionOrMap || isForString)) return + if (!(isForArrayOrCollection || isForMap || isForString)) return val matches = staticMethodCall.collectMethodCallsUpToStatement() - .mapNotNull { getMatch(it, isForArrayCollectionOrMap, isForString) } + .mapNotNull { getMatch(it, isForArrayOrCollection, isForMap, isForString) } .toList() if (matches.isNotEmpty()) { if (matches.size == 1) { @@ -116,9 +119,11 @@ class AssertThatSizeInspection : AbstractAssertJInspection() { if (!HAS_SIZE.test(expression)) return val actualExpression = expression.firstArg - val isForArrayCollectionOrMap = isArrayLength(actualExpression) || isCollectionSize(actualExpression) || isMapSize(actualExpression) + val isForArrayOrCollection = isArrayLength(actualExpression) || isCollectionSize(actualExpression) + val isForMap = isMapSize(actualExpression) val isForString = isCharSequenceLength(actualExpression) - if (!(isForArrayCollectionOrMap + if (!(isForArrayOrCollection + || (isForMap && checkAssertedType(expression, ABSTRACT_MAP_ASSERT_CLASSNAME)) || (isForString && checkAssertedType(expression, ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME))) ) return diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt index 500b732..c730d3a 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatSizeInspectionTest.kt @@ -24,13 +24,13 @@ internal class AssertThatSizeInspectionTest : AbstractCajonTest() { executeQuickFixes(myFixture, Regex.fromLiteral("Replace isGreaterThanOrEqualTo() with isNotEmpty()"), 5) executeQuickFixes(myFixture, Regex.fromLiteral("Replace isLessThan() with isEmpty()"), 5) executeQuickFixes(myFixture, Regex.fromLiteral("Replace isLessThanOrEqualTo() with isEmpty()"), 5) - executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with hasSameSizeAs()"), 19) - executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with hasSize()"), 11) + executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with hasSameSizeAs()"), 15) + executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with hasSize()"), 15) executeQuickFixes(myFixture, Regex.fromLiteral("Replace isGreaterThan() with hasSizeGreaterThan()"), 5) executeQuickFixes(myFixture, Regex.fromLiteral("Replace isGreaterThanOrEqualTo() with hasSizeGreaterThanOrEqualTo()"), 5) executeQuickFixes(myFixture, Regex.fromLiteral("Replace isLessThan() with hasSizeLessThan()"), 5) executeQuickFixes(myFixture, Regex.fromLiteral("Replace isLessThanOrEqualTo() with hasSizeLessThanOrEqualTo()"), 5) - executeQuickFixes(myFixture, Regex.fromLiteral("Remove size determination of expected expression and replace hasSize() with hasSameSizeAs()"), 21) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove size determination of expected expression and replace hasSize() with hasSameSizeAs()"), 17) myFixture.checkResultByFile("SizeAfter.java") } } \ No newline at end of file diff --git a/src/test/resources/inspections/Size/SizeAfter.java b/src/test/resources/inspections/Size/SizeAfter.java index dc1ebfa..288630e 100644 --- a/src/test/resources/inspections/Size/SizeAfter.java +++ b/src/test/resources/inspections/Size/SizeAfter.java @@ -26,9 +26,9 @@ public class Size { assertThat(list).isEmpty(); assertThat(list).hasSameSizeAs(otherList); assertThat(list).hasSameSizeAs(array); - assertThat(list).hasSize(string.length()); - assertThat(list).hasSize(stringBuilder.length()); - assertThat(list).hasSameSizeAs(map); + assertThat(list).hasSize(string.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(list).hasSize(stringBuilder.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(list).hasSize(map.size()); // does currently not support hasSameSizeAs() in assertj assertThat(list).hasSize(1); assertThat(list).hasSizeGreaterThan(otherList.size() * 2); assertThat(list).hasSizeGreaterThanOrEqualTo(otherList.size() * 2); @@ -36,9 +36,9 @@ public class Size { assertThat(list).hasSizeLessThanOrEqualTo(otherList.size() * 2); assertThat(list).hasSameSizeAs(otherList); assertThat(list).hasSameSizeAs(array); - assertThat(list).hasSize(string.length()); - assertThat(list).hasSize(stringBuilder.length()); - assertThat(list).hasSameSizeAs(map); + assertThat(list).hasSize(string.length()); // currently unsupported in assertj + assertThat(list).hasSize(stringBuilder.length()); // currently unsupported in assertj + assertThat(list).hasSize(map.size()); // currently unsupported in assertj assertThat(array).isEmpty(); assertThat(array).isEmpty(); @@ -49,9 +49,9 @@ public class Size { assertThat(array).isEmpty(); assertThat(array).hasSameSizeAs(list); assertThat(array).hasSameSizeAs(otherArray); - assertThat(array).hasSize(string.length()); - assertThat(array).hasSize(stringBuilder.length()); - assertThat(array).hasSameSizeAs(map); + assertThat(array).hasSize(string.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(array).hasSize(stringBuilder.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(array).hasSize(map.size()); // does currently not support hasSameSizeAs() in assertj assertThat(array).hasSize(1); assertThat(array).hasSizeGreaterThan(otherArray.length - 1); assertThat(array).hasSizeGreaterThanOrEqualTo(otherArray.length + 1); @@ -59,9 +59,9 @@ public class Size { assertThat(array).hasSizeLessThanOrEqualTo(1 - otherArray.length); assertThat(array).hasSameSizeAs(list); assertThat(array).hasSameSizeAs(otherArray); - assertThat(array).hasSize(string.length()); - assertThat(array).hasSize(stringBuilder.length()); - assertThat(array).hasSameSizeAs(map); + assertThat(array).hasSize(string.length()); // currently unsupported in assertj + assertThat(array).hasSize(stringBuilder.length()); // currently unsupported in assertj + assertThat(array).hasSize(map.size()); // currently unsupported in assertj assertThat(string).isEmpty(); assertThat(string).isEmpty(); @@ -74,7 +74,7 @@ public class Size { assertThat(string).hasSameSizeAs(array); assertThat(string).hasSameSizeAs(string); assertThat(string).hasSameSizeAs(stringBuilder); - assertThat(string).hasSameSizeAs(map); + assertThat(string).hasSize(map.size()); // currently unsupported in assertj assertThat(string).hasSize(1); assertThat(string).hasSizeGreaterThan(array.length - 1); assertThat(string).hasSizeGreaterThanOrEqualTo(array.length + 1); @@ -84,7 +84,7 @@ public class Size { assertThat(string).hasSameSizeAs(array); assertThat(string).hasSameSizeAs(string); assertThat(string).hasSameSizeAs(stringBuilder); - assertThat(string).hasSameSizeAs(map); + assertThat(string).hasSize(map.size()); // currently unsupported in assertj assertThat(stringBuilder).isEmpty(); assertThat(stringBuilder).isEmpty(); @@ -97,7 +97,7 @@ public class Size { assertThat(stringBuilder).hasSameSizeAs(array); assertThat(stringBuilder).hasSameSizeAs(string); assertThat(stringBuilder).hasSameSizeAs(stringBuilder); - assertThat(stringBuilder).hasSameSizeAs(map); + assertThat(stringBuilder).hasSize(map.size()); // does currently not support hasSameSizeAs() in assertj assertThat(stringBuilder).hasSize(1); assertThat(stringBuilder).hasSizeGreaterThan(array.length - 1); assertThat(stringBuilder).hasSizeGreaterThanOrEqualTo(array.length + 1); @@ -107,7 +107,7 @@ public class Size { assertThat(stringBuilder).hasSameSizeAs(array); assertThat(stringBuilder).hasSameSizeAs(string); assertThat(stringBuilder).hasSameSizeAs(stringBuilder); - assertThat(stringBuilder).hasSameSizeAs(map); + assertThat(stringBuilder).hasSize(map.size()); // currently unsupported in assertj assertThat(map).isEmpty(); assertThat(map).isEmpty(); @@ -118,8 +118,8 @@ public class Size { assertThat(map).isEmpty(); assertThat(map).hasSameSizeAs(list); assertThat(map).hasSameSizeAs(array); - assertThat(map).hasSize(string.length()); - assertThat(map).hasSize(stringBuilder.length()); + assertThat(map).hasSize(string.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(map).hasSize(stringBuilder.length()); // does currently not support hasSameSizeAs() in assertj assertThat(map).hasSameSizeAs(otherMap); assertThat(map).hasSize(1); assertThat(map).hasSizeGreaterThan(array.length - 1); @@ -128,8 +128,8 @@ public class Size { assertThat(map).hasSizeLessThanOrEqualTo(1 - array.length); assertThat(map).hasSameSizeAs(list); assertThat(map).hasSameSizeAs(array); - assertThat(map).hasSize(string.length()); - assertThat(map).hasSize(stringBuilder.length()); + assertThat(map).hasSize(string.length()); // currently unsupported in assertj + assertThat(map).hasSize(stringBuilder.length()); // currently unsupported in assertj assertThat(map).hasSameSizeAs(otherMap); assertThat(stringBuilder.length()).as("foo").isEqualTo(0).isZero().as("bar").isNotZero().isEqualTo(10); diff --git a/src/test/resources/inspections/Size/SizeBefore.java b/src/test/resources/inspections/Size/SizeBefore.java index 546bc56..f1aabc6 100644 --- a/src/test/resources/inspections/Size/SizeBefore.java +++ b/src/test/resources/inspections/Size/SizeBefore.java @@ -26,9 +26,9 @@ public class Size { assertThat(list.size()).isLessThanOrEqualTo(0); assertThat(list.size()).isEqualTo(otherList.size()); assertThat(list.size()).isEqualTo(array.length); - assertThat(list.size()).isEqualTo(string.length()); - assertThat(list.size()).isEqualTo(stringBuilder.length()); - assertThat(list.size()).isEqualTo(map.size()); + assertThat(list.size()).isEqualTo(string.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(list.size()).isEqualTo(stringBuilder.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(list.size()).isEqualTo(map.size()); // does currently not support hasSameSizeAs() in assertj assertThat(list.size()).isEqualTo(1); assertThat(list.size()).isGreaterThan(otherList.size() * 2); assertThat(list.size()).isGreaterThanOrEqualTo(otherList.size() * 2); @@ -36,9 +36,9 @@ public class Size { assertThat(list.size()).isLessThanOrEqualTo(otherList.size() * 2); assertThat(list).hasSize(otherList.size()); assertThat(list).hasSize(array.length); - assertThat(list).hasSize(string.length()); - assertThat(list).hasSize(stringBuilder.length()); - assertThat(list).hasSize(map.size()); + assertThat(list).hasSize(string.length()); // currently unsupported in assertj + assertThat(list).hasSize(stringBuilder.length()); // currently unsupported in assertj + assertThat(list).hasSize(map.size()); // currently unsupported in assertj assertThat(array.length).isEqualTo(0); assertThat(array.length).isZero(); @@ -49,9 +49,9 @@ public class Size { assertThat(array.length).isLessThanOrEqualTo(0); assertThat(array.length).isEqualTo(list.size()); assertThat(array.length).isEqualTo(otherArray.length); - assertThat(array.length).isEqualTo(string.length()); - assertThat(array.length).isEqualTo(stringBuilder.length()); - assertThat(array.length).isEqualTo(map.size()); + assertThat(array.length).isEqualTo(string.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(array.length).isEqualTo(stringBuilder.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(array.length).isEqualTo(map.size()); // does currently not support hasSameSizeAs() in assertj assertThat(array.length).isEqualTo(1); assertThat(array.length).isGreaterThan(otherArray.length - 1); assertThat(array.length).isGreaterThanOrEqualTo(otherArray.length + 1); @@ -59,9 +59,9 @@ public class Size { assertThat(array.length).isLessThanOrEqualTo(1 - otherArray.length); assertThat(array).hasSize(list.size()); assertThat(array).hasSize(otherArray.length); - assertThat(array).hasSize(string.length()); - assertThat(array).hasSize(stringBuilder.length()); - assertThat(array).hasSize(map.size()); + assertThat(array).hasSize(string.length()); // currently unsupported in assertj + assertThat(array).hasSize(stringBuilder.length()); // currently unsupported in assertj + assertThat(array).hasSize(map.size()); // currently unsupported in assertj assertThat(string.length()).isEqualTo(0); assertThat(string.length()).isZero(); @@ -74,7 +74,7 @@ public class Size { assertThat(string.length()).isEqualTo(array.length); assertThat(string.length()).isEqualTo(string.length()); assertThat(string.length()).isEqualTo(stringBuilder.length()); - assertThat(string.length()).isEqualTo(map.size()); + assertThat(string.length()).isEqualTo(map.size()); // currently unsupported in assertj assertThat(string.length()).isEqualTo(1); assertThat(string.length()).isGreaterThan(array.length - 1); assertThat(string.length()).isGreaterThanOrEqualTo(array.length + 1); @@ -84,7 +84,7 @@ public class Size { assertThat(string).hasSize(array.length); assertThat(string).hasSize(string.length()); assertThat(string).hasSize(stringBuilder.length()); - assertThat(string).hasSize(map.size()); + assertThat(string).hasSize(map.size()); // currently unsupported in assertj assertThat(stringBuilder.length()).isEqualTo(0); assertThat(stringBuilder.length()).isZero(); @@ -97,7 +97,7 @@ public class Size { assertThat(stringBuilder.length()).isEqualTo(array.length); assertThat(stringBuilder.length()).isEqualTo(string.length()); assertThat(stringBuilder.length()).isEqualTo(stringBuilder.length()); - assertThat(stringBuilder.length()).isEqualTo(map.size()); + assertThat(stringBuilder.length()).isEqualTo(map.size()); // does currently not support hasSameSizeAs() in assertj assertThat(stringBuilder.length()).isEqualTo(1); assertThat(stringBuilder.length()).isGreaterThan(array.length - 1); assertThat(stringBuilder.length()).isGreaterThanOrEqualTo(array.length + 1); @@ -107,7 +107,7 @@ public class Size { assertThat(stringBuilder).hasSize(array.length); assertThat(stringBuilder).hasSize(string.length()); assertThat(stringBuilder).hasSize(stringBuilder.length()); - assertThat(stringBuilder).hasSize(map.size()); + assertThat(stringBuilder).hasSize(map.size()); // currently unsupported in assertj assertThat(map.size()).isEqualTo(0); assertThat(map.size()).isZero(); @@ -118,8 +118,8 @@ public class Size { assertThat(map.size()).isLessThanOrEqualTo(0); assertThat(map.size()).isEqualTo(list.size()); assertThat(map.size()).isEqualTo(array.length); - assertThat(map.size()).isEqualTo(string.length()); - assertThat(map.size()).isEqualTo(stringBuilder.length()); + assertThat(map.size()).isEqualTo(string.length()); // does currently not support hasSameSizeAs() in assertj + assertThat(map.size()).isEqualTo(stringBuilder.length()); // does currently not support hasSameSizeAs() in assertj assertThat(map.size()).isEqualTo(otherMap.size()); assertThat(map.size()).isEqualTo(1); assertThat(map.size()).isGreaterThan(array.length - 1); @@ -128,8 +128,8 @@ public class Size { assertThat(map.size()).isLessThanOrEqualTo(1 - array.length); assertThat(map).hasSize(list.size()); assertThat(map).hasSize(array.length); - assertThat(map).hasSize(string.length()); - assertThat(map).hasSize(stringBuilder.length()); + assertThat(map).hasSize(string.length()); // currently unsupported in assertj + assertThat(map).hasSize(stringBuilder.length()); // currently unsupported in assertj assertThat(map).hasSize(otherMap.size()); assertThat(stringBuilder.length()).as("foo").isEqualTo(0).isZero().as("bar").isNotZero().isEqualTo(10);