New AssertThatInstanceOf inspection that moves instanceof expressions out of assertThat(). Fixes to documentation.

This commit is contained in:
Chris Hodges 2019-04-22 18:52:09 +02:00
parent db02f7fb93
commit 66508ceb2c
13 changed files with 215 additions and 24 deletions

View File

@ -1,15 +1,17 @@
# Cajon - Concise AssertJ Optimizing Nitpicker
Cajon is an IntelliJ IDEA Plugin for shortening and optimizing AssertJ assertions.
Cajon is an IntelliJ IDEA Plugin for shortening and optimizing [AssertJ](https://assertj.github.io/doc/) assertions.
## Why?
## Purpose
First, code is easier to read, when it is concise and reflects the intention clearly.
AssertJ has plenty of different convenience methods that describing various intentions precisely.
Why write longer, more complex code that can be expressed in brevity?
Second, AssertJ is able to output more meaningful descriptions when an assertion fails.
Second, when using the available special assertion methods of AssertJ, a failure of a condition
can be expressed in better detail and with more meaningful descriptions.
This makes finding bugs and fixing failed tests more efficient.
Nobody likes to read failures of the kind "failed because true is not false".
For example:
@ -50,9 +52,10 @@ The plugin will report inspections in your opened editor file as warnings.
You can then quick-fix these with your quick-fix hotkey (usually Alt-Return or Opt-Return).
Or, you can use the "Run Inspection by Name..." action to run one inspection on a bigger scope (e.g. the whole project).
Applying a quick fix might result in further optimization possibilities, so you might need to perform a couple of fixes before you get to the final result.
Applying a quick fix might result in further optimization possibilities, so
you might need to perform a couple of fixes before you get to the final result.
For example:
Check out this example where every line represents the result after a Cajon quickfix:
```
assertFalse(!(array.length == collection.size()));
@ -88,7 +91,18 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
```
from: assertThat(!booleanValue).isEqualTo(true/false/Boolean.TRUE/Boolean.FALSE);
from: assertThat(!booleanValue).isTrue()/isFalse();
to: assertThat(!booleanValue).isFalse()/isTrue();
to: assertThat(booleanValue).isFalse()/isTrue();
```
- AssertThatInstanceOf
```
from: assertThat(object instanceof classname).isEqualTo(true);
from: assertThat(object instanceof classname).isTrue();
to: assertThat(object).isInstanceOf(classname.class);
from: assertThat(object instanceof classname).isEqualTo(false);
from: assertThat(object instanceof classname).isFalse();
to: assertThat(object).isNotInstanceOf(classname.class);
```
- AssertThatStringIsEmpty
@ -297,11 +311,10 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
Cajon is written in Kotlin 1.3.
Cajon is probably the only plugin that uses JUnit 5 Jupiter for unit testing so far (or at least the only one that I'm aware of ;) ).
The IntelliJ framework actually uses the JUnit 3 TestCase for plugin testing and I took me quite a while to make it work with JUnit 5.
The IntelliJ framework actually uses the JUnit 3 TestCase for plugin testing and it took me quite a while to make it work with JUnit 5.
Feel free to use the code (in package de.platon42.intellij.jupiter) for your projects (with attribution).
## TODO
- AssertThatInstanceOf
- AssumeThatInsteadOfReturn
- Join consecutive assertThats
- Extraction with property names to lambda with Java 8
@ -314,14 +327,15 @@ Feel free to use the code (in package de.platon42.intellij.jupiter) for your pro
## Changelog
#### V0.6 (unreleased)
#### V0.6 (22-Apr-19)
- New AssertThatStringExpression inspection that will move ```isEmpty()```, ```equals()```, ```equalsIgnoreCase()```, ```contains()```,
```startsWith()```, and ```endsWith()``` out of actual expression.
- Extended AssertThatSize inspection to take ```String```s and ```CharSequences``` into account, too.
- New AssertThatInvertedBooleanCondition inspection that will remove inverted boolean expressions inside ```assertThat()```.
- Renamed a few inspections to better/shorter names.
- New AssertThatInstanceOf inspection that moves instanceof expressions out of ```assertThat()```.
#### V0.5 (13-Apr-19)
#### V0.5 (18-Apr-19)
- Fixed incompatibility with IDEA versions < 2018.2 (affected AssertThatSizeInspection). Minimal version is now 2017.3.
- Fixed missing Guava imports (if not already present) for AssertThatGuavaInspection. This was a major PITA to get right.
- Added support for referencing and refactoring inside ```.extracting()``` methods with fields, properties and methods (though

View File

@ -40,13 +40,14 @@ intellij {
patchPluginXml {
changeNotes """
<h4>V0.6 (xx-Apr-19)</h4>
<h4>V0.6 (22-Apr-19)</h4>
<ul>
<li>New AssertThatStringExpression inspection that will move isEmpty(), equals(), equalsIgnoreCase(), contains(),
startsWith(), and endsWith() out of actual expression.
<li>Extended AssertThatSize inspection to take strings and CharSequences into account, too.
<li>New AssertThatInvertedBooleanCondition inspection that will remove inverted boolean expressions inside assertThat().
<li>Renamed a few inspections to better/shorter names.
<li>New AssertThatInstanceOf inspection that moves instanceof expressions out of assertThat().
</ul>
<h4>V0.5 (18-Apr-19)</h4>
<ul>

View File

@ -45,6 +45,10 @@ class MethodNames {
const val IS_CLOSE_TO = "isCloseTo"
@NonNls
const val IS_NOT_CLOSE_TO = "isNotCloseTo"
@NonNls
const val IS_INSTANCE_OF = "isInstanceOf"
@NonNls
const val IS_NOT_INSTANCE_OF = "isNotInstanceOf"
@NonNls
const val IS_EMPTY = "isEmpty"

View File

@ -5,8 +5,6 @@ import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.*
import com.intellij.psi.util.TypeConversionUtil
import de.platon42.intellij.plugins.cajon.MethodNames
import de.platon42.intellij.plugins.cajon.MethodNames.Companion.IS_NOT_NULL
import de.platon42.intellij.plugins.cajon.MethodNames.Companion.IS_NULL
import de.platon42.intellij.plugins.cajon.findOutmostMethodCall
import de.platon42.intellij.plugins.cajon.firstArg
import de.platon42.intellij.plugins.cajon.map
@ -36,7 +34,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {
val assertThatArgument = expression.firstArg
if (assertThatArgument is PsiMethodCallExpression && OBJECT_EQUALS.test(assertThatArgument)) {
val replacementMethod = if (expectedResult) MethodNames.IS_EQUAL_TO else MethodNames.IS_NOT_EQUAL_TO
val replacementMethod = expectedResult.map(MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO)
registerSplitMethod(holder, expression, "${MethodNames.EQUALS}()", replacementMethod, ::MoveActualOuterExpressionMethodCallQuickFix)
return
}
@ -51,7 +49,7 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {
if (isLeftNull && isRightNull) {
return
} else if (isLeftNull || isRightNull) {
val replacementMethod = if (expectedResult) IS_NULL else IS_NOT_NULL
val replacementMethod = expectedResult.map(MethodNames.IS_NULL, MethodNames.IS_NOT_NULL)
registerSplitMethod(holder, expression, "binary", replacementMethod) { desc, method ->
SplitBinaryExpressionMethodCallQuickFix(desc, method, pickRightOperand = isLeftNull, noExpectedExpression = true)
}

View File

@ -0,0 +1,54 @@
package de.platon42.intellij.plugins.cajon.inspections
import com.intellij.codeInspection.LocalQuickFix
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.JavaElementVisitor
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.PsiInstanceOfExpression
import com.intellij.psi.PsiMethodCallExpression
import de.platon42.intellij.plugins.cajon.MethodNames
import de.platon42.intellij.plugins.cajon.findOutmostMethodCall
import de.platon42.intellij.plugins.cajon.firstArg
import de.platon42.intellij.plugins.cajon.map
import de.platon42.intellij.plugins.cajon.quickfixes.RemoveInstanceOfExpressionQuickFix
class AssertThatInstanceOfInspection : AbstractAssertJInspection() {
companion object {
private const val DISPLAY_NAME = "Asserting a class instance"
private const val REPLACE_INSTANCEOF_DESCRIPTION_TEMPLATE = "Replace instanceof expression by assertThat().%s()"
private const val MOVE_OUT_INSTANCEOF_MESSAGE = "instanceof expression could be moved out of assertThat()"
}
override fun getDisplayName() = DISPLAY_NAME
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
return object : JavaElementVisitor() {
override fun visitMethodCallExpression(expression: PsiMethodCallExpression) {
super.visitMethodCallExpression(expression)
if (!ASSERT_THAT_BOOLEAN.test(expression)) {
return
}
val expectedCallExpression = expression.findOutmostMethodCall() ?: return
val expectedResult = getExpectedBooleanResult(expectedCallExpression) ?: return
if (expression.firstArg is PsiInstanceOfExpression) {
val replacementMethod = expectedResult.map(MethodNames.IS_INSTANCE_OF, MethodNames.IS_NOT_INSTANCE_OF)
registerRemoveInstanceOfMethod(holder, expression, replacementMethod, ::RemoveInstanceOfExpressionQuickFix)
}
}
private fun registerRemoveInstanceOfMethod(
holder: ProblemsHolder,
expression: PsiMethodCallExpression,
replacementMethod: String,
quickFixSupplier: (String, String) -> LocalQuickFix
) {
val description = REPLACE_INSTANCEOF_DESCRIPTION_TEMPLATE.format(replacementMethod)
val quickfix = quickFixSupplier(description, replacementMethod)
holder.registerProblem(expression, MOVE_OUT_INSTANCEOF_MESSAGE, quickfix)
}
}
}
}

View File

@ -6,6 +6,7 @@ import com.intellij.psi.*
import de.platon42.intellij.plugins.cajon.MethodNames
import de.platon42.intellij.plugins.cajon.findOutmostMethodCall
import de.platon42.intellij.plugins.cajon.firstArg
import de.platon42.intellij.plugins.cajon.map
import de.platon42.intellij.plugins.cajon.quickfixes.RemoveUnaryExpressionQuickFix
class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection() {
@ -31,7 +32,7 @@ class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection()
val prefixExpression = expression.firstArg as? PsiPrefixExpression ?: return
if (prefixExpression.operationTokenType == JavaTokenType.EXCL) {
val replacementMethod = if (expectedResult) MethodNames.IS_FALSE else MethodNames.IS_TRUE
val replacementMethod = expectedResult.map(MethodNames.IS_FALSE, MethodNames.IS_TRUE)
registerInvertMethod(holder, expression, replacementMethod, ::RemoveUnaryExpressionQuickFix)
}
}

View File

@ -0,0 +1,36 @@
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.PsiInstanceOfExpression
import com.intellij.psi.PsiMethodCallExpression
import com.intellij.psi.PsiParenthesizedExpression
import de.platon42.intellij.plugins.cajon.createExpectedMethodCall
import de.platon42.intellij.plugins.cajon.findOutmostMethodCall
import de.platon42.intellij.plugins.cajon.firstArg
import de.platon42.intellij.plugins.cajon.replaceQualifierFromMethodCall
class RemoveInstanceOfExpressionQuickFix(description: String, private val replacementMethod: String) : AbstractCommonQuickFix(description) {
override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
val element = descriptor.startElement
val methodCallExpression = element as? PsiMethodCallExpression ?: return
val assertExpression = methodCallExpression.firstArg as? PsiInstanceOfExpression ?: return
val expectedClass = assertExpression.checkType ?: return
val factory = JavaPsiFacade.getElementFactory(project)
val classObjectAccess = factory.createExpressionFromText("${expectedClass.type.canonicalText}.class", null)
var operand = assertExpression.operand
while (operand is PsiParenthesizedExpression) {
operand = operand.expression ?: return
}
assertExpression.replace(operand)
val oldExpectedExpression = element.findOutmostMethodCall() ?: return
val expectedExpression = createExpectedMethodCall(element, replacementMethod, classObjectAccess)
expectedExpression.replaceQualifierFromMethodCall(oldExpectedExpression)
oldExpectedExpression.replace(expectedExpression)
}
}

View File

@ -5,8 +5,9 @@
<description><![CDATA[
Cajon is an IntelliJ IDEA Plugin for shortening and optimizing AssertJ assertions.
It adds inspections and quick fixes to fully make use of the AssertJ methods
to make the intention clear and concise. It can also convert JUnit 4 assertions to AssertJ.
It adds several inspections and quick fixes to fully use the fluent assertion methods
and thus makes the intention clear and concise, also generating better messages on test failures.
It can also be used to convert JUnit 4 assertions to AssertJ.
It supports referencing inside extracting()-methods with strings, adding refactoring safety.
]]></description>
@ -23,8 +24,12 @@
<psi.referenceContributor implementation="de.platon42.intellij.plugins.cajon.references.ExtractorReferenceContributor"/>
<localInspection groupPath="Java" shortName="AssertThatObjectIsNullOrNotNull" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatObjectIsNullOrNotNullInspection"/>
<localInspection groupPath="Java" shortName="AssertThatBooleanIsTrueOrFalse" enabledByDefault="true" level="WARNING"
<localInspection groupPath="Java" shortName="AssertThatBooleanCondition" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatBooleanConditionInspection"/>
<localInspection groupPath="Java" shortName="AssertThatInvertedBooleanCondition" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatInvertedBooleanConditionInspection"/>
<localInspection groupPath="Java" shortName="AssertThatInstanceOf" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatInstanceOfInspection"/>
<localInspection groupPath="Java" shortName="AssertThatStringIsEmpty" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatStringIsEmptyInspection"/>
<localInspection groupPath="Java" shortName="AssertThatEnumerableIsEmpty" enabledByDefault="true" level="WARNING"
@ -33,17 +38,15 @@
<localInspection groupPath="Java" shortName="AssertThatSize" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatSizeInspection"/>
<localInspection groupPath="Java" shortName="AssertThatBinaryExpressionIsTrueOrFalse" enabledByDefault="true" level="WARNING"
<localInspection groupPath="Java" shortName="AssertThatBinaryExpression" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatBinaryExpressionInspection"/>
<localInspection groupPath="Java" shortName="AssertThatNotPrefixExpression" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatInvertedBooleanConditionInspection"/>
<localInspection groupPath="Java" shortName="AssertThatStringExpression" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatStringExpressionInspection"/>
<localInspection groupPath="Java" shortName="AssertThatJava8Optional" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatJava8OptionalInspection"/>
<localInspection groupPath="Java" shortName="AssertThatGuavaOptional" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatGuavaOptionalInspection"/>
<localInspection groupPath="Java" shortName="AssertThatStringExpression" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatStringExpressionInspection"/>
<localInspection groupPath="Java" shortName="JUnitAssertToAssertJ" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.JUnitAssertToAssertJInspection"/>

View File

@ -0,0 +1,7 @@
<html>
<body>
Turns assertThat(object instanceof classname).isEqualTo(true/false) into assertThat(object).is(Not)InstanceOf(classname.class).
<!-- tooltip end -->
Also works with constant expressions and Boolean.TRUE/FALSE.
</body>
</html>

View File

@ -242,6 +242,13 @@ public class Playground {
assertThat(opt).isAbsent();
}
private void assertThatInstance() {
String foo = "foo";
assertThat(foo instanceof String).isTrue();
assertThat(foo).isInstanceOf(String.class);
assertThat(foo).isNotInstanceOf(String.class);
}
private void junitAssertions() {
assertFalse(!(new int[3].length == new ArrayList<Integer>().size()));
assertThat(!(new int[3].length == new ArrayList<Integer>().size())).isFalse();

View File

@ -0,0 +1,22 @@
package de.platon42.intellij.plugins.cajon.inspections
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.junit.jupiter.api.Test
internal class AssertThatInstanceOfInspectionTest : AbstractCajonTest() {
@Test
@TestDataSubPath("inspections/InstanceOf")
internal fun assertThat_with_instanceof_can_be_moved_out(@MyFixture myFixture: JavaCodeInsightTestFixture) {
runTest {
myFixture.enableInspections(AssertThatInstanceOfInspection::class.java)
myFixture.configureByFile("InstanceOfBefore.java")
executeQuickFixes(myFixture, Regex.fromLiteral("Replace instanceof expression by assertThat().isInstanceOf()"), 5)
executeQuickFixes(myFixture, Regex.fromLiteral("Replace instanceof expression by assertThat().isNotInstanceOf()"), 6)
myFixture.checkResultByFile("InstanceOfAfter.java")
}
}
}

View File

@ -0,0 +1,22 @@
import static org.assertj.core.api.Assertions.assertThat;
public class InstanceOf {
private void instanceOf() {
Boolean object = Boolean.TRUE;
assertThat(object).isInstanceOf(Boolean.class);
assertThat(object).isInstanceOf(Boolean.class);
assertThat(object).isInstanceOf(Boolean.class);
assertThat(object).isInstanceOf(Boolean.class);
assertThat(object).isInstanceOf(Boolean.class);
assertThat(object).isNotInstanceOf(Boolean.class);
assertThat(object).isNotInstanceOf(Boolean.class);
assertThat(object).isNotInstanceOf(Boolean.class);
assertThat(object).isNotInstanceOf(Boolean.class);
assertThat(object).isNotInstanceOf(Boolean.class);
assertThat(object).as("nah").isNotInstanceOf(Boolean.class);
}
}

View File

@ -0,0 +1,22 @@
import static org.assertj.core.api.Assertions.assertThat;
public class InstanceOf {
private void instanceOf() {
Boolean object = Boolean.TRUE;
assertThat(object instanceof Boolean).isEqualTo(Boolean.TRUE);
assertThat(object instanceof Boolean).isEqualTo(true);
assertThat(object instanceof Boolean).isNotEqualTo(Boolean.FALSE);
assertThat(object instanceof Boolean).isNotEqualTo(false);
assertThat(object instanceof Boolean).isTrue();
assertThat(object instanceof Boolean).isEqualTo(Boolean.FALSE);
assertThat(object instanceof Boolean).isEqualTo(false);
assertThat(object instanceof Boolean).isNotEqualTo(Boolean.TRUE);
assertThat(object instanceof Boolean).isNotEqualTo(true);
assertThat(object instanceof Boolean).isFalse();
assertThat(((object)) instanceof Boolean).as("nah").isEqualTo(true && !true);
}
}