From 42aea59253f21f914ad4e5a5e1f20c1ccdd8e359 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 3 Oct 2016 11:30:14 +0300 Subject: [PATCH] Simplify for intention refactoring: focus on the loop parameter, not on the loop itself --- .../inspectionDescriptions/Destructure.html | 5 ++ .../inspectionDescriptions/SimplifyFor.html | 5 -- .../after.kt.template | 0 .../before.kt.template | 0 .../DestructureIntention/description.html | 5 ++ .../SimplifyForIntention/description.html | 5 -- idea/src/META-INF/plugin.xml | 6 +- ...orIntention.kt => DestructureIntention.kt} | 50 +++++------ .../kotlin/idea/j2k/J2kPostProcessings.kt | 2 +- .../intentions/iterationOverMap/.intention | 2 +- .../inspectionData/expected.xml | 88 +++++++++---------- .../inspectionData/inspections.test | 2 +- 12 files changed, 83 insertions(+), 87 deletions(-) create mode 100644 idea/resources/inspectionDescriptions/Destructure.html delete mode 100644 idea/resources/inspectionDescriptions/SimplifyFor.html rename idea/resources/intentionDescriptions/{SimplifyForIntention => DestructureIntention}/after.kt.template (100%) rename idea/resources/intentionDescriptions/{SimplifyForIntention => DestructureIntention}/before.kt.template (100%) create mode 100644 idea/resources/intentionDescriptions/DestructureIntention/description.html delete mode 100644 idea/resources/intentionDescriptions/SimplifyForIntention/description.html rename idea/src/org/jetbrains/kotlin/idea/intentions/{SimplifyForIntention.kt => DestructureIntention.kt} (82%) diff --git a/idea/resources/inspectionDescriptions/Destructure.html b/idea/resources/inspectionDescriptions/Destructure.html new file mode 100644 index 00000000000..09c9edafbca --- /dev/null +++ b/idea/resources/inspectionDescriptions/Destructure.html @@ -0,0 +1,5 @@ + + +This inspection reports any declarations that can be destructured + + diff --git a/idea/resources/inspectionDescriptions/SimplifyFor.html b/idea/resources/inspectionDescriptions/SimplifyFor.html deleted file mode 100644 index 1b1131ae986..00000000000 --- a/idea/resources/inspectionDescriptions/SimplifyFor.html +++ /dev/null @@ -1,5 +0,0 @@ - - -This inspection reports any for expressions that can be simplified - - diff --git a/idea/resources/intentionDescriptions/SimplifyForIntention/after.kt.template b/idea/resources/intentionDescriptions/DestructureIntention/after.kt.template similarity index 100% rename from idea/resources/intentionDescriptions/SimplifyForIntention/after.kt.template rename to idea/resources/intentionDescriptions/DestructureIntention/after.kt.template diff --git a/idea/resources/intentionDescriptions/SimplifyForIntention/before.kt.template b/idea/resources/intentionDescriptions/DestructureIntention/before.kt.template similarity index 100% rename from idea/resources/intentionDescriptions/SimplifyForIntention/before.kt.template rename to idea/resources/intentionDescriptions/DestructureIntention/before.kt.template diff --git a/idea/resources/intentionDescriptions/DestructureIntention/description.html b/idea/resources/intentionDescriptions/DestructureIntention/description.html new file mode 100644 index 00000000000..c8ff7d13687 --- /dev/null +++ b/idea/resources/intentionDescriptions/DestructureIntention/description.html @@ -0,0 +1,5 @@ + + +This intention destructures declaration (of parameter or variable of data class / map entry type) + + diff --git a/idea/resources/intentionDescriptions/SimplifyForIntention/description.html b/idea/resources/intentionDescriptions/SimplifyForIntention/description.html deleted file mode 100644 index de61bdd34a1..00000000000 --- a/idea/resources/intentionDescriptions/SimplifyForIntention/description.html +++ /dev/null @@ -1,5 +0,0 @@ - - -This intention simplifies for expression by replacing iteration over map entries and collections of data classes with destructing declaration - - diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 7bbaced9786..0bd9fe2a37d 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -1251,7 +1251,7 @@ - org.jetbrains.kotlin.idea.intentions.SimplifyForIntention + org.jetbrains.kotlin.idea.intentions.DestructureIntention Kotlin @@ -1638,8 +1638,8 @@ language="kotlin" /> - (SimplifyForIntention::class) +class DestructureInspection : IntentionBasedInspection(DestructureIntention::class) -class SimplifyForIntention : SelfTargetingRangeIntention( - KtForExpression::class.java, - "Simplify 'for' using destructuring declaration", - "Simplify 'for'" +class DestructureIntention : SelfTargetingRangeIntention( + KtParameter::class.java, + "Simplify using destructuring declaration" ) { - override fun applyTo(element: KtForExpression, editor: Editor?) { - val (usagesToRemove, removeSelectorInLoopRange) = collectUsagesToRemove(element) ?: return + override fun applyTo(element: KtParameter, editor: Editor?) { + val forLoop = element.parent as? KtForExpression + val (usagesToRemove, removeSelectorInLoopRange) = collectUsagesToRemove(element, forLoop) ?: return - val loopRange = element.loopRange ?: return - val loopParameter = element.loopParameter ?: return + val loopRange = forLoop?.loopRange val factory = KtPsiFactory(element) val validator = NewDeclarationNameValidator(element.parent, element, NewDeclarationNameValidator.Target.VARIABLES, @@ -63,41 +62,38 @@ class SimplifyForIntention : SelfTargetingRangeIntention( } names.add(name) } - loopParameter.replace(factory.createDestructuringDeclarationInFor("(${names.joinToString()})")) + element.replace(factory.createDestructuringDeclarationInFor("(${names.joinToString()})")) if (removeSelectorInLoopRange && loopRange is KtDotQualifiedExpression) { loopRange.replace(loopRange.receiverExpression) } } - override fun applicabilityRange(element: KtForExpression): TextRange? { - if (element.destructuringParameter != null) return null - - val usagesToRemove = collectUsagesToRemove(element) + override fun applicabilityRange(element: KtParameter): TextRange? { + val forLoop = element.parent as? KtForExpression ?: return null + val usagesToRemove = collectUsagesToRemove(element, forLoop) if (usagesToRemove != null && usagesToRemove.first.isNotEmpty()) { - return element.loopParameter!!.textRange + return element.textRange } return null } // Note: list should contains properties in order to create destructuring declaration - private fun collectUsagesToRemove(element: KtForExpression): Pair, Boolean>? { - val loopParameter = element.loopParameter ?: return null + private fun collectUsagesToRemove(parameter: KtParameter, forLoop: KtForExpression?): Pair, Boolean>? { + val context = parameter.analyzeFullyAndGetResult().bindingContext - val context = element.analyzeFullyAndGetResult().bindingContext - - val loopParameterDescriptor = context.get(BindingContext.VALUE_PARAMETER, loopParameter) ?: return null - val loopParameterType = loopParameterDescriptor.type - if (loopParameterType.isMarkedNullable) return null - val classDescriptor = loopParameterType.constructor.declarationDescriptor as? ClassDescriptor ?: return null + val parameterDescriptor = context.get(BindingContext.VALUE_PARAMETER, parameter) ?: return null + val parameterType = parameterDescriptor.type + if (parameterType.isMarkedNullable) return null + val classDescriptor = parameterType.constructor.declarationDescriptor as? ClassDescriptor ?: return null var otherUsages = false val usagesToRemove : Array val mapEntry = classDescriptor.builtIns.mapEntry val removeSelectorInLoopRange: Boolean - if (DescriptorUtils.isSubclass(classDescriptor, mapEntry)) { - val loopRangeDescriptorName = element.loopRange.getResolvedCall(context)?.resultingDescriptor?.name + if (forLoop != null && DescriptorUtils.isSubclass(classDescriptor, mapEntry)) { + val loopRangeDescriptorName = forLoop.loopRange.getResolvedCall(context)?.resultingDescriptor?.name removeSelectorInLoopRange = loopRangeDescriptorName?.asString().let { it == "entries" || it == "entrySet" } usagesToRemove = Array(2, { @@ -105,7 +101,7 @@ class SimplifyForIntention : SelfTargetingRangeIntention( NoLookupLocation.FROM_BUILTINS).first()) }) - ReferencesSearch.search(loopParameter).iterateOverMapEntryPropertiesUsages( + ReferencesSearch.search(parameter).iterateOverMapEntryPropertiesUsages( context, { index, usageData -> usagesToRemove[index] += usageData }, { otherUsages = true } @@ -117,7 +113,7 @@ class SimplifyForIntention : SelfTargetingRangeIntention( val valueParameters = classDescriptor.unsubstitutedPrimaryConstructor?.valueParameters ?: return null usagesToRemove = Array(valueParameters.size, { UsageData(valueParameters[it] )}) - ReferencesSearch.search(loopParameter).iterateOverDataClassPropertiesUsagesWithIndex( + ReferencesSearch.search(parameter).iterateOverDataClassPropertiesUsagesWithIndex( context, classDescriptor, { index, usageData -> usagesToRemove[index] += usageData }, diff --git a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt index d6ce1613fce..b55560a5663 100644 --- a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt +++ b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt @@ -68,7 +68,7 @@ object J2KPostProcessingRegistrar { registerIntentionBasedProcessing(ObjectLiteralToLambdaIntention()) registerIntentionBasedProcessing(AnonymousFunctionToLambdaIntention()) registerIntentionBasedProcessing(RemoveUnnecessaryParenthesesIntention()) - registerIntentionBasedProcessing(SimplifyForIntention()) + registerIntentionBasedProcessing(DestructureIntention()) registerIntentionBasedProcessing(SimplifyAssertNotNullIntention()) registerDiagnosticBasedProcessing(Errors.USELESS_CAST) { element, diagnostic -> diff --git a/idea/testData/intentions/iterationOverMap/.intention b/idea/testData/intentions/iterationOverMap/.intention index 6cd5c448bf4..275f8c5a32b 100644 --- a/idea/testData/intentions/iterationOverMap/.intention +++ b/idea/testData/intentions/iterationOverMap/.intention @@ -1 +1 @@ -org.jetbrains.kotlin.idea.intentions.SimplifyForIntention +org.jetbrains.kotlin.idea.intentions.DestructureIntention diff --git a/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml b/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml index 6d786761c89..72952d43106 100644 --- a/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml +++ b/idea/testData/intentions/iterationOverMap/inspectionData/expected.xml @@ -4,175 +4,175 @@ 4 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration Simple.kt 5 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration PropertiesNames.kt 5 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration Getters.kt 5 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassParametersOrder.kt 5 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClass.kt 5 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration EntriesCallIsMissing.kt 6 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassFirstNPropertiesUsed.kt 5 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration CaretOffset.kt 6 light_idea_test_case - Simplify 'for' using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassNoVariablesInside.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassNoVariablesMultiUsages.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassPropertyBetweenUsages.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassSecondVariable.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassWithExternalUsage.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassDependentLocal.kt 6 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassFirstVariable.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration MapNoProperties.kt 4 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration ValueOnly.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration KeyOnly.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassNotAllPropertiesUsed.kt 5 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassLast.kt 6 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration DataClassNameConflict.kt 7 light_idea_test_case - 'for' that can be simplified using destructuring declaration - Simplify 'for' using destructuring declaration + Can be simplified using destructuring declaration + Simplify using destructuring declaration \ No newline at end of file diff --git a/idea/testData/intentions/iterationOverMap/inspectionData/inspections.test b/idea/testData/intentions/iterationOverMap/inspectionData/inspections.test index 5ce71a19f3f..911aacda32e 100644 --- a/idea/testData/intentions/iterationOverMap/inspectionData/inspections.test +++ b/idea/testData/intentions/iterationOverMap/inspectionData/inspections.test @@ -1 +1 @@ -// INSPECTION_CLASS: org.jetbrains.kotlin.idea.intentions.SimplifyForInspection +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.intentions.DestructureInspection