Simplifiable call inspection: add filter -> filterNotNull replacement

Related to KT-30501
2019-05-06 12:19:39 +03:00
parent 4a5cbe0443
commit 8913755e11
15 changed files with 161 additions and 11 deletions
@@ -10,16 +10,22 @@ import com.intellij.codeInspection.ProblemDescriptor
 import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.codeInspection.ProblemsHolder
 import com.intellij.openapi.project.Project
+import org.jetbrains.kotlin.KtNodeTypes
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
 import org.jetbrains.kotlin.idea.inspections.AbstractKotlinInspection
+import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
+import org.jetbrains.kotlin.types.typeUtil.builtIns

 class SimplifiableCallInspection : AbstractKotlinInspection() {
    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) =
        qualifiedExpressionVisitor(fun(expression) {
            val callExpression = expression.selectorExpression as? KtCallExpression ?: return
            val calleeExpression = callExpression.calleeExpression ?: return
-            val conversion = callExpression.findConversion() ?: return
+            val (conversion, resolvedCall) = callExpression.findConversionAndResolvedCall() ?: return
+            if (!conversion.callChecker(resolvedCall)) return
            val conversionSuffix = conversion.analyzer(callExpression) ?: return

            holder.registerProblem(
@@ -30,24 +36,73 @@ class SimplifiableCallInspection : AbstractKotlinInspection() {
            )
        })

-    private fun KtCallExpression.findConversion(): Conversion? = conversions.firstOrNull { isCalling(it.fqName) }
+    private fun KtCallExpression.findConversionAndResolvedCall(): Pair<Conversion, ResolvedCall<*>>? {
+        val calleeText = calleeExpression?.text ?: return null
+        val resolvedCall: ResolvedCall<*>? by lazy { this.resolveToCall() }
+        for (conversion in conversions) {
+            if (conversion.shortName != calleeText) continue
+            if (resolvedCall?.isCalling(conversion.fqName) == true) {
+                return conversion to resolvedCall!!
+            }
+        }
+        return null
+    }

-    private data class Conversion(val callFqName: String, val replacement: String, val analyzer: (KtCallExpression) -> String?) {
+    private data class Conversion(
+        val callFqName: String,
+        val replacement: String,
+        val analyzer: (KtCallExpression) -> String?,
+        val callChecker: (ResolvedCall<*>) -> Boolean = { true }
+    ) {
        val fqName = FqName(callFqName)
+
+        val shortName = fqName.shortName().asString()
    }

    companion object {
+        private fun KtCallExpression.singleLambdaExpression(): KtLambdaExpression? {
+            val argument = valueArguments.singleOrNull() ?: return null
+            return (argument as? KtLambdaArgument)?.getLambdaExpression() ?: argument.getArgumentExpression() as? KtLambdaExpression
+        }
+
+        private fun KtLambdaExpression.singleStatement(): KtExpression? = bodyExpression?.statements?.singleOrNull()
+
+        private fun KtLambdaExpression.singleLambdaParameterName(): String? {
+            val lambdaParameters = valueParameters
+            return if (lambdaParameters.isNotEmpty()) lambdaParameters.singleOrNull()?.name else "it"
+        }
+
+        private fun KtExpression.isNameReferenceTo(name: String): Boolean =
+            this is KtNameReferenceExpression && this.getReferencedName() == name
+
+        private fun KtExpression.isNull(): Boolean =
+            this is KtConstantExpression && this.node.elementType == KtNodeTypes.NULL
+
        private val conversions = listOf(
            Conversion("kotlin.collections.flatMap", "flatten", fun(callExpression: KtCallExpression): String? {
-                val argument = callExpression.valueArguments.singleOrNull() ?: return null
-                val lambdaExpression = (argument as? KtLambdaArgument)?.getLambdaExpression()
-                    ?: argument.getArgumentExpression() as? KtLambdaExpression
-                    ?: return null
-                val reference = lambdaExpression.bodyExpression?.statements?.singleOrNull() as? KtNameReferenceExpression ?: return null
-                val lambdaParameters = lambdaExpression.valueParameters
-                val lambdaParameterName = if (lambdaParameters.isNotEmpty()) lambdaParameters.singleOrNull()?.name else "it"
-                if (reference.text != lambdaParameterName) return null
+                val lambdaExpression = callExpression.singleLambdaExpression() ?: return null
+                val reference = lambdaExpression.singleStatement() ?: return null
+                val lambdaParameterName = lambdaExpression.singleLambdaParameterName() ?: return null
+                if (!reference.isNameReferenceTo(lambdaParameterName)) return null
                return "()"
+            }),
+
+            Conversion("kotlin.collections.filter", "filterNotNull", analyzer = fun(callExpression: KtCallExpression): String? {
+                val lambdaExpression = callExpression.singleLambdaExpression() ?: return null
+                val statement = lambdaExpression.singleStatement() as? KtBinaryExpression ?: return null
+                val lambdaParameterName = lambdaExpression.singleLambdaParameterName() ?: return null
+                if (statement.operationToken != KtTokens.EXCLEQ && statement.operationToken != KtTokens.EXCLEQEQEQ) return null
+                val left = statement.left ?: return null
+                val right = statement.right ?: return null
+                if (left.isNameReferenceTo(lambdaParameterName) && right.isNull()) {
+                    return "()"
+                } else if (right.isNameReferenceTo(lambdaParameterName) && left.isNull()) {
+                    return "()"
+                }
+                return null
+            }, callChecker = fun(resolvedCall: ResolvedCall<*>): Boolean {
+                val extensionReceiverType = resolvedCall.extensionReceiver?.type ?: return false
+                return extensionReceiverType.constructor.declarationDescriptor?.defaultType?.isMap(extensionReceiverType.builtIns) == false
            })
        )
    }
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.<caret>filter({ it != null })
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.filterNotNull()
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+// PROBLEM: none
+fun test(map: Map<String?, String?>) {
+    map.<caret>filter { (k, v) -> k != null && v != null }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.<caret>filter { arg -> arg != null }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.filterNotNull()
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+// PROBLEM: none
+fun test(list: List<Boolean>) {
+    list.<caret>filter { it }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.<caret>filter { it !== null }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.filterNotNull()
+}
@@ -0,0 +1,5 @@
+// WITH_RUNTIME
+// PROBLEM: none
+fun test(map: Map<String?, String?>) {
+    map.<caret>filter { it != null }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.<caret>filter { null != it }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.filterNotNull()
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.<caret>filter { it != null }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(list: List<String?>) {
+    list.filterNotNull()
+}
@@ -1108,6 +1108,46 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/explicitLambdaParameter.kt");
            }

+            @TestMetadata("filterNotNullBrackets.kt")
+            public void testFilterNotNullBrackets() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt");
+            }
+
+            @TestMetadata("filterNotNullDestructuring.kt")
+            public void testFilterNotNullDestructuring() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullDestructuring.kt");
+            }
+
+            @TestMetadata("filterNotNullExplicit.kt")
+            public void testFilterNotNullExplicit() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt");
+            }
+
+            @TestMetadata("filterNotNullFake.kt")
+            public void testFilterNotNullFake() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullFake.kt");
+            }
+
+            @TestMetadata("filterNotNullIdentity.kt")
+            public void testFilterNotNullIdentity() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt");
+            }
+
+            @TestMetadata("filterNotNullMap.kt")
+            public void testFilterNotNullMap() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullMap.kt");
+            }
+
+            @TestMetadata("filterNotNullReverse.kt")
+            public void testFilterNotNullReverse() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt");
+            }
+
+            @TestMetadata("filterNotNullSimple.kt")
+            public void testFilterNotNullSimple() throws Exception {
+                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt");
+            }
+
            @TestMetadata("notOnlyReference.kt")
            public void testNotOnlyReference() throws Exception {
                runTest("idea/testData/inspectionsLocal/collections/simplifiableCall/notOnlyReference.kt");