[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
2024-03-05 15:10:35 +01:00
parent 6b98602afc
commit 426d71b088
8 changed files with 144 additions and 9 deletions
@@ -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()
+
@@ -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,
 )
@@ -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<KtSymbol> {
 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<KtFunctionSymbol> {
    return getTopLevelCallableSymbols(packageFqName, callableName)
        .filterIsInstance<KtFunctionSymbol>()
+        .filter { symbol -> symbol in this }
 }

 /**
@@ -74,5 +78,23 @@ context(KtAnalysisSession)
 public fun KlibPropertyAddress.getPropertySymbols(): Sequence<KtPropertySymbol> {
    return getTopLevelCallableSymbols(packageFqName, callableName)
        .filterIsInstance<KtPropertySymbol>()
+        .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
+}
@@ -72,7 +72,7 @@ internal fun readKlibDeclarationAddresses(library: KotlinLibrary): Set<KlibDecla
    return packageMetadataSequence.flatMap { packageMetadata ->
        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<KlibClassAdd
        val classId = ClassId.fromString(nameResolver.getQualifiedClassName(classProto.fqName))
        if (classId.isNestedClass) return@mapNotNull null
        KlibClassAddress(
+            libraryPath = libraryPath,
            packageFqName = packageFqName,
            sourceFileName = classProto.getExtensionOrNull(KlibMetadataProtoBuf.classFile)?.let { fileNameId ->
                nameResolver.strings.getString(fileNameId)
@@ -102,6 +103,7 @@ internal fun ProtoBuf.PackageFragment.readKlibTypeAliasAddresses(): Set<KlibType
    return this.`package`.typeAliasList.map { typeAliasProto ->
        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<KlibPropertyAddress> {
    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<KlibFunctionAddress> {
    return `package`.functionList.map { functionProto ->
        KlibFunctionAddress(
+            libraryPath = libraryPath,
            sourceFileName = functionProto.getExtensionOrNull(KlibMetadataProtoBuf.functionFile)?.let { fileNameId ->
                nameResolver.getString(fileNameId)
            },
@@ -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<KlibFunctionAddress>()
+                .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<KlibPropertyAddress>()
+                .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]
     */
@@ -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"
@@ -7,4 +7,13 @@

 package org.jetbrains.sample.filePrivateSymbolsClash

+annotation class A
+
+/**
+ * Defined in A.kt, clashes with B.kt
+ */
+@A
 private fun foo() = 42
+
+@A
+private val fooProperty get() = 42
@@ -7,4 +7,13 @@

 package org.jetbrains.sample.filePrivateSymbolsClash

+annotation class B
+
+/**
+ * Defined in B.kt, clashes with A.kt
+ */
+@B
 private fun foo() = 42
+
+@B
+private val fooProperty get() = 42