Internal refactoring: Reduced code duplication by moving stuff into a common base class.

This commit is contained in:
Chris Hodges 2019-09-30 17:25:16 +02:00
parent a0ed4eab76
commit 1983750077
6 changed files with 127 additions and 139 deletions

View File

@ -25,7 +25,7 @@ import de.platon42.intellij.plugins.cajon.MethodNames
import de.platon42.intellij.plugins.cajon.qualifierExpression
import de.platon42.intellij.plugins.cajon.quickfixes.ReplaceSimpleMethodCallQuickFix
open class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
abstract class AbstractAssertJInspection : AbstractBaseJavaLocalInspectionTool() {
companion object {
const val SIMPLIFY_MESSAGE_TEMPLATE = "%s() can be simplified to %s()"

View File

@ -0,0 +1,53 @@
package de.platon42.intellij.plugins.cajon.inspections
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.PsiExpressionStatement
import com.intellij.psi.PsiMethodCallExpression
import com.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
abstract class AbstractMoveOutInspection : AbstractAssertJInspection() {
protected fun createInspectionsForMappings(
statement: PsiExpressionStatement,
holder: ProblemsHolder,
mappings: List<MoveOutMapping>
) {
if (!statement.hasAssertThat()) return
val staticMethodCall = statement.findStaticMethodCall() ?: return
val assertThatArgument = staticMethodCall.getArgOrNull(0) as? PsiMethodCallExpression ?: return
val expectedCallExpression = statement.findOutmostMethodCall() ?: return
for (mapping in mappings.filter { it.callMatcher.test(assertThatArgument) }) {
if (mapping.expectBoolean && ASSERT_THAT_BOOLEAN.test(staticMethodCall)) {
val expectedBooleanResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: continue
val replacementMethod = if (expectedBooleanResult) mapping.replacementForTrue else mapping.replacementForFalse ?: return
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
} else if (mapping.expectNullNonNull != null) {
val expectedNullNonNullResult = expectedCallExpression.getExpectedNullNonNullResult() ?: continue
val replacementMethod = if (expectedNullNonNullResult xor mapping.expectNullNonNull) mapping.replacementForTrue else mapping.replacementForFalse ?: continue
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true)
}
} else if (mapping.expectedMatcher?.test(expectedCallExpression) == true) {
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, mapping.replacementForTrue) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, replaceOnlyThisMethod = mapping.expectedMatcher)
}
}
}
}
class MoveOutMapping(
val callMatcher: CallMatcher,
val replacementForTrue: String,
val replacementForFalse: String? = null,
val expectBoolean: Boolean = false,
val expectNullNonNull: Boolean? = null,
val expectedMatcher: CallMatcher? = null
)
}

View File

@ -12,7 +12,7 @@ import javax.swing.JComponent
import javax.swing.JPanel
class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection() {
class AssertThatCollectionOrMapExpressionInspection : AbstractMoveOutInspection() {
companion object {
private const val DISPLAY_NAME = "Asserting a collection or map specific expression"
@ -39,28 +39,28 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
)
private val MAPPINGS = listOf(
Mapping(
MoveOutMapping(
CallMatcher.anyOf(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, MethodNames.IS_EMPTY).parameterCount(0),
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, MethodNames.IS_EMPTY).parameterCount(0)
),
MethodNames.IS_EMPTY, MethodNames.IS_NOT_EMPTY
MethodNames.IS_EMPTY, MethodNames.IS_NOT_EMPTY, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, MethodNames.CONTAINS).parameterCount(1),
MethodNames.CONTAINS, MethodNames.DOES_NOT_CONTAIN
MethodNames.CONTAINS, MethodNames.DOES_NOT_CONTAIN, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_COLLECTION, MethodNames.CONTAINS_ALL).parameterCount(1),
MethodNames.CONTAINS_ALL, null
MethodNames.CONTAINS_ALL, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, MethodNames.CONTAINS_KEY).parameterCount(1),
MethodNames.CONTAINS_KEY, MethodNames.DOES_NOT_CONTAIN_KEY
MethodNames.CONTAINS_KEY, MethodNames.DOES_NOT_CONTAIN_KEY, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, MethodNames.CONTAINS_VALUE).parameterCount(1),
MethodNames.CONTAINS_VALUE, MethodNames.DOES_NOT_CONTAIN_VALUE
MethodNames.CONTAINS_VALUE, MethodNames.DOES_NOT_CONTAIN_VALUE, expectBoolean = true
)
)
}
@ -78,8 +78,8 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
val staticMethodCall = statement.findStaticMethodCall() ?: return
val assertThatArgument = staticMethodCall.getArgOrNull(0) as? PsiMethodCallExpression ?: return
val expectedCallExpression = statement.findOutmostMethodCall() ?: return
if (MAP_GET_MATCHER.test(assertThatArgument)) {
val expectedCallExpression = statement.findOutmostMethodCall() ?: return
val nullOrNotNull = expectedCallExpression.getAllTheSameNullNotNullConstants()
if (nullOrNotNull == true) {
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.CONTAINS_KEY) { desc, method ->
@ -136,15 +136,7 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
}
}
} else {
if (!ASSERT_THAT_BOOLEAN.test(staticMethodCall)) return
val mapping = MAPPINGS.firstOrNull { it.callMatcher.test(assertThatArgument) } ?: return
val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return
val replacementMethod = if (expectedResult) mapping.replacementForTrue else mapping.replacementForFalse ?: return
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
createInspectionsForMappings(statement, holder, MAPPINGS)
}
}
}
@ -163,10 +155,4 @@ class AssertThatCollectionOrMapExpressionInspection : AbstractAssertJInspection(
)
return panel
}
private class Mapping(
val callMatcher: CallMatcher,
val replacementForTrue: String,
val replacementForFalse: String?
)
}

View File

@ -1,68 +1,71 @@
package de.platon42.intellij.plugins.cajon.inspections
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.*
import com.intellij.psi.CommonClassNames
import com.intellij.psi.JavaElementVisitor
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.PsiExpressionStatement
import com.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
import de.platon42.intellij.plugins.cajon.AssertJClassNames
import de.platon42.intellij.plugins.cajon.MethodNames
class AssertThatFileExpressionInspection : AbstractAssertJInspection() {
class AssertThatFileExpressionInspection : AbstractMoveOutInspection() {
companion object {
private const val DISPLAY_NAME = "Asserting a file specific expression"
private val MAPPINGS = listOf(
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "canRead"),
"canRead", expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "canWrite"),
"canWrite", expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "exists"),
"exists", "doesNotExist", expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "isAbsolute"),
"isAbsolute", "isRelative", expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "isDirectory"),
"isDirectory", expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "isFile"),
"isFile", expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "getName"),
"hasName",
expectedMatcher = CallMatcher.anyOf(IS_EQUAL_TO_OBJECT, IS_EQUAL_TO_STRING)
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "getParent", "getParentFile"),
"hasNoParent", expectNullNonNull = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "getParent"),
"hasParent",
expectedMatcher = CallMatcher.anyOf(IS_EQUAL_TO_OBJECT, IS_EQUAL_TO_STRING)
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "getParentFile"),
"hasParent",
expectedMatcher = IS_EQUAL_TO_OBJECT
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "list", "listFiles"),
"isEmptyDirectory",
expectedMatcher = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME, MethodNames.IS_EMPTY)
.parameterCount(0)!!
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_IO_FILE, "list", "listFiles"),
"isNotEmptyDirectory",
expectedMatcher = CallMatcher.instanceCall(AssertJClassNames.ABSTRACT_OBJECT_ARRAY_ASSERT_CLASSNAME, MethodNames.IS_NOT_EMPTY)
@ -77,41 +80,8 @@ class AssertThatFileExpressionInspection : AbstractAssertJInspection() {
return object : JavaElementVisitor() {
override fun visitExpressionStatement(statement: PsiExpressionStatement) {
super.visitExpressionStatement(statement)
if (!statement.hasAssertThat()) return
val staticMethodCall = statement.findStaticMethodCall() ?: return
val assertThatArgument = staticMethodCall.getArgOrNull(0) as? PsiMethodCallExpression ?: return
val expectedCallExpression = statement.findOutmostMethodCall() ?: return
for (mapping in MAPPINGS.filter { it.callMatcher.test(assertThatArgument) }) {
if (mapping.expectBoolean && ASSERT_THAT_BOOLEAN.test(staticMethodCall)) {
val expectedBooleanResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: continue
val replacementMethod = if (expectedBooleanResult) mapping.replacementForTrue else mapping.replacementForFalse ?: return
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
} else if (mapping.expectNullNonNull != null) {
val expectedNullNonNullResult = expectedCallExpression.getExpectedNullNonNullResult() ?: continue
val replacementMethod = if (expectedNullNonNullResult xor mapping.expectNullNonNull) mapping.replacementForTrue else mapping.replacementForFalse ?: continue
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, useNullNonNull = true)
}
} else if (mapping.expectedMatcher?.test(expectedCallExpression) == true) {
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, mapping.replacementForTrue) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method, replaceOnlyThisMethod = mapping.expectedMatcher)
}
}
}
createInspectionsForMappings(statement, holder, MAPPINGS)
}
}
}
private class Mapping(
val callMatcher: CallMatcher,
val replacementForTrue: String,
val replacementForFalse: String? = null,
val expectBoolean: Boolean = false,
val expectNullNonNull: Boolean? = null,
val expectedMatcher: CallMatcher? = null
)
}

View File

@ -5,10 +5,8 @@ import com.intellij.psi.*
import com.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
import de.platon42.intellij.plugins.cajon.quickfixes.HasHashCodeQuickFix
import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
import de.platon42.intellij.plugins.cajon.quickfixes.RemoveActualOutmostMethodCallQuickFix
class AssertThatObjectExpressionInspection : AbstractAssertJInspection() {
class AssertThatObjectExpressionInspection : AbstractMoveOutInspection() {
companion object {
private const val DISPLAY_NAME = "Asserting equals(), toString(), or hashCode()"
@ -16,6 +14,17 @@ class AssertThatObjectExpressionInspection : AbstractAssertJInspection() {
private val OBJECT_TO_STRING = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "toString").parameterCount(0)
private val OBJECT_HASHCODE = CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "hashCode").parameterCount(0)
private val MAPPINGS = listOf(
MoveOutMapping(
OBJECT_EQUALS,
MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO, expectBoolean = true
),
MoveOutMapping(
OBJECT_TO_STRING,
MethodNames.HAS_TO_STRING, expectedMatcher = CallMatcher.anyOf(IS_EQUAL_TO_OBJECT, IS_EQUAL_TO_STRING)
)
)
}
override fun getDisplayName() = DISPLAY_NAME
@ -25,31 +34,18 @@ class AssertThatObjectExpressionInspection : AbstractAssertJInspection() {
override fun visitExpressionStatement(statement: PsiExpressionStatement) {
super.visitExpressionStatement(statement)
if (!statement.hasAssertThat()) return
val staticMethodCall = statement.findStaticMethodCall() ?: return
val assertThatArgument = staticMethodCall.firstArg as? PsiMethodCallExpression ?: return
val expectedCallExpression = statement.findOutmostMethodCall() ?: return
when {
OBJECT_EQUALS.test(assertThatArgument) -> {
val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return
val replacementMethod = expectedResult.map(MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO)
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
}
OBJECT_TO_STRING.test(assertThatArgument) -> {
staticMethodCall.findFluentCallTo(IS_EQUAL_TO_OBJECT) ?: staticMethodCall.findFluentCallTo(IS_EQUAL_TO_STRING) ?: return
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, MethodNames.HAS_TO_STRING) { desc, method ->
RemoveActualOutmostMethodCallQuickFix(desc, method)
}
}
OBJECT_HASHCODE.test(assertThatArgument) -> {
val isEqualTo = staticMethodCall.findFluentCallTo(IS_EQUAL_TO_INT) ?: return
val expectedExpression = isEqualTo.firstArg as? PsiMethodCallExpression ?: return
if (OBJECT_HASHCODE.test(expectedExpression)) {
holder.registerProblem(expectedCallExpression, HASHCODE_MESSAGE_TEMPLATE, HasHashCodeQuickFix())
}
if (OBJECT_HASHCODE.test(assertThatArgument)) {
val expectedCallExpression = statement.findOutmostMethodCall() ?: return
val isEqualTo = staticMethodCall.findFluentCallTo(IS_EQUAL_TO_INT) ?: return
val expectedExpression = isEqualTo.firstArg as? PsiMethodCallExpression ?: return
if (OBJECT_HASHCODE.test(expectedExpression)) {
holder.registerProblem(expectedCallExpression, HASHCODE_MESSAGE_TEMPLATE, HasHashCodeQuickFix())
}
} else {
createInspectionsForMappings(statement, holder, MAPPINGS)
}
}
}

View File

@ -1,43 +1,45 @@
package de.platon42.intellij.plugins.cajon.inspections
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.psi.*
import com.intellij.psi.CommonClassNames
import com.intellij.psi.JavaElementVisitor
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.PsiExpressionStatement
import com.siyeh.ig.callMatcher.CallMatcher
import de.platon42.intellij.plugins.cajon.*
import de.platon42.intellij.plugins.cajon.quickfixes.MoveOutMethodCallExpressionQuickFix
import de.platon42.intellij.plugins.cajon.MethodNames
class AssertThatStringExpressionInspection : AbstractAssertJInspection() {
class AssertThatStringExpressionInspection : AbstractMoveOutInspection() {
companion object {
private const val DISPLAY_NAME = "Asserting a string specific expression"
private val MAPPINGS = listOf(
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "isEmpty").parameterCount(0),
MethodNames.IS_EMPTY, MethodNames.IS_NOT_EMPTY
MethodNames.IS_EMPTY, MethodNames.IS_NOT_EMPTY, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.anyOf(
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "equals").parameterCount(1),
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "contentEquals").parameterCount(1)
),
MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO
MethodNames.IS_EQUAL_TO, MethodNames.IS_NOT_EQUAL_TO, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "equalsIgnoreCase").parameterTypes(CommonClassNames.JAVA_LANG_STRING),
MethodNames.IS_EQUAL_TO_IC, MethodNames.IS_NOT_EQUAL_TO_IC
MethodNames.IS_EQUAL_TO_IC, MethodNames.IS_NOT_EQUAL_TO_IC, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "contains").parameterCount(1),
MethodNames.CONTAINS, MethodNames.DOES_NOT_CONTAIN
MethodNames.CONTAINS, MethodNames.DOES_NOT_CONTAIN, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "startsWith").parameterTypes(CommonClassNames.JAVA_LANG_STRING),
MethodNames.STARTS_WITH, MethodNames.DOES_NOT_START_WITH
MethodNames.STARTS_WITH, MethodNames.DOES_NOT_START_WITH, expectBoolean = true
),
Mapping(
MoveOutMapping(
CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "endsWith").parameterTypes(CommonClassNames.JAVA_LANG_STRING),
MethodNames.ENDS_WITH, MethodNames.DOES_NOT_END_WITH
MethodNames.ENDS_WITH, MethodNames.DOES_NOT_END_WITH, expectBoolean = true
)
)
}
@ -48,27 +50,8 @@ class AssertThatStringExpressionInspection : AbstractAssertJInspection() {
return object : JavaElementVisitor() {
override fun visitExpressionStatement(statement: PsiExpressionStatement) {
super.visitExpressionStatement(statement)
if (!statement.hasAssertThat()) return
val staticMethodCall = statement.findStaticMethodCall() ?: return
if (!ASSERT_THAT_BOOLEAN.test(staticMethodCall)) return
val assertThatArgument = staticMethodCall.firstArg as? PsiMethodCallExpression ?: return
val mapping = MAPPINGS.firstOrNull { it.callMatcher.test(assertThatArgument) } ?: return
val expectedCallExpression = statement.findOutmostMethodCall() ?: return
val expectedResult = expectedCallExpression.getAllTheSameExpectedBooleanConstants() ?: return
val replacementMethod = if (expectedResult) mapping.replacementForTrue else mapping.replacementForFalse
registerMoveOutMethod(holder, expectedCallExpression, assertThatArgument, replacementMethod) { desc, method ->
MoveOutMethodCallExpressionQuickFix(desc, method)
}
createInspectionsForMappings(statement, holder, MAPPINGS)
}
}
}
private class Mapping(
val callMatcher: CallMatcher,
val replacementForTrue: String,
val replacementForFalse: String
)
}