diff --git a/README.md b/README.md index 48ac224..0ddab9f 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,21 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the ``` Analogously with ```isFalse()```. +- AssertThatObjectExpression + + Handles equals(), toString() and hashCode() inside an expected expression. + + ``` + from: assertThat(objActual.equals(objExpected)).isTrue(); + to: assertThat(objActual).isEqualTo(objExpected); + + from: assertThat(objActual.toString()).isEqualTo(stringExpected); + to: assertThat(objActual).hasToString(stringExpected); + + from: assertThat(objActual.hashCode()).isEqualTo(objExpected.hashCode()); + to: assertThat(objActual).hasSameHashCodeAs(objExpected); + ``` + - AssertThatCollectionOrMapExpression Moves collection and map operations inside ```assertThat()``` out. @@ -273,9 +288,6 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the from: assertThat(null == objActual).isFalse(); to: assertThat(objActual).isNotNull(); - - from: assertThat(objActual.equals(objExpected).isTrue(); - to: assertThat(objActual).isEqualTo(objExpected); ``` ...and many, many more combinations (more than 150). @@ -494,9 +506,9 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo ## Planned features - Joining .contains() expressions -- Removing .isPresent().contains() combinations for Optionals - Converting ```foo.compareTo(bar) == 0``` to ```isEqualTo()``` (yes, I've *really* seen code like that) - Extraction with property names to lambda with Java 8 + ``` from: assertThat(object).extracting("propOne", "propNoGetter", "propTwo.innerProp")... to: assertThat(object).extracting(type::getPropOne, it -> it.propNoGetter, it -> it.getPropTwo().getInnerProp())... @@ -504,13 +516,15 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo ## Changelog -#### V1.1 (unreleased) +#### V1.1 (09-Jun-19) - Improved JoinAssertThatStatements detection of expressions with side-effects and added pre/post-increment/decrement detection. - Added Guava Optional ```opt.orNull() == null``` case. You know, I'm not making this stuff up, people actually write this kind of code. - Added Java 8 Optional ```opt.orElse(null) == null``` case, too. -- Extended JUnitAssertToAssertJ inspection to convert JUnit ```assume```-Statements, too. +- Extended JUnitAssertToAssertJ inspection to convert JUnit ```assume``` statements, too. - Improved JUnitAssertToAssertJ quick fix to swap expected and actual expressions if the actual one is a constant. - New ImplicitAssertion inspection for implicit ```isNotNull()```, ```isNotEmpty()``` and ```isPresent()``` assertions that will be covered by chained assertions. +- Fix for multiple JUnit Conversions in batch mode with and without delta creating an exception. +- Added new AssertThatObjectExpression inspection for ```toString()``` and ```hashCode()``` and moved ```equals()``` from AssertThatBinaryExpression there. #### V1.0 (06-May-19) - First release to be considered stable enough for production use. diff --git a/build.gradle b/build.gradle index dda7cf8..e9c106f 100644 --- a/build.gradle +++ b/build.gradle @@ -42,14 +42,16 @@ intellij { patchPluginXml { changeNotes """ -
Full changelog available at Github project site.
""" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBinaryExpressionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBinaryExpressionInspection.kt index b69853a..a87ff20 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBinaryExpressionInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBinaryExpressionInspection.kt @@ -5,15 +5,14 @@ import com.intellij.codeInspection.ProblemsHolder import com.intellij.psi.* import com.intellij.psi.util.TypeConversionUtil import de.platon42.intellij.plugins.cajon.* -import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix import de.platon42.intellij.plugins.cajon.quickfixes.SplitBinaryExpressionMethodCallQuickFix class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() { companion object { private const val DISPLAY_NAME = "Asserting a binary expression" - private const val SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE = "Split %s expression out of assertThat()" - private const val MORE_MEANINGFUL_MESSAGE_TEMPLATE = "Moving %s expression out of assertThat() would be more meaningful" + private const val SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE = "Split binary expression out of assertThat()" + private const val MORE_MEANINGFUL_MESSAGE_TEMPLATE = "Moving binary expression out of assertThat() would be more meaningful" } override fun getDisplayName() = DISPLAY_NAME @@ -33,16 +32,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() { val expectedCallExpression = statement.findOutmostMethodCall() ?: return val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return - val assertThatArgument = staticMethodCall.firstArg - if (assertThatArgument is PsiMethodCallExpression && OBJECT_EQUALS.test(assertThatArgument)) { - val replacementMethod = expectedResult.map(MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO) - registerSplitMethod(holder, expectedCallExpression, "${MethodNames.EQUALS}()", replacementMethod) { desc, method -> - MoveOutMethodCallExpressionQuickFix(desc, method) - } - return - } - - val binaryExpression = assertThatArgument as? PsiBinaryExpression ?: return + val binaryExpression = staticMethodCall.firstArg as? PsiBinaryExpression ?: return val leftType = binaryExpression.lOperand.type ?: return val rightType = binaryExpression.rOperand?.type ?: return @@ -53,7 +43,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() { return } else if (isLeftNull || isRightNull) { val replacementMethod = expectedResult.map(MethodNames.IS_NULL, MethodNames.IS_NOT_NULL) - registerSplitMethod(holder, expectedCallExpression, "binary", replacementMethod) { desc, method -> + registerSplitMethod(holder, expectedCallExpression, replacementMethod) { desc, method -> SplitBinaryExpressionMethodCallQuickFix(desc, method, pickRightOperand = isLeftNull, noExpectedExpression = true) } return @@ -75,7 +65,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() { (isPrimitive || isNumericType).map(TOKEN_TO_ASSERTJ_FOR_PRIMITIVE_MAP, TOKEN_TO_ASSERTJ_FOR_OBJECT_MAPPINGS) val replacementMethod = mappingToUse[tokenType] ?: return - registerSplitMethod(holder, expectedCallExpression, "binary", replacementMethod) { desc, method -> + registerSplitMethod(holder, expectedCallExpression, replacementMethod) { desc, method -> SplitBinaryExpressionMethodCallQuickFix(desc, method, pickRightOperand = swapExpectedAndActual) } } @@ -85,13 +75,10 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() { private fun registerSplitMethod( holder: ProblemsHolder, expression: PsiMethodCallExpression, - type: String, replacementMethod: String, quickFixSupplier: (String, String) -> LocalQuickFix ) { - val description = SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE.format(type) - val message = MORE_MEANINGFUL_MESSAGE_TEMPLATE.format(type) - val quickfix = quickFixSupplier(description, replacementMethod) - holder.registerProblem(expression, message, quickfix) + val quickfix = quickFixSupplier(SPLIT_EXPRESSION_DESCRIPTION_TEMPLATE, replacementMethod) + holder.registerProblem(expression, MORE_MEANINGFUL_MESSAGE_TEMPLATE, quickfix) } } \ No newline at end of file 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 329fbde..36207d1 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 @@ -34,11 +34,6 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection( Mapping( CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "containsValue").parameterCount(1), MethodNames.CONTAINS_VALUE, MethodNames.DOES_NOT_CONTAIN_VALUE - ), - // TODO not quite a collection or map expression, maybe move this out to new inspection together with equals and hashcode. - Mapping( - CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "toString").parameterCount(0), - MethodNames.HAS_TO_STRING, null ) ) } diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectExpressionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectExpressionInspection.kt new file mode 100644 index 0000000..a6b3737 --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectExpressionInspection.kt @@ -0,0 +1,59 @@ +package de.platon42.intellij.plugins.cajon.inspections + +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.psi.* +import com.siyeh.ig.callMatcher.CallMatcher +import de.platon42.intellij.plugins.cajon.* +import de.platon42.intellij.plugins.cajon.quickfixes.HasHashCodeQuickFix +import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix +import de.platon42.intellij.plugins.cajon.quickfixes.RemoveActualOutmostMethodCallQuickFix + +class AssertThatObjectExpressionInspection : AbstractAssertJInspection() { + + companion object { + private const val DISPLAY_NAME = "Asserting equals(), toString(), or hashCode()" + private const val HASHCODE_MESSAGE_TEMPLATE = "Replacing hashCode() expressions by hasSameHashCode() would be more concise" + + private val OBJECT_TO_STRING = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "toString").parameterCount(0) + private val OBJECT_HASHCODE = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "hashCode").parameterCount(0) + } + + override fun getDisplayName() = DISPLAY_NAME + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : JavaElementVisitor() { + override fun visitExpressionStatement(statement: PsiExpressionStatement) { + super.visitExpressionStatement(statement) + if (!statement.hasAssertThat()) { + return + } + val staticMethodCall = statement.findStaticMethodCall() ?: return + + val assertThatArgument = staticMethodCall.firstArg as? PsiMethodCallExpression ?: return + val expectedCallExpression = statement.findOutmostMethodCall() ?: return + when { + OBJECT_EQUALS.test(assertThatArgument) -> { + val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return + val replacementMethod = expectedResult.map(MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO) + registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method -> + MoveOutMethodCallExpressionQuickFix(desc, method) + } + } + OBJECT_TO_STRING.test(assertThatArgument) -> { + staticMethodCall.findFluentCallTo(IS_EQUAL_TO_OBJECT) ?: return + registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.HAS_TO_STRING) { desc, method -> + RemoveActualOutmostMethodCallQuickFix(desc, method) + } + } + OBJECT_HASHCODE.test(assertThatArgument) -> { + val isEqualTo = staticMethodCall.findFluentCallTo(IS_EQUAL_TO_INT) ?: return + val expectedExpression = isEqualTo.firstArg as? PsiMethodCallExpression ?: return + if (OBJECT_HASHCODE.test(expectedExpression)) { + holder.registerProblem(expectedCallExpression, HASHCODE_MESSAGE_TEMPLATE, HasHashCodeQuickFix()) + } + } + } + } + } + } +} \ No newline at end of file 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 eaa71ff..e2756c3 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 @@ -115,8 +115,7 @@ class AssertThatSizeInspection : AbstractAssertJInspection() { if (!expression.hasAssertThat()) { return } - val isHasSize = HAS_SIZE.test(expression) - if (!(isHasSize)) { + if (!HAS_SIZE.test(expression)) { return } val actualExpression = expression.firstArg diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/HasHashCodeQuickFix.kt b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/HasHashCodeQuickFix.kt new file mode 100644 index 0000000..0c1f718 --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/HasHashCodeQuickFix.kt @@ -0,0 +1,37 @@ +package de.platon42.intellij.plugins.cajon.quickfixes + +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiMethodCallExpression +import de.platon42.intellij.plugins.cajon.* + +class HasHashCodeQuickFix : + AbstractCommonQuickFix(HASHCODE_DESCRIPTION) { + + companion object { + private const val HASHCODE_DESCRIPTION = "Replace calls to hashCode() with hasSameHashCodeAs()" + } + + override fun getFamilyName(): String { + return HASHCODE_DESCRIPTION + } + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val outmostCallExpression = descriptor.startElement as? PsiMethodCallExpression ?: return + val assertThatMethodCall = outmostCallExpression.findStaticMethodCall() ?: return + + val assertExpression = assertThatMethodCall.firstArg as? PsiMethodCallExpression ?: return + + val methodsToFix = assertThatMethodCall.gatherAssertionCalls() + + assertExpression.replace(assertExpression.qualifierExpression) + + methodsToFix + .forEach { + val innerHashCodeObject = (it.firstArg as PsiMethodCallExpression).qualifierExpression + val expectedExpression = createExpectedMethodCall(it, "hasSameHashCodeAs", innerHashCodeObject) + expectedExpression.replaceQualifierFromMethodCall(it) + it.replace(expectedExpression) + } + } +} \ No newline at end of file diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/ReplaceJUnitDeltaAssertMethodCallQuickFix.kt b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/ReplaceJUnitDeltaAssertMethodCallQuickFix.kt index b1089d9..af65319 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/ReplaceJUnitDeltaAssertMethodCallQuickFix.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/ReplaceJUnitDeltaAssertMethodCallQuickFix.kt @@ -9,7 +9,7 @@ import de.platon42.intellij.plugins.cajon.AssertJClassNames.Companion.GUAVA_ASSE class ReplaceJUnitDeltaAssertMethodCallQuickFix(description: String, private val replacementMethod: String) : AbstractCommonQuickFix(description) { companion object { - private const val CONVERT_DESCRIPTION = "Convert JUnit assertions to assertJ" + private const val CONVERT_DESCRIPTION = "Convert JUnit assertions with delta to assertJ" } override fun getFamilyName(): String { diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 4e18c83..91e8075 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -40,6 +40,8 @@