diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt index 3a5db29dfe5..f158f854067 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt @@ -26,12 +26,15 @@ import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.idea.caches.resolve.analyzeFullyAndGetResult import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection +import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns +import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter import kotlin.collections.forEach class SimplifyForInspection : IntentionBasedInspection(SimplifyForIntention()) @@ -83,42 +86,46 @@ class SimplifyForIntention : SelfTargetingRangeIntention( val classDescriptor = loopParameterType.constructor.declarationDescriptor as? ClassDescriptor ?: return null var otherUsages = false - val usagesToRemove : Array + val usagesToRemove : Array - if (DescriptorUtils.isSubclass(classDescriptor, classDescriptor.builtIns.mapEntry)) { + val mapEntry = classDescriptor.builtIns.mapEntry + val removeSelectorInLoopRange: Boolean + if (DescriptorUtils.isSubclass(classDescriptor, mapEntry)) { val loopRangeDescriptorName = element.loopRange.getResolvedCall(context)?.resultingDescriptor?.name - val removeSelectorInLoopRange = loopRangeDescriptorName?.asString().let { it == "entries" || it == "entrySet" } + removeSelectorInLoopRange = loopRangeDescriptorName?.asString().let { it == "entries" || it == "entrySet" } - usagesToRemove = arrayOfNulls(2) + usagesToRemove = Array(2, { + UsageData(mapEntry.unsubstitutedMemberScope.getContributedVariables(Name.identifier(if (it == 0) "key" else "value"), + NoLookupLocation.FROM_BUILTINS).first()) + }) ReferencesSearch.search(loopParameter).iterateOverMapEntryPropertiesUsages( context, - { index, usageData -> usagesToRemove[index] += usageData.named(if (index == 0) "key" else "value") }, + { index, usageData -> usagesToRemove[index] += usageData }, { otherUsages = true } ) - - if (!otherUsages && usagesToRemove.all { it != null && it.name != null && it.properties.size <= 1 }) { - return usagesToRemove.mapNotNull { it } to removeSelectorInLoopRange - } } else if (classDescriptor.isData) { + removeSelectorInLoopRange = false + val valueParameters = classDescriptor.unsubstitutedPrimaryConstructor?.valueParameters ?: return null - usagesToRemove = arrayOfNulls(valueParameters.size) + usagesToRemove = Array(valueParameters.size, { UsageData(valueParameters[it] )}) ReferencesSearch.search(loopParameter).iterateOverDataClassPropertiesUsagesWithIndex( context, classDescriptor, - { index, usageData -> usagesToRemove[index] += usageData.named(valueParameters[index].name.asString()) }, + { index, usageData -> usagesToRemove[index] += usageData }, { otherUsages = true } ) + } + else { + return null + } + if (otherUsages) return null - if (otherUsages) return null - - val notNullUsages = usagesToRemove.filterNotNull().filter { it.name != null } - val droppedLastUnused = usagesToRemove.dropLastWhile { it == null } - if (droppedLastUnused.size == notNullUsages.size && droppedLastUnused.all { it != null && it.properties.size <= 1 } ) { - return notNullUsages to false - } + val droppedLastUnused = usagesToRemove.dropLastWhile { it.usersToReplace.isEmpty() && it.properties.isEmpty() } + if (droppedLastUnused.all { it.properties.size <= 1 } ) { + return droppedLastUnused to removeSelectorInLoopRange } return null @@ -197,17 +204,18 @@ class SimplifyForIntention : SelfTargetingRangeIntention( val descriptor: CallableDescriptor, val name: String? = properties.firstOrNull()?.name ) { + constructor(descriptor: CallableDescriptor): + this(emptyList(), emptyList(), descriptor, descriptor.name.asString()) + constructor(property: KtProperty?, user: KtExpression, descriptor: CallableDescriptor): this(listOfNotNull(property), if (property != null) emptyList() else listOf(user), descriptor) - - fun named(suggested: String) = if (name != null) this else copy(name = suggested) } private operator fun UsageData?.plus(newData: UsageData): UsageData { if (this == null) return newData val allUsersToReplace = usersToReplace + newData.usersToReplace val allProperties = properties + newData.properties - if (properties.isNotEmpty()) return copy(properties = allProperties, usersToReplace = allUsersToReplace) - else return newData.copy(properties = allProperties, usersToReplace = allUsersToReplace) + val name = if (properties.isNotEmpty()) name else newData.name ?: name + return UsageData(allProperties, allUsersToReplace, descriptor, name) } } \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassLast.kt b/idea/testData/intentions/iterationOverMap/DataClassLast.kt new file mode 100644 index 00000000000..48fe2df5ec7 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassLast.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME + +data class My(val first: String, val second: Int) + +fun foo(list: List) { + for (my in list) { + println(my.second) + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassLast.kt.after b/idea/testData/intentions/iterationOverMap/DataClassLast.kt.after new file mode 100644 index 00000000000..8762aec5d14 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassLast.kt.after @@ -0,0 +1,9 @@ +// WITH_RUNTIME + +data class My(val first: String, val second: Int) + +fun foo(list: List) { + for ((first, second) in list) { + println(second) + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/DataClassNotAllPropertiesUsed.kt b/idea/testData/intentions/iterationOverMap/DataClassNotAllPropertiesUsed.kt index 667af459166..62d6771f384 100644 --- a/idea/testData/intentions/iterationOverMap/DataClassNotAllPropertiesUsed.kt +++ b/idea/testData/intentions/iterationOverMap/DataClassNotAllPropertiesUsed.kt @@ -1,4 +1,3 @@ -// IS_APPLICABLE: false // WITH_RUNTIME fun main(args: Array) { @@ -6,6 +5,7 @@ fun main(args: Array) { for (klass in list) { val a = klass.a val c = klass.c + println("$a$c") } } diff --git a/idea/testData/intentions/iterationOverMap/DataClassNotAllPropertiesUsed.kt.after b/idea/testData/intentions/iterationOverMap/DataClassNotAllPropertiesUsed.kt.after new file mode 100644 index 00000000000..77dd84aff07 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/DataClassNotAllPropertiesUsed.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +fun main(args: Array) { + val list = listOf(MyClass(1, 2, 3, 4)) + for ((a, b, c) in list) { + println("$a$c") + } +} + +data class MyClass(val a: Int, val b: Int, val c: Int, val d: Int) \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/KeyOnly.kt b/idea/testData/intentions/iterationOverMap/KeyOnly.kt new file mode 100644 index 00000000000..53b45d5a137 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/KeyOnly.kt @@ -0,0 +1,8 @@ +// WITH_RUNTIME + +fun main(args: Array) { + val map = hashMapOf(1 to 1) + for (entry in map) { + println(entry.key) + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/KeyOnly.kt.after b/idea/testData/intentions/iterationOverMap/KeyOnly.kt.after new file mode 100644 index 00000000000..064033ac491 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/KeyOnly.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME + +fun main(args: Array) { + val map = hashMapOf(1 to 1) + for ((key) in map) { + println(key) + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/ValueOnly.kt b/idea/testData/intentions/iterationOverMap/ValueOnly.kt new file mode 100644 index 00000000000..ccf9f4a36ab --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/ValueOnly.kt @@ -0,0 +1,8 @@ +// WITH_RUNTIME + +fun main(args: Array) { + val map = hashMapOf(1 to 1) + for (entry in map) { + println(entry.value) + } +} \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/ValueOnly.kt.after b/idea/testData/intentions/iterationOverMap/ValueOnly.kt.after new file mode 100644 index 00000000000..ef549c73099 --- /dev/null +++ b/idea/testData/intentions/iterationOverMap/ValueOnly.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME + +fun main(args: Array) { + val map = hashMapOf(1 to 1) + for ((key, value) in map) { + println(value) + } +} \ 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 611911fdb07..8ea31054701 100644 --- a/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml +++ b/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml @@ -135,4 +135,36 @@ 'for' that can be simplified using destructing declaration Simplify 'for' using destructing declaration + + ValueOnly.kt + 5 + light_idea_test_case + + 'for' that can be simplified using destructing declaration + Simplify 'for' using destructing declaration + + + KeyOnly.kt + 5 + light_idea_test_case + + 'for' that can be simplified using destructing declaration + Simplify 'for' using destructing declaration + + + DataClassNotAllPropertiesUsed.kt + 5 + light_idea_test_case + + 'for' that can be simplified using destructing declaration + Simplify 'for' using destructing declaration + + + DataClassLast.kt + 6 + light_idea_test_case + + 'for' that can be simplified using destructing declaration + Simplify 'for' using destructing 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 9df162d79f4..fe34d136076 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -6549,6 +6549,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("DataClassLast.kt") + public void testDataClassLast() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassLast.kt"); + doTest(fileName); + } + @TestMetadata("DataClassNoVariablesInside.kt") public void testDataClassNoVariablesInside() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassNoVariablesInside.kt"); @@ -6639,6 +6645,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("KeyOnly.kt") + public void testKeyOnly() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/KeyOnly.kt"); + doTest(fileName); + } + @TestMetadata("MapNoProperties.kt") public void testMapNoProperties() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/MapNoProperties.kt"); @@ -6681,6 +6693,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("ValueOnly.kt") + public void testValueOnly() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/ValueOnly.kt"); + doTest(fileName); + } + } @TestMetadata("idea/testData/intentions/moveAssignmentToInitializer")