Refactoring + minor fixes: destructure intention

This commit is contained in:
Mikhail Glukhikh
2016-10-04 20:27:36 +03:00
parent dd7d7a5fc7
commit 70949d10da
11 changed files with 223 additions and 157 deletions
@@ -244,12 +244,7 @@ class KtPsiFactory(private val project: Project) {
return (createFunction("fun foo() {$text}").bodyExpression as KtBlockExpression).statements.first() as KtDestructuringDeclaration
}
fun createDestructuringParameterForLoop(text: String): KtParameter {
val dummyFun = createFunction("fun foo() { for ($text in foo) {} }")
return ((dummyFun.bodyExpression as KtBlockExpression).statements.first() as KtForExpression).loopParameter!!
}
fun createDestructuringParameterForLambda(text: String): KtParameter {
fun createDestructuringParameter(text: String): KtParameter {
val dummyFun = createFunction("fun foo() = { $text -> }")
return (dummyFun.bodyExpression as KtLambdaExpression).functionLiteral.valueParameters.first()
}
@@ -18,13 +18,14 @@ package org.jetbrains.kotlin.idea.intentions
import com.intellij.openapi.editor.Editor
import com.intellij.openapi.util.TextRange
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiReference
import com.intellij.psi.search.searches.ReferencesSearch
import com.intellij.util.Processor
import com.intellij.util.Query
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.caches.resolve.analyze
import org.jetbrains.kotlin.idea.core.KotlinNameSuggester
import org.jetbrains.kotlin.idea.core.NewDeclarationNameValidator
import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection
@@ -32,11 +33,14 @@ 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.psi.psiUtil.PsiChildRange
import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForReceiver
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.utils.singletonOrEmptyList
import java.util.*
class DestructureInspection : IntentionBasedInspection<KtDeclaration>(
@@ -52,147 +56,160 @@ class DestructureIntention : SelfTargetingRangeIntention<KtDeclaration>(
) {
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 &&
(element !is KtFunctionLiteral || element.hasParameterSpecification()) &&
(element !is KtVariableDeclaration)) {
return
}
val (usagesToRemove, removeSelectorInLoopRange) = collectUsagesToRemove(element, forLoop) ?: return
val loopRange = forLoop?.loopRange
val (usagesToRemove, removeSelectorInLoopRange) = collectUsagesToRemove(element, forLoop)!!
val factory = KtPsiFactory(element)
val validator = NewDeclarationNameValidator(element.parent, element, NewDeclarationNameValidator.Target.VARIABLES,
usagesToRemove.map { it.properties }.flatten())
val validator = NewDeclarationNameValidator(
element.parent, element, NewDeclarationNameValidator.Target.VARIABLES,
excludedDeclarations = usagesToRemove.map { it.variableToDrop.singletonOrEmptyList() }.flatten()
)
val names = ArrayList<String>()
usagesToRemove.forEach { p ->
val name = KotlinNameSuggester.suggestNameByName(p.name!!, validator)
p.properties.firstOrNull()?.delete()
p.usersToReplace.forEach {
it.replace(factory.createExpression(name))
usagesToRemove.forEach { (descriptor, usagesToReplace, variableToDrop, name) ->
val suggestedName = name ?: KotlinNameSuggester.suggestNameByName(descriptor.name.asString(), validator)
variableToDrop?.delete()
usagesToReplace.forEach {
it.replace(factory.createExpression(suggestedName))
}
names.add(name)
names.add(suggestedName)
}
val joinedNames = names.joinToString()
if (forLoop != null) {
element.replace(factory.createDestructuringParameterForLoop("($joinedNames)"))
if (removeSelectorInLoopRange && loopRange is KtDotQualifiedExpression) {
loopRange.replace(loopRange.receiverExpression)
val joinedNames = names.joinToString()
when (element) {
is KtParameter -> {
element.replace(factory.createDestructuringParameter("($joinedNames)"))
if (removeSelectorInLoopRange && loopRange is KtDotQualifiedExpression) {
loopRange.replace(loopRange.receiverExpression)
}
}
is KtFunctionLiteral -> {
val lambda = element.parent as KtLambdaExpression
SpecifyExplicitLambdaSignatureIntention().applyTo(lambda, editor)
lambda.functionLiteral.valueParameters.singleOrNull()?.replace(
factory.createDestructuringParameter("($joinedNames)")
)
}
is KtVariableDeclaration -> {
val rangeAfterEq = PsiChildRange(element.initializer, element.lastChild)
val modifierList = element.modifierList
if (modifierList == null) {
element.replace(factory.createDestructuringDeclarationByPattern(
"val ($joinedNames) = $0", rangeAfterEq))
}
else {
val rangeBeforeVal = PsiChildRange(element.firstChild, modifierList)
element.replace(factory.createDestructuringDeclarationByPattern(
"$0:'@xyz' val ($joinedNames) = $1", rangeBeforeVal, rangeAfterEq))
}
}
}
else if (element is KtFunctionLiteral) {
val lambda = element.parent as KtLambdaExpression
SpecifyExplicitLambdaSignatureIntention().applyTo(lambda, editor)
lambda.functionLiteral.valueParameters.singleOrNull()?.replace(
factory.createDestructuringParameterForLambda("($joinedNames)")
)
}
else if (functionLiteral != null) {
element.replace(factory.createDestructuringParameterForLambda("($joinedNames)"))
}
else if (element is KtVariableDeclaration) {
val modifiersText = element.modifierList?.text ?: ""
val initializerText = element.initializer!!.text
element.replace(factory.createDestructuringDeclaration("$modifiersText val ($joinedNames) = $initializerText"))
}
}
override fun applicabilityRange(element: KtDeclaration): TextRange? {
val forLoopIfAny = element.parent as? KtForExpression
val functionLiteral = element.parent?.parent as? KtFunctionLiteral
if (forLoopIfAny == null && functionLiteral == null &&
(element !is KtFunctionLiteral || element.hasParameterSpecification()) &&
(element !is KtVariableDeclaration)) {
return null
}
if (!isSuitableDeclaration(element)) return null
val usagesToRemove = collectUsagesToRemove(element, forLoopIfAny)
if (usagesToRemove != null && usagesToRemove.first.isNotEmpty()) {
return when (element) {
is KtFunctionLiteral -> element.lBrace.textRange
is KtVariableDeclaration -> element.nameIdentifier?.textRange ?: element.textRange
else -> element.textRange
}
val usagesToRemove = collectUsagesToRemove(element, element.parent as? KtForExpression)?.data ?: return null
if (usagesToRemove.isEmpty()) return null
return when (element) {
is KtFunctionLiteral -> element.lBrace.textRange
is KtNamedDeclaration -> element.nameIdentifier?.textRange
else -> null
}
return null
}
// Note: list should contains properties in order to create destructuring declaration
private fun collectUsagesToRemove(declaration: KtDeclaration, forLoop: KtForExpression?): Pair<List<UsageData>, Boolean>? {
val context = declaration.analyzeFullyAndGetResult().bindingContext
private fun isSuitableDeclaration(declaration: KtDeclaration) = when (declaration) {
is KtParameter -> {
val parent = declaration.parent
when {
parent is KtForExpression -> true
parent?.parent is KtFunctionLiteral -> true
else -> false
}
}
is KtVariableDeclaration -> true
is KtFunctionLiteral -> !declaration.hasParameterSpecification() // replace implicit 'it' with destructuring declaration
else -> false
}
private data class UsagesToRemove(val data: List<UsageData>, val removeSelectorInLoopRange: Boolean)
private fun collectUsagesToRemove(declaration: KtDeclaration, forLoop: KtForExpression?): UsagesToRemove? {
val context = declaration.analyze()
val variableDescriptor = when (declaration) {
is KtParameter ->
context.get(BindingContext.VALUE_PARAMETER, declaration)
is KtFunctionLiteral ->
if (declaration.hasParameterSpecification()) {
null
}
else {
context.get(BindingContext.FUNCTION, declaration)?.valueParameters?.singleOrNull()
}
is KtVariableDeclaration ->
context.get(BindingContext.VARIABLE, declaration)
is KtParameter -> context.get(BindingContext.VALUE_PARAMETER, declaration)
is KtFunctionLiteral -> context.get(BindingContext.FUNCTION, declaration)?.valueParameters?.singleOrNull()
is KtVariableDeclaration -> context.get(BindingContext.VARIABLE, declaration)
else -> null
} ?: return null
val variableType = variableDescriptor.type
if (variableType.isMarkedNullable) return null
val classDescriptor = variableType.constructor.declarationDescriptor as? ClassDescriptor ?: return null
var otherUsages = false
val usagesToRemove : Array<UsageData>
val mapEntryClassDescriptor = classDescriptor.builtIns.mapEntry
val mapEntry = classDescriptor.builtIns.mapEntry
// Note: list should contains properties in order to create destructuring declaration
val usagesToRemove = mutableListOf<UsageData>()
var badUsageFound = false
val removeSelectorInLoopRange: Boolean
if (forLoop != null && DescriptorUtils.isSubclass(classDescriptor, mapEntry)) {
if (forLoop != null && DescriptorUtils.isSubclass(classDescriptor, mapEntryClassDescriptor)) {
val loopRangeDescriptorName = forLoop.loopRange.getResolvedCall(context)?.resultingDescriptor?.name
removeSelectorInLoopRange = loopRangeDescriptorName?.asString().let { it == "entries" || it == "entrySet" }
usagesToRemove = Array(2, {
UsageData(mapEntry.unsubstitutedMemberScope.getContributedVariables(Name.identifier(if (it == 0) "key" else "value"),
NoLookupLocation.FROM_BUILTINS).first())
})
listOf("key", "value").mapTo(usagesToRemove) {
UsageData(mapEntryClassDescriptor.unsubstitutedMemberScope.getContributedVariables(
Name.identifier(it), NoLookupLocation.FROM_BUILTINS).single())
}
ReferencesSearch.search(declaration).iterateOverMapEntryPropertiesUsages(
context,
{ index, usageData -> usagesToRemove[index] += usageData },
{ otherUsages = true }
{ index, usageData -> usagesToRemove[index].add(usageData) },
{ badUsageFound = true }
)
}
else if (classDescriptor.isData) {
removeSelectorInLoopRange = false
val valueParameters = classDescriptor.unsubstitutedPrimaryConstructor?.valueParameters ?: return null
usagesToRemove = Array(valueParameters.size, { UsageData(valueParameters[it] )})
valueParameters.mapTo(usagesToRemove) { UsageData(it) }
val expressionToSearch = forLoop
?: (declaration as? KtFunctionLiteral)
?: (declaration.parent?.parent as? KtFunctionLiteral)
?: (declaration as? KtVariableDeclaration)?.parent
// inference bug: remove as? PsiElement when fixed
val usageScopeElement: PsiElement = forLoop as? PsiElement
?: (declaration as? KtFunctionLiteral)
?: (declaration.parent?.parent as? KtFunctionLiteral)
?: (declaration as? KtVariableDeclaration)?.parent
?: return null
if (expressionToSearch !is KtExpression) return null
val nameToSearch = when (declaration) {
is KtParameter -> declaration.nameAsName
is KtVariableDeclaration -> declaration.nameAsName
else -> Name.identifier("it")
} ?: return null
expressionToSearch.iterateOverDataClassPropertiesUsagesWithIndex(
val descriptorToIndex = mutableMapOf<CallableDescriptor, Int>()
for (valueParameter in valueParameters) {
val propertyDescriptor = context.get(BindingContext.VALUE_PARAMETER_AS_PROPERTY, valueParameter) ?: continue
descriptorToIndex[propertyDescriptor] = valueParameter.index
}
usageScopeElement.iterateOverDataClassPropertiesUsagesWithIndex(
context,
(declaration as? KtNamedDeclaration)?.nameAsName ?: Name.identifier("it"),
classDescriptor,
{ index, usageData -> usagesToRemove[index] += usageData },
{ otherUsages = true }
nameToSearch,
descriptorToIndex,
{ index, usageData -> usagesToRemove[index].add(usageData) },
{ badUsageFound = true }
)
}
else {
return null
}
if (otherUsages) return null
if (badUsageFound) return null
val droppedLastUnused = usagesToRemove.dropLastWhile { it.usersToReplace.isEmpty() && it.properties.isEmpty() }
if (droppedLastUnused.all { it.properties.size <= 1 } ) {
return droppedLastUnused to removeSelectorInLoopRange
}
return null
val droppedLastUnused = usagesToRemove.dropLastWhile { it.usagesToReplace.isEmpty() && it.variableToDrop == null }
return UsagesToRemove(droppedLastUnused, removeSelectorInLoopRange)
}
private fun Query<PsiReference>.iterateOverMapEntryPropertiesUsages(
@@ -204,14 +221,15 @@ class DestructureIntention : SelfTargetingRangeIntention<KtDeclaration>(
forEach(Processor forEach@{
val applicableUsage = getDataIfUsageIsApplicable(it, context)
if (applicableUsage != null) {
val descriptorName = applicableUsage.descriptor.name.asString()
if (descriptorName == "key" || descriptorName == "getKey") {
process(0, applicableUsage)
return@forEach true
}
else if (descriptorName == "value" || descriptorName == "getValue") {
process(1, applicableUsage)
return@forEach true
when (applicableUsage.descriptor.name.asString()) {
"key", "getKey" -> {
process(0, applicableUsage)
return@forEach true
}
"value", "getValue" -> {
process(1, applicableUsage)
return@forEach true
}
}
}
@@ -220,73 +238,66 @@ class DestructureIntention : SelfTargetingRangeIntention<KtDeclaration>(
})
}
private fun KtExpression.iterateOverDataClassPropertiesUsagesWithIndex(
private fun PsiElement.iterateOverDataClassPropertiesUsagesWithIndex(
context: BindingContext,
parameterName: Name,
dataClass: ClassDescriptor,
descriptorToIndex: Map<CallableDescriptor, Int>,
process: (Int, UsageData) -> Unit,
cancel: () -> Unit
) {
val valueParameters = dataClass.unsubstitutedPrimaryConstructor?.valueParameters ?: return
var stopTravelling = false
forEachDescendantOfType<KtNameReferenceExpression> {
if (!stopTravelling && it.getReferencedNameAsName() == parameterName) {
anyDescendantOfType<KtNameReferenceExpression> {
if (it.getReferencedNameAsName() != parameterName) false
else {
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
}
val index = descriptorToIndex[applicableUsage.descriptor]
if (index != null) {
process(index, applicableUsage)
return@anyDescendantOfType false
}
}
cancel()
stopTravelling = true
true
}
}
}
private fun getDataIfUsageIsApplicable(usage: PsiReference, context: BindingContext) =
(usage.element as? KtReferenceExpression)?.let { getDataIfUsageIsApplicable(it, context) }
private fun getDataIfUsageIsApplicable(dataClassUsage: PsiReference, context: BindingContext) =
(dataClassUsage.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 &&
userParent.left === parentCall) return null
}
if (userParent is KtUnaryExpression) {
if (userParent.operationToken === KtTokens.PLUSPLUS || userParent.operationToken === KtTokens.MINUSMINUS) return null
private fun getDataIfUsageIsApplicable(dataClassUsage: KtReferenceExpression, context: BindingContext): UsageData? {
val qualifiedExpression = dataClassUsage.getQualifiedExpressionForReceiver() ?: return null
val parent = qualifiedExpression.parent
when (parent) {
is KtBinaryExpression -> {
if (parent.operationToken in KtTokens.ALL_ASSIGNMENTS && parent.left == qualifiedExpression) return null
}
is KtUnaryExpression -> {
if (parent.operationToken == KtTokens.PLUSPLUS || parent.operationToken == KtTokens.MINUSMINUS) return null
}
}
val property = parentCall.parent as? KtProperty
val property = parent as? KtProperty // val x = d.y
if (property != null && property.isVar) return null
val resolvedCall = parentCall.getResolvedCall(context) ?: return null
val descriptor = resolvedCall.resultingDescriptor
return UsageData(property, parentCall, descriptor)
val descriptor = qualifiedExpression.getResolvedCall(context)?.resultingDescriptor ?: return null
return UsageData(
descriptor = descriptor,
usagesToReplace = mutableListOf(qualifiedExpression),
variableToDrop = property)
}
private data class UsageData(val properties: List<KtProperty>,
val usersToReplace: List<KtExpression>,
val descriptor: CallableDescriptor,
val name: String? = properties.firstOrNull()?.name
private data class UsageData(
val descriptor: CallableDescriptor,
val usagesToReplace: MutableList<KtExpression> = mutableListOf(),
var variableToDrop: KtProperty? = null,
var name: String? = variableToDrop?.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)
}
private operator fun UsageData?.plus(newData: UsageData): UsageData {
if (this == null) return newData
val allUsersToReplace = usersToReplace + newData.usersToReplace
val allProperties = properties + newData.properties
val name = if (properties.isNotEmpty()) name else newData.name ?: name
return UsageData(allProperties, allUsersToReplace, descriptor, name)
fun add(newData: UsageData) {
variableToDrop = variableToDrop ?: newData.variableToDrop
usagesToReplace.addAll(newData.usagesToReplace)
name = name ?: newData.name
}
}
}
@@ -81,7 +81,7 @@ class IterateExpressionIntention : SelfTargetingIntention<KtExpression>(KtExpres
}
val paramPattern = (names.singleOrNull()?.first()
?: psiFactory.createDestructuringParameterForLoop(names.indices.joinToString(prefix = "(", postfix = ")") { "p$it" }))
?: psiFactory.createDestructuringParameter(names.indices.joinToString(prefix = "(", postfix = ")") { "p$it" }))
var forExpression = psiFactory.createExpressionByPattern("for($0 in $1) {\nx\n}", paramPattern, element) as KtForExpression
forExpression = element.replaced(forExpression)
@@ -0,0 +1,10 @@
data class XY(val x: Int, val y: Int)
fun create() = XY(1, 2)
annotation class Ann
fun use(): Int {
@Ann val <caret>xy = create()
return xy.x + xy.y
}
@@ -0,0 +1,10 @@
data class XY(val x: Int, val y: Int)
fun create() = XY(1, 2)
annotation class Ann
fun use(): Int {
@Ann val (x, y) = create()
return x + y
}
@@ -1,4 +1,3 @@
// IS_APPLICABLE: false
// WITH_RUNTIME
data class XY(val x: String, val y: Int)
@@ -0,0 +1,10 @@
// WITH_RUNTIME
data class XY(val x: String, val y: Int)
fun test(xys: Array<XY>) {
for ((x) in xys) {
println(x)
val xx = x
println(xx)
}
}
@@ -1,4 +1,3 @@
// IS_APPLICABLE: false
// WITH_RUNTIME
fun main(args: Array<String>) {
@@ -0,0 +1,10 @@
// WITH_RUNTIME
fun main(args: Array<String>) {
val map = hashMapOf(1 to 1)
for ((key) in map) {
val key2 = key
println(key)
}
}
@@ -175,4 +175,20 @@
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Use destructuring declaration</problem_class>
<description>Use destructuring declaration</description>
</problem>
<problem>
<file>OnlyKeyUsed.kt</file>
<line>5</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="temp:///src/OnlyKeyUsed.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Use destructuring declaration</problem_class>
<description>Use destructuring declaration</description>
</problem>
<problem>
<file>DataClassTwoDifferentLocals.kt</file>
<line>5</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="temp:///src/DataClassTwoDifferentLocals.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Use destructuring declaration</problem_class>
<description>Use destructuring declaration</description>
</problem>
</problems>
@@ -6356,6 +6356,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringVariables/var.kt");
doTest(fileName);
}
@TestMetadata("withModifiers.kt")
public void testWithModifiers() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/destructuringVariables/withModifiers.kt");
doTest(fileName);
}
}
@TestMetadata("idea/testData/intentions/ifNullToElvis")