Fix code inlining for expression body with multiple occurrences

So #KT-17022 Fixed
This commit is contained in:
Mikhail Glukhikh
2017-03-22 22:24:38 +03:00
parent 96846d0b52
commit 081caadec1
7 changed files with 64 additions and 19 deletions
@@ -29,7 +29,6 @@ import org.jetbrains.kotlin.idea.core.*
import org.jetbrains.kotlin.idea.intentions.RemoveExplicitTypeArgumentsIntention import org.jetbrains.kotlin.idea.intentions.RemoveExplicitTypeArgumentsIntention
import org.jetbrains.kotlin.idea.util.* import org.jetbrains.kotlin.idea.util.*
import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.* import org.jetbrains.kotlin.psi.psiUtil.*
import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingContext
@@ -141,14 +140,10 @@ class CodeInliner<TCallElement : KtElement>(
private fun renameDuplicates(declarations: List<KtNamedDeclaration>, private fun renameDuplicates(declarations: List<KtNamedDeclaration>,
lexicalScope: LexicalScope) { lexicalScope: LexicalScope) {
fun String.hasConflicts() = lexicalScope.getAllAccessibleVariables(Name.identifier(this)).any { val validator = CollectingNameValidator { !it.nameHasConflictsInScope(lexicalScope) }
!it.isExtension && it.isVisible(lexicalScope.ownerDescriptor)
}
val validator = CollectingNameValidator { !it.hasConflicts() }
for (declaration in declarations) { for (declaration in declarations) {
val oldName = declaration.name val oldName = declaration.name
if (oldName != null && oldName.hasConflicts()) { if (oldName != null && oldName.nameHasConflictsInScope(lexicalScope)) {
val newName = KotlinNameSuggester.suggestNameByName(oldName, validator) val newName = KotlinNameSuggester.suggestNameByName(oldName, validator)
val renameProcessor = RenameProcessor(project, declaration, newName, false, false) val renameProcessor = RenameProcessor(project, declaration, newName, false, false)
renameProcessor.run() renameProcessor.run()
@@ -26,16 +26,21 @@ import com.intellij.psi.PsiElement
import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.search.GlobalSearchScope
import com.intellij.psi.search.searches.ReferencesSearch import com.intellij.psi.search.searches.ReferencesSearch
import com.intellij.ui.GuiUtils import com.intellij.ui.GuiUtils
import com.intellij.util.ui.UIUtil import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.idea.core.targetDescriptors import org.jetbrains.kotlin.idea.core.targetDescriptors
import org.jetbrains.kotlin.idea.references.KtSimpleNameReference import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
import org.jetbrains.kotlin.idea.search.usagesSearch.descriptor
import org.jetbrains.kotlin.idea.stubindex.KotlinSourceFilterScope import org.jetbrains.kotlin.idea.stubindex.KotlinSourceFilterScope
import org.jetbrains.kotlin.idea.util.application.executeWriteCommand import org.jetbrains.kotlin.idea.util.application.executeWriteCommand
import org.jetbrains.kotlin.idea.util.application.runReadAction import org.jetbrains.kotlin.idea.util.application.runReadAction
import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.psi.KtImportDirective import org.jetbrains.kotlin.psi.KtImportDirective
import org.jetbrains.kotlin.psi.KtNamedDeclaration
import org.jetbrains.kotlin.psi.KtSimpleNameExpression import org.jetbrains.kotlin.psi.KtSimpleNameExpression
import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
interface UsageReplacementStrategy { interface UsageReplacementStrategy {
fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement?)? fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement?)?
@@ -59,13 +64,14 @@ fun UsageReplacementStrategy.replaceUsagesInWholeProject(
.filterIsInstance<KtSimpleNameReference>() .filterIsInstance<KtSimpleNameReference>()
.map { ref -> ref.expression } .map { ref -> ref.expression }
} }
this@replaceUsagesInWholeProject.replaceUsages(usages, project, commandName, postAction) this@replaceUsagesInWholeProject.replaceUsages(usages, targetPsiElement, project, commandName, postAction)
} }
}) })
} }
private fun UsageReplacementStrategy.replaceUsages( private fun UsageReplacementStrategy.replaceUsages(
usages: Collection<KtSimpleNameExpression>, usages: Collection<KtSimpleNameExpression>,
targetPsiElement: PsiElement,
project: Project, project: Project,
commandName: String, commandName: String,
postAction: () -> Unit postAction: () -> Unit
@@ -74,10 +80,16 @@ private fun UsageReplacementStrategy.replaceUsages(
project.executeWriteCommand(commandName) { project.executeWriteCommand(commandName) {
// we should delete imports later to not affect other usages // we should delete imports later to not affect other usages
val importsToDelete = arrayListOf<KtImportDirective>() val importsToDelete = arrayListOf<KtImportDirective>()
val replacements = mutableListOf<KtElement>()
for (usage in usages) { var invalidUsagesFound = false
// NB: reversed order is better in case of composition like sqr(sqr(x))
for (usage in usages.reversed()) {
try { try {
if (!usage.isValid) continue // TODO: nested calls if (!usage.isValid) {
invalidUsagesFound = true
continue
}
//TODO: keep the import if we don't know how to replace some of the usages //TODO: keep the import if we don't know how to replace some of the usages
val importDirective = usage.getStrictParentOfType<KtImportDirective>() val importDirective = usage.getStrictParentOfType<KtImportDirective>()
@@ -88,13 +100,33 @@ private fun UsageReplacementStrategy.replaceUsages(
continue continue
} }
createReplacer(usage)?.invoke() val replacement = createReplacer(usage)?.invoke()
if (replacement != null) {
replacements += replacement
}
} }
catch (e: Throwable) { catch (e: Throwable) {
LOG.error(e) LOG.error(e)
} }
} }
if (invalidUsagesFound && targetPsiElement is KtNamedDeclaration) {
val name = targetPsiElement.name
val targetDescriptor = targetPsiElement.descriptor
if (name != null && targetDescriptor != null) {
for (replacement in replacements) {
replacement.forEachDescendantOfType<KtSimpleNameExpression> { usage ->
if (usage.isValid && usage.getReferencedName() == name) {
val context = usage.analyze(BodyResolveMode.PARTIAL)
if (targetDescriptor == context[BindingContext.REFERENCE_TARGET, usage]) {
createReplacer(usage)?.invoke()
}
}
}
}
}
}
importsToDelete.forEach { it.delete() } importsToDelete.forEach { it.delete() }
postAction() postAction()
@@ -20,7 +20,9 @@ import org.jetbrains.kotlin.idea.analysis.computeTypeInContext
import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
import org.jetbrains.kotlin.idea.core.KotlinNameSuggester import org.jetbrains.kotlin.idea.core.KotlinNameSuggester
import org.jetbrains.kotlin.idea.core.isVisible
import org.jetbrains.kotlin.idea.core.setType import org.jetbrains.kotlin.idea.core.setType
import org.jetbrains.kotlin.idea.util.getAllAccessibleVariables
import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.idea.util.getResolutionScope
import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.*
@@ -30,9 +32,9 @@ import org.jetbrains.kotlin.psi.psiUtil.isAncestor
import org.jetbrains.kotlin.renderer.render import org.jetbrains.kotlin.renderer.render
import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.bindingContextUtil.getDataFlowInfoBefore import org.jetbrains.kotlin.resolve.bindingContextUtil.getDataFlowInfoBefore
import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension
import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
import org.jetbrains.kotlin.resolve.scopes.LexicalScope import org.jetbrains.kotlin.resolve.scopes.LexicalScope
import org.jetbrains.kotlin.resolve.scopes.utils.findLocalVariable
import org.jetbrains.kotlin.types.ErrorUtils import org.jetbrains.kotlin.types.ErrorUtils
import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.KotlinType
@@ -85,9 +87,7 @@ internal fun MutableCodeToInline.introduceValue(
if (usages.isNotEmpty()) { if (usages.isNotEmpty()) {
val resolutionScope = expressionToBeReplaced.getResolutionScope(bindingContext, expressionToBeReplaced.getResolutionFacade()) val resolutionScope = expressionToBeReplaced.getResolutionScope(bindingContext, expressionToBeReplaced.getResolutionFacade())
val name = suggestName { name -> val name = suggestName { name -> !name.nameHasConflictsInScope(resolutionScope) && !isNameUsed(name) }
resolutionScope.findLocalVariable(Name.identifier(name)) == null && !isNameUsed(name)
}
val declaration = psiFactory.createDeclarationByPattern<KtVariableDeclaration>("val $0 = $1", name, value) val declaration = psiFactory.createDeclarationByPattern<KtVariableDeclaration>("val $0 = $1", name, value)
statementsBefore.add(0, declaration) statementsBefore.add(0, declaration)
@@ -134,6 +134,10 @@ internal fun MutableCodeToInline.introduceValue(
} }
} }
fun String.nameHasConflictsInScope(lexicalScope: LexicalScope) = lexicalScope.getAllAccessibleVariables(Name.identifier(this)).any {
!it.isExtension && it.isVisible(lexicalScope.ownerDescriptor)
}
private fun variableNeedsExplicitType( private fun variableNeedsExplicitType(
initializer: KtExpression, initializer: KtExpression,
initializerType: KotlinType, initializerType: KotlinType,
@@ -0,0 +1,4 @@
fun <caret>sqr(x: Double) = x * x
fun bisqr(x: Double) = sqr(sqr(x))
@@ -0,0 +1,4 @@
fun bisqr(x: Double): Double {
val x1 = x * x
return x1 * x1
}
@@ -1,8 +1,8 @@
class Point(val x: Double, val y: Double) { class Point(val x: Double, val y: Double) {
// After sqr inlining, only first usage of sqr is replaced // After sqr inlining, only first usage of sqr is replaced
fun distance(other: Point): Double { fun distance(other: Point): Double {
val x1 = x - other.x val x1 = y - other.y
// See KT-17022 val x2 = x - other.x
return Math.sqrt(x1 * x1 + sqr(y - other.y)) return Math.sqrt(x2 * x2 + x1 * x1)
} }
} }
@@ -130,6 +130,12 @@ public class InlineTestGenerated extends AbstractInlineTest {
doTest(fileName); doTest(fileName);
} }
@TestMetadata("MultipleInComposition.kt")
public void testMultipleInComposition() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/expressionBody/MultipleInComposition.kt");
doTest(fileName);
}
@TestMetadata("MultipleInExpression.kt") @TestMetadata("MultipleInExpression.kt")
public void testMultipleInExpression() throws Exception { public void testMultipleInExpression() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/expressionBody/MultipleInExpression.kt"); String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/expressionBody/MultipleInExpression.kt");