diff --git a/.travis.yml b/.travis.yml index ebc3f32..8f7e0c0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: java jdk: - - openjdk8 + - openjdk11 before_script: - chmod +x gradlew diff --git a/README.md b/README.md index ba6219f..29ba179 100644 --- a/README.md +++ b/README.md @@ -142,12 +142,19 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the - AssertThatInvertedBooleanCondition - Inverts the boolean condition to make it more readable. + Inverts the boolean condition in either ```assertThat()``` or ```isEqualTo()```/```isNotEqualTo()``` + to make it more readable. ``` from: assertThat(!booleanValue).isEqualTo(true/false/Boolean.TRUE/Boolean.FALSE); from: assertThat(!booleanValue).isTrue()/isFalse(); to: assertThat(booleanValue).isFalse()/isTrue(); + + from: assertThat(booleanValue).isEqualTo(!primitiveBooleanExpression); + to: assertThat(booleanValue).isNotEqualTo(primitiveBooleanExpression); + + from: assertThat(booleanValue).isNotEqualTo(!primitiveBooleanExpression); + to: assertThat(booleanValue).isEqualTo(primitiveBooleanExpression); ``` - AssertThatInstanceOf @@ -812,10 +819,11 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo #### V1.11 (03-Oct-20) Day of German Unity Edition - Now is being built with JDK 11 (with Java 8 target). -- Updated various dependencies and AssertJ 3.17.2. -- Fixed the ImplicitAssertionInspection that broke the plugin with IntelliJ 2020.3 EAP as reported by Frédéric Thomas. Thanks! +- Updated various dependencies (Kotlin 1.40.10) and AssertJ 3.17.2. +- Fixed the ImplicitAssertion inspection that broke the plugin with IntelliJ 2020.3 EAP as reported by Frédéric Thomas. Thanks! - Added new singleElement() from AssertJ >= 3.17.0 to ImplicitAssertionInspection. - Added several cases for ```hasSizeGreaterThan/LessThan/OrEqualTo()``` for EnumerablesEmpty inspection. +- Added inversion of boolean conditions inside ```isEqualTo()``` and ```isNotEqualTo()``` for InvertedBooleanCondition inspection. #### V1.10 (31-Jul-20) Friday the 31st Edition - Updated libraries to the latest versions (including AssertJ 3.16.1 and Kotlin 1.40-rc). diff --git a/build.gradle b/build.gradle index d0b61bd..e6ac0b2 100644 --- a/build.gradle +++ b/build.gradle @@ -50,10 +50,11 @@ patchPluginXml {

V1.11 (03-Oct-20) Day of German Unity Edition

Full changelog available at Github project site.

""" 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 0e38e17..6f3c85f 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 @@ -72,6 +72,8 @@ abstract class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() val ASSERT_THAT_BOOLEAN = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, MethodNames.ASSERT_THAT) .parameterTypes("boolean")!! + val ASSERT_THAT_BOOLEAN_OBJ = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, MethodNames.ASSERT_THAT) + .parameterTypes(CommonClassNames.JAVA_LANG_BOOLEAN)!! val ASSERT_THAT_ANY = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, MethodNames.ASSERT_THAT) .parameterCount(1)!! diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanConditionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanConditionInspection.kt index 442e551..6a424a2 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanConditionInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBooleanConditionInspection.kt @@ -21,18 +21,19 @@ class AssertThatBooleanConditionInspection : AbstractAssertJInspection() { override fun visitMethodCallExpression(expression: PsiMethodCallExpression) { super.visitMethodCallExpression(expression) if (!expression.hasAssertThat()) return - val matchingCalls = listOf( - IS_EQUAL_TO_OBJECT, IS_EQUAL_TO_BOOLEAN, - IS_NOT_EQUAL_TO_OBJECT, IS_NOT_EQUAL_TO_BOOLEAN - ).map { it.test(expression) } - if (matchingCalls.none { it }) return + val isEqualToObject = IS_EQUAL_TO_OBJECT.test(expression) + val isEqualToPrimitive = IS_EQUAL_TO_BOOLEAN.test(expression) + val isNotEqualToObject = IS_NOT_EQUAL_TO_OBJECT.test(expression) + val isNotEqualToPrimitive = IS_NOT_EQUAL_TO_BOOLEAN.test(expression) + + if (!(isEqualToObject || isEqualToPrimitive || isNotEqualToObject || isNotEqualToPrimitive)) return if (!checkAssertedType(expression, ABSTRACT_BOOLEAN_ASSERT_CLASSNAME)) return val expectedExpression = expression.firstArg if (!TypeConversionUtil.isBooleanType(expectedExpression.type)) return val expectedResult = expression.calculateConstantParameterValue(0) as? Boolean ?: return - val flippedBooleanTest = matchingCalls.drop(2).any { it } + val flippedBooleanTest = isNotEqualToObject || isNotEqualToPrimitive val replacementMethod = (expectedResult xor flippedBooleanTest).map(MethodNames.IS_TRUE, MethodNames.IS_FALSE) registerSimplifyMethod(holder, expression, replacementMethod) diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspection.kt b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspection.kt index 39b86d7..1919a61 100644 --- a/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspection.kt +++ b/src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspection.kt @@ -1,8 +1,13 @@ package de.platon42.intellij.plugins.cajon.inspections import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.util.TextRange import com.intellij.psi.* +import com.intellij.psi.util.PsiUtil +import com.intellij.psi.util.TypeConversionUtil +import com.siyeh.ig.callMatcher.CallMatcher import de.platon42.intellij.plugins.cajon.* +import de.platon42.intellij.plugins.cajon.quickfixes.InvertUnaryExpressionQuickFix import de.platon42.intellij.plugins.cajon.quickfixes.InvertUnaryStatementQuickFix class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection() { @@ -10,6 +15,8 @@ class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection() companion object { private const val DISPLAY_NAME = "Asserting an inverted boolean condition" private const val INVERT_CONDITION_MESSAGE = "Condition inside assertThat() could be inverted" + private const val REMOVE_INSTANCEOF_DESCRIPTION_TEMPLATE = "Invert condition in %s() and use %s() instead" + private const val INVERT_OUTSIDE_CONDITION_MESSAGE = "Condition outside assertThat() could be inverted" } override fun getDisplayName() = DISPLAY_NAME @@ -20,13 +27,27 @@ class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection() super.visitMethodCallExpression(expression) if (!expression.hasAssertThat()) return val staticMethodCall = expression.findStaticMethodCall() ?: return - if (!ASSERT_THAT_BOOLEAN.test(staticMethodCall)) return - expression.getExpectedBooleanResult() ?: return + if (expression.getExpectedBooleanResult() != null) { + if (!ASSERT_THAT_BOOLEAN.test(staticMethodCall)) return + val prefixExpression = staticMethodCall.firstArg as? PsiPrefixExpression ?: return + if (prefixExpression.operationTokenType == JavaTokenType.EXCL) { + val outmostMethodCall = expression.findOutmostMethodCall() ?: return + holder.registerProblem(outmostMethodCall, INVERT_CONDITION_MESSAGE, InvertUnaryStatementQuickFix()) + } + } else { + if (!CallMatcher.anyOf(ASSERT_THAT_BOOLEAN, ASSERT_THAT_BOOLEAN_OBJ).test(staticMethodCall)) return + val isEqualTo = CallMatcher.anyOf(IS_EQUAL_TO_BOOLEAN, IS_EQUAL_TO_OBJECT).test(expression) + val isNotEqualTo = CallMatcher.anyOf(IS_NOT_EQUAL_TO_BOOLEAN, IS_NOT_EQUAL_TO_OBJECT).test(expression) + if (!(isEqualTo || isNotEqualTo)) return + val prefixExpression = expression.firstArg as? PsiPrefixExpression ?: return + val operand = PsiUtil.skipParenthesizedExprDown(prefixExpression.operand) ?: return + if (!TypeConversionUtil.isPrimitiveAndNotNull(operand.type)) return + val originalMethod = getOriginalMethodName(expression) ?: return - val prefixExpression = staticMethodCall.firstArg as? PsiPrefixExpression ?: return - if (prefixExpression.operationTokenType == JavaTokenType.EXCL) { - val outmostMethodCall = expression.findOutmostMethodCall() ?: return - holder.registerProblem(outmostMethodCall, INVERT_CONDITION_MESSAGE, InvertUnaryStatementQuickFix()) + val replacementMethod = isEqualTo.map(MethodNames.IS_NOT_EQUAL_TO, MethodNames.IS_EQUAL_TO) + val description = REMOVE_INSTANCEOF_DESCRIPTION_TEMPLATE.format(originalMethod, replacementMethod) + val textRange = TextRange(staticMethodCall.textLength, expression.textLength) + holder.registerProblem(expression, textRange, INVERT_OUTSIDE_CONDITION_MESSAGE, InvertUnaryExpressionQuickFix(description, replacementMethod)) } } } diff --git a/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/InvertUnaryExpressionQuickFix.kt b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/InvertUnaryExpressionQuickFix.kt new file mode 100644 index 0000000..03894f7 --- /dev/null +++ b/src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/InvertUnaryExpressionQuickFix.kt @@ -0,0 +1,31 @@ +package de.platon42.intellij.plugins.cajon.quickfixes + +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiMethodCallExpression +import com.intellij.psi.PsiUnaryExpression +import com.intellij.psi.util.PsiUtil +import de.platon42.intellij.plugins.cajon.createExpectedMethodCall +import de.platon42.intellij.plugins.cajon.firstArg +import de.platon42.intellij.plugins.cajon.replaceQualifierFromMethodCall + +class InvertUnaryExpressionQuickFix(description: String, private val replacementMethod: String) : AbstractCommonQuickFix(description) { + + companion object { + private const val INVERT_CONDITION_DESCRIPTION = "Invert condition in isEqualTo()/isNotEqualTo() expressions" + } + + override fun getFamilyName(): String { + return INVERT_CONDITION_DESCRIPTION + } + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val methodCall = descriptor.startElement as? PsiMethodCallExpression ?: return + val assertExpression = methodCall.firstArg as? PsiUnaryExpression ?: return + val operand = PsiUtil.skipParenthesizedExprDown(assertExpression.operand) ?: return + + val expectedExpression = createExpectedMethodCall(assertExpression, replacementMethod, operand) + expectedExpression.replaceQualifierFromMethodCall(methodCall) + methodCall.replace(expectedExpression) + } +} \ No newline at end of file diff --git a/src/main/resources/inspectionDescriptions/AssertThatInvertedBooleanCondition.html b/src/main/resources/inspectionDescriptions/AssertThatInvertedBooleanCondition.html index 9229f86..054c628 100644 --- a/src/main/resources/inspectionDescriptions/AssertThatInvertedBooleanCondition.html +++ b/src/main/resources/inspectionDescriptions/AssertThatInvertedBooleanCondition.html @@ -1,6 +1,6 @@ -Turns assertThat(!condition).isEqualTo(true/false) into assertThat(condition).isFalse()/isTrue(). +Turns assertThat(!condition).isEqualTo(true/false) into assertThat(condition).isFalse()/isTrue() and negates .is(Not)EqualTo(!var).
Also works with constant expressions and Boolean.TRUE/FALSE. diff --git a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspectionTest.kt b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspectionTest.kt index 09a9379..c414e23 100644 --- a/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspectionTest.kt +++ b/src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatInvertedBooleanConditionInspectionTest.kt @@ -14,6 +14,8 @@ internal class AssertThatInvertedBooleanConditionInspectionTest : AbstractCajonT myFixture.enableInspections(AssertThatInvertedBooleanConditionInspection::class.java) myFixture.configureByFile("InvertedBooleanConditionBefore.java") executeQuickFixes(myFixture, Regex.fromLiteral("Invert condition in assertThat()"), 25) + executeQuickFixes(myFixture, Regex.fromLiteral("Invert condition in isEqualTo() and use isNotEqualTo() instead"), 4) + executeQuickFixes(myFixture, Regex.fromLiteral("Invert condition in isNotEqualTo() and use isEqualTo() instead"), 2) myFixture.checkResultByFile("InvertedBooleanConditionAfter.java") } } \ No newline at end of file diff --git a/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionAfter.java b/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionAfter.java index 1e34718..751292e 100644 --- a/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionAfter.java +++ b/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionAfter.java @@ -3,7 +3,7 @@ import static org.assertj.core.api.Assertions.fail; public class InvertedBooleanCondition { - private void invertedBooleanCondition() { + private void invertedBooleanCondition(boolean primitiveVariable, Boolean objectVariable) { boolean primitive = false; Boolean object = Boolean.TRUE; @@ -37,6 +37,23 @@ public class InvertedBooleanCondition { assertThat(primitive).as("foo").isFalse().as("bar").isFalse(); assertThat(primitive).as("foo").isFalse().as("bar").isTrue(); + assertThat(primitive).isNotEqualTo(primitiveVariable); + assertThat(primitive).isEqualTo(primitiveVariable); + + assertThat(object).isNotEqualTo(primitiveVariable); + assertThat(object).isEqualTo(primitiveVariable); + + assertThat(primitive).isEqualTo(!objectVariable); // not safe for null value + assertThat(primitive).isNotEqualTo(!objectVariable); // not safe for null value + + assertThat(object).isEqualTo(!objectVariable); // not safe for null value + assertThat(object).isNotEqualTo(!objectVariable); // not safe for null value + + assertThat(primitive).isNotEqualTo(primitiveVariable); + assertThat(primitive).isNotEqualTo(primitiveVariable); + + assertThat(primitive).isEqualTo(!primitiveVariable && objectVariable); + org.junit.Assert.assertThat(object, null); fail("oh no!"); } diff --git a/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionBefore.java b/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionBefore.java index 1376716..7fe0e19 100644 --- a/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionBefore.java +++ b/src/test/resources/inspections/InvertedBooleanCondition/InvertedBooleanConditionBefore.java @@ -3,7 +3,7 @@ import static org.assertj.core.api.Assertions.fail; public class InvertedBooleanCondition { - private void invertedBooleanCondition() { + private void invertedBooleanCondition(boolean primitiveVariable, Boolean objectVariable) { boolean primitive = false; Boolean object = Boolean.TRUE; @@ -37,6 +37,23 @@ public class InvertedBooleanCondition { assertThat(!primitive).as("foo").isEqualTo(Boolean.TRUE).as("bar").isNotEqualTo(false); assertThat(!primitive).as("foo").isEqualTo(Boolean.TRUE).as("bar").isNotEqualTo(true); + assertThat(primitive).isEqualTo(!primitiveVariable); + assertThat(primitive).isNotEqualTo(!primitiveVariable); + + assertThat(object).isEqualTo(!primitiveVariable); + assertThat(object).isNotEqualTo(!primitiveVariable); + + assertThat(primitive).isEqualTo(!objectVariable); // not safe for null value + assertThat(primitive).isNotEqualTo(!objectVariable); // not safe for null value + + assertThat(object).isEqualTo(!objectVariable); // not safe for null value + assertThat(object).isNotEqualTo(!objectVariable); // not safe for null value + + assertThat(primitive).isEqualTo((!primitiveVariable)); + assertThat(primitive).isEqualTo(!(primitiveVariable)); + + assertThat(primitive).isEqualTo(!primitiveVariable && objectVariable); + org.junit.Assert.assertThat(object, null); fail("oh no!"); }