Inline Variable: Remove invalid imports and perform reference lengthening/shortening when inlining non-local declaration

#KT-8884 Fixed
2015-12-14 12:24:28 +03:00
parent 44bdac6e11
commit 4e36edb5d6
25 changed files with 245 additions and 53 deletions
@@ -499,7 +499,7 @@ fun main(args: Array<String>) {
        }

        testClass<AbstractInlineTest>() {
-            model("refactoring/inline")
+            model("refactoring/inline", pattern = "^(\\w+)\\.kt$")
        }

        testClass<AbstractUnwrapRemoveTest>() {
@@ -25,6 +25,7 @@ import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.editor.colors.EditorColors
 import com.intellij.openapi.editor.colors.EditorColorsManager
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.util.Key
 import com.intellij.openapi.wm.WindowManager
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiWhiteSpace
@@ -33,17 +34,24 @@ import com.intellij.refactoring.HelpID
 import com.intellij.refactoring.RefactoringBundle
 import com.intellij.refactoring.util.CommonRefactoringUtil
 import com.intellij.refactoring.util.RefactoringMessageDialog
+import com.intellij.usageView.UsageInfo
 import com.intellij.util.containers.MultiMap
 import org.jetbrains.kotlin.diagnostics.Errors
 import org.jetbrains.kotlin.idea.KotlinLanguage
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
+import org.jetbrains.kotlin.idea.codeInsight.shorten.performDelayedShortening
 import org.jetbrains.kotlin.idea.core.refactoring.checkConflictsInteractively
 import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.idea.refactoring.move.PackageNameInfo
+import org.jetbrains.kotlin.idea.refactoring.move.lazilyProcessInternalReferencesToUpdateOnPackageNameChange
+import org.jetbrains.kotlin.idea.refactoring.move.postProcessMoveUsages
+import org.jetbrains.kotlin.idea.references.mainReference
 import org.jetbrains.kotlin.idea.resolve.ResolutionFacade
 import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
 import org.jetbrains.kotlin.idea.util.ShortenReferences
 import org.jetbrains.kotlin.idea.util.application.executeWriteCommand
+import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.*
 import org.jetbrains.kotlin.resolve.BindingContext
@@ -58,6 +66,11 @@ import org.jetbrains.kotlin.utils.sure
 import java.util.*

 class KotlinInlineValHandler : InlineActionHandler() {
+    companion object {
+        private var KtSimpleNameExpression.internalUsageInfos: MutableMap<FqName, (KtSimpleNameExpression) -> UsageInfo?>?
+                by CopyableUserDataProperty(Key.create("INTERNAL_USAGE_INFOS"))
+    }
+
    override fun isEnabledForLanguage(l: Language) = l == KotlinLanguage.INSTANCE

    override fun canInlineElement(element: PsiElement): Boolean {
@@ -146,14 +159,28 @@ class KotlinInlineValHandler : InlineActionHandler() {
        val typeArgumentsForCall = getTypeArgumentsStringForCall(initializer)
        val parametersForFunctionLiteral = getParametersForFunctionLiteral(initializer)

-        val canHighlight = referenceExpressions.all { it.containingFile === file }
-        if (canHighlight) {
+        val isSingleFile = referenceExpressions.all { it.containingFile === file }
+        if (isSingleFile) {
            highlightExpressions(project, editor, referenceExpressions)
        }

+        if (!isSingleFile) {
+            val targetPackages = referenceExpressions.mapNotNullTo(LinkedHashSet()) { (it.containingFile as? KtFile)?.packageFqName }
+            for (targetPackage in targetPackages) {
+                if (targetPackage == file.packageFqName) continue
+                val packageNameInfo = PackageNameInfo(file.packageFqName, targetPackage.toUnsafe())
+                initializer.lazilyProcessInternalReferencesToUpdateOnPackageNameChange(packageNameInfo) { expr, factory ->
+                    val infos = expr.internalUsageInfos
+                                ?: LinkedHashMap<FqName, (KtSimpleNameExpression) -> UsageInfo?>()
+                                        .apply { expr.internalUsageInfos = this }
+                    infos[targetPackage] = factory
+                }
+            }
+        }
+
        fun performRefactoring() {
            if (!showDialog(project, name, declaration, referenceExpressions)) {
-                if (canHighlight) {
+                if (isSingleFile) {
                    val statusBar = WindowManager.getInstance().getStatusBar(project)
                    statusBar?.info = RefactoringBundle.message("press.escape.to.remove.the.highlighting")
                }
@@ -161,23 +188,45 @@ class KotlinInlineValHandler : InlineActionHandler() {
            }

            project.executeWriteCommand(RefactoringBundle.message("inline.command", name)) {
-                val inlinedExpressions = referenceExpressions.flatMap { referenceExpression ->
-                    if (assignments.contains(referenceExpression.parent)) return@flatMap emptyList<KtExpression>()
-                    doReplace(referenceExpression, initializer)
-                }
+                val inlinedExpressions = referenceExpressions
+                        .flatMap { referenceExpression ->
+                            if (assignments.contains(referenceExpression.parent)) return@flatMap emptyList<KtExpression>()
+
+                            val importDirective = referenceExpression.getStrictParentOfType<KtImportDirective>()
+                            if (importDirective != null) {
+                                val reference = referenceExpression.getQualifiedElementSelector()?.mainReference
+                                if (reference != null && reference.multiResolve(false).size <= 1) {
+                                    importDirective.delete()
+                                }
+
+                                return@flatMap emptyList<KtExpression>()
+                            }
+
+                            doReplace(referenceExpression, initializer)
+                        }
+                        .mapNotNull { inlinedExpression ->
+                            val pointer = inlinedExpression.createSmartPointer()
+                            val targetPackage = inlinedExpression.getContainingKtFile().packageFqName
+                            val expressionsToProcess = inlinedExpression.collectDescendantsOfType<KtSimpleNameExpression> { it.internalUsageInfos != null }
+                            val internalUsages = expressionsToProcess.mapNotNull { it.internalUsageInfos!![targetPackage]?.invoke(it) }
+                            expressionsToProcess.forEach { it.internalUsageInfos = null }
+                            postProcessMoveUsages(internalUsages)
+                            pointer.element
+                        }

                assignments.forEach { it.delete() }
                declaration.delete()

-                if (inlinedExpressions.isEmpty()) return@executeWriteCommand
+                if (inlinedExpressions.isNotEmpty()) {
+                    typeArgumentsForCall?.let { addTypeArguments(it, inlinedExpressions) }

-                typeArgumentsForCall?.let { addTypeArguments(it, inlinedExpressions) }
+                    parametersForFunctionLiteral?.let { addFunctionLiteralParameterTypes(it, inlinedExpressions) }

-                parametersForFunctionLiteral?.let { addFunctionLiteralParameterTypes(it, inlinedExpressions) }
-
-                if (canHighlight) {
-                    highlightExpressions(project, editor, inlinedExpressions)
+                    if (isSingleFile) {
+                        highlightExpressions(project, editor, inlinedExpressions)
+                    }
                }
+                performDelayedShortening(project)
            }
        }

@@ -37,7 +37,6 @@ import com.intellij.refactoring.util.MoveRenameUsageInfo
 import com.intellij.refactoring.util.NonCodeUsageInfo
 import com.intellij.usageView.UsageInfo
 import com.intellij.util.IncorrectOperationException
-import com.intellij.util.containers.ContainerUtil
 import org.jetbrains.kotlin.asJava.namedUnwrappedElement
 import org.jetbrains.kotlin.asJava.unwrapped
 import org.jetbrains.kotlin.descriptors.*
@@ -63,14 +62,24 @@ import org.jetbrains.kotlin.psi.psiUtil.*
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.descriptorUtil.getImportableDescriptor
+import org.jetbrains.kotlin.utils.addIfNotNull
 import java.util.*

 val UNKNOWN_PACKAGE_FQ_NAME = FqNameUnsafe("org.jetbrains.kotlin.idea.refactoring.move.<unknown-package>")

 public class PackageNameInfo(val oldPackageName: FqName, val newPackageName: FqNameUnsafe)

-public fun KtElement.getInternalReferencesToUpdateOnPackageNameChange(packageNameInfo: PackageNameInfo): List<UsageInfo> {
-    val file = getContainingFile() as? KtFile ?: return listOf()
+fun KtElement.getInternalReferencesToUpdateOnPackageNameChange(packageNameInfo: PackageNameInfo): List<UsageInfo> {
+    val usages = ArrayList<UsageInfo>()
+    lazilyProcessInternalReferencesToUpdateOnPackageNameChange(packageNameInfo) { expr, factory -> usages.addIfNotNull(factory(expr)) }
+    return usages
+}
+
+fun KtElement.lazilyProcessInternalReferencesToUpdateOnPackageNameChange(
+        packageNameInfo: PackageNameInfo,
+        body: (originalRefExpr: KtSimpleNameExpression, usageFactory: (KtSimpleNameExpression) -> UsageInfo?) -> Unit
+) {
+    val file = getContainingFile() as? KtFile ?: return

    val importPaths = file.getImportDirectives().mapNotNull { it.getImportPath() }

@@ -85,7 +94,7 @@ public fun KtElement.getInternalReferencesToUpdateOnPackageNameChange(packageNam
        }
    }

-    fun processReference(refExpr: KtSimpleNameExpression, bindingContext: BindingContext): UsageInfo? {
+    fun processReference(refExpr: KtSimpleNameExpression, bindingContext: BindingContext): ((KtSimpleNameExpression) -> UsageInfo?)? {
        val descriptor = bindingContext[BindingContext.REFERENCE_TARGET, refExpr]?.getImportableDescriptor() ?: return null

        val declaration = DescriptorToSourceUtilsIde.getAnyDeclaration(getProject(), descriptor) ?: return null
@@ -95,11 +104,13 @@ public fun KtElement.getInternalReferencesToUpdateOnPackageNameChange(packageNam

        if (isCallable && !isExtension) {
            val containingDescriptor = descriptor.getContainingDeclaration()
-            val receiver = refExpr.getReceiverExpression()
-            if (receiver != null) {
-                val receiverRef = receiver.getQualifiedElementSelector() as? KtSimpleNameExpression ?: return null
-                if (bindingContext[BindingContext.QUALIFIER, receiverRef] == null) return null
-                return processReference(receiverRef, bindingContext)
+            if (refExpr.getReceiverExpression() != null) {
+                return fun(refExpr: KtSimpleNameExpression): UsageInfo? {
+                    val receiver = refExpr.getReceiverExpression() ?: return null
+                    val receiverRef = receiver.getQualifiedElementSelector() as? KtSimpleNameExpression ?: return null
+                    if (bindingContext[BindingContext.QUALIFIER, receiverRef] == null) return null
+                    return processReference(receiverRef, bindingContext)?.invoke(receiverRef)
+                }
            }
            if (containingDescriptor !is PackageFragmentDescriptor) return null
        }
@@ -113,41 +124,38 @@ public fun KtElement.getInternalReferencesToUpdateOnPackageNameChange(packageNam

        val oldPackageName = packageNameInfo.oldPackageName
        val newPackageName = packageNameInfo.newPackageName
-        return when {
-            isExtension ||
-            packageName == oldPackageName ||
-            packageName?.asString() == newPackageName.asString() ||
-            isImported(descriptor) -> {
-                if (isAncestor(declaration, false)) {
-                    if (descriptor.importableFqName == null) return null
-                    if (isUnqualifiedExtensionReference(refExpr.mainReference, declaration)) return null
-                    if (packageName == null || !newPackageName.isSafe) return null
-                    fqName.asString().let {
-                        val prefix = packageName.asString()
-                        val prefixOffset = it.indexOf(prefix)
-                        val newFqName = FqName(it.replaceRange(prefixOffset..prefixOffset + prefix.length() - 1, newPackageName.asString()))
-                        return MoveRenameSelfUsageInfo(refExpr.mainReference, declaration, newFqName)
-                    }
-                }

-                createMoveUsageInfoIfPossible(refExpr.mainReference, declaration, false)
+        fun doCreateUsageInfo(refExpr: KtSimpleNameExpression): UsageInfo? {
+            if (isAncestor(declaration, false)) {
+                if (descriptor.importableFqName == null) return null
+                if (isUnqualifiedExtensionReference(refExpr.mainReference, declaration)) return null
+                if (packageName == null || !newPackageName.isSafe) return null
+                return fqName.asString().let {
+                    val prefix = packageName.asString()
+                    val prefixOffset = it.indexOf(prefix)
+                    val newFqName = FqName(it.replaceRange(prefixOffset..prefixOffset + prefix.length() - 1, newPackageName.asString()))
+                    MoveRenameSelfUsageInfo(refExpr.mainReference, declaration, newFqName)
+                }
            }

-            else -> null
+            return createMoveUsageInfoIfPossible(refExpr.mainReference, declaration, false)
        }
+
+        if (!isExtension &&
+            packageName != oldPackageName &&
+            packageName?.asString() != newPackageName.asString() &&
+            !isImported(descriptor)) return null
+        return ::doCreateUsageInfo
    }

    val referenceToContext = KotlinFileReferencesResolver.resolve(file = file, elements = listOf(this))

-    val usages = ArrayList<UsageInfo>()
    for ((refExpr, bindingContext) in referenceToContext) {
        if (refExpr !is KtSimpleNameExpression || refExpr.getParent() is KtThisExpression) continue
        if (bindingContext[BindingContext.QUALIFIER, refExpr] != null) continue

-        processReference(refExpr, bindingContext)?.let { usages.add(it) }
+        processReference(refExpr, bindingContext)?.let { body(refExpr, it) }
    }
-
-    return usages
 }

 class MoveRenameUsageInfoForExtension(
@@ -0,0 +1,7 @@
+package boo
+
+import foo.bar.a
+
+fun main(args: Array<String>) {
+    println(a + a())
+}
@@ -0,0 +1,8 @@
+package boo
+
+import foo.bar.a
+import foo.bar.b
+
+fun main(args: Array<String>) {
+    println(b + a())
+}
@@ -0,0 +1,5 @@
+package foo.bar
+
+val b = 1
+val <caret>a = b
+fun a() = b
@@ -0,0 +1,4 @@
+package foo.bar
+
+val b = 1
+fun a() = b
@@ -0,0 +1,3 @@
+package bar
+
+val x = 1
@@ -0,0 +1,3 @@
+package bar
+
+val x = 1
@@ -0,0 +1,3 @@
+package baz
+
+val y = 2
@@ -0,0 +1,3 @@
+package baz
+
+val y = 2
@@ -0,0 +1,5 @@
+package bar
+
+import foo.z
+
+fun test() = z + 1
@@ -0,0 +1,5 @@
+package bar
+
+import baz.y
+
+fun test() = x + y + 1
@@ -0,0 +1,5 @@
+package baz
+
+import foo.z
+
+fun test() = z + 1
@@ -0,0 +1,5 @@
+package baz
+
+import bar.x
+
+fun test() = x + y + 1
@@ -0,0 +1,5 @@
+package other
+
+import foo.z
+
+fun test() = z + 1
@@ -0,0 +1,6 @@
+package other
+
+import bar.x
+import baz.y
+
+fun test() = x + y + 1
@@ -0,0 +1,8 @@
+package foo
+
+import bar.x
+import baz.y
+
+val <caret>z = x + y
+
+fun test() = z + 1
@@ -0,0 +1,6 @@
+package foo
+
+import bar.x
+import baz.y
+
+fun test() = x + y + 1
@@ -0,0 +1,7 @@
+package boo
+
+import foo.bar.a
+
+fun main(args: Array<String>) {
+    println(a)
+}
@@ -0,0 +1,7 @@
+package boo
+
+import foo.bar.b
+
+fun main(args: Array<String>) {
+    println(b)
+}
@@ -0,0 +1,4 @@
+package foo.bar
+
+val b = 1
+val <caret>a = b
@@ -0,0 +1,3 @@
+package foo.bar
+
+val b = 1
@@ -22,18 +22,30 @@ import com.intellij.codeInsight.TargetElementUtil.REFERENCED_ELEMENT_ACCEPTED
 import com.intellij.openapi.util.io.FileUtil
 import com.intellij.refactoring.util.CommonRefactoringUtil
 import com.intellij.testFramework.UsefulTestCase
+import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture
 import junit.framework.TestCase
 import org.jetbrains.kotlin.idea.test.KotlinLightCodeInsightFixtureTestCase
 import org.jetbrains.kotlin.idea.test.KotlinWithJdkAndRuntimeLightProjectDescriptor
 import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.test.InTextDirectivesUtils
+import org.jetbrains.kotlin.test.KotlinTestUtils
 import java.io.File

 abstract class AbstractInlineTest : KotlinLightCodeInsightFixtureTestCase() {
+    val fixture: JavaCodeInsightTestFixture
+        get() = myFixture
+
    protected fun doTest(path: String) {
+        val mainFile = File(path)
        val afterFile = File(path + ".after")

-        myFixture.configureByFile(path)
+        val mainFileName = mainFile.name
+        val mainFileBaseName = FileUtil.getNameWithoutExtension(mainFileName)
+        val extraFiles = mainFile.parentFile.listFiles { file, name ->
+            name != mainFileName && name.startsWith("$mainFileBaseName.") && (name.endsWith(".kt") || name.endsWith(".java"))
+        }
+        val extraFilesToPsi = extraFiles.toMapBy { fixture.configureByFile(path.replace(mainFileName, it.name)) }
+        val file = myFixture.configureByFile(path)

        val afterFileExists = afterFile.exists()

@@ -47,7 +59,10 @@ abstract class AbstractInlineTest : KotlinLightCodeInsightFixtureTestCase() {

                TestCase.assertTrue(afterFileExists)
                UsefulTestCase.assertEmpty(expectedErrors)
-                myFixture.checkResult(FileUtil.loadFile(afterFile, true))
+                KotlinTestUtils.assertEqualsToFile(afterFile, file.text)
+                for ((extraPsiFile, extraFile) in extraFilesToPsi) {
+                    KotlinTestUtils.assertEqualsToFile(File("${extraFile.path}.after"), extraPsiFile.text)
+                }
            }
            catch (e: CommonRefactoringUtil.RefactoringErrorHintException) {
                TestCase.assertFalse(afterFileExists)
@@ -32,7 +32,7 @@ import java.util.regex.Pattern;
@RunWith(JUnit3RunnerWithInners.class)
 public class InlineTestGenerated extends AbstractInlineTest {
    public void testAllFilesPresentInInline() throws Exception {
-        KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline"), Pattern.compile("^(.+)\\.kt$"), true);
+        KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline"), Pattern.compile("^(\\w+)\\.kt$"), true);
    }

    @TestMetadata("Basic.kt")
@@ -154,7 +154,7 @@ public class InlineTestGenerated extends AbstractInlineTest {
    @RunWith(JUnit3RunnerWithInners.class)
    public static class AddParenthesis extends AbstractInlineTest {
        public void testAllFilesPresentInAddParenthesis() throws Exception {
-            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/addParenthesis"), Pattern.compile("^(.+)\\.kt$"), true);
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/addParenthesis"), Pattern.compile("^(\\w+)\\.kt$"), true);
        }

        @TestMetadata("ArrayAccess.kt")
@@ -343,7 +343,7 @@ public class InlineTestGenerated extends AbstractInlineTest {
    @RunWith(JUnit3RunnerWithInners.class)
    public static class ExplicateParameterTypes extends AbstractInlineTest {
        public void testAllFilesPresentInExplicateParameterTypes() throws Exception {
-            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/explicateParameterTypes"), Pattern.compile("^(.+)\\.kt$"), true);
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/explicateParameterTypes"), Pattern.compile("^(\\w+)\\.kt$"), true);
        }

        @TestMetadata("EnoughDontExplicate.kt")
@@ -394,7 +394,7 @@ public class InlineTestGenerated extends AbstractInlineTest {
    @RunWith(JUnit3RunnerWithInners.class)
    public static class ExplicateTypeArgument extends AbstractInlineTest {
        public void testAllFilesPresentInExplicateTypeArgument() throws Exception {
-            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/explicateTypeArgument"), Pattern.compile("^(.+)\\.kt$"), true);
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/explicateTypeArgument"), Pattern.compile("^(\\w+)\\.kt$"), true);
        }

        @TestMetadata("DeeperNestedCall.kt")
@@ -457,7 +457,7 @@ public class InlineTestGenerated extends AbstractInlineTest {
    @RunWith(JUnit3RunnerWithInners.class)
    public static class Property extends AbstractInlineTest {
        public void testAllFilesPresentInProperty() throws Exception {
-            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/property"), Pattern.compile("^(.+)\\.kt$"), true);
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/property"), Pattern.compile("^(\\w+)\\.kt$"), true);
        }

        @TestMetadata("Basic.kt")
@@ -484,6 +484,18 @@ public class InlineTestGenerated extends AbstractInlineTest {
            doTest(fileName);
        }

+        @TestMetadata("keepImport.kt")
+        public void testKeepImport() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/property/keepImport.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("multiplePackages.kt")
+        public void testMultiplePackages() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/property/multiplePackages.kt");
+            doTest(fileName);
+        }
+
        @TestMetadata("ObjectProperty.kt")
        public void testObjectProperty() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/property/ObjectProperty.kt");
@@ -496,6 +508,12 @@ public class InlineTestGenerated extends AbstractInlineTest {
            doTest(fileName);
        }

+        @TestMetadata("removeImport.kt")
+        public void testRemoveImport() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/property/removeImport.kt");
+            doTest(fileName);
+        }
+
        @TestMetadata("WithGetter.kt")
        public void testWithGetter() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/property/WithGetter.kt");
@@ -520,7 +538,7 @@ public class InlineTestGenerated extends AbstractInlineTest {
        }

        public void testAllFilesPresentInStringTemplates() throws Exception {
-            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/stringTemplates"), Pattern.compile("^(.+)\\.kt$"), true);
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/stringTemplates"), Pattern.compile("^(\\w+)\\.kt$"), true);
        }

        @TestMetadata("blockEntry.kt")