From 47afd37596ab49bd474d26c45d3348c79ec7aeff Mon Sep 17 00:00:00 2001 From: Marco Pennekamp Date: Tue, 20 Feb 2024 22:59:54 +0100 Subject: [PATCH] [AA] IDE mode tests: Build and index stubs for binary libraries - Instead of indexing binary library declarations from decompiled PSI, the static declaration provider now builds and indexes stubs. As noted in KT-65960, this brings IDE mode tests much more in line with decompiled stubs indexing in the IDE. It should allow us to catch issues with the stub-based deserialized symbol provider outside of IDE tests. - In the Standalone mode, we can skip stub-indexing completely, as we provide FIR symbols via class-based deserialization. This also extends to shared binary roots. ^KT-65960 fixed --- .../AnalysisApiBaseTestServiceRegistrar.kt | 40 ++++- .../impl/KotlinStaticDeclarationProvider.kt | 169 +++++++++++------- 2 files changed, 131 insertions(+), 78 deletions(-) diff --git a/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/configurators/AnalysisApiBaseTestServiceRegistrar.kt b/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/configurators/AnalysisApiBaseTestServiceRegistrar.kt index a13ca00151c..03e96a6f8c7 100644 --- a/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/configurators/AnalysisApiBaseTestServiceRegistrar.kt +++ b/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/configurators/AnalysisApiBaseTestServiceRegistrar.kt @@ -23,8 +23,11 @@ import org.jetbrains.kotlin.analysis.project.structure.KtBinaryModule import org.jetbrains.kotlin.analysis.providers.* import org.jetbrains.kotlin.analysis.providers.impl.* import org.jetbrains.kotlin.analysis.test.framework.project.structure.ktModuleProvider +import org.jetbrains.kotlin.analysis.test.framework.services.configuration.AnalysisApiBinaryLibraryIndexingMode +import org.jetbrains.kotlin.analysis.test.framework.services.configuration.libraryIndexingConfiguration import org.jetbrains.kotlin.analysis.test.framework.services.environmentManager import org.jetbrains.kotlin.analysis.test.framework.test.configurators.AnalysisApiTestServiceRegistrar +import org.jetbrains.kotlin.analysis.test.framework.test.configurators.TestModuleKind import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.KtFileClassProvider @@ -66,32 +69,51 @@ object AnalysisApiBaseTestServiceRegistrar : AnalysisApiTestServiceRegistrar() { override fun registerProjectModelServices(project: MockProject, testServices: TestServices) { val moduleStructure = testServices.ktModuleProvider.getModuleStructure() - val allSourceKtFiles = moduleStructure.mainModules.flatMap { it.files.filterIsInstance() } - val binaryDependencies = moduleStructure.binaryModules.toMutableSet() + val testKtFiles = moduleStructure.mainModules.flatMap { it.files.filterIsInstance() } + + // We explicitly exclude decompiled libraries. Their decompiled PSI files are indexed by the declaration provider, so it shouldn't + // additionally build and index stubs for the library. + val mainBinaryModules = moduleStructure.mainModules + .filter { it.moduleKind == TestModuleKind.LibraryBinary } + .map { it.ktModule as KtBinaryModule } + + val sharedBinaryDependencies = moduleStructure.binaryModules.toMutableSet() for (mainModule in moduleStructure.mainModules) { val ktModule = mainModule.ktModule if (ktModule !is KtBinaryModule) continue - binaryDependencies -= ktModule + sharedBinaryDependencies -= ktModule } - val roots = StandaloneProjectFactory.getVirtualFilesForLibraryRoots( - binaryDependencies.flatMap { binary -> binary.getBinaryRoots() }, + val mainBinaryRoots = StandaloneProjectFactory.getVirtualFilesForLibraryRoots( + mainBinaryModules.flatMap { it.getBinaryRoots() }, + testServices.environmentManager.getProjectEnvironment(), + ).distinct() + + val sharedBinaryRoots = StandaloneProjectFactory.getVirtualFilesForLibraryRoots( + sharedBinaryDependencies.flatMap { binary -> binary.getBinaryRoots() }, testServices.environmentManager.getProjectEnvironment() ).distinct() project.apply { - registerService(KotlinAnnotationsResolverFactory::class.java, KotlinStaticAnnotationsResolverFactory(project, allSourceKtFiles)) + registerService(KotlinAnnotationsResolverFactory::class.java, KotlinStaticAnnotationsResolverFactory(project, testKtFiles)) val filter = BuiltInDefinitionFile.FILTER_OUT_CLASSES_EXISTING_AS_JVM_CLASS_FILES val ktFilesForBinaries: List try { BuiltInDefinitionFile.FILTER_OUT_CLASSES_EXISTING_AS_JVM_CLASS_FILES = false + + val shouldBuildStubsForBinaryLibraries = + testServices.libraryIndexingConfiguration.binaryLibraryIndexingMode == AnalysisApiBinaryLibraryIndexingMode.INDEX_STUBS + val declarationProviderFactory = KotlinStaticDeclarationProviderFactory( project, - allSourceKtFiles, - additionalRoots = roots, + testKtFiles, + binaryRoots = mainBinaryRoots, + sharedBinaryRoots = sharedBinaryRoots, skipBuiltins = testServices.moduleStructure.allDirectives.contains(NO_RUNTIME), + shouldBuildStubsForBinaryLibraries = shouldBuildStubsForBinaryLibraries, ) + ktFilesForBinaries = declarationProviderFactory.getAdditionalCreatedKtFiles() registerService( KotlinDeclarationProviderFactory::class.java, declarationProviderFactory @@ -102,7 +124,7 @@ object AnalysisApiBaseTestServiceRegistrar : AnalysisApiTestServiceRegistrar() { registerService(KotlinDeclarationProviderMerger::class.java, KotlinStaticDeclarationProviderMerger(project)) registerService( KotlinPackageProviderFactory::class.java, - KotlinStaticPackageProviderFactory(project, allSourceKtFiles + ktFilesForBinaries) + KotlinStaticPackageProviderFactory(project, testKtFiles + ktFilesForBinaries) ) registerService(KotlinPackageProviderMerger::class.java, KotlinStaticPackageProviderMerger(project)) registerService(KotlinResolutionScopeProvider::class.java, KotlinByModulesResolutionScopeProvider::class.java) diff --git a/analysis/analysis-api-providers/src/org/jetbrains/kotlin/analysis/providers/impl/KotlinStaticDeclarationProvider.kt b/analysis/analysis-api-providers/src/org/jetbrains/kotlin/analysis/providers/impl/KotlinStaticDeclarationProvider.kt index c93f9545937..21c7911ecc9 100644 --- a/analysis/analysis-api-providers/src/org/jetbrains/kotlin/analysis/providers/impl/KotlinStaticDeclarationProvider.kt +++ b/analysis/analysis-api-providers/src/org/jetbrains/kotlin/analysis/providers/impl/KotlinStaticDeclarationProvider.kt @@ -11,7 +11,6 @@ import com.intellij.openapi.project.Project import com.intellij.openapi.vfs.VfsUtilCore import com.intellij.openapi.vfs.VirtualFile import com.intellij.openapi.vfs.VirtualFileVisitor -import com.intellij.openapi.vfs.impl.jar.CoreJarFileSystem import com.intellij.psi.PsiElement import com.intellij.psi.PsiManager import com.intellij.psi.SingleRootFileViewProvider @@ -138,11 +137,22 @@ public class KotlinStaticDeclarationProvider internal constructor( } } +/** + * [binaryRoots] and [sharedBinaryRoots] are used to build stubs for symbols from binary libraries. They only need to be specified if + * [shouldBuildStubsForBinaryLibraries] is true. In Standalone mode, binary roots don't need to be specified because library symbols are + * provided via class-based deserialization, not stub-based deserialization. + * + * @param binaryRoots Binary roots of the binary libraries that are specific to [project]. + * @param sharedBinaryRoots Binary roots that are shared between multiple different projects. This allows Kotlin tests to cache stubs for + * shared libraries like the Kotlin stdlib. + */ public class KotlinStaticDeclarationProviderFactory( private val project: Project, - files: Collection, - additionalRoots: List = emptyList(), + testKtFiles: Collection, + binaryRoots: List = emptyList(), + sharedBinaryRoots: List = emptyList(), skipBuiltins: Boolean = false, + shouldBuildStubsForBinaryLibraries: Boolean = false, ) : KotlinDeclarationProviderFactory() { private val index = KotlinStaticDeclarationIndex() @@ -260,46 +270,6 @@ public class KotlinStaticDeclarationProviderFactory( init { val recorder = KtDeclarationRecorder() - fun indexStub(stub: StubElement<*>) { - when (stub) { - is KotlinClassStubImpl -> { - addToClassMap(stub.psi) - // member functions and properties - stub.childrenStubs.forEach(::indexStub) - } - is KotlinObjectStubImpl -> { - addToClassMap(stub.psi) - // member functions and properties - stub.childrenStubs.forEach(::indexStub) - } - is KotlinTypeAliasStubImpl -> addToTypeAliasMap(stub.psi) - is KotlinFunctionStubImpl -> addToFunctionMap(stub.psi) - is KotlinPropertyStubImpl -> addToPropertyMap(stub.psi) - is KotlinPlaceHolderStubImpl -> { - if (stub.stubType == KtStubElementTypes.CLASS_BODY) { - stub.getChildrenStubs().filterIsInstance>().forEach(::indexStub) - } - } - } - } - - fun processStub(ktFileStub: KotlinFileStubImpl) { - val ktFile: KtFile = ktFileStub.psi - addToFacadeFileMap(ktFile) - - val partNames = ktFileStub.facadePartSimpleNames - if (partNames != null) { - val packageFqName = ktFileStub.getPackageFqName() - for (partName in partNames) { - val multiFileClassPartFqName: FqName = packageFqName.child(Name.identifier(partName)) - index.multiFileClassPartMap.computeIfAbsent(multiFileClassPartFqName) { mutableSetOf() }.add(ktFile) - } - } - - // top-level functions and properties, built-in classes - ktFileStub.childrenStubs.forEach(::indexStub) - } - // Indexing built-ins val builtins = mutableSetOf() if (!skipBuiltins) { @@ -309,38 +279,86 @@ public class KotlinStaticDeclarationProviderFactory( } } - val binaryClassCache = ClsKotlinBinaryClassCache.getInstance() - for (root in additionalRoots) { - KotlinFakeClsStubsCache.processAdditionalRoot(root) { additionalRoot -> - val stubs = mutableMapOf() - VfsUtilCore.visitChildrenRecursively(additionalRoot, object : VirtualFileVisitor() { - override fun visitFile(file: VirtualFile): Boolean { - if (!file.isDirectory) { - val stub = buildStubByVirtualFile(file, binaryClassCache) ?: return true - stubs.put(file, stub) - } - return true - } - }) - stubs - }?.forEach { entry -> - val stub = entry.value - val fakeFile = object : KtFile(KtClassFileViewProvider(psiManager, entry.key), isCompiled = true) { - override fun getStub() = stub - override fun isPhysical() = false + // We only need to index binary roots if we deserialize compiled symbols from stubs. When deserializing from class files, we don't + // need these symbols in the declaration provider. + if (shouldBuildStubsForBinaryLibraries) { + val binaryClassCache = ClsKotlinBinaryClassCache.getInstance() + + for (root in sharedBinaryRoots) { + KotlinFakeClsStubsCache.processAdditionalRoot(root) { additionalRoot -> + collectStubsFromBinaryRoot(additionalRoot, binaryClassCache) + }?.let(::processCollectedStubs) + } + + // In contrast to `sharedBinaryRoots`, which are shared between many different projects (e.g. the Kotlin stdlib), `binaryRoots` + // come from binary libraries which are specific to the project. Caching them in `KotlinFakeClsStubsCache` won't have any + // performance advantage. + binaryRoots + .map { collectStubsFromBinaryRoot(it, binaryClassCache) } + .forEach { processCollectedStubs(it) } + } + + testKtFiles.forEach { file -> + file.accept(recorder) + } + } + + private fun indexStub(stub: StubElement<*>) { + when (stub) { + is KotlinClassStubImpl -> { + addToClassMap(stub.psi) + // member functions and properties + stub.childrenStubs.forEach(::indexStub) + } + is KotlinObjectStubImpl -> { + addToClassMap(stub.psi) + // member functions and properties + stub.childrenStubs.forEach(::indexStub) + } + is KotlinTypeAliasStubImpl -> addToTypeAliasMap(stub.psi) + is KotlinFunctionStubImpl -> addToFunctionMap(stub.psi) + is KotlinPropertyStubImpl -> addToPropertyMap(stub.psi) + is KotlinPlaceHolderStubImpl -> { + if (stub.stubType == KtStubElementTypes.CLASS_BODY) { + stub.getChildrenStubs().filterIsInstance>().forEach(::indexStub) } - stub.psi = fakeFile - createdFakeKtFiles.add(fakeFile) - processStub(stub) + } + } + } + + private fun processStub(ktFileStub: KotlinFileStubImpl) { + val ktFile: KtFile = ktFileStub.psi + addToFacadeFileMap(ktFile) + + val partNames = ktFileStub.facadePartSimpleNames + if (partNames != null) { + val packageFqName = ktFileStub.getPackageFqName() + for (partName in partNames) { + val multiFileClassPartFqName: FqName = packageFqName.child(Name.identifier(partName)) + index.multiFileClassPartMap.computeIfAbsent(multiFileClassPartFqName) { mutableSetOf() }.add(ktFile) } } - // Indexing user source files - files.forEach { - it.accept(recorder) - } + // top-level functions and properties, built-in classes + ktFileStub.childrenStubs.forEach(::indexStub) } + private fun collectStubsFromBinaryRoot( + binaryRoot: VirtualFile, + binaryClassCache: ClsKotlinBinaryClassCache, + ): Map = + buildMap { + VfsUtilCore.visitChildrenRecursively(binaryRoot, object : VirtualFileVisitor() { + override fun visitFile(file: VirtualFile): Boolean { + if (!file.isDirectory) { + val stub = buildStubByVirtualFile(file, binaryClassCache) ?: return true + put(file, stub) + } + return true + } + }) + } + private fun buildStubByVirtualFile(file: VirtualFile, binaryClassCache: ClsKotlinBinaryClassCache): KotlinFileStubImpl? { val fileContent = FileContentImpl.createByFile(file) val fileType = file.fileType @@ -358,6 +376,19 @@ public class KotlinStaticDeclarationProviderFactory( return stubBuilder.buildFileStub(fileContent) as? KotlinFileStubImpl } + private fun processCollectedStubs(stubs: Map) { + stubs.forEach { entry -> + val stub = entry.value + val fakeFile = object : KtFile(KtClassFileViewProvider(psiManager, entry.key), isCompiled = true) { + override fun getStub() = stub + override fun isPhysical() = false + } + stub.psi = fakeFile + createdFakeKtFiles.add(fakeFile) + processStub(stub) + } + } + override fun createDeclarationProvider(scope: GlobalSearchScope, contextualModule: KtModule?): KotlinDeclarationProvider { return KotlinStaticDeclarationProvider(index, scope) }