From 75abae23d8afd0942cf6f6f5cdb197efb5a92b42 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Tue, 7 Nov 2023 11:37:06 +0900 Subject: [PATCH] [Analysis API] Fix WITH_STDLIB for common modules in Analysis API tests The previous implementation in 'TestModuleStructureFactory' had very custom and redundant logic for binary dependency collection. In particular, JDK and kotlin-stdlib were specifically computed, although both already were in the module classpath, which was handled in 'createLibrariesByCompilerConfigurators()'. In addition, there was no support for common modules. The new behavior is much closer to what happens in production. --- .../AnalysisApiFe10TestConfigurator.kt | 8 + .../structure/TestModuleStructureFactory.kt | 217 ++++++------------ ...ysisApiFirLibraryBinaryTestConfigurator.kt | 8 +- ...nalysisApiFirSourceLikeTestConfigurator.kt | 10 +- .../JsEnvironmentConfigurator.kt | 3 + 5 files changed, 99 insertions(+), 147 deletions(-) diff --git a/analysis/analysis-api-fe10/tests/org/jetbrains/kotlin/analysis/api/fe10/test/configurator/AnalysisApiFe10TestConfigurator.kt b/analysis/analysis-api-fe10/tests/org/jetbrains/kotlin/analysis/api/fe10/test/configurator/AnalysisApiFe10TestConfigurator.kt index 2e8b595a192..64d6503277a 100644 --- a/analysis/analysis-api-fe10/tests/org/jetbrains/kotlin/analysis/api/fe10/test/configurator/AnalysisApiFe10TestConfigurator.kt +++ b/analysis/analysis-api-fe10/tests/org/jetbrains/kotlin/analysis/api/fe10/test/configurator/AnalysisApiFe10TestConfigurator.kt @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.analysis.api.standalone.base.project.structure.KtMod import org.jetbrains.kotlin.analysis.test.framework.project.structure.KtModuleFactory import org.jetbrains.kotlin.analysis.test.framework.project.structure.KtSourceModuleFactory import org.jetbrains.kotlin.analysis.test.framework.project.structure.TestModuleStructureFactory +import org.jetbrains.kotlin.analysis.test.framework.services.configuration.AnalysisApiJvmEnvironmentConfigurator import org.jetbrains.kotlin.analysis.test.framework.test.configurators.AnalysisApiTestConfigurator import org.jetbrains.kotlin.analysis.test.framework.test.configurators.AnalysisApiTestServiceRegistrar import org.jetbrains.kotlin.analysis.test.framework.test.configurators.FrontendKind @@ -24,6 +25,8 @@ import org.jetbrains.kotlin.test.model.TestModule import org.jetbrains.kotlin.test.services.TestModuleStructure import org.jetbrains.kotlin.test.services.TestServices import org.jetbrains.kotlin.test.services.compilerConfigurationProvider +import org.jetbrains.kotlin.test.services.configuration.CommonEnvironmentConfigurator +import org.jetbrains.kotlin.test.services.configuration.JsEnvironmentConfigurator import java.nio.file.Path import kotlin.io.path.extension import kotlin.io.path.nameWithoutExtension @@ -39,6 +42,11 @@ object AnalysisApiFe10TestConfigurator : AnalysisApiTestConfigurator() { override fun configureTest(builder: TestConfigurationBuilder, disposable: Disposable) { builder.apply { useAdditionalService { KtSourceModuleFactory } + useConfigurators( + ::CommonEnvironmentConfigurator, + ::AnalysisApiJvmEnvironmentConfigurator, + ::JsEnvironmentConfigurator + ) } } diff --git a/analysis/analysis-test-framework/tests/org/jetbrains/kotlin/analysis/test/framework/project/structure/TestModuleStructureFactory.kt b/analysis/analysis-test-framework/tests/org/jetbrains/kotlin/analysis/test/framework/project/structure/TestModuleStructureFactory.kt index 6b7653ac57e..82f67de043e 100644 --- a/analysis/analysis-test-framework/tests/org/jetbrains/kotlin/analysis/test/framework/project/structure/TestModuleStructureFactory.kt +++ b/analysis/analysis-test-framework/tests/org/jetbrains/kotlin/analysis/test/framework/project/structure/TestModuleStructureFactory.kt @@ -8,31 +8,28 @@ package org.jetbrains.kotlin.analysis.test.framework.project.structure import com.intellij.openapi.project.Project import com.intellij.psi.PsiFile import com.intellij.psi.PsiManager +import com.intellij.psi.search.GlobalSearchScope import org.jetbrains.kotlin.analysis.api.standalone.base.project.structure.KtModuleProjectStructure import org.jetbrains.kotlin.analysis.api.standalone.base.project.structure.KtModuleWithFiles import org.jetbrains.kotlin.analysis.api.standalone.base.project.structure.StandaloneProjectFactory import org.jetbrains.kotlin.analysis.project.structure.KtBinaryModule -import org.jetbrains.kotlin.analysis.project.structure.KtLibraryModule -import org.jetbrains.kotlin.analysis.project.structure.KtModule import org.jetbrains.kotlin.analysis.project.structure.KtNotUnderContentRootModule import org.jetbrains.kotlin.analysis.test.framework.services.environmentManager -import org.jetbrains.kotlin.analysis.utils.errors.requireIsInstance import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys import org.jetbrains.kotlin.cli.jvm.config.JvmClasspathRoot -import org.jetbrains.kotlin.platform.isCommon -import org.jetbrains.kotlin.platform.isJs +import org.jetbrains.kotlin.js.config.JSConfigurationKeys +import org.jetbrains.kotlin.library.KLIB_FILE_EXTENSION +import org.jetbrains.kotlin.platform.TargetPlatform +import org.jetbrains.kotlin.platform.js.JsPlatforms import org.jetbrains.kotlin.platform.jvm.JvmPlatforms import org.jetbrains.kotlin.platform.jvm.isJvm -import org.jetbrains.kotlin.test.TestInfrastructureInternals import org.jetbrains.kotlin.test.model.DependencyRelation import org.jetbrains.kotlin.test.model.TestModule import org.jetbrains.kotlin.test.services.* import org.jetbrains.kotlin.test.services.configuration.JvmEnvironmentConfigurator import org.jetbrains.kotlin.test.util.KtTestUtil -import org.jetbrains.kotlin.utils.PathUtil -import org.jetbrains.kotlin.utils.addIfNotNull import java.nio.file.Path -import kotlin.io.path.absolute +import java.nio.file.Paths import kotlin.io.path.exists import kotlin.io.path.extension import kotlin.io.path.nameWithoutExtension @@ -49,7 +46,7 @@ object TestModuleStructureFactory { val moduleEntriesByName = moduleEntries.associateByName() - val binaryModulesBySourceRoots = mutableMapOf, KtBinaryModule>() + val libraryCache = mutableMapOf, KtBinaryModule>() for (testModule in moduleStructure.modules) { val moduleWithFiles = moduleEntriesByName[testModule.name] ?: moduleEntriesByName.getValue(testModule.files.single().name) @@ -58,61 +55,21 @@ object TestModuleStructureFactory { // Not-under-content-root modules have no external dependencies on purpose } is KtModuleWithModifiableDependencies -> { - if (ktModule is KtBinaryModule) { - binaryModulesBySourceRoots.getOrPut(ktModule.getBinaryRoots().toSet()) { ktModule } - } - addModuleDependencies(testModule, moduleEntriesByName, ktModule) - stdlibAndSdkDependencies(ktModule, testModule, project, testServices).forEach { library -> - val cachedLibrary = binaryModulesBySourceRoots.getOrPut(library.getBinaryRoots().toSet()) { library } - ktModule.directRegularDependencies.add(cachedLibrary) - } + addLibraryDependencies(testModule, testServices, project, ktModule, libraryCache::getOrPut) } else -> error("Unexpected module type: " + ktModule.javaClass.name) } } - return KtModuleProjectStructure(moduleEntries, binaryModulesBySourceRoots.values) + return KtModuleProjectStructure(moduleEntries, libraryCache.values) } - private fun stdlibAndSdkDependencies( - module: KtModule, + private fun addModuleDependencies( testModule: TestModule, - project: Project, - testServices: TestServices, - ): List { - return when { - module.platform.isJvm() -> jvmStdlibAndJdkDependencies(testModule, project, testServices) - module.platform.isJs() -> jsStdlibDependencies(project, testServices) - module.platform.isCommon() -> jvmStdlibAndJdkDependencies(testModule, project, testServices) // TODO: KT-63769 - else -> error("Unsupported platform ${module.platform} in this test") - } - } - - private fun jvmStdlibAndJdkDependencies(testModule: TestModule, project: Project, testServices: TestServices): List { - return buildList { - addIfNotNull(getJdkModule(testModule, project, testServices)) - addAll(getStdlibModules(testModule, project, testServices)) - addAll(getLibraryModules(testServices, testModule, project)) - addAll(createLibrariesByCompilerConfigurators(testModule, testServices, project)) - } - } - - @OptIn(TestInfrastructureInternals::class) - private fun createLibrariesByCompilerConfigurators( - testModule: TestModule, - testServices: TestServices, - project: Project - ): List { - val compilerConfiguration = createCompilerConfiguration(testModule, testServices.environmentConfigurators) - val contentRoots = compilerConfiguration[CLIConfigurationKeys.CONTENT_ROOTS, emptyList()] - return contentRoots - .filterIsInstance() - .map { root -> createKtLibraryModuleByJar(root.file.toPath(), testServices, project) } - } - - private fun addModuleDependencies(testModule: TestModule, moduleByName: Map, ktModule: KtModule) { - requireIsInstance(ktModule) + moduleByName: Map, + ktModule: KtModuleWithModifiableDependencies + ) { testModule.allDependencies.forEach { dependency -> val dependencyKtModule = moduleByName.getValue(dependency.moduleName).ktModule when (dependency.relation) { @@ -123,104 +80,76 @@ object TestModuleStructureFactory { } } - private fun getLibraryModules( - testServices: TestServices, + private fun addLibraryDependencies( testModule: TestModule, - project: Project - ): List { - val configurationKind = JvmEnvironmentConfigurator.extractConfigurationKind(testModule.directives) - return JvmEnvironmentConfigurator - .getLibraryFilesExceptRealRuntime(testServices, configurationKind, testModule.directives) - .map { it.toPath().toAbsolutePath() } - .map { jar -> - createKtLibraryModuleByJar( - jar, - testServices, - project, - ) + testServices: TestServices, + project: Project, + ktModule: KtModuleWithModifiableDependencies, + libraryCache: (paths: Set, factory: () -> KtBinaryModule) -> KtBinaryModule + ) { + val compilerConfiguration = testServices.compilerConfigurationProvider.getCompilerConfiguration(testModule) + + val classpathRoots = compilerConfiguration[CLIConfigurationKeys.CONTENT_ROOTS, emptyList()] + .mapNotNull { (it as? JvmClasspathRoot)?.file?.toPath() } + + if (classpathRoots.isNotEmpty()) { + val jdkKind = JvmEnvironmentConfigurator.extractJdkKind(testModule.directives) + val jdkHome = JvmEnvironmentConfigurator.getJdkHome(jdkKind)?.toPath() + ?: JvmEnvironmentConfigurator.getJdkClasspathRoot(jdkKind)?.toPath() + ?: Paths.get(System.getProperty("java.home")) + + val (jdkRoots, libraryRoots) = classpathRoots.partition { jdkHome != null && it.startsWith(jdkHome) } + + if (testModule.targetPlatform.isJvm() && jdkRoots.isNotEmpty()) { + val jdkModule = libraryCache(jdkRoots.toSet()) { + val jdkScope = getScopeForLibraryByRoots(jdkRoots, testServices) + KtJdkModuleImpl("jdk", JvmPlatforms.defaultJvmPlatform, jdkScope, project, jdkRoots) + } + ktModule.directRegularDependencies.add(jdkModule) } - } - private fun createKtLibraryModuleByJar( - jar: Path, - testServices: TestServices, - project: Project, - libraryName: String = jar.nameWithoutExtension, - ): KtLibraryModuleImpl { - check(jar.extension == "jar") - check(jar.exists()) { - "library $jar does not exist" + for (libraryRoot in libraryRoots) { + check(libraryRoot.extension == "jar") + + val libraryModule = libraryCache(setOf(libraryRoot)) { + createLibraryModule(project, libraryRoot, JvmPlatforms.defaultJvmPlatform, testServices) + } + + ktModule.directRegularDependencies.add(libraryModule) + } + } + + val jsLibraryRootPaths = compilerConfiguration[JSConfigurationKeys.LIBRARIES].orEmpty() + + for (libraryRootPath in jsLibraryRootPaths) { + val libraryRoot = Paths.get(libraryRootPath) + check(libraryRoot.extension == KLIB_FILE_EXTENSION) + + val libraryModule = libraryCache(setOf(libraryRoot)) { + createLibraryModule(project, libraryRoot, JsPlatforms.defaultJsPlatform, testServices) + } + + ktModule.directRegularDependencies.add(libraryModule) } - return KtLibraryModuleImpl( - libraryName, - JvmPlatforms.defaultJvmPlatform, - StandaloneProjectFactory.createSearchScopeByLibraryRoots( - listOf(jar), - testServices.environmentManager.getProjectEnvironment() - ), - project, - listOf(jar), - librarySources = null, - ) } - private fun getStdlibModules( - testModule: TestModule, + private fun createLibraryModule( project: Project, + libraryFile: Path, + platform: TargetPlatform, testServices: TestServices, - ): List { - val configurationKind = JvmEnvironmentConfigurator.extractConfigurationKind(testModule.directives) - if (!configurationKind.withRuntime) return emptyList() - val lib = testServices.standardLibrariesPathProvider.runtimeJarForTests().toPath().absolute() - return listOf( - createKtLibraryModuleByJar(lib, testServices, project, PathUtil.KOTLIN_JAVA_STDLIB_NAME), - ) + ): KtLibraryModuleImpl { + check(libraryFile.exists()) { "Library $libraryFile does not exist" } + + val libraryName = libraryFile.nameWithoutExtension + val libraryScope = getScopeForLibraryByRoots(listOf(libraryFile), testServices) + return KtLibraryModuleImpl(libraryName, platform, libraryScope, project, listOf(libraryFile), librarySources = null) } - private fun getJdkModule( - testModule: TestModule, - project: Project, - testServices: TestServices, - ): KtJdkModuleImpl? { - val jdkKind = JvmEnvironmentConfigurator.extractJdkKind(testModule.directives) - - val jdkSourceRoots = buildList { - JvmEnvironmentConfigurator.getJdkHome(jdkKind)?.let { add(it.toPath()) } - JvmEnvironmentConfigurator.getJdkClasspathRoot(jdkKind)?.let { add(it.toPath()) } - }.mapTo(mutableListOf()) { it.toAbsolutePath() } - - if (jdkSourceRoots.isEmpty()) return null - - return KtJdkModuleImpl( - "jdk", - JvmPlatforms.defaultJvmPlatform, - StandaloneProjectFactory.createSearchScopeByLibraryRoots( - jdkSourceRoots, - testServices.environmentManager.getProjectEnvironment() - ), - project, - jdkSourceRoots - ) - } - - private fun jsStdlibDependencies(project: Project, testServices: TestServices): List { - return listOf( - jsStdlib(project, testServices), - ) - } - - private fun jsStdlib(project: Project, testServices: TestServices): KtBinaryModule { - val jar = testServices.standardLibrariesPathProvider.fullJsStdlib().toPath() - return KtLibraryModuleImpl( - PathUtil.JS_LIB_NAME, - JvmPlatforms.defaultJvmPlatform, - StandaloneProjectFactory.createSearchScopeByLibraryRoots( - listOf(jar), - testServices.environmentManager.getProjectEnvironment() - ), - project, - listOf(jar), - librarySources = null, + fun getScopeForLibraryByRoots(roots: Collection, testServices: TestServices): GlobalSearchScope { + return StandaloneProjectFactory.createSearchScopeByLibraryRoots( + roots, + testServices.environmentManager.getProjectEnvironment() ) } diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirLibraryBinaryTestConfigurator.kt b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirLibraryBinaryTestConfigurator.kt index e55dc0fd206..95a097abf7d 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirLibraryBinaryTestConfigurator.kt +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirLibraryBinaryTestConfigurator.kt @@ -23,6 +23,8 @@ import org.jetbrains.kotlin.analysis.test.framework.test.configurators.FrontendK import org.jetbrains.kotlin.test.builders.TestConfigurationBuilder import org.jetbrains.kotlin.test.services.TestModuleStructure import org.jetbrains.kotlin.test.services.TestServices +import org.jetbrains.kotlin.test.services.configuration.CommonEnvironmentConfigurator +import org.jetbrains.kotlin.test.services.configuration.JsEnvironmentConfigurator object AnalysisApiFirLibraryBinaryTestConfigurator : AnalysisApiTestConfigurator() { override val analyseInDependentSession: Boolean get() = false @@ -39,7 +41,11 @@ object AnalysisApiFirLibraryBinaryTestConfigurator : AnalysisApiTestConfigurator builder.apply { useAdditionalService { DispatchingTestModuleCompiler() } useAdditionalService { TestModuleDecompilerJar() } - useConfigurators(::AnalysisApiJvmEnvironmentConfigurator) + useConfigurators( + ::CommonEnvironmentConfigurator, + ::AnalysisApiJvmEnvironmentConfigurator, + ::JsEnvironmentConfigurator + ) } } diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirSourceLikeTestConfigurator.kt b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirSourceLikeTestConfigurator.kt index 8881310842e..91f233ac936 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirSourceLikeTestConfigurator.kt +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/test/configurators/AnalysisApiFirSourceLikeTestConfigurator.kt @@ -23,6 +23,8 @@ import org.jetbrains.kotlin.test.preprocessors.ExternalAnnotationsSourcePreproce import org.jetbrains.kotlin.test.services.TestModuleStructure import org.jetbrains.kotlin.test.services.TestServices import org.jetbrains.kotlin.test.services.configuration.ExternalAnnotationsEnvironmentConfigurator +import org.jetbrains.kotlin.test.services.configuration.CommonEnvironmentConfigurator +import org.jetbrains.kotlin.test.services.configuration.JsEnvironmentConfigurator abstract class AnalysisApiFirSourceLikeTestConfigurator(override val analyseInDependentSession: Boolean) : AnalysisApiTestConfigurator() { override val frontendKind: FrontendKind get() = FrontendKind.Fir @@ -32,8 +34,12 @@ abstract class AnalysisApiFirSourceLikeTestConfigurator(override val analyseInDe useDirectives(SealedClassesInheritorsCaclulatorPreAnalysisHandler.Directives) usePreAnalysisHandlers(::SealedClassesInheritorsCaclulatorPreAnalysisHandler) configureOptionalTestCompilerPlugin() - useConfigurators(::AnalysisApiJvmEnvironmentConfigurator) - useConfigurators(::ExternalAnnotationsEnvironmentConfigurator) + useConfigurators( + ::CommonEnvironmentConfigurator, + ::AnalysisApiJvmEnvironmentConfigurator, + ::JsEnvironmentConfigurator, + ::ExternalAnnotationsEnvironmentConfigurator + ) useSourcePreprocessor(::ExternalAnnotationsSourcePreprocessor) } } diff --git a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/services/configuration/JsEnvironmentConfigurator.kt b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/services/configuration/JsEnvironmentConfigurator.kt index 9f881f5c767..6be3be199a6 100644 --- a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/services/configuration/JsEnvironmentConfigurator.kt +++ b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/services/configuration/JsEnvironmentConfigurator.kt @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.ir.backend.js.transformers.irToJs.TranslationMode import org.jetbrains.kotlin.js.config.* import org.jetbrains.kotlin.js.facade.MainCallParameters import org.jetbrains.kotlin.library.KotlinLibrary +import org.jetbrains.kotlin.platform.js.JsPlatforms import org.jetbrains.kotlin.resolve.CompilerEnvironment import org.jetbrains.kotlin.resolve.TargetEnvironment import org.jetbrains.kotlin.serialization.js.JsModuleDescriptor @@ -236,6 +237,8 @@ class JsEnvironmentConfigurator(testServices: TestServices) : EnvironmentConfigu } override fun configureCompilerConfiguration(configuration: CompilerConfiguration, module: TestModule) { + if (module.targetPlatform !in JsPlatforms.allJsPlatforms) return + val registeredDirectives = module.directives val moduleKinds = registeredDirectives[MODULE_KIND] val moduleKind = when (moduleKinds.size) {