Added inversion of boolean conditions inside isEqualTo() and isNotEqualTo() for InvertedBooleanCondition inspection.

This commit is contained in:
Chris Hodges 2020-10-02 22:34:35 +02:00
parent d48c71d4ea
commit ce0b5ce872
11 changed files with 121 additions and 21 deletions

View File

@ -1,6 +1,6 @@
language: java language: java
jdk: jdk:
- openjdk8 - openjdk11
before_script: before_script:
- chmod +x gradlew - chmod +x gradlew

View File

@ -142,12 +142,19 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
- AssertThatInvertedBooleanCondition - 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).isEqualTo(true/false/Boolean.TRUE/Boolean.FALSE);
from: assertThat(!booleanValue).isTrue()/isFalse(); from: assertThat(!booleanValue).isTrue()/isFalse();
to: assertThat(booleanValue).isFalse()/isTrue(); 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 - 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 #### V1.11 (03-Oct-20) Day of German Unity Edition
- Now is being built with JDK 11 (with Java 8 target). - Now is being built with JDK 11 (with Java 8 target).
- Updated various dependencies and AssertJ 3.17.2. - Updated various dependencies (Kotlin 1.40.10) 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! - 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 new singleElement() from AssertJ >= 3.17.0 to ImplicitAssertionInspection.
- Added several cases for ```hasSizeGreaterThan/LessThan/OrEqualTo()``` for EnumerablesEmpty inspection. - 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 #### 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). - Updated libraries to the latest versions (including AssertJ 3.16.1 and Kotlin 1.40-rc).

View File

@ -50,10 +50,11 @@ patchPluginXml {
<h4>V1.11 (03-Oct-20) Day of German Unity Edition</h4> <h4>V1.11 (03-Oct-20) Day of German Unity Edition</h4>
<ul> <ul>
<li>Now is being built with JDK 11 (with Java 8 target). <li>Now is being built with JDK 11 (with Java 8 target).
<li>Updated various dependencies and AssertJ 3.17.2. <li>Updated various dependencies (Kotlin 1.40.10) and AssertJ 3.17.2.
<li>Fixed the ImplicitAssertionInspection that broke the plugin with IntelliJ 2020.3 EAP as reported by Frédéric Thomas. Thanks! <li>Fixed the ImplicitAssertion inspection that broke the plugin with IntelliJ 2020.3 EAP as reported by Frédéric Thomas. Thanks!
<li>Added new singleElement() from AssertJ >= 3.17.0 to ImplicitAssertionInspection. <li>Added new singleElement() from AssertJ >= 3.17.0 to ImplicitAssertionInspection.
<li>Added several cases for hasSizeGreaterThan/LessThan/OrEqualTo() for EnumerablesEmpty inspection. <li>Added several cases for hasSizeGreaterThan/LessThan/OrEqualTo() for EnumerablesEmpty inspection.
<li>Added inversion of boolean conditions inside isEqualTo() and isNotEqualTo() for InvertedBooleanCondition inspection.
</ul> </ul>
<p>Full changelog available at <a href="https://github.com/chrisly42/cajon-plugin#changelog">Github project site</a>.</p> <p>Full changelog available at <a href="https://github.com/chrisly42/cajon-plugin#changelog">Github project site</a>.</p>
""" """

View File

@ -72,6 +72,8 @@ abstract class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool()
val ASSERT_THAT_BOOLEAN = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, MethodNames.ASSERT_THAT) val ASSERT_THAT_BOOLEAN = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, MethodNames.ASSERT_THAT)
.parameterTypes("boolean")!! .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) val ASSERT_THAT_ANY = CallMatcher.staticCall(ASSERTIONS_CLASSNAME, MethodNames.ASSERT_THAT)
.parameterCount(1)!! .parameterCount(1)!!

View File

@ -21,18 +21,19 @@ class AssertThatBooleanConditionInspection : AbstractAssertJInspection() {
override fun visitMethodCallExpression(expression: PsiMethodCallExpression) { override fun visitMethodCallExpression(expression: PsiMethodCallExpression) {
super.visitMethodCallExpression(expression) super.visitMethodCallExpression(expression)
if (!expression.hasAssertThat()) return if (!expression.hasAssertThat()) return
val matchingCalls = listOf( val isEqualToObject = IS_EQUAL_TO_OBJECT.test(expression)
IS_EQUAL_TO_OBJECT, IS_EQUAL_TO_BOOLEAN, val isEqualToPrimitive = IS_EQUAL_TO_BOOLEAN.test(expression)
IS_NOT_EQUAL_TO_OBJECT, IS_NOT_EQUAL_TO_BOOLEAN val isNotEqualToObject = IS_NOT_EQUAL_TO_OBJECT.test(expression)
).map { it.test(expression) } val isNotEqualToPrimitive = IS_NOT_EQUAL_TO_BOOLEAN.test(expression)
if (matchingCalls.none { it }) return
if (!(isEqualToObject || isEqualToPrimitive || isNotEqualToObject || isNotEqualToPrimitive)) return
if (!checkAssertedType(expression, ABSTRACT_BOOLEAN_ASSERT_CLASSNAME)) return if (!checkAssertedType(expression, ABSTRACT_BOOLEAN_ASSERT_CLASSNAME)) return
val expectedExpression = expression.firstArg val expectedExpression = expression.firstArg
if (!TypeConversionUtil.isBooleanType(expectedExpression.type)) return if (!TypeConversionUtil.isBooleanType(expectedExpression.type)) return
val expectedResult = expression.calculateConstantParameterValue(0) as? Boolean ?: 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) val replacementMethod = (expectedResult xor flippedBooleanTest).map(MethodNames.IS_TRUE, MethodNames.IS_FALSE)
registerSimplifyMethod(holder, expression, replacementMethod) registerSimplifyMethod(holder, expression, replacementMethod)

View File

@ -1,8 +1,13 @@
package de.platon42.intellij.plugins.cajon.inspections package de.platon42.intellij.plugins.cajon.inspections
import com.intellij.codeInspection.ProblemsHolder import com.intellij.codeInspection.ProblemsHolder
import com.intellij.openapi.util.TextRange
import com.intellij.psi.* 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.*
import de.platon42.intellij.plugins.cajon.quickfixes.InvertUnaryExpressionQuickFix
import de.platon42.intellij.plugins.cajon.quickfixes.InvertUnaryStatementQuickFix import de.platon42.intellij.plugins.cajon.quickfixes.InvertUnaryStatementQuickFix
class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection() { class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection() {
@ -10,6 +15,8 @@ class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection()
companion object { companion object {
private const val DISPLAY_NAME = "Asserting an inverted boolean condition" 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 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 override fun getDisplayName() = DISPLAY_NAME
@ -20,13 +27,27 @@ class AssertThatInvertedBooleanConditionInspection : AbstractAssertJInspection()
super.visitMethodCallExpression(expression) super.visitMethodCallExpression(expression)
if (!expression.hasAssertThat()) return if (!expression.hasAssertThat()) return
val staticMethodCall = expression.findStaticMethodCall() ?: return val staticMethodCall = expression.findStaticMethodCall() ?: return
if (!ASSERT_THAT_BOOLEAN.test(staticMethodCall)) return if (expression.getExpectedBooleanResult() != null) {
expression.getExpectedBooleanResult() ?: return 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 val replacementMethod = isEqualTo.map(MethodNames.IS_NOT_EQUAL_TO, MethodNames.IS_EQUAL_TO)
if (prefixExpression.operationTokenType == JavaTokenType.EXCL) { val description = REMOVE_INSTANCEOF_DESCRIPTION_TEMPLATE.format(originalMethod, replacementMethod)
val outmostMethodCall = expression.findOutmostMethodCall() ?: return val textRange = TextRange(staticMethodCall.textLength, expression.textLength)
holder.registerProblem(outmostMethodCall, INVERT_CONDITION_MESSAGE, InvertUnaryStatementQuickFix()) holder.registerProblem(expression, textRange, INVERT_OUTSIDE_CONDITION_MESSAGE, InvertUnaryExpressionQuickFix(description, replacementMethod))
} }
} }
} }

View File

@ -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)
}
}

View File

@ -1,6 +1,6 @@
<html> <html>
<body> <body>
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).
<!-- tooltip end --> <!-- tooltip end -->
<br>Also works with constant expressions and Boolean.TRUE/FALSE. <br>Also works with constant expressions and Boolean.TRUE/FALSE.
</body> </body>

View File

@ -14,6 +14,8 @@ internal class AssertThatInvertedBooleanConditionInspectionTest : AbstractCajonT
myFixture.enableInspections(AssertThatInvertedBooleanConditionInspection::class.java) myFixture.enableInspections(AssertThatInvertedBooleanConditionInspection::class.java)
myFixture.configureByFile("InvertedBooleanConditionBefore.java") myFixture.configureByFile("InvertedBooleanConditionBefore.java")
executeQuickFixes(myFixture, Regex.fromLiteral("Invert condition in assertThat()"), 25) 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") myFixture.checkResultByFile("InvertedBooleanConditionAfter.java")
} }
} }

View File

@ -3,7 +3,7 @@ import static org.assertj.core.api.Assertions.fail;
public class InvertedBooleanCondition { public class InvertedBooleanCondition {
private void invertedBooleanCondition() { private void invertedBooleanCondition(boolean primitiveVariable, Boolean objectVariable) {
boolean primitive = false; boolean primitive = false;
Boolean object = Boolean.TRUE; 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").isFalse();
assertThat(primitive).as("foo").isFalse().as("bar").isTrue(); 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); org.junit.Assert.assertThat(object, null);
fail("oh no!"); fail("oh no!");
} }

View File

@ -3,7 +3,7 @@ import static org.assertj.core.api.Assertions.fail;
public class InvertedBooleanCondition { public class InvertedBooleanCondition {
private void invertedBooleanCondition() { private void invertedBooleanCondition(boolean primitiveVariable, Boolean objectVariable) {
boolean primitive = false; boolean primitive = false;
Boolean object = Boolean.TRUE; 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(false);
assertThat(!primitive).as("foo").isEqualTo(Boolean.TRUE).as("bar").isNotEqualTo(true); 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); org.junit.Assert.assertThat(object, null);
fail("oh no!"); fail("oh no!");
} }