Do not add JS/JVM libraries to common library dependencies in LibraryDependenciesCache

Currently kotlin-stdlib-common doesn't have serialized built-ins inside, so in a multiplatform project, it was possible that either JVM or JS built-ins (depending on the order of entries in the project configuration) were going to be used for the analysis of a common module depending on kotlin-stdlib-common. This resulted, for example, in additional built-in member calls (e.g. "List.stream()") being unresolved in JVM modules when they used the List instance coming from the common module (see the added test)
2017-04-14 15:27:53 +03:00
parent 4c2c734733
commit 9a8dc14616
10 changed files with 114 additions and 20 deletions
@@ -66,6 +66,11 @@ public class ForTestCompileRuntime {
        return assertExists(new File("dist/common/kotlin-stdlib-common.jar"));
    }

+    @NotNull
+    public static File stdlibJsForTests() {
+        return assertExists(new File("dist/kotlinc/lib/kotlin-stdlib-js.jar"));
+    }
+
    // TODO: Do not use these classes, remove them after stdlib tests are merged in the same build as the compiler
    @NotNull
    @Deprecated
@@ -32,15 +32,12 @@ import com.intellij.util.SmartList
 import org.jetbrains.kotlin.analyzer.ModuleInfo
 import org.jetbrains.kotlin.caches.resolve.LibraryModuleInfo
 import org.jetbrains.kotlin.descriptors.ModuleDescriptor
-import org.jetbrains.kotlin.idea.framework.CommonLibraryDetectionUtil
-import org.jetbrains.kotlin.idea.framework.KotlinJavaScriptLibraryDetectionUtil
+import org.jetbrains.kotlin.idea.project.TargetPlatformDetector
 import org.jetbrains.kotlin.idea.stubindex.KotlinSourceFilterScope
 import org.jetbrains.kotlin.idea.util.isInSourceContentWithoutInjected
-import org.jetbrains.kotlin.js.resolve.JsPlatform
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.resolve.TargetPlatform
 import org.jetbrains.kotlin.resolve.jvm.GlobalSearchScopeWithModuleSources
-import org.jetbrains.kotlin.resolve.jvm.platform.JvmPlatform
 import java.util.*

 interface IdeaModuleInfo : ModuleInfo {
@@ -205,11 +202,7 @@ class LibraryInfo(val project: Project, val library: Library) : IdeaModuleInfo,
    }

    override val libraryPlatform: TargetPlatform
-        get() = when {
-            KotlinJavaScriptLibraryDetectionUtil.isKotlinJavaScriptLibrary(library) -> JsPlatform
-            CommonLibraryDetectionUtil.isCommonLibrary(library) -> TargetPlatform.Default
-            else -> JvmPlatform
-        }
+        get() = TargetPlatformDetector.getPlatform(library)

    override val sourcesModuleInfo: SourceForBinaryModuleInfo
        get() = LibrarySourceInfo(project, library)
@@ -26,6 +26,8 @@ import com.intellij.openapi.util.Condition
 import com.intellij.psi.util.CachedValueProvider
 import com.intellij.psi.util.CachedValuesManager
 import com.intellij.util.containers.MultiMap
+import org.jetbrains.kotlin.idea.project.TargetPlatformDetector
+import org.jetbrains.kotlin.resolve.TargetPlatform
 import org.jetbrains.kotlin.utils.addIfNotNull
 import java.util.*

@@ -47,6 +49,8 @@ class LibraryDependenciesCache(private val project: Project) {
        val libraries = LinkedHashSet<Library>()
        val sdks = LinkedHashSet<Sdk>()

+        val platform = TargetPlatformDetector.getPlatform(library)
+
        for (module in getLibraryUsageIndex().modulesLibraryIsUsedIn[library]) {
            if (!processedModules.add(module)) continue

@@ -56,7 +60,10 @@ class LibraryDependenciesCache(private val project: Project) {
                }

                override fun visitLibraryOrderEntry(libraryOrderEntry: LibraryOrderEntry, value: Unit) {
-                    libraries.addIfNotNull(libraryOrderEntry.library)
+                    val otherLibrary = libraryOrderEntry.library
+                    if (otherLibrary != null && compatiblePlatforms(platform, TargetPlatformDetector.getPlatform(otherLibrary))) {
+                        libraries.add(otherLibrary)
+                    }
                }

                override fun visitJdkOrderEntry(jdkOrderEntry: JdkOrderEntry, value: Unit) {
@@ -68,6 +75,13 @@ class LibraryDependenciesCache(private val project: Project) {
        return Pair(libraries.toList(), sdks.toList())
    }

+    /**
+     * @return true if it's OK to add a dependency from a library with platform [from] to a library with platform [to]
+     */
+    private fun compatiblePlatforms(from: TargetPlatform, to: TargetPlatform): Boolean {
+        return from == to || to == TargetPlatform.Default
+    }
+
    private fun getLibraryUsageIndex(): LibraryUsageIndex {
        return CachedValuesManager.getManager(project).getCachedValue(project) {
            CachedValueProvider.Result(LibraryUsageIndex(), ProjectRootModificationTracker.getInstance(project))
@@ -25,6 +25,7 @@ import org.jetbrains.kotlin.idea.caches.JarUserDataManager
 import org.jetbrains.kotlin.serialization.deserialization.MetadataPackageFragment

 object CommonLibraryDetectionUtil {
+    @JvmStatic
    fun isCommonLibrary(library: Library): Boolean {
        if (library is LibraryEx && library.isDisposed) return false

@@ -19,10 +19,14 @@ package org.jetbrains.kotlin.idea.project;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.module.Module;
 import com.intellij.openapi.roots.ProjectFileIndex;
+import com.intellij.openapi.roots.libraries.Library;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiFile;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.kotlin.idea.framework.CommonLibraryDetectionUtil;
+import org.jetbrains.kotlin.idea.framework.KotlinJavaScriptLibraryDetectionUtil;
+import org.jetbrains.kotlin.js.resolve.JsPlatform;
 import org.jetbrains.kotlin.psi.KtCodeFragment;
 import org.jetbrains.kotlin.psi.KtFile;
 import org.jetbrains.kotlin.psi.KtPsiFactoryKt;
@@ -70,4 +74,15 @@ public class TargetPlatformDetector {
    public static TargetPlatform getPlatform(@NotNull Module module) {
        return ProjectStructureUtil.getCachedPlatformForModule(module);
    }
+
+    @NotNull
+    public static TargetPlatform getPlatform(@NotNull Library library) {
+        if (KotlinJavaScriptLibraryDetectionUtil.isKotlinJavaScriptLibrary(library)) {
+            return JsPlatform.INSTANCE;
+        }
+        if (CommonLibraryDetectionUtil.isCommonLibrary(library)) {
+            return TargetPlatform.Default.INSTANCE;
+        }
+        return JvmPlatform.INSTANCE;
+    }
 }
@@ -0,0 +1,3 @@
+package test
+
+fun getEmptyList() = emptyList<String>()
@@ -0,0 +1,6 @@
+package test
+
+fun testJs() {
+    // This is a JavaScript module, so `List.stream` should be unresolved
+    getEmptyList().<error>stream</error>()
+}
@@ -0,0 +1,6 @@
+package test
+
+fun testJvm() {
+    // This is a JVM 8 module, so `List.stream` should be resolved correctly
+    getEmptyList().stream()
+}
@@ -16,16 +16,20 @@

 package org.jetbrains.kotlin.idea.caches.resolve

-import com.intellij.testFramework.ModuleTestCase
-import com.intellij.openapi.module.StdModuleTypes
+import com.intellij.openapi.command.WriteCommandAction
 import com.intellij.openapi.module.Module
-import com.intellij.openapi.roots.ModuleRootModificationUtil
+import com.intellij.openapi.module.StdModuleTypes
 import com.intellij.openapi.roots.DependencyScope
-import org.junit.Assert
+import com.intellij.openapi.roots.ModuleRootModificationUtil
+import com.intellij.openapi.roots.OrderRootType
 import com.intellij.openapi.roots.impl.libraries.ProjectLibraryTable
 import com.intellij.openapi.roots.libraries.Library
-import com.intellij.openapi.command.WriteCommandAction
+import com.intellij.openapi.vfs.VfsUtil
+import com.intellij.testFramework.ModuleTestCase
 import com.intellij.testFramework.UsefulTestCase
+import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime
+import org.junit.Assert
+import java.io.File

 class IdeaModuleInfoTest : ModuleTestCase() {

@@ -267,6 +271,24 @@ class IdeaModuleInfoTest : ModuleTestCase() {
        lib3.classes.assertAdditionalLibraryDependencies(lib1.classes, lib2.classes)
    }

+    fun testCommonLibraryDoesNotDependOnPlatform() {
+        val stdlibCommon = stdlibCommon()
+        val stdlibJvm = stdlibJvm()
+        val stdlibJs = stdlibJs()
+
+        val a = module("a")
+        a.addDependency(stdlibCommon)
+        a.addDependency(stdlibJvm)
+
+        val b = module("b")
+        b.addDependency(stdlibCommon)
+        b.addDependency(stdlibJs)
+
+        stdlibCommon.classes.assertAdditionalLibraryDependencies()
+        stdlibJvm.classes.assertAdditionalLibraryDependencies(stdlibCommon.classes)
+        stdlibJs.classes.assertAdditionalLibraryDependencies(stdlibCommon.classes)
+    }
+
    private fun Module.addDependency(
            other: Module,
            dependencyScope: DependencyScope = DependencyScope.COMPILE,
@@ -308,10 +330,22 @@ class IdeaModuleInfoTest : ModuleTestCase() {
        UsefulTestCase.assertSameElements(this.getDependentModules(), expected.toList())
    }

-    private fun projectLibrary(name: String = "lib"): Library {
+    private fun projectLibrary(name: String = "lib", vararg roots: File): Library {
        val libraryTable = ProjectLibraryTable.getInstance(myProject)!!
        return WriteCommandAction.runWriteCommandAction<Library>(myProject) {
-            libraryTable.createLibrary(name)
+            libraryTable.createLibrary(name).apply {
+                for (root in roots) {
+                    val model = modifiableModel
+                    model.addRoot(VfsUtil.getUrlForLibraryRoot(root), OrderRootType.CLASSES)
+                    model.commit()
+                }
+            }
        }!!
    }
+
+    private fun stdlibCommon(): Library = projectLibrary("kotlin-stdlib-common", ForTestCompileRuntime.stdlibCommonForTests())
+
+    private fun stdlibJvm(): Library = projectLibrary("kotlin-stdlib", ForTestCompileRuntime.runtimeJarForTests())
+
+    private fun stdlibJs(): Library = projectLibrary("kotlin-stdlib-js", ForTestCompileRuntime.runtimeJarForTests())
 }
@@ -16,6 +16,7 @@

 package org.jetbrains.kotlin.idea.caches.resolve

+import com.intellij.openapi.module.Module
 import com.intellij.openapi.roots.DependencyScope
 import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime
 import org.jetbrains.kotlin.config.JvmTarget
@@ -65,8 +66,13 @@ class MultiModuleHighlightingTest : AbstractMultiModuleHighlightingTest() {
    class MultiPlatform : AbstractMultiModuleHighlightingTest() {
        override val testPath get() = super.testPath + "multiplatform/"

-        private fun doTest(vararg platforms: TargetPlatformKind<*>, withStdlibCommon: Boolean = false) {
-            val commonModule = module("common")
+        private fun doTest(
+                vararg platforms: TargetPlatformKind<*>,
+                withStdlibCommon: Boolean = false,
+                configureModule: (Module, TargetPlatformKind<*>) -> Unit = { _, _ -> },
+                useFullJdk: Boolean = false
+        ) {
+            val commonModule = module("common", useFullJdk = useFullJdk)
            commonModule.createFacet(TargetPlatformKind.Common)
            if (withStdlibCommon) {
                commonModule.addLibrary(ForTestCompileRuntime.stdlibCommonForTests())
@@ -78,10 +84,11 @@ class MultiModuleHighlightingTest : AbstractMultiModuleHighlightingTest() {
                    is TargetPlatformKind.JavaScript -> "js"
                    else -> error("Unsupported platform: $platform")
                }
-                val platformModule = module(path)
+                val platformModule = module(path, useFullJdk = useFullJdk)
                platformModule.createFacet(platform)
                platformModule.enableMultiPlatform()
                platformModule.addDependency(commonModule)
+                configureModule(platformModule, platform)
            }

            checkHighlightingInAllFiles()
@@ -114,5 +121,15 @@ class MultiModuleHighlightingTest : AbstractMultiModuleHighlightingTest() {
        fun testCatchHeaderExceptionInPlatformModule() {
            doTest(TargetPlatformKind.Jvm[JvmTarget.JVM_1_6], withStdlibCommon = true)
        }
+
+        fun testUseCorrectBuiltInsForCommonModule() {
+            doTest(TargetPlatformKind.Jvm[JvmTarget.JVM_1_8], TargetPlatformKind.JavaScript,
+                   withStdlibCommon = true, useFullJdk = true, configureModule = { module, platform ->
+                if (platform == TargetPlatformKind.JavaScript) {
+                    module.addLibrary(ForTestCompileRuntime.stdlibJsForTests())
+                    module.addLibrary(ForTestCompileRuntime.stdlibCommonForTests())
+                }
+            })
+        }
    }
 }