From 5113cc15ab47a1168a07e9d277dc7f22d955fb09 Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Mon, 18 Nov 2019 21:13:30 +0100 Subject: [PATCH] Added hasSize(), isEmpty() and isNotEmpty() for AssertThatFileExpression when using AssertJ >= 3.14.0. --- README.md | 16 +++++++ build.gradle | 1 + .../plugins/cajon/AssertJClassNames.kt | 2 + .../inspections/AbstractAssertJInspection.kt | 8 +++- .../inspections/AbstractMoveOutInspection.kt | 4 +- .../AssertThatComparableInspection.kt | 4 +- .../AssertThatFileExpressionInspection.kt | 48 +++++++++++++++---- .../inspections/AssertThatSizeInspection.kt | 4 +- .../AssertThatStringExpressionInspection.kt | 8 ++-- .../MoveOutMethodCallExpressionQuickFix.kt | 2 +- .../AssertThatFileExpressionInspectionTest.kt | 3 ++ .../FileExpression/FileExpressionAfter.java | 6 +++ .../FileExpression/FileExpressionBefore.java | 6 +++ 13 files changed, 92 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 79441b2..78c7958 100644 --- a/README.md +++ b/README.md @@ -362,6 +362,21 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the to: assertThat(file).isNotEmptyDirectory(); ``` + and additionally with AssertJ 3.14.0 or later + + ``` + from: assertThat(file.length()).isEqualTo(0); + from: assertThat(file.length()).isZero(); + to: assertThat(file).isEmpty(); + + from: assertThat(file.length()).isNotEqualTo(0); + from: assertThat(file.length()).isNotZero(); + to: assertThat(file).isNotEmpty(); + + from: assertThat(file.length()).isEqualTo(len); + to: assertThat(file).hasSize(len); + ``` + - AssertThatPathExpression Moves ```Path``` method calls inside ```assertThat()``` out. @@ -701,6 +716,7 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo - Added first version of AssertThatPathExpression for a limited number transformations (more stuff is possible, but requires detection and transformation of static ```Files```-methods). - Added AssertThatComparableExpression for funny ```compareTo()``` uses. +- Added ```hasSize(), isEmpty()``` and ```isNotEmpty()``` for AssertThatFileExpression when using AssertJ >= 3.14.0. #### V1.6 (30-Sep-19) - Really fixed AssertThatGuavaOptional inspections to avoid conversions from ```.get()``` to ```.contains()``` diff --git a/build.gradle b/build.gradle index 3f3766d..b6f8f26 100644 --- a/build.gradle +++ b/build.gradle @@ -49,6 +49,7 @@ patchPluginXml {
  • Added first version of AssertThatPathExpression for a limited number transformations (more stuff is possible, but requires detection and transformation of static Files-methods).
  • Added AssertThatComparableExpression for funny compareTo() uses. +
  • Added hasSize(), isEmpty() and isNotEmpty() for AssertThatFileExpression when using AssertJ >= 3.14.0.

    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 3b93318..c780987 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/AssertJClassNames.kt @@ -48,6 +48,8 @@ class AssertJClassNames { @NonNls const val ABSTRACT_ITERABLE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractIterableAssert" @NonNls + const val ABSTRACT_FILE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractFileAssert" + @NonNls const val ABSTRACT_OPTIONAL_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractOptionalAssert" @NonNls const val EXTRACTORS_CLASSNAME = "org.assertj.core.extractor.Extractors" 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 1f963de..67b7745 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 @@ -137,9 +137,13 @@ abstract class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() val IS_LESS_THAN_OR_EQUAL_TO_INT = CallMatcher.instanceCall(ABSTRACT_COMPARABLE_ASSERT_CLASSNAME, MethodNames.IS_LESS_THAN_OR_EQUAL_TO) .parameterTypes("int")!! - val IS_ZERO = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_ZERO) + val IS_ZERO_INT = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_ZERO) .parameterCount(0)!! - val IS_NOT_ZERO = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_NOT_ZERO) + val IS_NOT_ZERO_INT = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_NOT_ZERO) + .parameterCount(0)!! + val IS_ZERO_LONG = CallMatcher.instanceCall(ABSTRACT_LONG_ASSERT_CLASSNAME, MethodNames.IS_ZERO) + .parameterCount(0)!! + val IS_NOT_ZERO_LONG = CallMatcher.instanceCall(ABSTRACT_LONG_ASSERT_CLASSNAME, MethodNames.IS_NOT_ZERO) .parameterCount(0)!! val IS_ONE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isOne") .parameterCount(0)!! diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractMoveOutInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractMoveOutInspection.kt index 10aae5c..b865adb 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractMoveOutInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractMoveOutInspection.kt @@ -42,7 +42,8 @@ abstract class AbstractMoveOutInspection : AbstractAssertJInspection() { MoveOutMethodCallExpressionQuickFix( desc, method, replaceOnlyThisMethod = mapping.expectedMatcher, - replaceFromOriginalMethod = mapping.replaceFromOriginalMethod + replaceFromOriginalMethod = mapping.replaceFromOriginalMethod, + noExpectedExpression = mapping.noExpectedExpression ) } } @@ -57,6 +58,7 @@ abstract class AbstractMoveOutInspection : AbstractAssertJInspection() { val expectNullNonNull: Boolean? = null, val expectedMatcher: CallMatcher? = null, val replaceFromOriginalMethod: Boolean = false, + val noExpectedExpression: Boolean = false, val additionalCondition: ((PsiExpressionStatement, PsiMethodCallExpression) -> Boolean)? = null ) } \ No newline at end of file diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatComparableInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatComparableInspection.kt index 163dcdb..6b6d117 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatComparableInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatComparableInspection.kt @@ -27,7 +27,7 @@ class AssertThatComparableInspection : AbstractMoveOutInspection() { ), MoveOutMapping( COMPARABLE_COMPARE_TO, - "isEqualByComparingTo", expectedMatcher = IS_ZERO, replaceFromOriginalMethod = true + "isEqualByComparingTo", expectedMatcher = IS_ZERO_INT, replaceFromOriginalMethod = true ), MoveOutMapping( @@ -37,7 +37,7 @@ class AssertThatComparableInspection : AbstractMoveOutInspection() { ), MoveOutMapping( COMPARABLE_COMPARE_TO, - "isNotEqualByComparingTo", expectedMatcher = IS_NOT_ZERO, replaceFromOriginalMethod = true + "isNotEqualByComparingTo", expectedMatcher = IS_NOT_ZERO_INT, replaceFromOriginalMethod = true ), MoveOutMapping( diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspection.kt index cac70e6..4870d8f 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspection.kt @@ -1,13 +1,9 @@ package de.platon42.intellij.plugins.cajon.inspections import com.intellij.codeInspection.ProblemsHolder -import com.intellij.psi.CommonClassNames -import com.intellij.psi.JavaElementVisitor -import com.intellij.psi.PsiElementVisitor -import com.intellij.psi.PsiExpressionStatement +import com.intellij.psi.* import com.siyeh.ig.callMatcher.CallMatcher -import de.platon42.intellij.plugins.cajon.AssertJClassNames -import de.platon42.intellij.plugins.cajon.MethodNames +import de.platon42.intellij.plugins.cajon.* class AssertThatFileExpressionInspection : AbstractMoveOutInspection() { @@ -15,6 +11,13 @@ class AssertThatFileExpressionInspection : AbstractMoveOutInspection() { companion object { private const val DISPLAY_NAME = "Asserting a file specific expression" + private val ARG_IS_ZERO_CONST: (PsiExpressionStatement, PsiMethodCallExpression) -> Boolean = { _, call -> call.firstArg.calculateConstantValue() == 0 } + private val ARG_IS_NOT_ZERO_CONST: (PsiExpressionStatement, PsiMethodCallExpression) -> Boolean = { _, call -> + val constant = + call.firstArg.calculateConstantValue() + (constant != null) && (constant != 0) + } + private val MAPPINGS = listOf( MoveOutMapping( CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "canRead").parameterCount(0), @@ -63,13 +66,39 @@ class AssertThatFileExpressionInspection : AbstractMoveOutInspection() { CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "list", "listFiles").parameterCount(0), "isEmptyDirectory", expectedMatcher = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME, MethodNames.IS_EMPTY) - .parameterCount(0)!! + .parameterCount(0) ), MoveOutMapping( CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "list", "listFiles").parameterCount(0), "isNotEmptyDirectory", expectedMatcher = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME, MethodNames.IS_NOT_EMPTY) - .parameterCount(0)!! + .parameterCount(0) + ) + ) + + private val MAPPINGS_SINCE_ASSERTJ_3_14_0 = listOf( + MoveOutMapping( + CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "length").parameterCount(0), + "isEmpty", expectedMatcher = IS_ZERO_LONG, noExpectedExpression = true + ), + MoveOutMapping( + CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "length").parameterCount(0), + "isEmpty", expectedMatcher = IS_EQUAL_TO_LONG, noExpectedExpression = true, + additionalCondition = ARG_IS_ZERO_CONST + ), + MoveOutMapping( + CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "length").parameterCount(0), + "isNotEmpty", expectedMatcher = IS_NOT_ZERO_LONG, noExpectedExpression = true + ), + MoveOutMapping( + CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "length").parameterCount(0), + "isNotEmpty", expectedMatcher = IS_NOT_EQUAL_TO_LONG, noExpectedExpression = true, + additionalCondition = ARG_IS_ZERO_CONST + ), + MoveOutMapping( + CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "length").parameterCount(0), + "hasSize", expectedMatcher = IS_EQUAL_TO_LONG, + additionalCondition = ARG_IS_NOT_ZERO_CONST ) ) } @@ -81,6 +110,9 @@ class AssertThatFileExpressionInspection : AbstractMoveOutInspection() { override fun visitExpressionStatement(statement: PsiExpressionStatement) { super.visitExpressionStatement(statement) createInspectionsForMappings(statement, holder, MAPPINGS) + if (hasAssertJMethod(statement, AssertJClassNames.ABSTRACT_FILE_ASSERT_CLASSNAME, MethodNames.HAS_SIZE)) { + createInspectionsForMappings(statement, holder, MAPPINGS_SINCE_ASSERTJ_3_14_0) + } } } } 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 eea3631..cfa21f6 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 @@ -57,10 +57,10 @@ class AssertThatSizeInspection : AbstractAssertJInspection() { } else { val isTestForEmpty = ((IS_LESS_THAN_OR_EQUAL_TO_INT.test(expression) && (constValue == 0)) || (IS_LESS_THAN_INT.test(expression) && (constValue == 1)) - || IS_ZERO.test(expression)) + || IS_ZERO_INT.test(expression)) val isTestForNotEmpty = ((IS_GREATER_THAN_INT.test(expression) && (constValue == 0)) || (IS_GREATER_THAN_OR_EQUAL_TO_INT.test(expression) && (constValue == 1)) - || IS_NOT_ZERO.test(expression)) + || IS_NOT_ZERO_INT.test(expression)) if ((isTestForEmpty && isLastExpression) || isTestForNotEmpty) { val replacementMethod = isTestForEmpty.map(MethodNames.IS_EMPTY, MethodNames.IS_NOT_EMPTY) return Match(expression, replacementMethod, noExpectedExpression = true) diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringExpressionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringExpressionInspection.kt index 479c72d..f81fa20 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringExpressionInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringExpressionInspection.kt @@ -60,7 +60,7 @@ class AssertThatStringExpressionInspection : AbstractMoveOutInspection() { ), MoveOutMapping( STRING_COMPARE_TO_IGNORE_CASE, - MethodNames.IS_EQUAL_TO_IC, expectedMatcher = IS_ZERO, replaceFromOriginalMethod = true + MethodNames.IS_EQUAL_TO_IC, expectedMatcher = IS_ZERO_INT, replaceFromOriginalMethod = true ), MoveOutMapping( STRING_COMPARE_TO_IGNORE_CASE, @@ -69,7 +69,7 @@ class AssertThatStringExpressionInspection : AbstractMoveOutInspection() { ), MoveOutMapping( STRING_COMPARE_TO_IGNORE_CASE, - MethodNames.IS_NOT_EQUAL_TO_IC, expectedMatcher = IS_NOT_ZERO, replaceFromOriginalMethod = true + MethodNames.IS_NOT_EQUAL_TO_IC, expectedMatcher = IS_NOT_ZERO_INT, replaceFromOriginalMethod = true ), MoveOutMapping( @@ -79,7 +79,7 @@ class AssertThatStringExpressionInspection : AbstractMoveOutInspection() { ), MoveOutMapping( STRING_INDEX_OF, - MethodNames.STARTS_WITH, expectedMatcher = IS_ZERO, replaceFromOriginalMethod = true + MethodNames.STARTS_WITH, expectedMatcher = IS_ZERO_INT, replaceFromOriginalMethod = true ), MoveOutMapping( @@ -89,7 +89,7 @@ class AssertThatStringExpressionInspection : AbstractMoveOutInspection() { ), MoveOutMapping( STRING_INDEX_OF, - MethodNames.DOES_NOT_START_WITH, expectedMatcher = IS_NOT_ZERO, replaceFromOriginalMethod = true + MethodNames.DOES_NOT_START_WITH, expectedMatcher = IS_NOT_ZERO_INT, replaceFromOriginalMethod = true ), MoveOutMapping( 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 4b59fb4..6fe3dd8 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 @@ -46,7 +46,7 @@ class MoveOutMethodCallExpressionQuickFix( val expectedExpression = createExpectedMethodCall( it, replacementMethod, - *if (replaceFromOriginalMethod) arrayOf(assertExpressionArg!!) else it.argumentList.expressions + *if (replaceFromOriginalMethod || noExpectedExpression) listOfNotNull(assertExpressionArg).toTypedArray() else it.argumentList.expressions ) expectedExpression.replaceQualifierFromMethodCall(it) it.replace(expectedExpression) diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspectionTest.kt index 698974a..d729ea7 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatFileExpressionInspectionTest.kt @@ -30,6 +30,9 @@ internal class AssertThatFileExpressionInspectionTest : AbstractCajonTest() { executeQuickFixes(myFixture, Regex.fromLiteral("Remove listFiles() of actual expression and use assertThat().isNotEmptyDirectory() instead"), 1) executeQuickFixes(myFixture, Regex.fromLiteral("Remove list() of actual expression and use assertThat().isEmptyDirectory() instead"), 1) executeQuickFixes(myFixture, Regex.fromLiteral("Remove list() of actual expression and use assertThat().isNotEmptyDirectory() instead"), 1) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove length() of actual expression and use assertThat().isEmpty() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove length() of actual expression and use assertThat().isNotEmpty() instead"), 2) + executeQuickFixes(myFixture, Regex.fromLiteral("Remove length() of actual expression and use assertThat().hasSize() instead"), 1) myFixture.checkResultByFile("FileExpressionAfter.java") } } \ No newline at end of file diff --git a/src/test/resources/inspections/FileExpression/FileExpressionAfter.java b/src/test/resources/inspections/FileExpression/FileExpressionAfter.java index 9433649..ad19462 100644 --- a/src/test/resources/inspections/FileExpression/FileExpressionAfter.java +++ b/src/test/resources/inspections/FileExpression/FileExpressionAfter.java @@ -75,6 +75,12 @@ public class FileExpression { assertThat(file.getParentFile()).isNotEqualTo(null); assertThat(file.getParentFile()).isNotNull(); + assertThat(file).isEmpty(); + assertThat(file).isEmpty(); + assertThat(file).isNotEmpty(); + assertThat(file).isNotEmpty(); + assertThat(file).hasSize(2); + assertThat(file.listFiles()).isNull(); assertThat(file.listFiles()).isNullOrEmpty(); assertThat(file).isEmptyDirectory(); diff --git a/src/test/resources/inspections/FileExpression/FileExpressionBefore.java b/src/test/resources/inspections/FileExpression/FileExpressionBefore.java index 88176ce..02c0789 100644 --- a/src/test/resources/inspections/FileExpression/FileExpressionBefore.java +++ b/src/test/resources/inspections/FileExpression/FileExpressionBefore.java @@ -75,6 +75,12 @@ public class FileExpression { assertThat(file.getParentFile()).isNotEqualTo(null); assertThat(file.getParentFile()).isNotNull(); + assertThat(file.length()).isEqualTo(0); + assertThat(file.length()).isZero(); + assertThat(file.length()).isNotEqualTo(0); + assertThat(file.length()).isNotZero(); + assertThat(file.length()).isEqualTo(2); + assertThat(file.listFiles()).isNull(); assertThat(file.listFiles()).isNullOrEmpty(); assertThat(file.listFiles()).isEmpty();