From 48437d5965a22e74843a05c5f892c015903bd042 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 3 Oct 2016 14:36:25 +0300 Subject: [PATCH] KT-13941 related: "Simplify using destructuring declaration" is now applicable for function literals without parameter specification --- .../idea/intentions/DestructureIntention.kt | 92 +++++++++++++------ .../intentions/destructuringInLambda/caret.kt | 7 ++ .../intentions/destructuringInLambda/noIt.kt | 5 + .../destructuringInLambda/noIt.kt.after | 5 + .../destructuringInLambda/noItVariables.kt | 11 +++ .../noItVariables.kt.after | 10 ++ .../intentions/IntentionTestGenerated.java | 18 ++++ 7 files changed, 121 insertions(+), 27 deletions(-) create mode 100644 idea/testData/intentions/destructuringInLambda/caret.kt create mode 100644 idea/testData/intentions/destructuringInLambda/noIt.kt create mode 100644 idea/testData/intentions/destructuringInLambda/noIt.kt.after create mode 100644 idea/testData/intentions/destructuringInLambda/noItVariables.kt create mode 100644 idea/testData/intentions/destructuringInLambda/noItVariables.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 314916838c9..9e3b1934bd4 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt @@ -32,22 +32,25 @@ 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.psi.psiUtil.forEachDescendantOfType 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 java.util.* -class DestructureInspection : IntentionBasedInspection(DestructureIntention::class) +class DestructureInspection : IntentionBasedInspection(DestructureIntention::class) -class DestructureIntention : SelfTargetingRangeIntention( - KtParameter::class.java, +class DestructureIntention : SelfTargetingRangeIntention( + KtDeclaration::class.java, "Simplify using destructuring declaration" ) { - override fun applyTo(element: KtParameter, editor: Editor?) { + override fun applyTo(element: KtDeclaration, editor: Editor?) { val forLoop = element.parent as? KtForExpression val functionLiteral = element.parent?.parent as? KtFunctionLiteral - if (forLoop == null && functionLiteral == null) return + if (forLoop == null && functionLiteral == null && (element !is KtFunctionLiteral || element.hasParameterSpecification())) { + return + } val (usagesToRemove, removeSelectorInLoopRange) = collectUsagesToRemove(element, forLoop) ?: return val loopRange = forLoop?.loopRange @@ -71,27 +74,48 @@ class DestructureIntention : SelfTargetingRangeIntention( loopRange.replace(loopRange.receiverExpression) } } + else if (element is KtFunctionLiteral) { + val lambda = element.parent as KtLambdaExpression + SpecifyExplicitLambdaSignatureIntention().applyTo(lambda, editor) + lambda.functionLiteral.valueParameters.singleOrNull()?.replace( + factory.createDestructuringParameterForLambda("(${names.joinToString()})") + ) + } else if (functionLiteral != null) { element.replace(factory.createDestructuringParameterForLambda("(${names.joinToString()})")) } } - override fun applicabilityRange(element: KtParameter): TextRange? { + override fun applicabilityRange(element: KtDeclaration): TextRange? { val forLoopIfAny = element.parent as? KtForExpression - if (forLoopIfAny == null && element.parent?.parent !is KtFunctionLiteral) return null + val functionLiteral = element.parent?.parent as? KtFunctionLiteral + if (forLoopIfAny == null && functionLiteral == null && (element !is KtFunctionLiteral || element.hasParameterSpecification())) { + return null + } val usagesToRemove = collectUsagesToRemove(element, forLoopIfAny) if (usagesToRemove != null && usagesToRemove.first.isNotEmpty()) { - return element.textRange + return if (element is KtFunctionLiteral) element.lBrace.textRange else element.textRange } return null } // Note: list should contains properties in order to create destructuring declaration - private fun collectUsagesToRemove(parameter: KtParameter, forLoop: KtForExpression?): Pair, Boolean>? { - val context = parameter.analyzeFullyAndGetResult().bindingContext + private fun collectUsagesToRemove(declaration: KtDeclaration, forLoop: KtForExpression?): Pair, Boolean>? { + val context = declaration.analyzeFullyAndGetResult().bindingContext - val parameterDescriptor = context.get(BindingContext.VALUE_PARAMETER, parameter) ?: return null + val parameterDescriptor = when (declaration) { + is KtParameter -> + context.get(BindingContext.VALUE_PARAMETER, declaration) + is KtFunctionLiteral -> + if (declaration.hasParameterSpecification()) { + null + } + else { + context.get(BindingContext.FUNCTION, declaration)?.valueParameters?.singleOrNull() + } + else -> null + } ?: return null val parameterType = parameterDescriptor.type if (parameterType.isMarkedNullable) return null val classDescriptor = parameterType.constructor.declarationDescriptor as? ClassDescriptor ?: return null @@ -110,7 +134,7 @@ class DestructureIntention : SelfTargetingRangeIntention( NoLookupLocation.FROM_BUILTINS).first()) }) - ReferencesSearch.search(parameter).iterateOverMapEntryPropertiesUsages( + ReferencesSearch.search(declaration).iterateOverMapEntryPropertiesUsages( context, { index, usageData -> usagesToRemove[index] += usageData }, { otherUsages = true } @@ -122,8 +146,15 @@ class DestructureIntention : SelfTargetingRangeIntention( val valueParameters = classDescriptor.unsubstitutedPrimaryConstructor?.valueParameters ?: return null usagesToRemove = Array(valueParameters.size, { UsageData(valueParameters[it] )}) - ReferencesSearch.search(parameter).iterateOverDataClassPropertiesUsagesWithIndex( + val expressionToSearch = forLoop + ?: (declaration as? KtFunctionLiteral) + ?: (declaration.parent?.parent as? KtFunctionLiteral) + + if (expressionToSearch !is KtExpression) return null + + expressionToSearch.iterateOverDataClassPropertiesUsagesWithIndex( context, + (declaration as? KtParameter)?.nameAsName ?: Name.identifier("it"), classDescriptor, { index, usageData -> usagesToRemove[index] += usageData }, { otherUsages = true } @@ -167,32 +198,39 @@ class DestructureIntention : SelfTargetingRangeIntention( }) } - private fun Query.iterateOverDataClassPropertiesUsagesWithIndex( + private fun KtExpression.iterateOverDataClassPropertiesUsagesWithIndex( context: BindingContext, + parameterName: Name, dataClass: ClassDescriptor, process: (Int, UsageData) -> Unit, cancel: () -> Unit ) { val valueParameters = dataClass.unsubstitutedPrimaryConstructor?.valueParameters ?: return - forEach(Processor forEach@{ - val applicableUsage = getDataIfUsageIsApplicable(it, context) - if (applicableUsage != null) { - for (valueParameter in valueParameters) { - if (context.get(BindingContext.VALUE_PARAMETER_AS_PROPERTY, valueParameter) == applicableUsage.descriptor) { - process(valueParameter.index, applicableUsage) - return@forEach true + var stopTravelling = false + forEachDescendantOfType { + if (!stopTravelling && it.getReferencedNameAsName() == parameterName) { + val applicableUsage = getDataIfUsageIsApplicable(it, context) + if (applicableUsage != null) { + for (valueParameter in valueParameters) { + if (context.get(BindingContext.VALUE_PARAMETER_AS_PROPERTY, valueParameter) == applicableUsage.descriptor) { + process(valueParameter.index, applicableUsage) + return@forEachDescendantOfType + } } } - } - cancel() - return@forEach false - }) + cancel() + stopTravelling = true + } + } } - private fun getDataIfUsageIsApplicable(usage: PsiReference, context: BindingContext): UsageData? { - val parentCall = usage.element.parent as? KtExpression ?: return null + private fun getDataIfUsageIsApplicable(usage: PsiReference, context: BindingContext) = + (usage.element as? KtReferenceExpression)?.let { getDataIfUsageIsApplicable(it, context) } + + private fun getDataIfUsageIsApplicable(referenceExpression: KtReferenceExpression, context: BindingContext): UsageData? { + val parentCall = referenceExpression.parent as? KtExpression ?: return null val userParent = parentCall.parent if (userParent is KtBinaryExpression) { if (userParent.operationToken in KtTokens.ALL_ASSIGNMENTS && diff --git a/idea/testData/intentions/destructuringInLambda/caret.kt b/idea/testData/intentions/destructuringInLambda/caret.kt new file mode 100644 index 00000000000..23a59663188 --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/caret.kt @@ -0,0 +1,7 @@ +// IS_APPLICABLE: false + +data class XY(val x: String, val y: String) + +fun convert(xy: XY, foo: (XY) -> String) = foo(xy) + +fun foo(xy: XY) = convert(xy) { it.x + it.y } \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/noIt.kt b/idea/testData/intentions/destructuringInLambda/noIt.kt new file mode 100644 index 00000000000..72d303b81fe --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/noIt.kt @@ -0,0 +1,5 @@ +data class XY(val x: String, val y: String) + +fun convert(xy: XY, foo: (XY) -> String) = foo(xy) + +fun foo(xy: XY) = convert(xy) { it.x + it.y } \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/noIt.kt.after b/idea/testData/intentions/destructuringInLambda/noIt.kt.after new file mode 100644 index 00000000000..0ea3cb83715 --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/noIt.kt.after @@ -0,0 +1,5 @@ +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/destructuringInLambda/noItVariables.kt b/idea/testData/intentions/destructuringInLambda/noItVariables.kt new file mode 100644 index 00000000000..b1e24ae2b30 --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/noItVariables.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME + +data class XY(val x: String, val y: String) + +fun convert(xy: XY, foo: (XY) -> Unit) = foo(xy) + +fun foo(xy: XY) = convert(xy) { + val x = it.x + val y = it.y + println(x + y) +} \ No newline at end of file diff --git a/idea/testData/intentions/destructuringInLambda/noItVariables.kt.after b/idea/testData/intentions/destructuringInLambda/noItVariables.kt.after new file mode 100644 index 00000000000..5bc011c2388 --- /dev/null +++ b/idea/testData/intentions/destructuringInLambda/noItVariables.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME + +data class XY(val x: String, val y: String) + +fun convert(xy: XY, foo: (XY) -> Unit) = foo(xy) + +fun foo(xy: XY) = convert(xy) { + (x, y) -> + println(x + y) +} \ 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 636ce36ba93..fff8baf6fe8 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -6252,6 +6252,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/destructuringInLambda"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true); } + @TestMetadata("caret.kt") + public void testCaret() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/caret.kt"); + doTest(fileName); + } + @TestMetadata("dependentLocal.kt") public void testDependentLocal() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/dependentLocal.kt"); @@ -6282,6 +6288,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("noIt.kt") + public void testNoIt() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/noIt.kt"); + doTest(fileName); + } + + @TestMetadata("noItVariables.kt") + public void testNoItVariables() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/noItVariables.kt"); + doTest(fileName); + } + @TestMetadata("nullable.kt") public void testNullable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringInLambda/nullable.kt");