From feb9dd4d8311c835aea27bf87b4a3550962d878d Mon Sep 17 00:00:00 2001 From: mglukhikh Date: Fri, 31 Mar 2017 13:10:40 +0300 Subject: [PATCH] Don't suggest destructuring if at least half components not used So #KT-16828 Fixed --- .../idea/intentions/DestructureIntention.kt | 8 ++++++-- .../inspectionData/expected.xml | 8 ++++---- .../mapIndexedExceptFirst.kt | 5 +++++ .../mapIndexedExceptFirst.kt.after | 5 +++++ .../destructuringInLambda/twoOfThree.kt | 10 ++++++++++ .../destructuringInLambda/twoOfThree.kt.after | 10 ++++++++++ .../iterationOverMap/inspectionData/expected.xml | 16 ---------------- .../idea/intentions/IntentionTestGenerated.java | 12 ++++++++++++ 8 files changed, 52 insertions(+), 22 deletions(-) create mode 100644 idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt create mode 100644 idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt.after create mode 100644 idea/testData/intentions/destructuringInLambda/twoOfThree.kt create mode 100644 idea/testData/intentions/destructuringInLambda/twoOfThree.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 07c9e98cb7a..8bdf150e53a 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt @@ -48,9 +48,13 @@ import java.util.* class DestructureInspection : IntentionBasedInspection( DestructureIntention::class, { element, _ -> - if (element is KtParameter) true + val usagesToRemove = DestructureIntention.collectUsagesToRemove(element)?.data + if (element is KtParameter) { + usagesToRemove != null && + (usagesToRemove.any { it.declarationToDrop is KtDestructuringDeclaration } || + usagesToRemove.filter { it.usagesToReplace.isNotEmpty() }.size > usagesToRemove.size / 2) + } else { - val usagesToRemove = DestructureIntention.collectUsagesToRemove(element)?.data usagesToRemove?.any { it.declarationToDrop is KtDestructuringDeclaration } ?: false } } diff --git a/idea/testData/intentions/destructuringInLambda/inspectionData/expected.xml b/idea/testData/intentions/destructuringInLambda/inspectionData/expected.xml index 2b9d0973a28..8bc9c151b25 100644 --- a/idea/testData/intentions/destructuringInLambda/inspectionData/expected.xml +++ b/idea/testData/intentions/destructuringInLambda/inspectionData/expected.xml @@ -24,10 +24,10 @@ Use destructuring declaration - last.kt + twoOfThree.kt 6 light_idea_test_case - + Use destructuring declaration Use destructuring declaration @@ -64,10 +64,10 @@ Use destructuring declaration - mapIndexedLast.kt + mapIndexedExceptFirst.kt 5 light_idea_test_case - + Use destructuring declaration Use destructuring declaration diff --git a/idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt b/idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt new file mode 100644 index 00000000000..18c5bf2d881 --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt @@ -0,0 +1,5 @@ +// WITH_RUNTIME + +data class Package(val name: String, val version: String, val source: String, val id: Int) + +val pkgs = listOf().mapIndexed { i, p -> "${p.name} ${p.version}" to i + p.id }.toMap() \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt.after b/idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt.after new file mode 100644 index 00000000000..6a3afce9c1e --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt.after @@ -0,0 +1,5 @@ +// WITH_RUNTIME + +data class Package(val name: String, val version: String, val source: String, val id: Int) + +val pkgs = listOf().mapIndexed { i, (name, version, _, id) -> "${name} ${version}" to i + id }.toMap() \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/twoOfThree.kt b/idea/testData/intentions/destructuringInLambda/twoOfThree.kt new file mode 100644 index 00000000000..14a5bb6adeb --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/twoOfThree.kt @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +data class My(val first: String, val second: Int, val third: Boolean) + +fun foo(list: List) { + list.forEach { my -> + println(my.second) + println(my.third) + } +} \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/twoOfThree.kt.after b/idea/testData/intentions/destructuringInLambda/twoOfThree.kt.after new file mode 100644 index 00000000000..2430a3e1668 --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/twoOfThree.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +data class My(val first: String, val second: Int, val third: Boolean) + +fun foo(list: List) { + list.forEach { (_, second, third) -> + println(second) + println(third) + } +} \ 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 1f2211c2b66..0b6d06403f9 100644 --- a/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml +++ b/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml @@ -135,14 +135,6 @@ Use destructuring declaration Use destructuring declaration - - ValueOnly.kt - 5 - light_idea_test_case - - Use destructuring declaration - Use destructuring declaration - KeyOnly.kt 5 @@ -159,14 +151,6 @@ Use destructuring declaration Use destructuring declaration - - DataClassLast.kt - 6 - light_idea_test_case - - Use destructuring declaration - Use destructuring declaration - DataClassNameConflict.kt 7 diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 2bc875550bc..05930664ba0 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -7645,6 +7645,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("mapIndexedExceptFirst.kt") + public void testMapIndexedExceptFirst() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/mapIndexedExceptFirst.kt"); + doTest(fileName); + } + @TestMetadata("mapIndexedLast.kt") public void testMapIndexedLast() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/mapIndexedLast.kt"); @@ -7687,6 +7693,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("twoOfThree.kt") + public void testTwoOfThree() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/twoOfThree.kt"); + doTest(fileName); + } + @TestMetadata("variables.kt") public void testVariables() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/variables.kt");