From fdca96634e43a34a8d58483ccc7ac9210945e3d7 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 27 Jun 2017 14:47:31 +0300 Subject: [PATCH] Add restrictions for "simplifiable call chain" inspection Do not use on maps; do not suggest for lambdas with return inside Related to KT-18274 --- .../kotlin/builtins/KotlinBuiltIns.java | 5 ++++ .../SimplifiableCallChainInspection.kt | 17 ++++++++++--- .../simplifiableCallChain/joinToForHashMap.kt | 4 ++++ .../simplifiableCallChain/joinToForMap.kt | 4 ++++ .../joinToStringViaBuilder.kt | 7 ++++++ .../joinToStringViaBuilder.kt.after | 7 ++++++ .../simplifiableCallChain/mapWithReturn.kt | 9 +++++++ .../LocalInspectionTestGenerated.java | 24 +++++++++++++++++++ 8 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForHashMap.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForMap.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt.after create mode 100644 idea/testData/inspectionsLocal/collections/simplifiableCallChain/mapWithReturn.kt diff --git a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java index f491515be20..6e885cbd030 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java +++ b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java @@ -615,6 +615,11 @@ public abstract class KotlinBuiltIns { return getBuiltInClassByName("String"); } + @NotNull + public ClassDescriptor getCharSequence() { + return getBuiltInClassByName("CharSequence"); + } + @NotNull public ClassDescriptor getComparable() { return getBuiltInClassByName("Comparable"); diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallChainInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallChainInspection.kt index 2a6c0137249..9dd893654b7 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallChainInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/collections/SimplifiableCallChainInspection.kt @@ -19,7 +19,6 @@ package org.jetbrains.kotlin.idea.inspections.collections import com.intellij.codeInspection.ProblemHighlightType import com.intellij.codeInspection.ProblemsHolder import com.intellij.openapi.util.TextRange -import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.inspections.AbstractKotlinInspection import org.jetbrains.kotlin.psi.* @@ -29,6 +28,8 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.model.DefaultValueArgument import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull +import org.jetbrains.kotlin.types.typeUtil.builtIns +import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf class SimplifiableCallChainInspection : AbstractKotlinInspection() { @@ -46,11 +47,19 @@ class SimplifiableCallChainInspection : AbstractKotlinInspection() { firstCallExpression.calleeExpression?.text to secondCallExpression.calleeExpression?.text ] ?: return - val context = expression.analyze(BodyResolveMode.PARTIAL) + val context = expression.analyze() val firstResolvedCall = firstQualifiedExpression.getResolvedCall(context) ?: return val conversion = actualConversions.firstOrNull { firstResolvedCall.resultingDescriptor.fqNameOrNull()?.asString() == it.firstFqName } ?: return + // Do not apply on maps due to lack of relevant stdlib functions + val firstReceiverType = firstResolvedCall.extensionReceiver?.type ?: return + val builtIns = firstReceiverType.builtIns + val mapType = builtIns.map.defaultType + if (firstReceiverType.isSubtypeOf(mapType)) return + // Do not apply for lambdas with return inside + val lambdaArgument = firstCallExpression.lambdaArguments.firstOrNull() + if (lambdaArgument?.anyDescendantOfType() == true) return val secondResolvedCall = expression.getResolvedCall(context) ?: return val secondResultingDescriptor = secondResolvedCall.resultingDescriptor @@ -62,7 +71,9 @@ class SimplifiableCallChainInspection : AbstractKotlinInspection() { if (conversion.replacement.startsWith("joinTo")) { // Function parameter in map must have String result type - if (!firstResolvedCall.hasLastFunctionalParameterWithResult(context) { KotlinBuiltIns.isString(it) }) return + if (!firstResolvedCall.hasLastFunctionalParameterWithResult(context) { + it.isSubtypeOf(builtIns.charSequence.defaultType) + }) return } val descriptor = holder.manager.createProblemDescriptor( diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForHashMap.kt b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForHashMap.kt new file mode 100644 index 00000000000..a58f081b878 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForHashMap.kt @@ -0,0 +1,4 @@ +// PROBLEM: none +// WITH_RUNTIME + +val x = hashMapOf(1 to 2, 3 to 4).map { (key, value) -> key + value }.joinToString() \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForMap.kt b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForMap.kt new file mode 100644 index 00000000000..b4e1bd4db52 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForMap.kt @@ -0,0 +1,4 @@ +// PROBLEM: none +// WITH_RUNTIME + +val x = mapOf(1 to 2, 3 to 4).map { (key, value) -> key + value }.joinToString() \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt new file mode 100644 index 00000000000..2bb1dd6d505 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt @@ -0,0 +1,7 @@ +// WITH_RUNTIME + +val x = listOf(1, 2, 3).map { + val sb = StringBuilder() + sb.append(it).append(" + ").append(it) + sb +}.joinToString(prefix = "= ", separator = " + ") \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt.after b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt.after new file mode 100644 index 00000000000..3611c4d5858 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt.after @@ -0,0 +1,7 @@ +// WITH_RUNTIME + +val x = listOf(1, 2, 3).joinToString(prefix = "= ", separator = " + ") { + val sb = StringBuilder() + sb.append(it).append(" + ").append(it) + sb +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/collections/simplifiableCallChain/mapWithReturn.kt b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/mapWithReturn.kt new file mode 100644 index 00000000000..60a54555b48 --- /dev/null +++ b/idea/testData/inspectionsLocal/collections/simplifiableCallChain/mapWithReturn.kt @@ -0,0 +1,9 @@ +// PROBLEM: none +// WITH_RUNTIME + +fun test(args: List): String { + return args.map { + if (it == 0) return "" + "$it * $it" + }.joinToString(separator = " + ") +} \ 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 ba59914e14d..052721aab38 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -139,12 +139,30 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { doTest(fileName); } + @TestMetadata("joinToForHashMap.kt") + public void testJoinToForHashMap() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForHashMap.kt"); + doTest(fileName); + } + + @TestMetadata("joinToForMap.kt") + public void testJoinToForMap() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToForMap.kt"); + doTest(fileName); + } + @TestMetadata("joinToString.kt") public void testJoinToString() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToString.kt"); doTest(fileName); } + @TestMetadata("joinToStringViaBuilder.kt") + public void testJoinToStringViaBuilder() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringViaBuilder.kt"); + doTest(fileName); + } + @TestMetadata("joinToStringWithReference.kt") public void testJoinToStringWithReference() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/collections/simplifiableCallChain/joinToStringWithReference.kt"); @@ -162,6 +180,12 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/collections/simplifiableCallChain/mapNotNull.kt"); doTest(fileName); } + + @TestMetadata("mapWithReturn.kt") + public void testMapWithReturn() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/collections/simplifiableCallChain/mapWithReturn.kt"); + doTest(fileName); + } } @TestMetadata("idea/testData/inspectionsLocal/collections/uselessCallOnCollection")