Move: Fix conflict checking when switching between similar libraries

#KT-17006 Fixed
2017-03-23 21:07:34 +03:00
parent 45b8cd29e1
commit 298ee266c3
25 changed files with 338 additions and 16 deletions
@@ -18,7 +18,7 @@ package org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations

 import com.intellij.openapi.module.ModuleUtilCore
 import com.intellij.openapi.project.Project
-import com.intellij.openapi.roots.ModuleRootManager
+import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiMember
 import com.intellij.psi.PsiMethod
@@ -35,13 +35,19 @@ import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.descriptors.impl.MutablePackageFragmentDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.*
 import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
+import org.jetbrains.kotlin.idea.imports.importableFqName
+import org.jetbrains.kotlin.idea.project.TargetPlatformDetector
 import org.jetbrains.kotlin.idea.refactoring.getUsageContext
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.*
+import org.jetbrains.kotlin.renderer.ClassifierNamePolicy
+import org.jetbrains.kotlin.renderer.DescriptorRenderer
+import org.jetbrains.kotlin.renderer.ParameterNameRenderingPolicy
 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.getImportableDescriptor
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver
 import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
@@ -67,6 +73,9 @@ class MoveConflictChecker(
        }
    }

+    private fun getModuleDescriptor(sourceFile: VirtualFile) =
+            resolutionFacade.findModuleDescriptor(getModuleInfoByVirtualFile(project, sourceFile))
+
    private fun KotlinMoveTarget.getContainerDescriptor(): DeclarationDescriptor? {
        return when (this) {
            is KotlinMoveTargetForExistingElement -> {
@@ -87,14 +96,7 @@ class MoveConflictChecker(
                val packageFqName = targetContainerFqName ?: return null
                val targetDir = directory?.virtualFile ?: targetFile
                val targetModuleDescriptor = if (targetDir != null) {
-                    val targetModule = ModuleUtilCore.findModuleForFile(targetDir, project) ?: return null
-                    val moduleFileIndex = ModuleRootManager.getInstance(targetModule).fileIndex
-                    val targetModuleInfo = when {
-                        moduleFileIndex.isInSourceContent(targetDir) -> targetModule.productionSourceInfo()
-                        moduleFileIndex.isInTestSourceContent(targetDir) -> targetModule.testSourceInfo()
-                        else -> return null
-                    }
-                    resolutionFacade.findModuleDescriptor(targetModuleInfo) ?: return null
+                    getModuleDescriptor(targetDir) ?: return null
                }
                else {
                    resolutionFacade.moduleDescriptor
@@ -156,6 +158,19 @@ class MoveConflictChecker(
        }
    }

+    companion object {
+        private val DESCRIPTOR_RENDERER_FOR_COMPARISON = DescriptorRenderer.withOptions {
+            withDefinedIn = true
+            classifierNamePolicy = ClassifierNamePolicy.FULLY_QUALIFIED
+            modifiers = emptySet()
+            withoutTypeParameters = true
+            parameterNameRenderingPolicy = ParameterNameRenderingPolicy.NONE
+            includeAdditionalModifiers = false
+            renderUnabbreviatedType = false
+            withoutSuperTypes = true
+        }
+    }
+
    private fun checkModuleConflictsInDeclarations(
            internalUsages: MutableSet<UsageInfo>,
            conflicts: MultiMap<PsiElement, String>
@@ -163,6 +178,47 @@ class MoveConflictChecker(
        val sourceRoot = moveTarget.targetFile ?: return
        val targetModule = ModuleUtilCore.findModuleForFile(sourceRoot, project) ?: return
        val resolveScope = GlobalSearchScope.moduleWithDependenciesAndLibrariesScope(targetModule)
+
+        fun isInScope(targetElement: PsiElement, targetDescriptor: DeclarationDescriptor): Boolean {
+            if (targetElement in resolveScope) return true
+            if (targetElement.manager.isInProject(targetElement)) return false
+
+            val fqName = targetDescriptor.importableFqName ?: return true
+            val importableDescriptor = targetDescriptor.getImportableDescriptor()
+            val renderedImportableTarget = DESCRIPTOR_RENDERER_FOR_COMPARISON.render(importableDescriptor)
+            val renderedTarget by lazy { DESCRIPTOR_RENDERER_FOR_COMPARISON.render(targetDescriptor) }
+
+            val targetModuleInfo = getModuleInfoByVirtualFile(project, sourceRoot)
+            val dummyFile = KtPsiFactory(targetElement.project).createFile("dummy.kt", "").apply {
+                moduleInfo = targetModuleInfo
+                targetPlatform = TargetPlatformDetector.getPlatform(targetModule)
+            }
+
+            val newTargetDescriptors = dummyFile.resolveImportReference(fqName)
+
+            return newTargetDescriptors.any {
+                if (DESCRIPTOR_RENDERER_FOR_COMPARISON.render(it) != renderedImportableTarget) return@any false
+                if (importableDescriptor == targetDescriptor) return@any true
+
+                val candidateDescriptors: Collection<DeclarationDescriptor> = when (targetDescriptor) {
+                    is ConstructorDescriptor -> {
+                        (it as? ClassDescriptor)?.constructors ?: emptyList<DeclarationDescriptor>()
+                    }
+
+                    is PropertyAccessorDescriptor -> {
+                        (it as? PropertyDescriptor)
+                                ?.let { if (targetDescriptor is PropertyGetterDescriptor) it.getter else it.setter }
+                                ?.let { listOf(it) }
+                        ?: emptyList<DeclarationDescriptor>()
+                    }
+
+                    else -> emptyList()
+                }
+
+                candidateDescriptors.any { DESCRIPTOR_RENDERER_FOR_COMPARISON.render(it) == renderedTarget }
+            }
+        }
+
        val referencesToSkip = HashSet<KtReferenceExpression>()
        for (declaration in elementsToMove - doNotGoIn) {
            declaration.forEachDescendantOfType<KtReferenceExpression> { refExpr ->
@@ -173,12 +229,13 @@ class MoveConflictChecker(
                val target = DescriptorToSourceUtilsIde.getAnyDeclaration(project, targetDescriptor) ?: return@forEachDescendantOfType

                if (target.isInsideOf(elementsToMove)) return@forEachDescendantOfType
-                if (target in resolveScope) return@forEachDescendantOfType
+
+                if (isInScope(target, targetDescriptor)) return@forEachDescendantOfType
                if (target is KtTypeParameter) return@forEachDescendantOfType

                val superMethods = SmartSet.create<PsiMethod>()
                target.toLightMethods().forEach { superMethods += it.findDeepestSuperMethods() }
-                if (superMethods.any { it in resolveScope }) return@forEachDescendantOfType
+                if (superMethods.any { isInScope(it, targetDescriptor) }) return@forEachDescendantOfType

                val refContainer = refExpr.getStrictParentOfType<KtNamedDeclaration>() ?: return@forEachDescendantOfType
                val scopeDescription = RefactoringUIUtil.getDescription(refContainer, true)
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,14 @@
+package packJvm
+
+class ClassUsageJs {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    @JsName("bar")
+    fun foo() {
+
+    }
+}
@@ -0,0 +1,17 @@
+package packJvm
+
+class ClassUsageJvm {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    companion object {
+        @JvmStatic fun foo() {
+
+        }
+    }
+}
+
+class Foo
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,3 @@
+package packJs
+
+class Foo
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,17 @@
+package packJvm
+
+class ClassUsageJvm {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    companion object {
+        @JvmStatic fun foo() {
+
+        }
+    }
+}
+
+class Foo
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,16 @@
+package packJs
+
+class <caret>ClassUsageJs {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    @JsName("bar")
+    fun foo() {
+
+    }
+}
+
+class Foo
@@ -0,0 +1 @@
+Constructor JsName(String), referenced in function packJs.ClassUsageJs.foo(), will not be accessible in module A
@@ -0,0 +1,10 @@
+{
+  "mainFile": "B/src/packJs/testJs.kt",
+  "type": "MOVE_KOTLIN_TOP_LEVEL_DECLARATIONS",
+  "targetPackage": "packJvm",
+  "targetSourceRoot": "A/src",
+  "isMultiModule": "true",
+  "withRuntime": "true",
+  "modulesWithRuntime": ["A"],
+  "modulesWithJsRuntime": ["B"]
+}
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,3 @@
+package packJvm
+
+class Foo
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,15 @@
+package packJs
+
+class ClassUsageJvm {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    companion object {
+        @JvmStatic fun foo() {
+
+        }
+    }
+}
@@ -0,0 +1,16 @@
+package packJs
+
+class ClassUsageJs {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    @JsName("bar")
+    fun foo() {
+
+    }
+}
+
+class Foo
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,17 @@
+package packJvm
+
+class <caret>ClassUsageJvm {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    companion object {
+        @JvmStatic fun foo() {
+
+        }
+    }
+}
+
+class Foo
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="inheritedJdk" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>
@@ -0,0 +1,16 @@
+package packJs
+
+class ClassUsageJs {
+    init {
+        listOf("example")
+        arrayOf("another")
+        Pair(1, "2")
+    }
+
+    @JsName("bar")
+    fun foo() {
+
+    }
+}
+
+class Foo
@@ -0,0 +1 @@
+Constructor JvmStatic(), referenced in function packJvm.ClassUsageJvm.Companion.foo(), will not be accessible in module B
@@ -0,0 +1,10 @@
+{
+  "mainFile": "A/src/packJvm/testJvm.kt",
+  "type": "MOVE_KOTLIN_TOP_LEVEL_DECLARATIONS",
+  "targetPackage": "packJs",
+  "targetSourceRoot": "B/src",
+  "isMultiModule": "true",
+  "withRuntime": "true",
+  "modulesWithRuntime": ["A"],
+  "modulesWithJsRuntime": ["B"]
+}
@@ -72,18 +72,24 @@ abstract class AbstractMoveTest : KotlinMultiFileTestCase() {
        isMultiModule = config["isMultiModule"]?.asBoolean ?: false

        doTest({ rootDir, _ ->
-            val modulesWithRuntime: List<Module>
+            val modulesWithJvmRuntime: List<Module>
+            val modulesWithJsRuntime: List<Module>

            val withRuntime = config["withRuntime"]?.asBoolean ?: false
            if (withRuntime) {
                val moduleManager = ModuleManager.getInstance(project)
-                modulesWithRuntime =
+                modulesWithJvmRuntime =
                        (config["modulesWithRuntime"]?.asJsonArray?.map { moduleManager.findModuleByName(it.asString!!)!! }
                         ?: moduleManager.modules.toList())
-                modulesWithRuntime.forEach { ConfigLibraryUtil.configureKotlinRuntimeAndSdk(it, PluginTestCaseBase.mockJdk()) }
+                modulesWithJvmRuntime.forEach { ConfigLibraryUtil.configureKotlinRuntimeAndSdk(it, PluginTestCaseBase.mockJdk()) }
+                modulesWithJsRuntime =
+                        (config["modulesWithJsRuntime"]?.asJsonArray?.map { moduleManager.findModuleByName(it.asString!!)!! }
+                         ?: emptyList())
+                modulesWithJsRuntime.forEach { ConfigLibraryUtil.configureKotlinJsRuntimeAndSdk(it, PluginTestCaseBase.mockJdk()) }
            }
            else {
-                modulesWithRuntime = emptyList()
+                modulesWithJvmRuntime = emptyList()
+                modulesWithJsRuntime = emptyList()
            }

            val mainFile = rootDir.findFileByRelativePath(mainFilePath)!!
@@ -122,9 +128,12 @@ abstract class AbstractMoveTest : KotlinMultiFileTestCase() {

                EditorFactory.getInstance()!!.releaseEditor(editor)

-                modulesWithRuntime.forEach {
+                modulesWithJvmRuntime.forEach {
                    ConfigLibraryUtil.unConfigureKotlinRuntimeAndSdk(it, PluginTestCaseBase.mockJdk())
                }
+                modulesWithJsRuntime.forEach {
+                    ConfigLibraryUtil.unConfigureKotlinJsRuntimeAndSdk(it, PluginTestCaseBase.mockJdk())
+                }
            }
        },
        getTestDirName(true))
@@ -636,6 +636,18 @@ public class MoveTestGenerated extends AbstractMoveTest {
        doTest(fileName);
    }

+    @TestMetadata("kotlin/moveTopLevelDeclarations/moveFromJsModuleToJvmModule/moveFromJsModuleToJvmModule.test")
+    public void testKotlin_moveTopLevelDeclarations_moveFromJsModuleToJvmModule_MoveFromJsModuleToJvmModule() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFromJsModuleToJvmModule/moveFromJsModuleToJvmModule.test");
+        doTest(fileName);
+    }
+
+    @TestMetadata("kotlin/moveTopLevelDeclarations/moveFromJvmModuleToJsModule/moveFromJvmModuleToJsModule.test")
+    public void testKotlin_moveTopLevelDeclarations_moveFromJvmModuleToJsModule_MoveFromJvmModuleToJsModule() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFromJvmModuleToJsModule/moveFromJvmModuleToJsModule.test");
+        doTest(fileName);
+    }
+
    @TestMetadata("kotlin/moveTopLevelDeclarations/moveFunctionToFile/moveFunctionToFile.test")
    public void testKotlin_moveTopLevelDeclarations_moveFunctionToFile_MoveFunctionToFile() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToFile/moveFunctionToFile.test");
				`@@ -0,0 +1 @@`
				`Constructor JsName(String), referenced in function packJs.ClassUsageJs.foo(), will not be accessible in module A`
				`@@ -0,0 +1 @@`
				`Constructor JvmStatic(), referenced in function packJvm.ClassUsageJvm.Companion.foo(), will not be accessible in module B`