chrisly42/cajon-plugin
1
0
Fork 0
Code Issues 4 Pull Requests 1 Packages Projects Releases Wiki Activity

Added Guava Optional opt.orNull() == null case. Added Java 8 Optional opt.orElse(null) == null case, too.

Browse Source
This commit is contained in:
Chris Hodges 2019-05-15 20:50:30 +02:00
parent 94c695617a
commit 7133c55710
16 changed files with 134 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
README.md
View File

@ -265,7 +265,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
to: assertThat(primActual).isEqualTo(primExpected);
from: assertThat(10 < primActual).isNotEqualTo(false);
to: assertThat(primActual).isGreaterThan(primExpected);
to: assertThat(primActual).isGreaterThan(10);
from: assertThat(objActual != objExpected).isEqualTo(true);
to: assertThat(objActual).isNotSameAs(objExpected);
@ -301,6 +301,12 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
from: assertThat(opt.get()).isSameAs("foo");
to: assertThat(opt).containsSame("foo");
from: assertThat(opt.orElse(null)).isEqualTo(null);
to: assertThat(opt).isNotPresent();
from: assertThat(opt.orElse(null)).isNotEqualTo(null);
to: assertThat(opt).isPresent();
from: assertThat(opt).isEqualTo(Optional.of("foo"));
from: assertThat(opt).isEqualTo(Optional.ofNullable("foo"));
to: assertThat(opt).contains("foo");
@ -332,6 +338,12 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
from: assertThat(opt.get()).isEqualTo("foo");
to: assertThat(opt).contains("foo");
from: assertThat(opt.orNull()).isEqualTo(null);
to: assertThat(opt).isAbsent();
from: assertThat(opt.orNull()).isNotEqualTo(null);
to: assertThat(opt).isPresent();
from: assertThat(opt).isEqualTo(Optional.of("foo"));
from: assertThat(opt).isEqualTo(Optional.fromNullable("foo"));
to: assertThat(opt).contains("foo");
@ -436,8 +448,8 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
.flatExtracting(Extractors.resultOf("bareMethod")
```
Works on both POJOs and ```Iterable```s/```Array```s.
Implementation is very basic though and does not work with fancy cascaded .extracting() sequences.
If there's demand, I will add it.
Implementation is very basic though and does not work with fancy cascaded ```.extracting()``` sequences.
If there's demand, I could add it.
## Development notice
@ -450,6 +462,7 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo
## Planned features
- Joining .contains() expressions
- Removing .isPresent().contains() combinations for Optionals
- Converting ```foo.compareTo(bar) == 0``` to ```isEqualTo()``` (yes, I've *really* seen code like that)
- Extraction with property names to lambda with Java 8
```
from: assertThat(object).extracting("propOne", "propNoGetter", "propTwo.innerProp")...
@ -460,6 +473,8 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo
#### V1.1 (unreleased)
- Improved JoinAssertThatStatements detection of expressions with side-effects and added pre/post-increment/decrement detection.
- Added Guava Optional ```opt.orNull() == null``` case. You know, I'm not making this stuff up, people actually write this kind of code.
- Added Java 8 Optional ```opt.orElse(null) == null``` case, too.
#### V1.0 (06-May-19)
- First release to be considered stable enough for production use.

4
build.gradle
View File

@ -35,7 +35,7 @@ compileTestKotlin {
kotlinOptions.jvmTarget = "1.8"
}
intellij {
version '2019.1.1'
version '2019.1.2'
// pluginName 'Concise AssertJ Optimizing Nitpicker (Cajon)'
updateSinceUntilBuild false
}
@ -45,6 +45,8 @@ patchPluginXml {
<h4>V1.1 (unreleased)</h4>
<ul>
<li>Improved JoinAssertThatStatements detection of expressions with side-effects and added pre/post-increment/decrement detection.
<li>Added Guava Optional opt.orNull() == null case. You know, I'm not making this stuff up, people actually write this kind of code.
<li>Added Java 8 Optional opt.orElse(null) == null case, too.
</ul>
<h4>V1.0 (06-May-19)</h4>
<ul>

42
src/main/java/de/platon42/intellij/plugins/cajon/Extensions.kt
View File

@ -100,6 +100,21 @@ fun PsiMethodCallExpression.getExpectedBooleanResult(): Boolean? {
return null
}
fun PsiMethodCallExpression.getExpectedNullNonNullResult(): Boolean? {
val isNull = AbstractAssertJInspection.IS_NULL.test(this)
val isNotNull = AbstractAssertJInspection.IS_NOT_NULL.test(this)
if (isNull || isNotNull) {
return isNotNull
} else {
val isEqualTo = AbstractAssertJInspection.IS_EQUAL_TO_OBJECT.test(this)
val isNotEqualTo = AbstractAssertJInspection.IS_NOT_EQUAL_TO_OBJECT.test(this)
if ((isEqualTo || isNotEqualTo) && firstArg.type == PsiType.NULL) {
return isNotEqualTo
}
}
return null
}
fun PsiMethodCallExpression.calculateConstantParameterValue(argIndex: Int): Any? {
if (argIndex >= argumentList.expressions.size) return null
val valueExpression = getArg(argIndex)
@ -145,4 +160,31 @@ fun PsiExpression.getAllTheSameExpectedBooleanConstants(): Boolean? {
}
}
return lockedResult
}
fun PsiExpression.getAllTheSameNullNotNullConstants(): Boolean? {
val assertThatMethodCall = findStaticMethodCall() ?: return null
var lockedResult: Boolean? = null
val methodsToView = generateSequence(assertThatMethodCall) { PsiTreeUtil.getParentOfType(it, PsiMethodCallExpression::class.java) }
for (methodCall in methodsToView) {
val expectedResult = methodCall.getExpectedNullNonNullResult()
if (expectedResult != null) {
if ((lockedResult != null) && (lockedResult != expectedResult)) {
return null
}
lockedResult = expectedResult
} else {
val isNotConstant = CallMatcher.anyOf(
EXTENSION_POINTS,
MORE_EXTENSION_POINTS,
AbstractAssertJInspection.IS_EQUAL_TO_OBJECT,
AbstractAssertJInspection.IS_NOT_EQUAL_TO_OBJECT
).test(methodCall)
if (isNotConstant) {
return null
}
}
}
return lockedResult
}

9
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AbstractAssertJInspection.kt
View File

@ -93,6 +93,11 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
val IS_NOT_SAME_AS_OBJECT = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_NOT_SAME_AS)
.parameterTypes(CommonClassNames.JAVA_LANG_OBJECT)!!
val IS_NULL = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_NULL)
.parameterCount(0)!!
val IS_NOT_NULL = CallMatcher.instanceCall(ASSERT_INTERFACE, MethodNames.IS_NOT_NULL)
.parameterCount(0)!!
val HAS_SIZE = CallMatcher.instanceCall(ABSTRACT_ENUMERABLE_ASSERT_CLASSNAME, MethodNames.HAS_SIZE)
.parameterTypes("int")!!
@ -129,6 +134,8 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
.parameterCount(0)!!
val OPTIONAL_IS_PRESENT = CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_OPTIONAL, "isPresent")
.parameterCount(0)!!
val OPTIONAL_OR_ELSE = CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_OPTIONAL, "orElse")
.parameterCount(1)!!
val OPTIONAL_OF = CallMatcher.staticCall(CommonClassNames.JAVA_UTIL_OPTIONAL, "of")
.parameterCount(1)!!
@ -141,6 +148,8 @@ open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
.parameterCount(0)!!
val GUAVA_OPTIONAL_IS_PRESENT = CallMatcher.instanceCall(GUAVA_OPTIONAL_CLASSNAME, "isPresent")
.parameterCount(0)!!
val GUAVA_OPTIONAL_OR_NULL = CallMatcher.instanceCall(GUAVA_OPTIONAL_CLASSNAME, "orNull")
.parameterCount(0)!!
val GUAVA_OPTIONAL_OF = CallMatcher.staticCall(GUAVA_OPTIONAL_CLASSNAME, "of")
.parameterCount(1)!!

4
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatBinaryExpressionInspection.kt
View File

@ -36,7 +36,9 @@ class AssertThatBinaryExpressionInspection : AbstractAssertJInspection() {
val assertThatArgument = staticMethodCall.firstArg
if (assertThatArgument is PsiMethodCallExpression && OBJECT_EQUALS.test(assertThatArgument)) {
val replacementMethod = expectedResult.map(MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO)
registerSplitMethod(holder, expectedCallExpression, "${MethodNames.EQUALS}()", replacementMethod, ::MoveOutMethodCallExpressionQuickFix)
registerSplitMethod(holder, expectedCallExpression, "${MethodNames.EQUALS}()", replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
return
}

4
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatCollectionOrMapExpressionInspection.kt
View File

@ -58,7 +58,9 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return
val replacementMethod = if (expectedResult) mapping.replacementForTrue else mapping.replacementForFalse ?: return
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod, ::MoveOutMethodCallExpressionQuickFix)
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
}
}
}

9
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspection.kt
View File

@ -51,6 +51,15 @@ class AssertThatGuavaOptionalInspection : AbstractAssertJInspection() {
ForGuavaPostFix.REPLACE_BY_GUAVA_ASSERT_THAT_AND_STATIC_IMPORT
)
}
} else if (GUAVA_OPTIONAL_OR_NULL.test(actualExpression)) {
val expectedPresence = outmostMethodCall.getAllTheSameNullNotNullConstants() ?: return
val replacementMethod = expectedPresence.map(MethodNames.IS_PRESENT, MethodNames.IS_ABSENT)
registerMoveOutMethod(holder, outmostMethodCall, actualExpression, replacementMethod) { desc, method ->
QuickFixWithPostfixDelegate(
MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true),
ForGuavaPostFix.REPLACE_BY_GUAVA_ASSERT_THAT_AND_STATIC_IMPORT
)
}
}
}

11
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatJava8OptionalInspection.kt
View File

@ -1,10 +1,7 @@
package de.platon42.intellij.plugins.cajon.inspections
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.JavaElementVisitor
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.PsiExpressionStatement
import com.intellij.psi.PsiMethodCallExpression
import com.intellij.psi.*
import com.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
@ -50,6 +47,12 @@ class AssertThatJava8OptionalInspection : AbstractAssertJInspection() {
registerMoveOutMethod(holder, outmostMethodCall, actualExpression, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
} else if (OPTIONAL_OR_ELSE.test(actualExpression) && (actualExpression.firstArg.type == PsiType.NULL)) {
val expectedPresence = outmostMethodCall.getAllTheSameNullNotNullConstants() ?: return
val replacementMethod = expectedPresence.map(MethodNames.IS_PRESENT, MethodNames.IS_NOT_PRESENT)
registerMoveOutMethod(holder, outmostMethodCall, actualExpression, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true, noExpectedExpression = true)
}
}
}

4
src/main/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatStringExpressionInspection.kt
View File

@ -62,7 +62,9 @@ class AssertThatStringExpressionInspection : AbstractAssertJInspection() {
val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return
val replacementMethod = if (expectedResult) mapping.replacementForTrue else mapping.replacementForFalse
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod, ::MoveOutMethodCallExpressionQuickFix)
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
}
}
}

12
src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/MoveOutMethodCallExpressionQuickFix.kt
View File

@ -5,7 +5,13 @@ import com.intellij.openapi.project.Project
import com.intellij.psi.PsiMethodCallExpression
import de.platon42.intellij.plugins.cajon.*
class MoveOutMethodCallExpressionQuickFix(description: String, private val replacementMethod: String) : AbstractCommonQuickFix(description) {
class MoveOutMethodCallExpressionQuickFix(
description: String,
private val replacementMethod: String,
private val useNullNonNull: Boolean = false,
private val noExpectedExpression: Boolean = false
) :
AbstractCommonQuickFix(description) {
companion object {
private const val REMOVE_ACTUAL_EXPRESSION_DESCRIPTION = "Move method calls in actual expressions out of assertThat()"
@ -19,10 +25,10 @@ class MoveOutMethodCallExpressionQuickFix(description: String, private val repla
val outmostCallExpression = descriptor.startElement as? PsiMethodCallExpression ?: return
val assertThatMethodCall = outmostCallExpression.findStaticMethodCall() ?: return
val assertExpression = assertThatMethodCall.firstArg as? PsiMethodCallExpression ?: return
val assertExpressionArg = assertExpression.getArgOrNull(0)?.copy()
val assertExpressionArg = if (noExpectedExpression) null else assertExpression.getArgOrNull(0)?.copy()
val methodsToFix = assertThatMethodCall.collectMethodCallsUpToStatement()
.filter { it.getExpectedBooleanResult() != null }
.filter { (if (useNullNonNull) it.getExpectedNullNonNullResult() else it.getExpectedBooleanResult()) != null }
.toList()
assertExpression.replace(assertExpression.qualifierExpression)

2
src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatGuavaOptionalInspectionTest.kt
View File

@ -19,6 +19,8 @@ internal class AssertThatGuavaOptionalInspectionTest : AbstractCajonTest() {
myFixture.configureByFile("GuavaOptionalBefore.java")
executeQuickFixes(myFixture, Regex.fromLiteral("Remove isPresent() of actual expression and use assertThat().isPresent() instead"), 6)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove isPresent() of actual expression and use assertThat().isAbsent() instead"), 5)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove orNull() of actual expression and use assertThat().isPresent() instead"), 2)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove orNull() of actual expression and use assertThat().isAbsent() instead"), 2)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().contains() instead"), 1)
executeQuickFixes(myFixture, Regex.fromLiteral("Unwrap expected expression and replace isEqualTo() with contains()"), 6)
executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with Guava assertThat().isAbsent()"), 3)

2
src/test/java/de/platon42/intellij/plugins/cajon/inspections/AssertThatJava8OptionalInspectionTest.kt
View File

@ -16,6 +16,8 @@ internal class AssertThatJava8OptionalInspectionTest : AbstractCajonTest() {
myFixture.configureByFile("Java8OptionalBefore.java")
executeQuickFixes(myFixture, Regex.fromLiteral("Remove isPresent() of actual expression and use assertThat().isPresent() instead"), 6)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove isPresent() of actual expression and use assertThat().isNotPresent() instead"), 5)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove orElse() of actual expression and use assertThat().isPresent() instead"), 2)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove orElse() of actual expression and use assertThat().isNotPresent() instead"), 2)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().contains() instead"), 1)
executeQuickFixes(myFixture, Regex.fromLiteral("Remove get() of actual expression and use assertThat().containsSame() instead"), 1)
executeQuickFixes(myFixture, Regex.fromLiteral("Replace isEqualTo() with isNotPresent()"), 1)

5
src/test/resources/inspections/GuavaOptional/GuavaOptionalAfter.java
View File

@ -25,6 +25,11 @@ public class GuavaOptional {
assertThat(opt.get()).isNotEqualTo("foo");
assertThat(opt.get()).isNotSameAs("foo");
assertThat(opt).as("foo").isAbsent();
assertThat(opt).isAbsent();
assertThat(opt).isPresent();
assertThat(opt).isPresent();
assertThat(opt).as("foo").contains("foo");
assertThat(opt).contains("foo");
assertThat(opt).isNotEqualTo(Optional.of("foo"));

5
src/test/resources/inspections/GuavaOptional/GuavaOptionalBefore.java
View File

@ -25,6 +25,11 @@ public class GuavaOptional {
assertThat(opt.get()).isNotEqualTo("foo");
assertThat(opt.get()).isNotSameAs("foo");
assertThat(opt.orNull()).as("foo").isEqualTo(null);
assertThat(opt.orNull()).isNull();
assertThat(opt.orNull()).isNotEqualTo(null);
assertThat(opt.orNull()).isNotNull();
assertThat(opt).as("foo").isEqualTo(Optional.of("foo"));
assertThat(opt).isEqualTo(Optional.fromNullable("foo"));
assertThat(opt).isNotEqualTo(Optional.of("foo"));

7
src/test/resources/inspections/Java8Optional/Java8OptionalAfter.java
View File

@ -24,6 +24,11 @@ public class Java8Optional {
assertThat(opt.get()).isNotEqualTo("foo");
assertThat(opt.get()).isNotSameAs("foo");
assertThat(opt).as("foo").isNotPresent();
assertThat(opt).isNotPresent();
assertThat(opt).isPresent();
assertThat(opt).isPresent();
assertThat(opt).as("foo").contains("foo");
assertThat(opt).contains("foo");
assertThat(opt).isNotEqualTo(Optional.of("foo"));
@ -36,5 +41,7 @@ public class Java8Optional {
assertThat(opt.isPresent()).as("foo").isEqualTo(false).as("bar").isTrue();
assertThat(opt.get()).isEqualTo("foo").isSameAs("foo").isNotEqualTo("foo").isNotSameAs("foo");
assertThat(opt.orElse("foo")).as("foo").isEqualTo(null);
}
}

7
src/test/resources/inspections/Java8Optional/Java8OptionalBefore.java
View File

@ -24,6 +24,11 @@ public class Java8Optional {
assertThat(opt.get()).isNotEqualTo("foo");
assertThat(opt.get()).isNotSameAs("foo");
assertThat(opt.orElse(null)).as("foo").isEqualTo(null);
assertThat(opt.orElse(null)).isNull();
assertThat(opt.orElse(null)).isNotEqualTo(null);
assertThat(opt.orElse(null)).isNotNull();
assertThat(opt).as("foo").isEqualTo(Optional.of("foo"));
assertThat(opt).isEqualTo(Optional.ofNullable("foo"));
assertThat(opt).isNotEqualTo(Optional.of("foo"));
@ -36,5 +41,7 @@ public class Java8Optional {
assertThat(opt.isPresent()).as("foo").isEqualTo(false).as("bar").isTrue();
assertThat(opt.get()).isEqualTo("foo").isSameAs("foo").isNotEqualTo("foo").isNotSameAs("foo");
assertThat(opt.orElse("foo")).as("foo").isEqualTo(null);
}
}