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

New JoinVarArgsContains inspection that will detect multiple .contains(), .doesNotContain(), and .containsOnlyOnce() calls within the same statement that could be joined together using variadic arguments.

Browse Source
This commit is contained in:
Chris Hodges 2019-08-03 20:57:34 +02:00
parent 4420a0a392
commit df11939589
16 changed files with 236 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
README.md
View File

@ -92,6 +92,23 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
The behavior regarding the insertion of line breaks between the expressions can be configured in the
inspection settings.
- JoinVarArgsContains
Looks for ```.contains()```, ```.doesNotContain()```, and .```containsOnlyOnce()``` calls for iterables
within the same statement. The available quickfix can join the arguments to variadic version of the call
and remove the surplus one.
```
from: assertThat(expected).contains("foo").doesNotContain("bar").contains("etc").doesNotContain("huh");
to: assertThat(expected).contains("foo", "etc").doesNotContain("bar", "huh");
```
Will not be performed on more complex statements with ```.extracting()``` or ```.as()``` to avoid
changing semantics or losing descriptions.
Note that the quickfix does not handle comments very well and might remove them during the operation.
You may need to perform some manual reformatting, if the line gets too long after applying the fix.
- AssertThatObjectIsNullOrNotNull
Uses ```isNull()``` and ```isNotNull()``` instead.
@ -343,7 +360,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
to: assertThat(opt).isPresent();
from: assertThat(opt).isEqualTo(Optional.of("foo"));
from: assertThat(opt).isEqualTo(Optional.ofNullable("foo"));
from: assertThat(opt).isEqualTo(Optional.ofNullable("foo")); // only for constant "foo"
to: assertThat(opt).contains("foo");
from: assertThat(opt).isEqualTo(Optional.empty());
@ -380,7 +397,7 @@ You can toggle the various inspections in the Settings/Editor/Inspections in the
to: assertThat(opt).isPresent();
from: assertThat(opt).isEqualTo(Optional.of("foo"));
from: assertThat(opt).isEqualTo(Optional.fromNullable("foo"));
from: assertThat(opt).isEqualTo(Optional.fromNullable("foo")); // only for constant "foo"
to: assertThat(opt).contains("foo");
from: assertThat(opt).isEqualTo(Optional.absent());
@ -509,7 +526,7 @@ The IntelliJ framework actually uses the JUnit 3 TestCase for plugin testing and
Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for your projects (with attribution).
## Planned features
- Joining ```.contains()``` expressions
- More Optional fixes such as opt1.get() == opt2.get() etc.
- Converting ```foo.compareTo(bar) == 0``` to ```isEqualTo()``` (yes, I've *really* seen code like that)
- Extraction with property names to lambda with Java 8
@ -520,7 +537,9 @@ Feel free to use the code (in package ```de.platon42.intellij.jupiter```) for yo
## Changelog
#### V1.3 (02-Aug-19)
#### V1.3 (03-Aug-19)
- New JoinVarArgsContains inspection that will detect multiple ```.contains()```, ```.doesNotContain()```,
and ```.containsOnlyOnce()``` calls within the same statement that could be joined together using variadic arguments.
- AssertJ 3.13.0 broke some inspections due to new ```AbstractStringAssert::isEqualTo()``` method.
- AssertThatJava8Optional and AssertThatGuavaOptional inspections do not longer try to fix
```assertThat(optional).isEqualTo(Optional.fromNullable(expression))``` to ```contains()```

4
build.gradle
View File

@ -43,8 +43,10 @@ intellij {
patchPluginXml {
changeNotes """
<h4>V1.3 (02-Aug-19)</h4>
<h4>V1.3 (03-Aug-19)</h4>
<ul>
<li>New JoinVarArgsContains inspection that will detect multiple .contains(), .doesNotContain(), and .containsOnlyOnce()
calls within the same statement that could be joined together using variadic arguments.
<li>AssertJ 3.13.0 broke some inspections due to new AbstractStringAssert::isEqualTo() method.
<li>AssertThatJava8Optional and AssertThatGuavaOptional inspections do not longer try to fix
assertThat(optional).isEqualTo(Optional.fromNullable(expression)) to contains()

8
src/main/java/de/platon42/intellij/plugins/cajon/CommonMatchers.kt
View File

@ -36,14 +36,18 @@ val MORE_EXTENSION_POINTS = CallMatcher.instanceCall(
"hasOnlyOneElementSatisfying", "anyMatch", "noneMatch", "anySatisfy", "noneSatisfy"
)!!
val NOT_ACTUAL_ASSERTIONS = CallMatcher.anyOf(
ALL_ASSERT_THAT_MATCHERS,
val COMPLEX_CALLS_THAT_MAKES_STUFF_TRICKY = CallMatcher.anyOf(
DESCRIBED_AS,
WITH_REPRESENTATION_AND_SUCH,
USING_COMPARATOR,
IN_HEXADECIMAL_OR_BINARY
)!!
val NOT_ACTUAL_ASSERTIONS = CallMatcher.anyOf(
ALL_ASSERT_THAT_MATCHERS,
COMPLEX_CALLS_THAT_MAKES_STUFF_TRICKY
)!!
val KNOWN_METHODS_WITH_SIDE_EFFECTS = CallMatcher.anyOf(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_ITERATOR, "next")
)!!

2
src/main/java/de/platon42/intellij/plugins/cajon/MethodNames.kt
View File

@ -86,6 +86,8 @@ class MethodNames {
@NonNls
const val CONTAINS = "contains"
@NonNls
const val CONTAINS_ONLY_ONCE = "containsOnlyOnce"
@NonNls
const val DOES_NOT_CONTAIN = "doesNotContain"
@NonNls
const val CONTAINS_EXACTLY = "containsExactly"

2
src/main/java/de/platon42/intellij/plugins/cajon/inspections/ImplicitAssertionInspection.kt
View File

@ -29,7 +29,7 @@ class ImplicitAssertionInspection : AbstractAssertJInspection() {
private val OBJECT_ENUMERABLE_ANY_CONTENT_ASSERTIONS = CallMatcher.instanceCall(
AssertJClassNames.OBJECT_ENUMERABLE_ASSERT_INTERFACE,
MethodNames.CONTAINS, "containsOnly", "containsOnlyNulls", "containsOnlyOnce",
MethodNames.CONTAINS, "containsOnly", "containsOnlyNulls", MethodNames.CONTAINS_ONLY_ONCE,
"containsExactly", "containsExactlyInAnyOrder", "containsExactlyInAnyOrderElementsOf",
"containsAll", "containsAnyOf",
"containsAnyElementsOf", "containsExactlyElementsOf", "containsOnlyElementsOf",

53
src/main/java/de/platon42/intellij/plugins/cajon/inspections/JoinVarArgsContainsInspection.kt Normal file
View File

@ -0,0 +1,53 @@
package de.platon42.intellij.plugins.cajon.inspections
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.PsiExpressionStatement
import com.intellij.psi.PsiMethodCallExpression
import com.intellij.psi.util.PsiTreeUtil
import com.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
import de.platon42.intellij.plugins.cajon.quickfixes.JoinVarArgsContainsQuickFix
class JoinVarArgsContainsInspection : AbstractAssertJInspection() {
companion object {
private const val DISPLAY_NAME = "Join variadic arguments of contains()/containsOnlyOnce()/doesNotContain()"
private const val JOIN_VARARGS_MESSAGE = "Calls to same methods may be joined to variadic version"
private val MATCHERS = listOf(MethodNames.CONTAINS, MethodNames.CONTAINS_ONLY_ONCE, MethodNames.DOES_NOT_CONTAIN)
.map { CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_ITERABLE_ASSERT_CLASSNAME, it) }
}
override fun getDisplayName() = DISPLAY_NAME
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
return object : JavaElementVisitor() {
override fun visitExpressionStatement(statement: PsiExpressionStatement) {
super.visitStatement(statement)
if (!statement.hasAssertThat()) return
val assertThatCall = PsiTreeUtil.findChildrenOfType(statement, PsiMethodCallExpression::class.java).find { ALL_ASSERT_THAT_MATCHERS.test(it) } ?: return
val allCalls = assertThatCall.collectMethodCallsUpToStatement().toList()
if (allCalls.find(COMPLEX_CALLS_THAT_MAKES_STUFF_TRICKY::test) != null) return
val onlyAssertionCalls = allCalls
.filterNot { NOT_ACTUAL_ASSERTIONS.test(it) }
.toList()
for (methodMatcher in MATCHERS) {
if (onlyAssertionCalls.count(methodMatcher::test) > 1) {
val outmostMethodCall = statement.findOutmostMethodCall() ?: return
val quickFix = JoinVarArgsContainsQuickFix(MATCHERS)
val textRange = TextRange(outmostMethodCall.qualifierExpression.textLength, outmostMethodCall.textLength)
holder.registerProblem(outmostMethodCall, textRange, JOIN_VARARGS_MESSAGE, quickFix)
return
}
}
}
}
}
}

38
src/main/java/de/platon42/intellij/plugins/cajon/quickfixes/JoinVarArgsContainsQuickFix.kt Normal file
View File

@ -0,0 +1,38 @@
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.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
class JoinVarArgsContainsQuickFix(private val matchers: Iterable<CallMatcher>) : AbstractCommonQuickFix(JOIN_VARARGS_DESCRIPTION) {
companion object {
private const val JOIN_VARARGS_DESCRIPTION = "Join multiple arguments to variadic argument method calls"
}
override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
var outmostCallExpression = descriptor.startElement as? PsiMethodCallExpression ?: return
for (matcher in matchers) {
val assertThatMethodCall = outmostCallExpression.findStaticMethodCall() ?: return
val methodsToFix = assertThatMethodCall.gatherAssertionCalls()
val matchedCalls = methodsToFix.filter(matcher::test)
if (matchedCalls.size > 1) {
val mainCall = matchedCalls.first()
val args = mutableListOf(*mainCall.argumentList.expressions)
for (secondaryCall in matchedCalls.asSequence().drop(1)) {
args.addAll(secondaryCall.argumentList.expressions)
}
val newMainCall = createExpectedMethodCall(mainCall, mainCall.methodExpression.qualifiedName, *args.toTypedArray())
newMainCall.replaceQualifierFromMethodCall(mainCall)
mainCall.replace(newMainCall)
for (secondaryCall in matchedCalls.asSequence().drop(1)) {
val newQualifier = secondaryCall.qualifierExpression
outmostCallExpression = secondaryCall.replace(newQualifier).findOutmostMethodCall() ?: return
}
}
}
}
}

2
src/main/resources/META-INF/plugin.xml
View File

@ -48,6 +48,8 @@
<localInspection groupPath="Java" shortName="JoinAssertThatStatements" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.JoinAssertThatStatementsInspection"/>
<localInspection groupPath="Java" shortName="JoinVarArgsContains" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.JoinVarArgsContainsInspection"/>
<localInspection groupPath="Java" shortName="AssumeThatInsteadOfReturn" enabledByDefault="true" level="WARNING"
implementationClass="de.platon42.intellij.plugins.cajon.inspections.AssumeThatInsteadOfReturnInspection"/>

11
src/main/resources/inspectionDescriptions/JoinVarArgsContains.html Normal file
View File

@ -0,0 +1,11 @@
<html>
<body>
Finds assertions where multiple .contains(), .containsOnlyOnce() or .doesNotContain() are
used in a single statement that could be joined together.
<!-- tooltip end -->
Only works when variadic arguments are possible and will not be performed on more complex
statements with .extracting() or .as() to avoid changing semantics.
<br>
Note that the quickfix does not handle comments very well and might remove them during the operation.
</body>
</html>

19
src/test/java/de/platon42/intellij/plugins/cajon/inspections/JoinVarArgsContainsInspectionTest.kt Normal file
View File

@ -0,0 +1,19 @@
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 JoinVarArgsContainsInspectionTest : AbstractCajonTest() {
@Test
@TestDataSubPath("inspections/JoinVarArgsContains")
internal fun join_contains_and_doesNotContain_together_where_possible(@MyFixture myFixture: JavaCodeInsightTestFixture) {
myFixture.enableInspections(JoinVarArgsContainsInspection::class.java)
myFixture.configureByFile("JoinVarArgsContainsBefore.java")
executeQuickFixes(myFixture, Regex.fromLiteral("Join multiple arguments to variadic argument method calls"), 3)
myFixture.checkResultByFile("JoinVarArgsContainsAfter.java")
}
}

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

@ -31,6 +31,13 @@ public class GuavaOptional {
assertThat(opt).isPresent();
assertThat(opt).isPresent();
//assertThat(opt.get()).isEqualTo(opt.get()); // there's a better version than contains(opt.get())
assertThat(opt.orNull()).isEqualTo(opt.get());
//assertThat(opt.get()).isEqualTo(opt.orNull()); // there's a better version than contains(opt.orNull())
assertThat(opt).contains(opt.get());
assertThat(opt).contains(opt.orNull());
String possibleNullString = System.getProperty("username");
String notNullString = "Narf";

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

@ -31,6 +31,13 @@ public class GuavaOptional {
assertThat(opt.orNull()).isNotEqualTo(null);
assertThat(opt.orNull()).isNotNull();
//assertThat(opt.get()).isEqualTo(opt.get()); // there's a better version than contains(opt.get())
assertThat(opt.orNull()).isEqualTo(opt.get());
//assertThat(opt.get()).isEqualTo(opt.orNull()); // there's a better version than contains(opt.orNull())
assertThat(opt).contains(opt.get());
assertThat(opt).contains(opt.orNull());
String possibleNullString = System.getProperty("username");
String notNullString = "Narf";

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

@ -30,6 +30,13 @@ public class Java8Optional {
assertThat(opt).isPresent();
assertThat(opt).isPresent();
//assertThat(opt.get()).isEqualTo(opt.get()); // there's a better version than contains(opt.get())
assertThat(opt.orElse(null)).isEqualTo(opt.get());
//assertThat(opt.get()).isEqualTo(opt.orElse(null)); // there's a better version than contains(opt.orElse(null))
assertThat(opt).contains(opt.get());
assertThat(opt).contains(opt.orElse(null));
String possibleNullString = System.getProperty("username");
String notNullString = "Narf";

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

@ -30,6 +30,13 @@ public class Java8Optional {
assertThat(opt.orElse(null)).isNotEqualTo(null);
assertThat(opt.orElse(null)).isNotNull();
//assertThat(opt.get()).isEqualTo(opt.get()); // there's a better version than contains(opt.get())
assertThat(opt.orElse(null)).isEqualTo(opt.get());
//assertThat(opt.get()).isEqualTo(opt.orElse(null)); // there's a better version than contains(opt.orElse(null))
assertThat(opt).contains(opt.get());
assertThat(opt).contains(opt.orElse(null));
String possibleNullString = System.getProperty("username");
String notNullString = "Narf";

22
src/test/resources/inspections/JoinVarArgsContains/JoinVarArgsContainsAfter.java Normal file
View File

@ -0,0 +1,22 @@
import java.util.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
public class JoinVarArgsContains {
private void joinVarArgsContains() {
List<String> list = new ArrayList<>();
assertThat(list).contains("foo", "bar", "etc").hasSize(2);
assertThat(list).contains("foo").as("narf").contains("bar");
assertThat(list).doesNotContain("foo", "bar");
assertThat(list).containsOnlyOnce("foo", "etc") // will we lose this comment?
.hasSize(2).contains("bar", "narf", "1", "2", "3").doesNotContain("puit", "Jens Stoltenberg is a war-monger", "and an atomic playboy") /* inline */; // the final comment
assertThat(list).contains("foo").doesNotContain("bar").containsOnlyOnce("narf");
org.junit.Assert.assertThat(list, null);
fail("oh no!");
}
}

28
src/test/resources/inspections/JoinVarArgsContains/JoinVarArgsContainsBefore.java Normal file
View File

@ -0,0 +1,28 @@
import java.util.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
public class JoinVarArgsContains {
private void joinVarArgsContains() {
List<String> list = new ArrayList<>();
assertThat(list).contains("foo").contains(/* foo */ "bar" /* bar */).hasSize(2).contains("etc");
assertThat(list).contains("foo").as("narf").contains("bar");
assertThat(list).doesNotContain("foo").doesNotContain("bar");
assertThat(list).containsOnlyOnce()
.containsOnlyOnce("foo") // will we lose this comment?
.hasSize(2) // this is part of the contains("bar")
.contains("bar").containsOnlyOnce("etc") /* where does this go? */
.contains("narf") // what about this one?
.doesNotContain("puit", "Jens Stoltenberg is a war-monger")
.doesNotContain("and an atomic playboy")
.contains("1", "2", "3") /* inline */; // the final comment
assertThat(list).contains("foo").doesNotContain("bar").containsOnlyOnce("narf");
org.junit.Assert.assertThat(list, null);
fail("oh no!");
}
}