From 8913755e113d4455f2199c7fd6e28529d9b59d21 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 6 May 2019 12:19:39 +0300 Subject: [PATCH] Simplifiable call inspection: add filter -> filterNotNull replacement Related to KT-30501 --- .../collections/SimplifiableCallInspection.kt | 77 ++++++++++++++++--- .../simplifiableCall/filterNotNullBrackets.kt | 4 + .../filterNotNullBrackets.kt.after | 4 + .../filterNotNullDestructuring.kt | 5 ++ .../simplifiableCall/filterNotNullExplicit.kt | 4 + .../filterNotNullExplicit.kt.after | 4 + .../simplifiableCall/filterNotNullFake.kt | 5 ++ .../simplifiableCall/filterNotNullIdentity.kt | 4 + .../filterNotNullIdentity.kt.after | 4 + .../simplifiableCall/filterNotNullMap.kt | 5 ++ .../simplifiableCall/filterNotNullReverse.kt | 4 + .../filterNotNullReverse.kt.after | 4 + .../simplifiableCall/filterNotNullSimple.kt | 4 + .../filterNotNullSimple.kt.after | 4 + .../LocalInspectionTestGenerated.java | 40 ++++++++++ 15 files changed, 161 insertions(+), 11 deletions(-) create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt.after create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullDestructuring.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt.after create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullFake.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt.after create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullMap.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt.after create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallInspection.kt index 00cdb965937..c094e9501f8 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallInspection.kt @@ -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>? { + 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 }) ) } diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt new file mode 100644 index 00000000000..3ae84f39d31 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filter({ it != null }) +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt.after b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt.after new file mode 100644 index 00000000000..36c607db47e --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullBrackets.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filterNotNull() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullDestructuring.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullDestructuring.kt new file mode 100644 index 00000000000..56c2beeb89b --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullDestructuring.kt @@ -0,0 +1,5 @@ +// WITH_RUNTIME +// PROBLEM: none +fun test(map: Map) { + map.filter { (k, v) -> k != null && v != null } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt new file mode 100644 index 00000000000..2c89c829333 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filter { arg -> arg != null } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt.after b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt.after new file mode 100644 index 00000000000..36c607db47e --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullExplicit.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filterNotNull() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullFake.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullFake.kt new file mode 100644 index 00000000000..97efe536ce0 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullFake.kt @@ -0,0 +1,5 @@ +// WITH_RUNTIME +// PROBLEM: none +fun test(list: List) { + list.filter { it } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt new file mode 100644 index 00000000000..1f8418aef9e --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filter { it !== null } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt.after b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt.after new file mode 100644 index 00000000000..36c607db47e --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullIdentity.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filterNotNull() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullMap.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullMap.kt new file mode 100644 index 00000000000..3ddd7d0e4bf --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullMap.kt @@ -0,0 +1,5 @@ +// WITH_RUNTIME +// PROBLEM: none +fun test(map: Map) { + map.filter { it != null } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt new file mode 100644 index 00000000000..b633653ec23 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filter { null != it } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt.after b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt.after new file mode 100644 index 00000000000..36c607db47e --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullReverse.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filterNotNull() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt new file mode 100644 index 00000000000..23725115608 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filter { it != null } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt.after b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt.after new file mode 100644 index 00000000000..36c607db47e --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCall/filterNotNullSimple.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(list: List) { + list.filterNotNull() +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 5b3650b9157..3857d4cfe8f 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -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");