From 475d5548c4fc90b1f6f0efbfd05d825c31b89340 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 7 Oct 2016 18:43:58 +0300 Subject: [PATCH] Destructure intention now handles the case with manual destructuring inside #KT-13943 Fixed --- .../idea/intentions/DestructureIntention.kt | 78 +++++++++++++------ .../noItWithDestructuring.kt | 8 ++ .../noItWithDestructuring.kt.after | 8 ++ .../DataClassWithDestructuring.kt | 9 +++ .../DataClassWithDestructuring.kt.after | 8 ++ .../DataClassWithDestructuringConflict.kt | 11 +++ .../DataClassWithDestructuringFakeConflict.kt | 10 +++ ...lassWithDestructuringFakeConflict.kt.after | 8 ++ .../DataClassWithDestructuringPartial.kt | 13 ++++ ...DataClassWithDestructuringPartial.kt.after | 12 +++ .../KeyOnlyWithDestructuring.kt | 7 ++ .../KeyOnlyWithDestructuring.kt.after | 6 ++ .../KeyValueWithDestructuring.kt | 7 ++ .../KeyValueWithDestructuring.kt.after | 6 ++ .../inspectionData/expected.xml | 40 ++++++++++ .../intentions/IntentionTestGenerated.java | 42 ++++++++++ 16 files changed, 250 insertions(+), 23 deletions(-) create mode 100644 idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt create mode 100644 idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt.after create mode 100644 idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt create mode 100644 idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt.after create mode 100644 idea/testData/intentions/iterationOverMap/DataClassWithDestructuringConflict.kt create mode 100644 idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt create mode 100644 idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt.after create mode 100644 idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt create mode 100644 idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt.after create mode 100644 idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt create mode 100644 idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt.after create mode 100644 idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt create mode 100644 idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt index b001e5c6fbd..12631afe44d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt @@ -62,7 +62,7 @@ class DestructureIntention : SelfTargetingRangeIntention( val factory = KtPsiFactory(element) val validator = NewDeclarationNameValidator( element.parent, element, NewDeclarationNameValidator.Target.VARIABLES, - excludedDeclarations = usagesToRemove.map { it.variableToDrop.singletonOrEmptyList() }.flatten() + excludedDeclarations = usagesToRemove.map { it.declarationToDrop.singletonOrEmptyList() }.flatten() ) val names = ArrayList() usagesToRemove.forEach { (descriptor, usagesToReplace, variableToDrop, name) -> @@ -154,7 +154,7 @@ class DestructureIntention : SelfTargetingRangeIntention( // Note: list should contains properties in order to create destructuring declaration val usagesToRemove = mutableListOf() - var badUsageFound = false + var noBadUsages = true val removeSelectorInLoopRange: Boolean if (forLoop != null && DescriptorUtils.isSubclass(classDescriptor, mapEntryClassDescriptor)) { val loopRangeDescriptorName = forLoop.loopRange.getResolvedCall(context)?.resultingDescriptor?.name @@ -167,8 +167,8 @@ class DestructureIntention : SelfTargetingRangeIntention( ReferencesSearch.search(declaration).iterateOverMapEntryPropertiesUsages( context, - { index, usageData -> usagesToRemove[index].add(usageData) }, - { badUsageFound = true } + { index, usageData -> noBadUsages = usagesToRemove[index].add(usageData, index) && noBadUsages }, + { noBadUsages = false } ) } else if (classDescriptor.isData) { @@ -199,29 +199,35 @@ class DestructureIntention : SelfTargetingRangeIntention( context, nameToSearch, descriptorToIndex, - { index, usageData -> usagesToRemove[index].add(usageData) }, - { badUsageFound = true } + { index, usageData -> noBadUsages = usagesToRemove[index].add(usageData, index) && noBadUsages }, + { noBadUsages = false } ) } else { return null } - if (badUsageFound) return null + if (!noBadUsages) return null - val droppedLastUnused = usagesToRemove.dropLastWhile { it.usagesToReplace.isEmpty() && it.variableToDrop == null } + val droppedLastUnused = usagesToRemove.dropLastWhile { it.usagesToReplace.isEmpty() && it.declarationToDrop == null } return UsagesToRemove(droppedLastUnused, removeSelectorInLoopRange) } private fun Query.iterateOverMapEntryPropertiesUsages( context: BindingContext, - process: (Int, UsageData) -> Unit, + process: (Int, SingleUsageData) -> Unit, cancel: () -> Unit ) { // TODO: Remove SAM-constructor when KT-11265 will be fixed forEach(Processor forEach@{ val applicableUsage = getDataIfUsageIsApplicable(it, context) if (applicableUsage != null) { - when (applicableUsage.descriptor.name.asString()) { + val usageDescriptor = applicableUsage.descriptor + if (usageDescriptor == null) { + process(0, applicableUsage) + process(1, applicableUsage) + return@forEach true + } + when (usageDescriptor.name.asString()) { "key", "getKey" -> { process(0, applicableUsage) return@forEach true @@ -242,7 +248,7 @@ class DestructureIntention : SelfTargetingRangeIntention( context: BindingContext, parameterName: Name, descriptorToIndex: Map, - process: (Int, UsageData) -> Unit, + process: (Int, SingleUsageData) -> Unit, cancel: () -> Unit ) { anyDescendantOfType { @@ -250,7 +256,14 @@ class DestructureIntention : SelfTargetingRangeIntention( else { val applicableUsage = getDataIfUsageIsApplicable(it, context) if (applicableUsage != null) { - val index = descriptorToIndex[applicableUsage.descriptor] + val usageDescriptor = applicableUsage.descriptor + if (usageDescriptor == null) { + for (index in descriptorToIndex.values) { + process(index, applicableUsage) + } + return@anyDescendantOfType false + } + val index = descriptorToIndex[usageDescriptor] if (index != null) { process(index, applicableUsage) return@anyDescendantOfType false @@ -266,7 +279,11 @@ class DestructureIntention : SelfTargetingRangeIntention( private fun getDataIfUsageIsApplicable(dataClassUsage: PsiReference, context: BindingContext) = (dataClassUsage.element as? KtReferenceExpression)?.let { getDataIfUsageIsApplicable(it, context) } - private fun getDataIfUsageIsApplicable(dataClassUsage: KtReferenceExpression, context: BindingContext): UsageData? { + private fun getDataIfUsageIsApplicable(dataClassUsage: KtReferenceExpression, context: BindingContext): SingleUsageData? { + val destructuringDecl = dataClassUsage.parent as? KtDestructuringDeclaration + if (destructuringDecl != null && destructuringDecl.initializer == dataClassUsage) { + return SingleUsageData(descriptor = null, usageToReplace = null, declarationToDrop = destructuringDecl) + } val qualifiedExpression = dataClassUsage.getQualifiedExpressionForReceiver() ?: return null val parent = qualifiedExpression.parent when (parent) { @@ -282,22 +299,37 @@ class DestructureIntention : SelfTargetingRangeIntention( if (property != null && property.isVar) return null val descriptor = qualifiedExpression.getResolvedCall(context)?.resultingDescriptor ?: return null - return UsageData( - descriptor = descriptor, - usagesToReplace = mutableListOf(qualifiedExpression), - variableToDrop = property) + return SingleUsageData(descriptor = descriptor, usageToReplace = qualifiedExpression, declarationToDrop = property) } + private data class SingleUsageData( + val descriptor: CallableDescriptor?, + val usageToReplace: KtExpression?, + val declarationToDrop: KtDeclaration? + ) + private data class UsageData( val descriptor: CallableDescriptor, val usagesToReplace: MutableList = mutableListOf(), - var variableToDrop: KtProperty? = null, - var name: String? = variableToDrop?.name + var declarationToDrop: KtDeclaration? = null, + var name: String? = declarationToDrop?.name ) { - fun add(newData: UsageData) { - variableToDrop = variableToDrop ?: newData.variableToDrop - usagesToReplace.addAll(newData.usagesToReplace) - name = name ?: newData.name + // Returns true if data is successfully added, false otherwise + fun add(newData: SingleUsageData, componentIndex: Int): Boolean { + if (newData.declarationToDrop is KtDestructuringDeclaration) { + val destructuringEntries = newData.declarationToDrop.entries + if (componentIndex < destructuringEntries.size) { + if (declarationToDrop != null) return false + name = destructuringEntries[componentIndex].name ?: return false + declarationToDrop = newData.declarationToDrop + } + } + else { + name = name ?: newData.declarationToDrop?.name + declarationToDrop = declarationToDrop ?: newData.declarationToDrop + } + newData.usageToReplace?.let { usagesToReplace.add(it) } + return true } } } \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt b/idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt new file mode 100644 index 00000000000..22efa2d529a --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt @@ -0,0 +1,8 @@ +data class XY(val x: String, val y: String) + +fun convert(xy: XY, foo: (XY) -> String) = foo(xy) + +fun foo(xy: XY) = convert(xy) { + val (x, y) = it + x + y +} \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt.after b/idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt.after new file mode 100644 index 00000000000..7dfd536c73c --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt.after @@ -0,0 +1,8 @@ +data class XY(val x: String, val y: String) + +fun convert(xy: XY, foo: (XY) -> String) = foo(xy) + +fun foo(xy: XY) = convert(xy) { + (x, y) -> + x + y +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt new file mode 100644 index 00000000000..03df6a3cd93 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME + +data class XY(val x: Int, val y: Int) + +fun foo(list: List) { + for (element in list) { + val (x, y) = element + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt.after b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt.after new file mode 100644 index 00000000000..7df8609f971 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME + +data class XY(val x: Int, val y: Int) + +fun foo(list: List) { + for ((x, y) in list) { + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringConflict.kt b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringConflict.kt new file mode 100644 index 00000000000..1c6c27a7a27 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringConflict.kt @@ -0,0 +1,11 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME + +data class XY(val x: Int, val y: Int) + +fun foo(list: List) { + for (element in list) { + val z = element.y + val (x, y) = element + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt new file mode 100644 index 00000000000..ac15a364c82 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +data class XY(val x: Int, val y: Int) + +fun foo(list: List) { + for (element in list) { + val z = element.y + val (x) = element + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt.after b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt.after new file mode 100644 index 00000000000..d5cbb8503f3 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME + +data class XY(val x: Int, val y: Int) + +fun foo(list: List) { + for ((x, z) in list) { + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt new file mode 100644 index 00000000000..7774b7878ec --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt @@ -0,0 +1,13 @@ +// WITH_RUNTIME + +data class XY(val x: Int, val y: Int) + +fun foo(list: List): Int { + var result = 0 + for (element in list) { + val (x) = element + result += x + result += element.y + } + return result +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt.after b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt.after new file mode 100644 index 00000000000..d4d5ff370ea --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt.after @@ -0,0 +1,12 @@ +// WITH_RUNTIME + +data class XY(val x: Int, val y: Int) + +fun foo(list: List): Int { + var result = 0 + for ((x, y) in list) { + result += x + result += y + } + return result +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt b/idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt new file mode 100644 index 00000000000..e9da1d03e94 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt @@ -0,0 +1,7 @@ +// WITH_RUNTIME + +fun foo(map: Map) { + for (entry in map.entries) { + val (key) = entry + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt.after b/idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt.after new file mode 100644 index 00000000000..5116e54d59d --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo(map: Map) { + for ((key) in map) { + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt b/idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt new file mode 100644 index 00000000000..da3a898a6be --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt @@ -0,0 +1,7 @@ +// WITH_RUNTIME + +fun foo(map: Map) { + for (entry in map.entries) { + val (key, value) = entry + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt.after b/idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt.after new file mode 100644 index 00000000000..4945886a857 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo(map: Map) { + for ((key, value) in map) { + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml b/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml index ab0bfd74a2d..b613095ddf0 100644 --- a/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml +++ b/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml @@ -191,4 +191,44 @@ Use destructuring declaration Use destructuring declaration + + DataClassWithDestructuring.kt + 6 + light_idea_test_case + + Use destructuring declaration + Use destructuring declaration + + + DataClassWithDestructuringFakeConflict.kt + 6 + light_idea_test_case + + Use destructuring declaration + Use destructuring declaration + + + DataClassWithDestructuringPartial.kt + 7 + light_idea_test_case + + Use destructuring declaration + Use destructuring declaration + + + KeyOnlyWithDestructuring.kt + 4 + light_idea_test_case + + Use destructuring declaration + Use destructuring declaration + + + KeyValueWithDestructuring.kt + 4 + light_idea_test_case + + Use destructuring declaration + Use destructuring declaration + \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index a09b6a18550..5a50ebe58ac 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -6306,6 +6306,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("noItWithDestructuring.kt") + public void testNoItWithDestructuring() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/noItWithDestructuring.kt"); + doTest(fileName); + } + @TestMetadata("nullable.kt") public void testNullable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/nullable.kt"); @@ -7644,6 +7650,30 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("DataClassWithDestructuring.kt") + public void testDataClassWithDestructuring() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassWithDestructuring.kt"); + doTest(fileName); + } + + @TestMetadata("DataClassWithDestructuringConflict.kt") + public void testDataClassWithDestructuringConflict() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassWithDestructuringConflict.kt"); + doTest(fileName); + } + + @TestMetadata("DataClassWithDestructuringFakeConflict.kt") + public void testDataClassWithDestructuringFakeConflict() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassWithDestructuringFakeConflict.kt"); + doTest(fileName); + } + + @TestMetadata("DataClassWithDestructuringPartial.kt") + public void testDataClassWithDestructuringPartial() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassWithDestructuringPartial.kt"); + doTest(fileName); + } + @TestMetadata("DataClassWithExternalUsage.kt") public void testDataClassWithExternalUsage() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassWithExternalUsage.kt"); @@ -7680,6 +7710,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("KeyOnlyWithDestructuring.kt") + public void testKeyOnlyWithDestructuring() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/KeyOnlyWithDestructuring.kt"); + doTest(fileName); + } + + @TestMetadata("KeyValueWithDestructuring.kt") + public void testKeyValueWithDestructuring() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/KeyValueWithDestructuring.kt"); + doTest(fileName); + } + @TestMetadata("MapNoProperties.kt") public void testMapNoProperties() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/MapNoProperties.kt");