Added AssertThatComparableExpression for funny compareTo() uses.
This commit is contained in:
parent
8133f3850f
commit
42429c0f72
33
README.md
33
README.md
@ -234,6 +234,35 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
|
||||
to: assertThat(objActual).hasSameHashCodeAs(objExpected);
|
||||
```
|
||||
|
||||
- AssertThatComparableExpression
|
||||
|
||||
Handles ```compareTo()``` inside an expected expression.
|
||||
|
||||
```
|
||||
from: assertThat(obj1.compareTo(obj2)).isEqualTo(0);
|
||||
to: assertThat(obj1).isEqualByComparingTo(obj2);
|
||||
|
||||
from: assertThat(obj1.compareTo(obj2)).isNotZero();
|
||||
to: assertThat(obj1).isNotEqualByComparingTo(obj2);
|
||||
|
||||
from: assertThat(obj1.compareTo(obj2)).isNotEqualTo(-1);
|
||||
from: assertThat(obj1.compareTo(obj2)).isGreaterThanOrEqualTo(0);
|
||||
from: assertThat(obj1.compareTo(obj2)).isGreaterThan(-1);
|
||||
from: assertThat(obj1.compareTo(obj2)).isNotNegative();
|
||||
to: assertThat(obj1).isGreaterThanOrEqualTo(obj2);
|
||||
|
||||
from: assertThat(obj1.compareTo(obj2)).isOne();
|
||||
to: assertThat(obj1).isGreaterThan(obj2);
|
||||
|
||||
from: assertThat(obj1.compareTo(obj2)).isNotPositive();
|
||||
to: assertThat(obj1).isLessThanOrEqualTo(obj2);
|
||||
|
||||
from: assertThat(obj1.compareTo(obj2)).isLessThan(0);
|
||||
to: assertThat(obj1).isLessThan(obj2);
|
||||
```
|
||||
|
||||
Several more combinations omitted...
|
||||
|
||||
- AssertThatCollectionOrMapExpression
|
||||
|
||||
Moves ```Collection``` and ```Map``` operations inside ```assertThat()``` out.
|
||||
@ -657,8 +686,7 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo
|
||||
|
||||
## Planned features
|
||||
- More Optional fixes such as ```opt1.get() == opt2.get()``` etc.
|
||||
- More moving out of methods for Path, LocalDate/Time etc.
|
||||
- Converting ```foo.compareTo(bar) == 0``` to ```isEqualTo()``` (yes, I've *really* seen code like that)
|
||||
- More moving out of methods for LocalDate/Time etc.
|
||||
- Extraction with property names to lambda/method reference with Java 8
|
||||
|
||||
```
|
||||
@ -672,6 +700,7 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo
|
||||
- Fixed a lapsuus in AssertThatFileExpression also transforming ```.listFiles()``` with a filter argument.
|
||||
- Added first version of AssertThatPathExpression for a limited number transformations (more stuff is possible,
|
||||
but requires detection and transformation of static ```Files```-methods).
|
||||
- Added AssertThatComparableExpression for funny ```compareTo()``` uses.
|
||||
|
||||
#### V1.6 (30-Sep-19)
|
||||
- Really fixed AssertThatGuavaOptional inspections to avoid conversions from ```.get()``` to ```.contains()```
|
||||
|
||||
@ -48,6 +48,7 @@ patchPluginXml {
|
||||
<li>Fixed a lapsuus in AssertThatFileExpression also transforming listFiles() with a filter argument.
|
||||
<li>Added first version of AssertThatPathExpression for a limited number transformations (more stuff is possible,
|
||||
but requires detection and transformation of static Files-methods).
|
||||
<li>Added AssertThatComparableExpression for funny compareTo() uses.
|
||||
</ul>
|
||||
<p>Full changelog available at <a href="https://github.com/chrisly42/cajon-plugin#changelog">Github project site</a>.</p>
|
||||
"""
|
||||
|
||||
@ -141,6 +141,8 @@ abstract class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool()
|
||||
.parameterCount(0)!!
|
||||
val IS_NOT_ZERO = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, MethodNames.IS_NOT_ZERO)
|
||||
.parameterCount(0)!!
|
||||
val IS_ONE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isOne")
|
||||
.parameterCount(0)!!
|
||||
val IS_NEGATIVE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isNegative")
|
||||
.parameterCount(0)!!
|
||||
val IS_NOT_NEGATIVE = CallMatcher.instanceCall(ABSTRACT_INTEGER_ASSERT_CLASSNAME, "isNotNegative")
|
||||
|
||||
@ -0,0 +1,115 @@
|
||||
package de.platon42.intellij.plugins.cajon.inspections
|
||||
|
||||
import com.intellij.codeInspection.ProblemsHolder
|
||||
import com.intellij.psi.*
|
||||
import com.siyeh.ig.callMatcher.CallMatcher
|
||||
import de.platon42.intellij.plugins.cajon.MethodNames
|
||||
import de.platon42.intellij.plugins.cajon.calculateConstantValue
|
||||
import de.platon42.intellij.plugins.cajon.firstArg
|
||||
|
||||
class AssertThatComparableInspection : AbstractMoveOutInspection() {
|
||||
|
||||
companion object {
|
||||
private const val DISPLAY_NAME = "Asserting a compareTo() expression"
|
||||
|
||||
private val ARG_IS_ZERO_CONST: (PsiExpressionStatement, PsiMethodCallExpression) -> Boolean = { _, call -> call.firstArg.calculateConstantValue() == 0 }
|
||||
private val ARG_IS_PLUS_ONE_CONST: (PsiExpressionStatement, PsiMethodCallExpression) -> Boolean = { _, call -> call.firstArg.calculateConstantValue() == 1 }
|
||||
private val ARG_IS_MINUS_ONE_CONST: (PsiExpressionStatement, PsiMethodCallExpression) -> Boolean = { _, call -> call.firstArg.calculateConstantValue() == -1 }
|
||||
|
||||
private val COMPARABLE_COMPARE_TO =
|
||||
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_COMPARABLE, "compareTo").parameterCount(1)
|
||||
|
||||
private val MAPPINGS = listOf(
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
"isEqualByComparingTo", expectedMatcher = IS_EQUAL_TO_INT, replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_ZERO_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
"isEqualByComparingTo", expectedMatcher = IS_ZERO, replaceFromOriginalMethod = true
|
||||
),
|
||||
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
"isNotEqualByComparingTo", expectedMatcher = IS_NOT_EQUAL_TO_INT, replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_ZERO_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
"isNotEqualByComparingTo", expectedMatcher = IS_NOT_ZERO, replaceFromOriginalMethod = true
|
||||
),
|
||||
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_GREATER_THAN_OR_EQUAL_TO, expectedMatcher = IS_GREATER_THAN_OR_EQUAL_TO_INT, replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_ZERO_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_GREATER_THAN_OR_EQUAL_TO, expectedMatcher = CallMatcher.anyOf(IS_NOT_EQUAL_TO_INT, IS_GREATER_THAN_INT), replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_MINUS_ONE_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_GREATER_THAN_OR_EQUAL_TO, expectedMatcher = IS_NOT_NEGATIVE, replaceFromOriginalMethod = true
|
||||
),
|
||||
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_GREATER_THAN, expectedMatcher = CallMatcher.anyOf(IS_EQUAL_TO_INT, IS_GREATER_THAN_OR_EQUAL_TO_INT), replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_PLUS_ONE_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_GREATER_THAN, expectedMatcher = IS_GREATER_THAN_INT, replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_ZERO_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_GREATER_THAN, expectedMatcher = CallMatcher.anyOf(IS_POSITIVE, IS_ONE), replaceFromOriginalMethod = true
|
||||
),
|
||||
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_LESS_THAN_OR_EQUAL_TO, expectedMatcher = IS_LESS_THAN_OR_EQUAL_TO_INT, replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_ZERO_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_LESS_THAN_OR_EQUAL_TO, expectedMatcher = CallMatcher.anyOf(IS_NOT_EQUAL_TO_INT, IS_LESS_THAN_INT), replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_PLUS_ONE_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_LESS_THAN_OR_EQUAL_TO, expectedMatcher = IS_NOT_POSITIVE, replaceFromOriginalMethod = true
|
||||
),
|
||||
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_LESS_THAN, expectedMatcher = CallMatcher.anyOf(IS_EQUAL_TO_INT, IS_LESS_THAN_OR_EQUAL_TO_INT), replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_MINUS_ONE_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_LESS_THAN, expectedMatcher = IS_LESS_THAN_INT, replaceFromOriginalMethod = true,
|
||||
additionalCondition = ARG_IS_ZERO_CONST
|
||||
),
|
||||
MoveOutMapping(
|
||||
COMPARABLE_COMPARE_TO,
|
||||
MethodNames.IS_LESS_THAN, expectedMatcher = IS_NEGATIVE, replaceFromOriginalMethod = true
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
override fun getDisplayName() = DISPLAY_NAME
|
||||
|
||||
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
|
||||
return object : JavaElementVisitor() {
|
||||
override fun visitExpressionStatement(statement: PsiExpressionStatement) {
|
||||
super.visitExpressionStatement(statement)
|
||||
createInspectionsForMappings(statement, holder, MAPPINGS)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -41,6 +41,8 @@
|
||||
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatBinaryExpressionInspection"/>
|
||||
<localInspection groupPath="Java" shortName="AssertThatObjectExpression" enabledByDefault="true" level="WARNING"
|
||||
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatObjectExpressionInspection"/>
|
||||
<localInspection groupPath="Java" shortName="AssertThatComparable" enabledByDefault="true" level="WARNING"
|
||||
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatComparableInspection"/>
|
||||
<localInspection groupPath="Java" shortName="AssertThatStringExpression" enabledByDefault="true" level="WARNING"
|
||||
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssertThatStringExpressionInspection"/>
|
||||
<localInspection groupPath="Java" shortName="AssertThatCollectionOrMapExpression" enabledByDefault="true" level="WARNING"
|
||||
|
||||
@ -0,0 +1,5 @@
|
||||
<html>
|
||||
<body>
|
||||
Turns assertThat(obj1.compareTo(obj2)) into assertThat(obj1).someMethod(obj2).
|
||||
</body>
|
||||
</html>
|
||||
@ -0,0 +1,24 @@
|
||||
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 AssertThatComparableInspectionTest : AbstractCajonTest() {
|
||||
|
||||
@Test
|
||||
@TestDataSubPath("inspections/Comparable")
|
||||
internal fun assertThat_with_compareTo_method(@MyFixture myFixture: JavaCodeInsightTestFixture) {
|
||||
myFixture.enableInspections(AssertThatComparableInspection::class.java)
|
||||
myFixture.configureByFile("ComparableBefore.java")
|
||||
executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareTo() of actual expression and use assertThat().isEqualByComparingTo() instead"), 2)
|
||||
executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareTo() of actual expression and use assertThat().isNotEqualByComparingTo() instead"), 2)
|
||||
executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareTo() of actual expression and use assertThat().isGreaterThanOrEqualTo() instead"), 4)
|
||||
executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareTo() of actual expression and use assertThat().isGreaterThan() instead"), 5)
|
||||
executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareTo() of actual expression and use assertThat().isLessThanOrEqualTo() instead"), 4)
|
||||
executeQuickFixes(myFixture, Regex.fromLiteral("Remove compareTo() of actual expression and use assertThat().isLessThan() instead"), 4)
|
||||
myFixture.checkResultByFile("ComparableAfter.java")
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,41 @@
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.fail;
|
||||
|
||||
public class CompareTo {
|
||||
|
||||
private void comparable() {
|
||||
String string = "string";
|
||||
assertThat(string).isEqualByComparingTo("foo");
|
||||
assertThat(string).isEqualByComparingTo("foo");
|
||||
|
||||
assertThat(string).isNotEqualByComparingTo("foo");
|
||||
assertThat(string).isNotEqualByComparingTo("foo");
|
||||
|
||||
assertThat(string).isGreaterThanOrEqualTo("foo");
|
||||
assertThat(string).isGreaterThanOrEqualTo("foo");
|
||||
assertThat(string).isGreaterThanOrEqualTo("foo");
|
||||
assertThat(string).isGreaterThanOrEqualTo("foo");
|
||||
|
||||
assertThat(string).isGreaterThan("foo");
|
||||
assertThat(string).isGreaterThan("foo");
|
||||
assertThat(string).isGreaterThan("foo");
|
||||
assertThat(string).isGreaterThan("foo");
|
||||
assertThat(string).isGreaterThan("foo");
|
||||
|
||||
assertThat(string).isLessThanOrEqualTo("foo");
|
||||
assertThat(string).isLessThanOrEqualTo("foo");
|
||||
assertThat(string).isLessThanOrEqualTo("foo");
|
||||
assertThat(string).isLessThanOrEqualTo("foo");
|
||||
|
||||
assertThat(string).isLessThan("foo");
|
||||
assertThat(string).isLessThan("foo");
|
||||
assertThat(string).isLessThan("foo");
|
||||
assertThat(string).isLessThan("foo");
|
||||
|
||||
assertThat(string.compareTo("foo")).isNotEqualTo(2);
|
||||
assertThat(string.compareTo("foo")).isEqualTo(2);
|
||||
|
||||
org.junit.Assert.assertThat(string, null);
|
||||
fail("oh no!");
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,41 @@
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.fail;
|
||||
|
||||
public class CompareTo {
|
||||
|
||||
private void comparable() {
|
||||
String string = "string";
|
||||
assertThat(string.compareTo("foo")).isEqualTo(0);
|
||||
assertThat(string.compareTo("foo")).isZero();
|
||||
|
||||
assertThat(string.compareTo("foo")).isNotEqualTo(0);
|
||||
assertThat(string.compareTo("foo")).isNotZero();
|
||||
|
||||
assertThat(string.compareTo("foo")).isNotEqualTo(-1);
|
||||
assertThat(string.compareTo("foo")).isGreaterThanOrEqualTo(0);
|
||||
assertThat(string.compareTo("foo")).isGreaterThan(-1);
|
||||
assertThat(string.compareTo("foo")).isNotNegative();
|
||||
|
||||
assertThat(string.compareTo("foo")).isEqualTo(1);
|
||||
assertThat(string.compareTo("foo")).isOne();
|
||||
assertThat(string.compareTo("foo")).isGreaterThan(0);
|
||||
assertThat(string.compareTo("foo")).isPositive();
|
||||
assertThat(string.compareTo("foo")).isGreaterThanOrEqualTo(1);
|
||||
|
||||
assertThat(string.compareTo("foo")).isNotEqualTo(1);
|
||||
assertThat(string.compareTo("foo")).isLessThanOrEqualTo(0);
|
||||
assertThat(string.compareTo("foo")).isLessThan(1);
|
||||
assertThat(string.compareTo("foo")).isNotPositive();
|
||||
|
||||
assertThat(string.compareTo("foo")).isEqualTo(-1);
|
||||
assertThat(string.compareTo("foo")).isLessThan(0);
|
||||
assertThat(string.compareTo("foo")).isNegative();
|
||||
assertThat(string.compareTo("foo")).isLessThanOrEqualTo(-1);
|
||||
|
||||
assertThat(string.compareTo("foo")).isNotEqualTo(2);
|
||||
assertThat(string.compareTo("foo")).isEqualTo(2);
|
||||
|
||||
org.junit.Assert.assertThat(string, null);
|
||||
fail("oh no!");
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user