From b96c1a644f32d46ba766fa0daa100e846b406107 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Tue, 22 Aug 2023 16:48:28 +0900 Subject: [PATCH] [LL API] Fix code fragment compilation in library sources (KT-61383) In complex projects, there might be several library copies (with the same or different versions). As there is no way to build a reliable dependency graph between libraries, a project library depends on all other libraries. As a result, there might be several declarations in the classpath with the same name and signature. Normally, K2 issues a 'resolution ambiguity' error on calls to such libraries. It is acceptable for resolution, as resolution errors are never shown in the library code. However, the backend, to which 'evaluate expression' needs to pass FIR afterwards, is not designed for compiling ambiguous (and non-completed) calls. --- ...ceModuleCompilerFacilityTestGenerated.java | 6 ++ ...ceModuleCompilerFacilityTestGenerated.java | 6 ++ .../AbstractCompilerFacilityTest.kt | 16 ++- .../duplicateLibrary.fragment.kt | 1 + .../codeFragments/duplicateLibrary.ir.txt | 33 ++++++ .../codeFragments/duplicateLibrary.kt | 7 ++ .../codeFragments/duplicateLibrary.txt | 5 + ...braryScopeAwareConeCallConflictResolver.kt | 102 ++++++++++++++++++ .../fir/sessions/LLFirJvmSessionFactory.kt | 10 ++ 9 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.fragment.kt create mode 100644 analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.ir.txt create mode 100644 analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.kt create mode 100644 analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.txt create mode 100644 analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/resolve/extensions/LLLibraryScopeAwareConeCallConflictResolver.kt diff --git a/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisLibrarySourceModuleCompilerFacilityTestGenerated.java b/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisLibrarySourceModuleCompilerFacilityTestGenerated.java index c4fc101e39c..a882be13d45 100644 --- a/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisLibrarySourceModuleCompilerFacilityTestGenerated.java +++ b/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisLibrarySourceModuleCompilerFacilityTestGenerated.java @@ -85,6 +85,12 @@ public class FirIdeNormalAnalysisLibrarySourceModuleCompilerFacilityTestGenerate runTest("analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/customName.kt"); } + @Test + @TestMetadata("duplicateLibrary.kt") + public void testDuplicateLibrary() throws Exception { + runTest("analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.kt"); + } + @Test @TestMetadata("imports.kt") public void testImports() throws Exception { diff --git a/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisSourceModuleCompilerFacilityTestGenerated.java b/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisSourceModuleCompilerFacilityTestGenerated.java index 1f50d4f3682..0c1e2b542fd 100644 --- a/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisSourceModuleCompilerFacilityTestGenerated.java +++ b/analysis/analysis-api-fir/tests-gen/org/jetbrains/kotlin/analysis/api/fir/test/cases/generated/cases/components/compilerFacility/FirIdeNormalAnalysisSourceModuleCompilerFacilityTestGenerated.java @@ -85,6 +85,12 @@ public class FirIdeNormalAnalysisSourceModuleCompilerFacilityTestGenerated exten runTest("analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/customName.kt"); } + @Test + @TestMetadata("duplicateLibrary.kt") + public void testDuplicateLibrary() throws Exception { + runTest("analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.kt"); + } + @Test @TestMetadata("imports.kt") public void testImports() throws Exception { diff --git a/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/cases/components/compilerFacility/AbstractCompilerFacilityTest.kt b/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/cases/components/compilerFacility/AbstractCompilerFacilityTest.kt index 39b15efc1cc..0d04f037015 100644 --- a/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/cases/components/compilerFacility/AbstractCompilerFacilityTest.kt +++ b/analysis/analysis-api-impl-base/tests/org/jetbrains/kotlin/analysis/api/impl/base/test/cases/components/compilerFacility/AbstractCompilerFacilityTest.kt @@ -17,12 +17,14 @@ import org.jetbrains.kotlin.analysis.test.framework.base.AbstractAnalysisApiBase import org.jetbrains.kotlin.analysis.test.framework.services.expressionMarkerProvider import org.jetbrains.kotlin.backend.common.extensions.IrGenerationExtension import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext +import org.jetbrains.kotlin.cli.common.CLIConfigurationKeys +import org.jetbrains.kotlin.cli.jvm.config.JvmClasspathRoot import org.jetbrains.kotlin.codegen.BytecodeListingTextCollectingVisitor import org.jetbrains.kotlin.codegen.ClassBuilderFactories +import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.config.CommonConfigurationKeys import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.config.JVMConfigurationKeys -import org.jetbrains.kotlin.fileClasses.javaFileFacadeFqName import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.ir.declarations.IrModuleFragment import org.jetbrains.kotlin.ir.util.DumpIrTreeOptions @@ -33,6 +35,7 @@ import org.jetbrains.kotlin.test.directives.ConfigurationDirectives import org.jetbrains.kotlin.test.directives.JvmEnvironmentConfigurationDirectives import org.jetbrains.kotlin.test.directives.model.SimpleDirectivesContainer import org.jetbrains.kotlin.test.model.TestModule +import org.jetbrains.kotlin.test.services.EnvironmentConfigurator import org.jetbrains.kotlin.test.services.TestServices import org.jetbrains.kotlin.test.services.assertions import org.jetbrains.org.objectweb.asm.ClassReader @@ -110,7 +113,7 @@ abstract class AbstractCompilerFacilityTest : AbstractAnalysisApiBasedSingleModu super.configureTest(builder) with(builder) { useDirectives(Directives) - useConfigurators() + useConfigurators(::CompilerFacilityEnvironmentConfigurator) defaultDirectives { +ConfigurationDirectives.WITH_STDLIB +JvmEnvironmentConfigurationDirectives.FULL_JDK @@ -168,6 +171,15 @@ abstract class AbstractCompilerFacilityTest : AbstractAnalysisApiBasedSingleModu val CODE_FRAGMENT_IMPORT by stringDirective("Import directive for a code fragment") val CODE_FRAGMENT_CLASS_NAME by stringDirective("Short name of a code fragment class") val CODE_FRAGMENT_METHOD_NAME by stringDirective("Name of a code fragment facade method") + val ATTACH_DUPLICATE_STDLIB by directive("Attach the 'stdlib-jvm-minimal-for-test' library to simulate duplicate stdlib dependency") + } +} + +private class CompilerFacilityEnvironmentConfigurator(testServices: TestServices) : EnvironmentConfigurator(testServices) { + override fun configureCompilerConfiguration(configuration: CompilerConfiguration, module: TestModule) { + if (module.directives.contains(AbstractCompilerFacilityTest.Directives.ATTACH_DUPLICATE_STDLIB)) { + configuration.add(CLIConfigurationKeys.CONTENT_ROOTS, JvmClasspathRoot(ForTestCompileRuntime.minimalRuntimeJarForTests())) + } } } diff --git a/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.fragment.kt b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.fragment.kt new file mode 100644 index 00000000000..47ff212e7a3 --- /dev/null +++ b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.fragment.kt @@ -0,0 +1 @@ +text.takeUnless { it.isEmpty() }?.length \ No newline at end of file diff --git a/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.ir.txt b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.ir.txt new file mode 100644 index 00000000000..e01d7f65556 --- /dev/null +++ b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.ir.txt @@ -0,0 +1,33 @@ +MODULE_FRAGMENT + FILE fqName: fileName:fragment.kt + CLASS CLASS name:CodeFragment modality:FINAL visibility:public superTypes:[kotlin.Any] + $this: VALUE_PARAMETER INSTANCE_RECEIVER name: type:.CodeFragment + CONSTRUCTOR visibility:public <> () returnType:.CodeFragment [primary] + BLOCK_BODY + DELEGATING_CONSTRUCTOR_CALL 'public constructor () [primary] declared in kotlin.Any' + FUN name:run visibility:public modality:FINAL <> (p0:kotlin.String) returnType:kotlin.Int? + VALUE_PARAMETER name:p0 index:0 type:kotlin.String + EXPRESSION_BODY + BLOCK type=kotlin.Int? origin=null + BLOCK type=kotlin.Int? origin=SAFE_CALL + VAR IR_TEMPORARY_VARIABLE name:tmp_0 type:kotlin.String? [val] + CALL 'public final fun takeUnless (predicate: kotlin.Function1): T of kotlin.takeUnless? [inline] declared in kotlin' type=kotlin.String? origin=null + : kotlin.String + $receiver: GET_VAR 'p0: kotlin.String declared in .CodeFragment.run' type=kotlin.String origin=null + predicate: FUN_EXPR type=kotlin.Function1 origin=LAMBDA + FUN LOCAL_FUNCTION_FOR_LAMBDA name: visibility:local modality:FINAL <> (it:kotlin.String) returnType:kotlin.Boolean + VALUE_PARAMETER name:it index:0 type:kotlin.String + BLOCK_BODY + RETURN type=kotlin.Nothing from='local final fun (it: kotlin.String): kotlin.Boolean declared in .CodeFragment.run' + CALL 'public final fun isEmpty (): kotlin.Boolean [inline] declared in kotlin.text' type=kotlin.Boolean origin=null + $receiver: GET_VAR 'it: kotlin.String declared in .CodeFragment.run.' type=kotlin.String origin=null + WHEN type=kotlin.Int? origin=null + BRANCH + if: CALL 'public final fun EQEQ (arg0: kotlin.Any?, arg1: kotlin.Any?): kotlin.Boolean declared in kotlin.internal.ir' type=kotlin.Boolean origin=EQEQ + arg0: GET_VAR 'val tmp_0: kotlin.String? [val] declared in .CodeFragment.run' type=kotlin.String? origin=null + arg1: CONST Null type=kotlin.Nothing? value=null + then: CONST Null type=kotlin.Nothing? value=null + BRANCH + if: CONST Boolean type=kotlin.Boolean value=true + then: CALL 'public open fun (): kotlin.Int declared in kotlin.String' type=kotlin.Int origin=GET_PROPERTY + $this: GET_VAR 'val tmp_0: kotlin.String? [val] declared in .CodeFragment.run' type=kotlin.String? origin=null diff --git a/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.kt b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.kt new file mode 100644 index 00000000000..eaaedc1a674 --- /dev/null +++ b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.kt @@ -0,0 +1,7 @@ +// ATTACH_DUPLICATE_STDLIB + +fun test(text: String) { + consume(text) +} + +fun consume(text: String) {} \ No newline at end of file diff --git a/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.txt b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.txt new file mode 100644 index 00000000000..84093cdaa51 --- /dev/null +++ b/analysis/analysis-api/testData/components/compilerFacility/compilation/codeFragments/duplicateLibrary.txt @@ -0,0 +1,5 @@ +public final class CodeFragment { + // source: 'fragment.kt' + public method (): void + public final static method run(p0: java.lang.String): java.lang.Integer +} diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/resolve/extensions/LLLibraryScopeAwareConeCallConflictResolver.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/resolve/extensions/LLLibraryScopeAwareConeCallConflictResolver.kt new file mode 100644 index 00000000000..2780713797b --- /dev/null +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/resolve/extensions/LLLibraryScopeAwareConeCallConflictResolver.kt @@ -0,0 +1,102 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.analysis.low.level.api.fir.resolve.extensions + +import com.intellij.openapi.vfs.VirtualFile +import org.jetbrains.kotlin.fir.psi +import org.jetbrains.kotlin.fir.resolve.BodyResolveComponents +import org.jetbrains.kotlin.fir.resolve.calls.Candidate +import org.jetbrains.kotlin.fir.resolve.calls.ConeCallConflictResolver +import org.jetbrains.kotlin.fir.resolve.calls.ConeCallConflictResolverFactory +import org.jetbrains.kotlin.fir.resolve.inference.InferenceComponents +import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol +import org.jetbrains.kotlin.name.CallableId +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.psi.KtCodeFragment +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.resolve.calls.results.TypeSpecificityComparator + +internal class LLLibraryScopeAwareCallConflictResolverFactory( + private val delegateFactory: ConeCallConflictResolverFactory +) : ConeCallConflictResolverFactory() { + override fun create( + typeSpecificityComparator: TypeSpecificityComparator, + components: InferenceComponents, + transformerComponents: BodyResolveComponents, + ): ConeCallConflictResolver { + val delegate = delegateFactory.create(typeSpecificityComparator, components, transformerComponents) + return LLLibraryScopeAwareConeCallConflictResolver(delegate, transformerComponents) + } +} + +private class LLLibraryScopeAwareConeCallConflictResolver( + private val delegate: ConeCallConflictResolver, + private val bodyResolveComponents: BodyResolveComponents +) : ConeCallConflictResolver() { + override fun chooseMaximallySpecificCandidates(candidates: Set, discriminateAbstracts: Boolean): Set { + if (candidates.size > 1 && bodyResolveComponents.file.psi is KtCodeFragment) { + val filteredCandidates = filterCodeFragmentCandidates(candidates) + return delegate.chooseMaximallySpecificCandidates(filteredCandidates, discriminateAbstracts) + } + + return delegate.chooseMaximallySpecificCandidates(candidates, discriminateAbstracts) + } + + // In complex projects, there might be several library copies (with the same or different versions). + // As there is no way to build a reliable dependency graph between libraries, a project library depends on all other libraries. + // As a result, there might be several declarations in the classpath with the same name and signature. + // Normally, K2 issues a 'resolution ambiguity' error on calls to such libraries. It is sort of acceptable for resolution, as + // resolution errors are never shown in the library code. However, the backend, to which 'evaluate expression' needs to pass FIR + // afterwards, is not designed for compiling ambiguous (and non-completed) calls. + // The code below scans for declaration duplicates, and chooses a set of them from a single class input (a JAR or a directory). + // The logic is not ideal, as, in theory, versions might differ non-trivially: each artifact may have unique declarations. + // However, such cases should be relatively rare, and to provide the candidate list more precisely, one would need to also compare + // signatures of each declaration. + private fun filterCodeFragmentCandidates(candidates: Set): Set { + val binaryCallableCandidates = LinkedHashMap>>() + val otherCandidates = ArrayList() + + for (candidate in candidates) { + val symbol = candidate.symbol + + if (symbol is FirCallableSymbol<*> && symbol.callableId.className == null) { + val callableId = symbol.callableId + + val symbolFile = symbol.fir.psi?.containingFile + val symbolVirtualFile = symbolFile?.virtualFile + if (symbolFile is KtFile && symbolFile.isCompiled && symbolVirtualFile != null) { + val symbolRootVirtualFile = getSymbolRootFile(symbolVirtualFile, symbolFile.packageFqName) + if (symbolRootVirtualFile != null) { + binaryCallableCandidates + .getOrPut(callableId, ::LinkedHashMap) + .getOrPut(symbolRootVirtualFile, ::ArrayList) + .add(candidate) + continue + } + } + } + + otherCandidates.add(candidate) + } + + if (binaryCallableCandidates.isNotEmpty()) { + return buildSet { + addAll(otherCandidates) + for (binaryCallableCandidateGroup in binaryCallableCandidates.values) { + val chosenGroupKey = binaryCallableCandidateGroup.keys.maxBy { it.path } + addAll(binaryCallableCandidateGroup[chosenGroupKey].orEmpty()) + } + } + } + + return candidates + } + + private fun getSymbolRootFile(virtualFile: VirtualFile, packageFqName: FqName): VirtualFile? { + val nestingLevel = packageFqName.pathSegments().size + return generateSequence(virtualFile) { it.parent }.drop(nestingLevel + 1).firstOrNull() + } +} \ No newline at end of file diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirJvmSessionFactory.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirJvmSessionFactory.kt index ce406a38454..587b347f251 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirJvmSessionFactory.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirJvmSessionFactory.kt @@ -9,14 +9,18 @@ import com.intellij.openapi.project.Project import com.intellij.psi.search.GlobalSearchScope import org.jetbrains.kotlin.analysis.low.level.api.fir.project.structure.* import org.jetbrains.kotlin.analysis.low.level.api.fir.providers.* +import org.jetbrains.kotlin.analysis.low.level.api.fir.resolve.extensions.LLLibraryScopeAwareCallConflictResolverFactory import org.jetbrains.kotlin.analysis.project.structure.* import org.jetbrains.kotlin.analysis.providers.createPackagePartProvider import org.jetbrains.kotlin.fir.BuiltinTypes +import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.SessionConfiguration import org.jetbrains.kotlin.fir.backend.jvm.FirJvmTypeMapper import org.jetbrains.kotlin.fir.deserialization.SingleModuleDataProvider import org.jetbrains.kotlin.fir.java.JavaSymbolProvider import org.jetbrains.kotlin.fir.java.deserialization.OptionalAnnotationClassesProvider +import org.jetbrains.kotlin.fir.resolve.calls.ConeCallConflictResolverFactory +import org.jetbrains.kotlin.fir.resolve.calls.callConflictResolverFactory import org.jetbrains.kotlin.fir.resolve.providers.FirSymbolProvider import org.jetbrains.kotlin.fir.resolve.scopes.wrapScopeWithJvmMapped import org.jetbrains.kotlin.fir.scopes.FirKotlinScopeProvider @@ -71,6 +75,7 @@ internal class LLFirJvmSessionFactory(project: Project) : LLFirAbstractSessionFa ) register(JavaSymbolProvider::class, javaSymbolProvider) register(FirJvmTypeMapper::class, FirJvmTypeMapper(this)) + registerLibraryScopeAwareCallConflictResolverFactory() } } @@ -79,9 +84,14 @@ internal class LLFirJvmSessionFactory(project: Project) : LLFirAbstractSessionFa registerCommonJavaComponents(JavaModuleResolver.getInstance(project)) registerJavaSpecificResolveComponents() register(FirJvmTypeMapper::class, FirJvmTypeMapper(this)) + registerLibraryScopeAwareCallConflictResolverFactory() } } + private fun FirSession.registerLibraryScopeAwareCallConflictResolverFactory() { + register(ConeCallConflictResolverFactory::class, LLLibraryScopeAwareCallConflictResolverFactory(callConflictResolverFactory)) + } + override fun createProjectLibraryProvidersForScope( session: LLFirSession, moduleData: LLFirModuleData,