diff --git a/README.md b/README.md index 510018c..c20e86a 100644 --- a/README.md +++ b/README.md @@ -19,12 +19,12 @@ This makes finding bugs and fixing failed tests easier. - AssertThatObjectIsNotNull > from: assertThat(object).isNotEqualTo(null); > to: assertThat(object).isNotNull(); - -## TODO - AssertThatBooleanIsTrueOrFalse > from: assertThat(booleanValue).isEqualTo(true/false/Boolean.TRUE/Boolean.FALSE); > to: assertThat(booleanValue).isTrue()/isFalse(); -- AssertThatStringEmpty + +## TODO +- AssertThatStringIsEmpty > from: assertThat(string).isEqualTo("") > to: assertThat(string).isEmpty(); - AssertThatArrayHasLiteralSize 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 adc0f1b..1ecc873 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 @@ -7,14 +7,19 @@ import com.siyeh.ig.callMatcher.CallMatcher open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() { companion object { - private const val ABSTRACT_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractAssert" - private const val IS_EQUAL_TO = "isEqualTo" - private const val IS_NOT_EQUAL_TO = "isNotEqualTo" + const val ABSTRACT_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractAssert" + const val ABSTRACT_BOOLEAN_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractBooleanAssert" + const val IS_EQUAL_TO = "isEqualTo" + const val IS_NOT_EQUAL_TO = "isNotEqualTo" val IS_EQUAL_TO_OBJECT = CallMatcher.instanceCall(ABSTRACT_ASSERT_CLASSNAME, IS_EQUAL_TO) .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!! val IS_NOT_EQUAL_TO_OBJECT = CallMatcher.instanceCall(ABSTRACT_ASSERT_CLASSNAME, IS_NOT_EQUAL_TO) .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!! + val IS_EQUAL_TO_BOOLEAN = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, IS_EQUAL_TO) + .parameterTypes("boolean")!! + val IS_NOT_EQUAL_TO_BOOLEAN = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, IS_NOT_EQUAL_TO) + .parameterTypes("boolean")!! } override fun getGroupDisplayName(): String { diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspection.kt new file mode 100644 index 0000000..89afeb3 --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspection.kt @@ -0,0 +1,96 @@ +package de.platon42.intellij.plugins.cajon.inspections + +import com.intellij.codeInspection.LocalQuickFix +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.project.Project +import com.intellij.openapi.util.TextRange +import com.intellij.psi.* +import com.intellij.psi.util.TypeConversionUtil +import org.jetbrains.annotations.NonNls + +class AssertThatBooleanIsTrueOrFalseInspection : AbstractAssertJInspection() { + + companion object { + @NonNls + private val DISPLAY_NAME = "Asserting true or false" + + @NonNls + private val INSPECTION_MESSAGE = "isEqualTo(true/false) can be simplified to isTrue()/isFalse()" + + @NonNls + private val QUICKFIX_DESCRIPTION_IS_TRUE = "Replace isEqualTo(true/false) with isTrue()/isFalse()" + + @NonNls + private val QUICKFIX_DESCRIPTION_NOT_IS_TRUE = "Replace isNotEqualTo(true/false) with isFalse()/isTrue()" + } + + override fun getDisplayName() = DISPLAY_NAME + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : JavaElementVisitor() { + override fun visitMethodCallExpression(expression: PsiMethodCallExpression) { + super.visitMethodCallExpression(expression) + val isEqualToObject = IS_EQUAL_TO_OBJECT.test(expression) + val isEqualToBoolean = IS_EQUAL_TO_BOOLEAN.test(expression) + val isNotEqualToObject = IS_NOT_EQUAL_TO_OBJECT.test(expression) + val isNotEqualToBoolean = IS_NOT_EQUAL_TO_BOOLEAN.test(expression) + val normalBooleanTest = isEqualToObject || isEqualToBoolean + val flippedBooleanTest = isNotEqualToObject || isNotEqualToBoolean + if (!(normalBooleanTest || flippedBooleanTest)) { + return + } + var assertedType = expression.methodExpression.qualifierExpression?.type + if (assertedType is PsiCapturedWildcardType) { + assertedType = assertedType.upperBound + } + val assertedTypeIsBoolean = + assertedType?.canonicalText?.startsWith(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME) ?: false + val equalToExpression = expression.argumentList.expressions[0]!! + if (!TypeConversionUtil.isBooleanType(equalToExpression.type) || !assertedTypeIsBoolean) { + return + } + val constantEvaluationHelper = JavaPsiFacade.getInstance(holder.project).constantEvaluationHelper + var result = constantEvaluationHelper.computeConstantExpression(equalToExpression) + if (result == null) { + val field = (equalToExpression as? PsiReferenceExpression)?.resolve() as? PsiField + if (field?.containingClass?.qualifiedName == CommonClassNames.JAVA_LANG_BOOLEAN) { + when { + field.name == "TRUE" -> result = true + field.name == "FALSE" -> result = false + } + } + } + val expectedResult = result as? Boolean ?: return + holder.registerProblem( + expression, + INSPECTION_MESSAGE, + ProblemHighlightType.INFORMATION, + null as TextRange?, + ReplaceWithIsNullQuickFix( + expectedResult xor flippedBooleanTest, + if (flippedBooleanTest) QUICKFIX_DESCRIPTION_NOT_IS_TRUE else QUICKFIX_DESCRIPTION_IS_TRUE + ) + ) + } + } + } + + + private class ReplaceWithIsNullQuickFix(val expectedResult: Boolean, val quickfixName: String) : LocalQuickFix { + override fun getFamilyName() = quickfixName + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val element = descriptor.startElement + val factory = JavaPsiFacade.getElementFactory(element.project) + val methodCallExpression = element as? PsiMethodCallExpression ?: return + val oldQualifier = methodCallExpression.methodExpression.qualifierExpression ?: return + val methodName = if (expectedResult) "isTrue()" else "isFalse()" + val isNullExpression = + factory.createExpressionFromText("a.$methodName", null) as PsiMethodCallExpression + isNullExpression.methodExpression.qualifierExpression!!.replace(oldQualifier) + element.replace(isNullExpression) + } + } +} \ No newline at end of file diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspection.kt index 6cc77f0..b24905d 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspection.kt @@ -13,7 +13,7 @@ class AssertThatObjectIsNotNullInspection : AbstractAssertJInspection() { companion object { @NonNls - private val DISPLAY_NAME = "Comparing to non-null" + private val DISPLAY_NAME = "Asserting non-null" @NonNls private val INSPECTION_MESSAGE = "isNotEqualTo(null) can be simplified to isNotNull()" diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspection.kt index 35b5cfc..4bce1c0 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspection.kt @@ -13,7 +13,7 @@ class AssertThatObjectIsNullInspection : AbstractAssertJInspection() { companion object { @NonNls - private val DISPLAY_NAME = "Comparing to null" + private val DISPLAY_NAME = "Asserting null" @NonNls private val INSPECTION_MESSAGE = "isEqualTo(null) can be simplified to isNull()" diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index e6c41e7..cf81350 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -22,6 +22,9 @@ implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatObjectIsNullInspection"/> + diff --git a/src/main/resources/inspectionDescriptions/AssertThatBooleanIsTrueOrFalse.html b/src/main/resources/inspectionDescriptions/AssertThatBooleanIsTrueOrFalse.html new file mode 100644 index 0000000..d87d6de --- /dev/null +++ b/src/main/resources/inspectionDescriptions/AssertThatBooleanIsTrueOrFalse.html @@ -0,0 +1,7 @@ + + +Turns assertThat(booleanExpression).isEqualTo(true/false) or assertThat(booleanExpression).isNotEqualTo(true/false) +into assertThat(booleanExpression).isTrue() or assertThat(booleanExpression).isFalse(). + + + \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/playground/Playground.java b/src/test/java/de/platon42/intellij/playground/Playground.java index bd298bf..a0ada4b 100644 --- a/src/test/java/de/platon42/intellij/playground/Playground.java +++ b/src/test/java/de/platon42/intellij/playground/Playground.java @@ -14,5 +14,38 @@ public class Playground { assertThat(new String[1].length).isLessThanOrEqualTo(1); assertThat(new String[1]).hasSameSizeAs(new Object()); assertThat("").isEqualTo(null); + assertThat(true).isTrue(); + assertThat(true).isEqualTo(true); + assertThat(Boolean.TRUE).isEqualTo(Boolean.FALSE); + assertThat(Boolean.TRUE).isEqualTo(true); } + + private void booleanIsTrueOrFalse() { + boolean primitive = false; + Boolean object = java.lang.Boolean.TRUE; + + assertThat(primitive).isEqualTo(Boolean.TRUE); + assertThat(primitive).isEqualTo(Boolean.FALSE); + assertThat(object).isEqualTo(Boolean.TRUE); + assertThat(object).isEqualTo(Boolean.FALSE); + assertThat(primitive).isEqualTo(true); + assertThat(primitive).isEqualTo(false); + assertThat(object).isEqualTo(true); + assertThat(object).isEqualTo(false); + + assertThat(primitive).isNotEqualTo(Boolean.TRUE); + assertThat(primitive).isNotEqualTo(Boolean.FALSE); + assertThat(object).isNotEqualTo(Boolean.TRUE); + assertThat(object).isNotEqualTo(Boolean.FALSE); + assertThat(primitive).isNotEqualTo(true); + assertThat(primitive).isNotEqualTo(false); + assertThat(object).isNotEqualTo(true); + assertThat(object).isNotEqualTo(false); + + assertThat(primitive).as("nah").isEqualTo(true && !true); + assertThat(object).isEqualTo(Boolean.TRUE && !Boolean.TRUE); + + assertThat("").isEqualTo(Boolean.TRUE); + } + } 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 b877670..cb14ed9 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/AbstractCajonTest.kt @@ -59,8 +59,8 @@ abstract class AbstractCajonTest { } } - protected fun executeQuickFixes(myFixture: JavaCodeInsightTestFixture, expectedFixes: Int) { - val quickfixes = myFixture.getAllQuickFixes() + protected fun executeQuickFixes(myFixture: JavaCodeInsightTestFixture, regex: Regex, expectedFixes: Int) { + val quickfixes = myFixture.getAllQuickFixes().filter { it.familyName.matches(regex) } assertThat(quickfixes).hasSize(expectedFixes) quickfixes.forEach(myFixture::launchAction) } diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspectionTest.kt new file mode 100644 index 0000000..b0c0114 --- /dev/null +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspectionTest.kt @@ -0,0 +1,21 @@ +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 AssertThatBooleanIsTrueOrFalseInspectionTest : AbstractCajonTest() { + + @Test + @TestDataSubPath("inspections/BooleanIsTrueOrFalse") + internal fun assertThat_with_isEqualTo_true_or_false_can_use_isTrue_or_isFalse(@MyFixture myFixture: JavaCodeInsightTestFixture) { + runTest { + myFixture.enableInspections(AssertThatBooleanIsTrueOrFalseInspection::class.java) + myFixture.configureByFile("BooleanIsTrueOrFalseBefore.java") + executeQuickFixes(myFixture, Regex("Replace is.*"), 17) + myFixture.checkResultByFile("BooleanIsTrueOrFalseAfter.java") + } + } +} \ No newline at end of file diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspectionTest.kt index 421cd13..c14ddb4 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNotNullInspectionTest.kt @@ -14,7 +14,7 @@ internal class AssertThatObjectIsNotNullInspectionTest : AbstractCajonTest() { runTest { myFixture.enableInspections(AssertThatObjectIsNotNullInspection::class.java) myFixture.configureByFile("ObjectIsNotNullBefore.java") - executeQuickFixes(myFixture, 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Replace isNotEqualTo(null) with isNotNull()"), 3) myFixture.checkResultByFile("ObjectIsNotNullAfter.java") } } diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspectionTest.kt index b934fa6..a5bed5e 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatObjectIsNullInspectionTest.kt @@ -14,7 +14,7 @@ internal class AssertThatObjectIsNullInspectionTest : AbstractCajonTest() { runTest { myFixture.enableInspections(AssertThatObjectIsNullInspection::class.java) myFixture.configureByFile("ObjectIsNullBefore.java") - executeQuickFixes(myFixture, 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo(null) with isNull()"), 3) myFixture.checkResultByFile("ObjectIsNullAfter.java") } } diff --git a/src/test/resources/inspections/BooleanIsTrueOrFalse/BooleanIsTrueOrFalseAfter.java b/src/test/resources/inspections/BooleanIsTrueOrFalse/BooleanIsTrueOrFalseAfter.java new file mode 100644 index 0000000..afe2840 --- /dev/null +++ b/src/test/resources/inspections/BooleanIsTrueOrFalse/BooleanIsTrueOrFalseAfter.java @@ -0,0 +1,32 @@ +import static org.assertj.core.api.Assertions.assertThat; + +public class BooleanIsTrueOrFalse { + + private void booleanIsTrueOrFalse() { + boolean primitive = false; + Boolean object = Boolean.TRUE; + + assertThat(primitive).isTrue(); + assertThat(primitive).isFalse(); + assertThat(object).isTrue(); + assertThat(object).isFalse(); + assertThat(primitive).isTrue(); + assertThat(primitive).isFalse(); + assertThat(object).isTrue(); + assertThat(object).isFalse(); + + assertThat(primitive).isFalse(); + assertThat(primitive).isTrue(); + assertThat(object).isFalse(); + assertThat(object).isTrue(); + assertThat(primitive).isFalse(); + assertThat(primitive).isTrue(); + assertThat(object).isFalse(); + assertThat(object).isTrue(); + + assertThat(primitive).as("nah").isFalse(); + assertThat(object).isEqualTo(Boolean.TRUE && !Boolean.TRUE); + + assertThat("").isEqualTo(Boolean.TRUE); + } +} diff --git a/src/test/resources/inspections/BooleanIsTrueOrFalse/BooleanIsTrueOrFalseBefore.java b/src/test/resources/inspections/BooleanIsTrueOrFalse/BooleanIsTrueOrFalseBefore.java new file mode 100644 index 0000000..6da7ac6 --- /dev/null +++ b/src/test/resources/inspections/BooleanIsTrueOrFalse/BooleanIsTrueOrFalseBefore.java @@ -0,0 +1,32 @@ +import static org.assertj.core.api.Assertions.assertThat; + +public class BooleanIsTrueOrFalse { + + private void booleanIsTrueOrFalse() { + boolean primitive = false; + Boolean object = Boolean.TRUE; + + assertThat(primitive).isEqualTo(Boolean.TRUE); + assertThat(primitive).isEqualTo(Boolean.FALSE); + assertThat(object).isEqualTo(Boolean.TRUE); + assertThat(object).isEqualTo(Boolean.FALSE); + assertThat(primitive).isEqualTo(true); + assertThat(primitive).isEqualTo(false); + assertThat(object).isEqualTo(true); + assertThat(object).isEqualTo(false); + + assertThat(primitive).isNotEqualTo(Boolean.TRUE); + assertThat(primitive).isNotEqualTo(Boolean.FALSE); + assertThat(object).isNotEqualTo(Boolean.TRUE); + assertThat(object).isNotEqualTo(Boolean.FALSE); + assertThat(primitive).isNotEqualTo(true); + assertThat(primitive).isNotEqualTo(false); + assertThat(object).isNotEqualTo(true); + assertThat(object).isNotEqualTo(false); + + assertThat(primitive).as("nah").isEqualTo(true && !true); + assertThat(object).isEqualTo(Boolean.TRUE && !Boolean.TRUE); + + assertThat("").isEqualTo(Boolean.TRUE); + } +}