From e2ffce753cc10079c25092362c4543de2fbeff7e Mon Sep 17 00:00:00 2001 From: chrisly42 Date: Sun, 24 Mar 2019 13:03:53 +0100 Subject: [PATCH] Another fix for AssertThatStringIsEmpty, added support for CharSequences, added hasSize(0) case, refactored stuff. Added missing html. --- README.md | 5 +- .../inspections/AbstractAssertJInspection.kt | 47 +++++++++++++++++-- ...ssertThatBooleanIsTrueOrFalseInspection.kt | 23 +-------- .../AssertThatObjectIsNotNullInspection.kt | 17 +------ .../AssertThatObjectIsNullInspection.kt | 17 +------ .../AssertThatStringIsEmptyInspection.kt | 34 ++++++-------- .../AssertThatStringIsEmpty.html | 7 +++ .../intellij/playground/Playground.java | 6 +-- ...AssertThatObjectIsNotNullInspectionTest.kt | 2 +- .../AssertThatObjectIsNullInspectionTest.kt | 2 +- .../AssertThatStringIsEmptyInspectionTest.kt | 2 +- .../StringIsEmpty/StringIsEmptyAfter.java | 7 +++ .../StringIsEmpty/StringIsEmptyBefore.java | 7 +++ 13 files changed, 91 insertions(+), 85 deletions(-) create mode 100644 src/main/resources/inspectionDescriptions/AssertThatStringIsEmpty.html diff --git a/README.md b/README.md index fd125e3..29f37f5 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,9 @@ Then AssertJ would tell you the contents of the collection on failure. > from: assertThat(booleanValue).isEqualTo(true/false/Boolean.TRUE/Boolean.FALSE); > to: assertThat(booleanValue).isTrue()/isFalse(); - AssertThatStringIsEmpty - > from: assertThat(string).isEqualTo("") - > to: assertThat(string).isEmpty(); + > from: assertThat(charSequence/string).isEqualTo(""); + > from: assertThat(charSequence/string).hasSize(0); + > to: assertThat(charSequence/string).isEmpty(); ## TODO - 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 1cd7b95..26fbc6b 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 @@ -1,19 +1,35 @@ package de.platon42.intellij.plugins.cajon.inspections import com.intellij.codeInspection.AbstractBaseJavaLocalInspectionTool +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.util.TextRange import com.intellij.psi.CommonClassNames +import com.intellij.psi.JavaPsiFacade import com.intellij.psi.PsiCapturedWildcardType import com.intellij.psi.PsiMethodCallExpression +import com.intellij.psi.search.GlobalSearchScope +import com.intellij.psi.util.PsiTypesUtil import com.siyeh.ig.callMatcher.CallMatcher +import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceSimpleMethodCallQuickFix +import org.jetbrains.annotations.NonNls open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() { companion object { + @NonNls + const val SIMPLIFY_MESSAGE_TEMPLATE = "%s can be simplified to %s" + + @NonNls + const val REPLACE_DESCRIPTION_TEMPLATE = "Replace %s with %s" + const val ABSTRACT_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractAssert" const val ABSTRACT_BOOLEAN_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractBooleanAssert" const val ABSTRACT_STRING_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractStringAssert" + const val ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME = "org.assertj.core.api.AbstractCharSequenceAssert" const val IS_EQUAL_TO = "isEqualTo" const val IS_NOT_EQUAL_TO = "isNotEqualTo" + const val HAS_SIZE = "hasSize" val IS_EQUAL_TO_OBJECT = CallMatcher.instanceCall(ABSTRACT_ASSERT_CLASSNAME, IS_EQUAL_TO) .parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!! @@ -23,17 +39,42 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() { .parameterTypes("boolean")!! val IS_NOT_EQUAL_TO_BOOLEAN = CallMatcher.instanceCall(ABSTRACT_BOOLEAN_ASSERT_CLASSNAME, IS_NOT_EQUAL_TO) .parameterTypes("boolean")!! + val CHAR_SEQUENCE_HAS_SIZE = CallMatcher.instanceCall(ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME, HAS_SIZE) + .parameterTypes("int")!! } override fun getGroupDisplayName(): String { return "AssertJ" } - protected fun checkAssertedType(expression: PsiMethodCallExpression, prefix: String): Boolean { - var assertedType = expression.methodExpression.qualifierExpression?.type + protected fun checkAssertedType(expression: PsiMethodCallExpression, classname: String): Boolean { + var assertedType = expression.methodExpression.qualifierExpression?.type ?: return false if (assertedType is PsiCapturedWildcardType) { assertedType = assertedType.upperBound } - return assertedType?.canonicalText?.startsWith(prefix) ?: false + val assertedClass = PsiTypesUtil.getPsiClass(assertedType) ?: return false + val expectedClass = JavaPsiFacade.getInstance(expression.project) + .findClass(classname, GlobalSearchScope.allScope(expression.project)) ?: return false + return assertedClass.isEquivalentTo(expectedClass) || assertedClass.isInheritor(expectedClass, true) + } + + protected fun getOriginalMethodName(expression: PsiMethodCallExpression) = + expression.resolveMethod()?.name?.plus("()") + + protected fun registerSimplifyMethod( + holder: ProblemsHolder, + expression: PsiMethodCallExpression, + replacementMethod: String + ) { + val originalMethod = getOriginalMethodName(expression) ?: return + val description = String.format(REPLACE_DESCRIPTION_TEMPLATE, originalMethod, replacementMethod) + val message = String.format(SIMPLIFY_MESSAGE_TEMPLATE, originalMethod, replacementMethod) + holder.registerProblem( + expression, + message, + ProblemHighlightType.INFORMATION, + null as TextRange?, + ReplaceSimpleMethodCallQuickFix(description, replacementMethod) + ) } } \ No newline at end of file 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 index 2305846..92935ad 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanIsTrueOrFalseInspection.kt @@ -1,11 +1,8 @@ package de.platon42.intellij.plugins.cajon.inspections -import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder -import com.intellij.openapi.util.TextRange import com.intellij.psi.* import com.intellij.psi.util.TypeConversionUtil -import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceSimpleMethodCallQuickFix import org.jetbrains.annotations.NonNls class AssertThatBooleanIsTrueOrFalseInspection : AbstractAssertJInspection() { @@ -13,15 +10,6 @@ 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 @@ -59,16 +47,9 @@ class AssertThatBooleanIsTrueOrFalseInspection : AbstractAssertJInspection() { } } val expectedResult = result as? Boolean ?: return - val description = - if (flippedBooleanTest) QUICKFIX_DESCRIPTION_NOT_IS_TRUE else QUICKFIX_DESCRIPTION_IS_TRUE + val replacementMethod = if (expectedResult xor flippedBooleanTest) "isTrue()" else "isFalse()" - holder.registerProblem( - expression, - INSPECTION_MESSAGE, - ProblemHighlightType.INFORMATION, - null as TextRange?, - ReplaceSimpleMethodCallQuickFix(description, replacementMethod) - ) + registerSimplifyMethod(holder, expression, replacementMethod) } } } 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 3d4b237..e89e06f 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 @@ -1,13 +1,10 @@ package de.platon42.intellij.plugins.cajon.inspections -import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder -import com.intellij.openapi.util.TextRange import com.intellij.psi.JavaElementVisitor import com.intellij.psi.PsiElementVisitor import com.intellij.psi.PsiMethodCallExpression import com.intellij.psi.PsiType -import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceSimpleMethodCallQuickFix import org.jetbrains.annotations.NonNls class AssertThatObjectIsNotNullInspection : AbstractAssertJInspection() { @@ -15,12 +12,6 @@ class AssertThatObjectIsNotNullInspection : AbstractAssertJInspection() { companion object { @NonNls private val DISPLAY_NAME = "Asserting non-null" - - @NonNls - private val INSPECTION_MESSAGE = "isNotEqualTo(null) can be simplified to isNotNull()" - - @NonNls - private val QUICKFIX_DESCRIPTION = "Replace isNotEqualTo(null) with isNotNull()" } override fun getDisplayName() = DISPLAY_NAME @@ -34,13 +25,7 @@ class AssertThatObjectIsNotNullInspection : AbstractAssertJInspection() { } if (expression.argumentList.expressions[0].type == PsiType.NULL) { - holder.registerProblem( - expression, - INSPECTION_MESSAGE, - ProblemHighlightType.INFORMATION, - null as TextRange?, - ReplaceSimpleMethodCallQuickFix(QUICKFIX_DESCRIPTION, "isNotNull()") - ) + registerSimplifyMethod(holder, expression, "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 eb59be7..f963031 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 @@ -1,13 +1,10 @@ package de.platon42.intellij.plugins.cajon.inspections -import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder -import com.intellij.openapi.util.TextRange import com.intellij.psi.JavaElementVisitor import com.intellij.psi.PsiElementVisitor import com.intellij.psi.PsiMethodCallExpression import com.intellij.psi.PsiType -import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceSimpleMethodCallQuickFix import org.jetbrains.annotations.NonNls class AssertThatObjectIsNullInspection : AbstractAssertJInspection() { @@ -15,12 +12,6 @@ class AssertThatObjectIsNullInspection : AbstractAssertJInspection() { companion object { @NonNls private val DISPLAY_NAME = "Asserting null" - - @NonNls - private val INSPECTION_MESSAGE = "isEqualTo(null) can be simplified to isNull()" - - @NonNls - private val QUICKFIX_DESCRIPTION = "Replace isEqualTo(null) with isNull()" } override fun getDisplayName() = DISPLAY_NAME @@ -34,13 +25,7 @@ class AssertThatObjectIsNullInspection : AbstractAssertJInspection() { } if (expression.argumentList.expressions[0].type == PsiType.NULL) { - holder.registerProblem( - expression, - INSPECTION_MESSAGE, - ProblemHighlightType.INFORMATION, - null as TextRange?, - ReplaceSimpleMethodCallQuickFix(QUICKFIX_DESCRIPTION, "isNull()") - ) + registerSimplifyMethod(holder, expression, "isNull()") } } } diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspection.kt index b604128..f6af10c 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspection.kt @@ -1,13 +1,10 @@ package de.platon42.intellij.plugins.cajon.inspections -import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder -import com.intellij.openapi.util.TextRange import com.intellij.psi.JavaElementVisitor import com.intellij.psi.PsiElementVisitor import com.intellij.psi.PsiLiteralExpression import com.intellij.psi.PsiMethodCallExpression -import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceSimpleMethodCallQuickFix import org.jetbrains.annotations.NonNls class AssertThatStringIsEmptyInspection : AbstractAssertJInspection() { @@ -15,12 +12,6 @@ class AssertThatStringIsEmptyInspection : AbstractAssertJInspection() { companion object { @NonNls private val DISPLAY_NAME = "Asserting an empty string" - - @NonNls - private val INSPECTION_MESSAGE = "isEqualTo(\"\") can be simplified to isEmpty()" - - @NonNls - private val QUICKFIX_DESCRIPTION = "Replace isEqualTo(\"\") with isEmpty()" } override fun getDisplayName() = DISPLAY_NAME @@ -29,24 +20,27 @@ class AssertThatStringIsEmptyInspection : AbstractAssertJInspection() { return object : JavaElementVisitor() { override fun visitMethodCallExpression(expression: PsiMethodCallExpression) { super.visitMethodCallExpression(expression) - if (!IS_EQUAL_TO_OBJECT.test(expression)) { + val isEqual = IS_EQUAL_TO_OBJECT.test(expression) + val hasSize = CHAR_SEQUENCE_HAS_SIZE.test(expression) + if (!(isEqual || hasSize)) { return } - if (!checkAssertedType(expression, ABSTRACT_STRING_ASSERT_CLASSNAME)) { + if (!checkAssertedType(expression, ABSTRACT_CHAR_SEQUENCE_ASSERT_CLASSNAME)) { return } - val psiExpression = expression.argumentList.expressions[0] as? PsiLiteralExpression ?: return + if (isEqual) { + val psiExpression = expression.argumentList.expressions[0] as? PsiLiteralExpression ?: return - if (psiExpression.value == "") { - holder.registerProblem( - expression, - INSPECTION_MESSAGE, - ProblemHighlightType.INFORMATION, - null as TextRange?, - ReplaceSimpleMethodCallQuickFix(QUICKFIX_DESCRIPTION, "isEmpty()") - ) + if (psiExpression.value == "") { + registerSimplifyMethod(holder, expression, "isEmpty()") + } + } else { + val psiExpression = expression.argumentList.expressions[0] as? PsiLiteralExpression ?: return + if (psiExpression.value == 0) { + registerSimplifyMethod(holder, expression, "isEmpty()") + } } } } diff --git a/src/main/resources/inspectionDescriptions/AssertThatStringIsEmpty.html b/src/main/resources/inspectionDescriptions/AssertThatStringIsEmpty.html new file mode 100644 index 0000000..c3df8c2 --- /dev/null +++ b/src/main/resources/inspectionDescriptions/AssertThatStringIsEmpty.html @@ -0,0 +1,7 @@ + + +Turns assertThat(string).isEqualTo("") or assertThat(string).hasSize(0) into assertThat(string).isEmpty(). + +Also works with CharSequences. + + \ 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 bab2561..8e64fa7 100644 --- a/src/test/java/de/platon42/intellij/playground/Playground.java +++ b/src/test/java/de/platon42/intellij/playground/Playground.java @@ -1,7 +1,5 @@ package de.platon42.intellij.playground; -import org.assertj.core.api.AbstractStringAssert; - import java.util.ArrayList; import static org.assertj.core.api.Assertions.assertThat; @@ -52,8 +50,8 @@ public class Playground { private void stringIsEmpty() { String foo = "bar"; - AbstractStringAssert abstractStringAssert = assertThat(foo); - abstractStringAssert.isEqualTo(""); + assertThat(foo).isEqualTo(""); + assertThat(foo).hasSize(0); } } 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 c14ddb4..9b39b9a 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, Regex.fromLiteral("Replace isNotEqualTo(null) with isNotNull()"), 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Replace isNotEqualTo() 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 a5bed5e..0a29ee8 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, Regex.fromLiteral("Replace isEqualTo(null) with isNull()"), 3) + executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with isNull()"), 3) myFixture.checkResultByFile("ObjectIsNullAfter.java") } } diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspectionTest.kt index 7b8c1af..b621a7d 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringIsEmptyInspectionTest.kt @@ -14,7 +14,7 @@ internal class AssertThatStringIsEmptyInspectionTest : AbstractCajonTest() { runTest { myFixture.enableInspections(AssertThatStringIsEmptyInspection::class.java) myFixture.configureByFile("StringIsEmptyBefore.java") - executeQuickFixes(myFixture, Regex("Replace is.*"), 1) + executeQuickFixes(myFixture, Regex("Replace .*"), 4) myFixture.checkResultByFile("StringIsEmptyAfter.java") } } diff --git a/src/test/resources/inspections/StringIsEmpty/StringIsEmptyAfter.java b/src/test/resources/inspections/StringIsEmpty/StringIsEmptyAfter.java index 7b17cad..441fec0 100644 --- a/src/test/resources/inspections/StringIsEmpty/StringIsEmptyAfter.java +++ b/src/test/resources/inspections/StringIsEmpty/StringIsEmptyAfter.java @@ -4,9 +4,16 @@ public class StringIsEmpty { private void stringIsEmpty() { String string = "string"; + StringBuilder stringBuilder = new StringBuilder(); assertThat(string).isEqualTo("foo"); assertThat(string).as("foo").isEmpty(); + assertThat(string).as("bar").isEmpty(); + + assertThat(stringBuilder).isEqualTo("foo"); + assertThat(stringBuilder).as("foo").isEmpty(); + assertThat(stringBuilder).as("bar").isEmpty(); + assertThat(new Object()).isEqualTo(""); } } diff --git a/src/test/resources/inspections/StringIsEmpty/StringIsEmptyBefore.java b/src/test/resources/inspections/StringIsEmpty/StringIsEmptyBefore.java index f8b7dab..34226c2 100644 --- a/src/test/resources/inspections/StringIsEmpty/StringIsEmptyBefore.java +++ b/src/test/resources/inspections/StringIsEmpty/StringIsEmptyBefore.java @@ -4,9 +4,16 @@ public class StringIsEmpty { private void stringIsEmpty() { String string = "string"; + StringBuilder stringBuilder = new StringBuilder(); assertThat(string).isEqualTo("foo"); assertThat(string).as("foo").isEqualTo(""); + assertThat(string).as("bar").hasSize(0); + + assertThat(stringBuilder).isEqualTo("foo"); + assertThat(stringBuilder).as("foo").isEqualTo(""); + assertThat(stringBuilder).as("bar").hasSize(0); + assertThat(new Object()).isEqualTo(""); } }