From 426d71b088925435d9e9ce5dcab965812e90e23a Mon Sep 17 00:00:00 2001 From: Sebastian Sellmair Date: Tue, 5 Mar 2024 15:10:35 +0100 Subject: [PATCH] [aa-klib-reader] Only resolve symbols from the associated library/sourceFile Previously the addresses were resolved by only respecting CallableId or ClassId. This however, could lead to resolving symbols that were not defined in the klib which initially provided the declaration address. E.g. this commit adds a test in `GetSymbolsTest`, where ``` // A.kt private fun foo() = 42 // B.kt private fun foo() = 42 ``` In this case the two source files (A.kt and B.kt) defined two distinct addresses for `foo`. However: Resolving any of those two addresses would resolve both functions (A&B), which is not expected. ^KT-66271 Fixed --- .../api/klib/reader/KlibDeclarationAddress.kt | 15 +++-- .../reader/PackageFragmentReadingContext.kt | 8 ++- .../analysis/api/klib/reader/getSymbols.kt | 22 +++++++ .../reader/readKlibDeclarationAddresses.kt | 6 +- .../api/klib/reader/tests/GetSymbolsTest.kt | 60 +++++++++++++++++++ .../testData/!testProject.addresses | 20 +++++++ .../sample/filePrivateSymbolsClash/A.kt | 11 +++- .../sample/filePrivateSymbolsClash/B.kt | 11 +++- 8 files changed, 144 insertions(+), 9 deletions(-) diff --git a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/KlibDeclarationAddress.kt b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/KlibDeclarationAddress.kt index 1ede34ec823..2b009660d12 100644 --- a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/KlibDeclarationAddress.kt +++ b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/KlibDeclarationAddress.kt @@ -8,8 +8,10 @@ package org.jetbrains.kotlin.analysis.api.klib.reader import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name +import java.nio.file.Path public sealed class KlibDeclarationAddress { + internal abstract val libraryPath: Path public abstract val sourceFileName: String? public abstract val packageFqName: FqName } @@ -18,13 +20,15 @@ public sealed class KlibClassifierAddress : KlibDeclarationAddress() { public abstract val classId: ClassId } -public data class KlibClassAddress( +public data class KlibClassAddress internal constructor( + override val libraryPath: Path, public override val sourceFileName: String?, public override val packageFqName: FqName, public override val classId: ClassId, ) : KlibClassifierAddress() -public data class KlibTypeAliasAddress( +public data class KlibTypeAliasAddress internal constructor( + override val libraryPath: Path, override val packageFqName: FqName, override val classId: ClassId, ) : KlibClassifierAddress() { @@ -38,14 +42,17 @@ public sealed class KlibCallableAddress : KlibDeclarationAddress() { public abstract val callableName: Name } -public data class KlibPropertyAddress( +public data class KlibPropertyAddress internal constructor( + override val libraryPath: Path, override val sourceFileName: String?, override val packageFqName: FqName, override val callableName: Name, ) : KlibCallableAddress() -public data class KlibFunctionAddress( +public data class KlibFunctionAddress internal constructor( + override val libraryPath: Path, override val sourceFileName: String?, override val packageFqName: FqName, override val callableName: Name, ) : KlibCallableAddress() + diff --git a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/PackageFragmentReadingContext.kt b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/PackageFragmentReadingContext.kt index c05b37994c5..15f9ad3d435 100644 --- a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/PackageFragmentReadingContext.kt +++ b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/PackageFragmentReadingContext.kt @@ -5,20 +5,24 @@ package org.jetbrains.kotlin.analysis.api.klib.reader +import org.jetbrains.kotlin.library.KotlinLibrary import org.jetbrains.kotlin.library.metadata.KlibMetadataProtoBuf import org.jetbrains.kotlin.metadata.ProtoBuf import org.jetbrains.kotlin.metadata.deserialization.NameResolverImpl import org.jetbrains.kotlin.metadata.deserialization.getExtensionOrNull import org.jetbrains.kotlin.name.FqName +import java.nio.file.Path +import kotlin.io.path.Path -internal fun PackageFragmentReadingContext(packageFragmentProto: ProtoBuf.PackageFragment): PackageFragmentReadingContext? { +internal fun PackageFragmentReadingContext(library: KotlinLibrary, packageFragmentProto: ProtoBuf.PackageFragment): PackageFragmentReadingContext? { val nameResolver = NameResolverImpl(packageFragmentProto.strings, packageFragmentProto.qualifiedNames) val packageFqName = packageFragmentProto.`package`.getExtensionOrNull(KlibMetadataProtoBuf.packageFqName) ?.let { packageFqNameStringIndex -> nameResolver.getPackageFqName(packageFqNameStringIndex) } ?: return null - return PackageFragmentReadingContext(FqName(packageFqName), nameResolver) + return PackageFragmentReadingContext(Path(library.libraryFile.path), FqName(packageFqName), nameResolver) } internal class PackageFragmentReadingContext( + val libraryPath: Path, val packageFqName: FqName, val nameResolver: NameResolverImpl, ) diff --git a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/getSymbols.kt b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/getSymbols.kt index ade8537df39..a35de5f389b 100644 --- a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/getSymbols.kt +++ b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/getSymbols.kt @@ -7,6 +7,7 @@ package org.jetbrains.kotlin.analysis.api.klib.reader import org.jetbrains.kotlin.analysis.api.KtAnalysisSession import org.jetbrains.kotlin.analysis.api.symbols.* +import org.jetbrains.kotlin.analysis.project.structure.KtLibraryModule /** * Note: A single [KlibDeclarationAddress] can be shared by multiple symbols. @@ -40,11 +41,13 @@ public fun KlibDeclarationAddress.getSymbols(): Sequence { context(KtAnalysisSession) public fun KlibClassAddress.getClassOrObjectSymbol(): KtClassOrObjectSymbol? { return getClassOrObjectSymbolByClassId(classId) + ?.takeIf { symbol -> symbol in this } } context(KtAnalysisSession) public fun KlibTypeAliasAddress.getTypeAliasSymbol(): KtTypeAliasSymbol? { return getTypeAliasByClassId(classId) + ?.takeIf { symbol -> symbol in this } } /** @@ -65,6 +68,7 @@ context(KtAnalysisSession) public fun KlibFunctionAddress.getFunctionSymbols(): Sequence { return getTopLevelCallableSymbols(packageFqName, callableName) .filterIsInstance() + .filter { symbol -> symbol in this } } /** @@ -74,5 +78,23 @@ context(KtAnalysisSession) public fun KlibPropertyAddress.getPropertySymbols(): Sequence { return getTopLevelCallableSymbols(packageFqName, callableName) .filterIsInstance() + .filter { symbol -> symbol in this } } +context(KtAnalysisSession) +private operator fun KlibDeclarationAddress.contains(symbol: KtDeclarationSymbol): Boolean { + val symbolKlibSourceFileName = symbol.getKlibSourceFileName() + val symbolLibraryModule = symbol.getContainingModule() as? KtLibraryModule ?: return false + + /* check if symbol comes from the same klib library: symbolKlibSourceFile not known -> checking library module */ + if (libraryPath !in symbolLibraryModule.getBinaryRoots()) { + return false + } + + /* Check if symbol comes from the same source file (if known) */ + if (this.sourceFileName != null && symbolKlibSourceFileName != sourceFileName) { + return false + } + + return true +} diff --git a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/readKlibDeclarationAddresses.kt b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/readKlibDeclarationAddresses.kt index 7f0662f840f..b2c66954cb3 100644 --- a/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/readKlibDeclarationAddresses.kt +++ b/native/analysis-api-klib-reader/src/org/jetbrains/kotlin/analysis/api/klib/reader/readKlibDeclarationAddresses.kt @@ -72,7 +72,7 @@ internal fun readKlibDeclarationAddresses(library: KotlinLibrary): Set val packageFragmentProto = parsePackageFragment(packageMetadata) - with(PackageFragmentReadingContext(packageFragmentProto) ?: return@flatMap emptySet()) { + with(PackageFragmentReadingContext(library, packageFragmentProto) ?: return@flatMap emptySet()) { packageFragmentProto.readKlibClassAddresses() + packageFragmentProto.readKlibTypeAliasAddresses() + packageFragmentProto.readKlibPropertyAddresses() + @@ -88,6 +88,7 @@ internal fun ProtoBuf.PackageFragment.readKlibClassAddresses(): Set nameResolver.strings.getString(fileNameId) @@ -102,6 +103,7 @@ internal fun ProtoBuf.PackageFragment.readKlibTypeAliasAddresses(): Set val name = Name.identifier(nameResolver.getString(typeAliasProto.name)) KlibTypeAliasAddress( + libraryPath = libraryPath, packageFqName = packageFqName, classId = ClassId(packageFqName, name) ) @@ -112,6 +114,7 @@ context(PackageFragmentReadingContext) internal fun ProtoBuf.PackageFragment.readKlibPropertyAddresses(): Set { return `package`.propertyList.map { propertyProto -> KlibPropertyAddress( + libraryPath = libraryPath, sourceFileName = propertyProto.getExtensionOrNull(KlibMetadataProtoBuf.propertyFile)?.let { fileNameId -> nameResolver.strings.getString(fileNameId) }, @@ -125,6 +128,7 @@ context(PackageFragmentReadingContext) internal fun ProtoBuf.PackageFragment.readKlibFunctionAddresses(): Set { return `package`.functionList.map { functionProto -> KlibFunctionAddress( + libraryPath = libraryPath, sourceFileName = functionProto.getExtensionOrNull(KlibMetadataProtoBuf.functionFile)?.let { fileNameId -> nameResolver.getString(fileNameId) }, diff --git a/native/analysis-api-klib-reader/test/org/jetbrains/kotlin/analysis/api/klib/reader/tests/GetSymbolsTest.kt b/native/analysis-api-klib-reader/test/org/jetbrains/kotlin/analysis/api/klib/reader/tests/GetSymbolsTest.kt index 4c0fce8f5a9..9445066f9b1 100644 --- a/native/analysis-api-klib-reader/test/org/jetbrains/kotlin/analysis/api/klib/reader/tests/GetSymbolsTest.kt +++ b/native/analysis-api-klib-reader/test/org/jetbrains/kotlin/analysis/api/klib/reader/tests/GetSymbolsTest.kt @@ -130,6 +130,66 @@ class GetSymbolsTest { } } + @Test + fun `test - filePrivateSymbolsClash - function`() { + withTestProjectLibraryAnalysisSession { + val addresses = (useSiteModule as KtLibraryModule).readKlibDeclarationAddresses() ?: fail("Failed reading addresses") + val clashingAddresses = addresses.filterIsInstance() + .filter { it.callableName == Name.identifier("foo") } + + val fooInAKt = clashingAddresses.find { it.sourceFileName == "A.kt" } ?: fail("Missing `fun foo()` in A.kt") + val fooInBKt = clashingAddresses.find { it.sourceFileName == "B.kt" } ?: fail("Missing `fun foo()` in B.kt") + + val fooInASymbols = fooInAKt.getFunctionSymbols().toList() + if (fooInASymbols.size != 1) fail( + "Expected exactly one 'foo' symbol in A.kt. Found ${fooInASymbols.joinToString { it.render() }}" + ) + + val fooInBSymbols = fooInBKt.getFunctionSymbols().toList() + if (fooInASymbols.size != 1) fail( + "Expected exactly one 'foo' symbol in B.kt. Found ${fooInBSymbols.joinToString { it.render() }}" + ) + + val fooInASymbol = fooInASymbols.first() + if (!fooInASymbol.annotationsList.hasAnnotation(ClassId.fromString("org/jetbrains/sample/filePrivateSymbolsClash/A"))) + fail("Missing annotation 'A' on 'fun foo()' in A.kt") + + val fooInBSymbol = fooInBSymbols.first() + if (!fooInBSymbol.annotationsList.hasAnnotation(ClassId.fromString("org/jetbrains/sample/filePrivateSymbolsClash/B"))) + fail("Missing annotation 'B' on 'fun foo()' in B.kt") + } + } + + @Test + fun `test - filePrivateSymbolsClash - property`() { + withTestProjectLibraryAnalysisSession { + val addresses = (useSiteModule as KtLibraryModule).readKlibDeclarationAddresses() ?: fail("Failed reading addresses") + val clashingAddresses = addresses.filterIsInstance() + .filter { it.callableName == Name.identifier("fooProperty") } + + val fooInAKt = clashingAddresses.find { it.sourceFileName == "A.kt" } ?: fail("Missing `val fooProperty` in A.kt") + val fooInBKt = clashingAddresses.find { it.sourceFileName == "B.kt" } ?: fail("Missing `val fooProperty` in B.kt") + + val fooInASymbols = fooInAKt.getPropertySymbols().toList() + if (fooInASymbols.size != 1) fail( + "Expected exactly one 'fooProperty' symbol in A.kt. Found ${fooInASymbols.joinToString { it.render() }}" + ) + + val fooInBSymbols = fooInBKt.getPropertySymbols().toList() + if (fooInASymbols.size != 1) fail( + "Expected exactly one 'fooProperty' symbol in B.kt. Found ${fooInBSymbols.joinToString { it.render() }}" + ) + + val fooInASymbol = fooInASymbols.first() + if (!fooInASymbol.annotationsList.hasAnnotation(ClassId.fromString("org/jetbrains/sample/filePrivateSymbolsClash/A"))) + fail("Missing annotation 'A' on 'val fooProperty' in A.kt") + + val fooInBSymbol = fooInBSymbols.first() + if (!fooInBSymbol.annotationsList.hasAnnotation(ClassId.fromString("org/jetbrains/sample/filePrivateSymbolsClash/B"))) + fail("Missing annotation 'B' on 'val fooProperty' in B.kt") + } + } + /** * Runs the given [block] in an analysis session that will have the built library as [KtAnalysisSession.useSiteModule] */ diff --git a/native/analysis-api-klib-reader/testData/!testProject.addresses b/native/analysis-api-klib-reader/testData/!testProject.addresses index a5f3bc7fb71..54c7d505f6e 100644 --- a/native/analysis-api-klib-reader/testData/!testProject.addresses +++ b/native/analysis-api-klib-reader/testData/!testProject.addresses @@ -111,11 +111,31 @@ Class (org.jetbrains.sample.b.BClass) Package Name : "org.jetbrains.sample.b" ClassId : "org/jetbrains/sample/b/BClass" +Class (org.jetbrains.sample.filePrivateSymbolsClash.A) + Source File Name : "A.kt" + Package Name : "org.jetbrains.sample.filePrivateSymbolsClash" + ClassId : "org/jetbrains/sample/filePrivateSymbolsClash/A" + +Property (org.jetbrains.sample.filePrivateSymbolsClash.fooProperty) + Source File Name : "A.kt" + Package Name : "org.jetbrains.sample.filePrivateSymbolsClash" + Property Name : "fooProperty" + Function (org.jetbrains.sample.filePrivateSymbolsClash.foo) Source File Name : "A.kt" Package Name : "org.jetbrains.sample.filePrivateSymbolsClash" Function Name : "foo" +Class (org.jetbrains.sample.filePrivateSymbolsClash.B) + Source File Name : "B.kt" + Package Name : "org.jetbrains.sample.filePrivateSymbolsClash" + ClassId : "org/jetbrains/sample/filePrivateSymbolsClash/B" + +Property (org.jetbrains.sample.filePrivateSymbolsClash.fooProperty) + Source File Name : "B.kt" + Package Name : "org.jetbrains.sample.filePrivateSymbolsClash" + Property Name : "fooProperty" + Function (org.jetbrains.sample.filePrivateSymbolsClash.foo) Source File Name : "B.kt" Package Name : "org.jetbrains.sample.filePrivateSymbolsClash" diff --git a/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/A.kt b/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/A.kt index cfbf1f27143..c9a7ffc2d54 100644 --- a/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/A.kt +++ b/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/A.kt @@ -7,4 +7,13 @@ package org.jetbrains.sample.filePrivateSymbolsClash -private fun foo() = 42 \ No newline at end of file +annotation class A + +/** + * Defined in A.kt, clashes with B.kt + */ +@A +private fun foo() = 42 + +@A +private val fooProperty get() = 42 diff --git a/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/B.kt b/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/B.kt index cfbf1f27143..55b30093305 100644 --- a/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/B.kt +++ b/native/analysis-api-klib-reader/testProject/src/nativeMain/kotlin/org/jetbrains/sample/filePrivateSymbolsClash/B.kt @@ -7,4 +7,13 @@ package org.jetbrains.sample.filePrivateSymbolsClash -private fun foo() = 42 \ No newline at end of file +annotation class B + +/** + * Defined in B.kt, clashes with A.kt + */ +@B +private fun foo() = 42 + +@B +private val fooProperty get() = 42 \ No newline at end of file