Gradle plugin: The artifact walker skips internal packages and honours excludePackages

docs/SPEC.md

@@ -473,6 +473,16 @@ All in `PM/commonTest`, so they run wherever the interpreter loads.
   with the notebook's own state spelling (`main.App.messages.getValue()` / `setValue(...)`, #174)
   against pythonx-compose f6dd3e2. No CI
   workflow runs the module yet.
+- **B-11** (#192) The artifact walker skips every declaration whose package has an `internal` segment
+  (`*.internal` and `*.internal.*`, as in `androidx.compose.ui.internal`), on jars and on klibs. That is the
+  Kotlin `.api` dump convention: a library that puts a declaration in such a package has declared it
+  non-public API even though it is public in bytecode. The walker also honours
+  `pythonBindings.excludePackages`, with the namespace matching the KSP side uses (a package equal to an
+  entry, or below it). A skipped declaration gets no binding and no `.pyi` stub, and no declined
+  `DeclarationModel` either: it is not part of the library's surface. The rule is a blacklist (decision 12.7
+  in `AGENTS.md`): everything else that is public stays bound; it is not a list of what to expose. A segment
+  must equal `internal` exactly (`internals` and `internal_api` are ordinary packages).
+  `Status: implemented`, `GP/artifact/InternalPackageExclusionTest.kt`.
 
 ## 7. Threading and builds
 

python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/PythonBindingsPlugin.kt

@@ -362,6 +362,7 @@ class PythonBindingsPlugin : Plugin<Project> {
                     },
                 )
                 includePrefixes.set(includes)
+                excludePackages.set(extension.excludePackages.getOrElse(emptyList()))
                 this.klibReaderClasspath.from(klibReader)
                 outputDirectory.set(
                     project.layout.buildDirectory.dir("generated/pythonArtifactBindings/$sourceSetName"),
@@ -508,6 +509,7 @@ class PythonBindingsPlugin : Plugin<Project> {
             description = "Emits .pyi stubs for the declarations the artefact walker bound."
             artifacts.from(resolved.artifactFiles)
             includePrefixes.set(includes)
+            excludePackages.set(extension.excludePackages.getOrElse(emptyList()))
             klibReaderClasspath.from(klibReader)
             outputDirectory.set(project.layout.buildDirectory.dir("generated/pythonStubs/$sourceSetName"))
         }

python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/ArtifactScanner.kt

@@ -196,9 +196,16 @@ internal object ArtifactScanner {
      *   test fixtures, false for `androidx.compose.foundation.layout`, whose `Dp` lives in
      *   `androidx.compose.ui.unit`. [PythonArtifactBindingsTask] passes the whole resolved
      *   configuration.
+     * @param excludePackages packages to leave out, on top of every `internal` package (SPEC B-11):
+     *   an entry covers the package it names and the ones below it.
      */
-    fun scanJar(target: File, includePrefixes: List<String> = emptyList(), classpath: List<File> = listOf(target)): List<ArtifactCallable> =
-        scan(target, includePrefixes, classpath).mapNotNull { it.callable }.sortedBy { it.name }
+    fun scanJar(
+        target: File,
+        includePrefixes: List<String> = emptyList(),
+        classpath: List<File> = listOf(target),
+        excludePackages: List<String> = emptyList(),
+    ): List<ArtifactCallable> =
+        scan(target, includePrefixes, classpath, excludePackages).mapNotNull { it.callable }.sortedBy { it.name }
 
     /**
      * The same walk, with the half `docs/design/pyi-generation-design.md` §2.2 asks for kept: one
@@ -214,11 +221,17 @@ internal object ArtifactScanner {
         target: File,
         includePrefixes: List<String> = emptyList(),
         classpath: List<File> = listOf(target),
-    ): List<DeclarationModel> = scan(target, includePrefixes, classpath)
+        excludePackages: List<String> = emptyList(),
+    ): List<DeclarationModel> = scan(target, includePrefixes, classpath, excludePackages)
         .map { it.declaration }
         .sortedWith(compareBy({ it.owner }, { it.simpleName }, { it.bindingName ?: "" }))
 
-    private fun scan(target: File, includePrefixes: List<String>, classpath: List<File>): List<Candidate> {
+    private fun scan(
+        target: File,
+        includePrefixes: List<String>,
+        classpath: List<File>,
+        excludePackages: List<String>,
+    ): List<Candidate> {
         val artifactClasspath = ArtifactClasspath(classpath)
         val entries = mutableListOf<Candidate>()
         forEachClassEntry(target) { relativePath, bytes ->
@@ -228,6 +241,7 @@ internal object ArtifactScanner {
             if ('$' in relativePath) return@forEachClassEntry
             val qualified = binaryNameToQualified(relativePath.removeSuffix(".class"))
             if (!matchesInclude(qualified, includePrefixes)) return@forEachClassEntry
+            if (isExcludedPackage(packageNameOf(relativePath.removeSuffix(".class")), excludePackages)) return@forEachClassEntry
             val node = readClassNodeOrNull(bytes) ?: return@forEachClassEntry
             entries += scanClassNode(node, artifactClasspath)
         }
@@ -380,6 +394,15 @@ internal object ArtifactScanner {
     private fun packageNameOf(binaryName: String): String =
         binaryNameToQualified(binaryName.substringBeforeLast('/', missingDelimiterValue = ""))
 
+    /**
+     * SPEC B-11: a package is out when one of its segments is exactly `internal` (`*.internal`,
+     * `*.internal.*`, the Kotlin `.api` dump convention) or when it equals, or lies below, an
+     * `excludePackages` entry (the KSP side's namespace rule). Shared with [KlibScanner].
+     */
+    internal fun isExcludedPackage(packageName: String, excludePackages: List<String>): Boolean =
+        packageName.split('.').any { it == "internal" } ||
+            excludePackages.any { packageName == it || packageName.startsWith("$it.") }
+
     /** A namespace match, not a text one: `junit.run` is a prefix of the string `junit.runner` and
      * of no package or class. */
     private fun matchesInclude(qualifiedName: String, includePrefixes: List<String>): Boolean =

python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/KlibScanWorkAction.kt

@@ -51,15 +51,17 @@ internal abstract class KlibScanWorkAction : WorkAction<KlibScanWorkAction.Param
     internal interface Params : WorkParameters {
         val klibFile: RegularFileProperty
         val includePrefixes: ListProperty<String>
+        val excludePackages: ListProperty<String>
         val resultFile: RegularFileProperty
     }
 
     override fun execute() {
         val klib = parameters.klibFile.get().asFile
         val includes = parameters.includePrefixes.get()
+        val excludes = parameters.excludePackages.get()
         val result = KlibScanResult(
-            callables = KlibScanner.scanKlib(klib, includes),
-            declarations = KlibScanner.scanKlibDeclarations(klib, includes),
+            callables = KlibScanner.scanKlib(klib, includes, excludes),
+            declarations = KlibScanner.scanKlibDeclarations(klib, includes, excludes),
         )
         ObjectOutputStream(parameters.resultFile.get().asFile.outputStream()).use { it.writeObject(result) }
     }
@@ -138,6 +140,7 @@ private fun pluginCodeSource(): File {
 internal fun WorkerExecutor.scanKlibIsolated(
     klib: File,
     includes: List<String>,
+    excludes: List<String>,
     scratchDirectory: File,
     readerClasspath: Collection<File>,
 ): KlibScanResult {
@@ -153,6 +156,7 @@ internal fun WorkerExecutor.scanKlibIsolated(
     queue.submit(KlibScanWorkAction::class.java) {
         klibFile.set(klib)
         includePrefixes.set(includes)
+        excludePackages.set(excludes)
         this.resultFile.set(resultFile)
     }
     queue.await()

python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/KlibScanner.kt

@@ -164,18 +164,18 @@ internal object KlibScanner {
     private val PUBLISHED_API = AbiQualifiedName(AbiCompoundName("kotlin"), AbiCompoundName("PublishedApi"))
 
     /** Every binding this klib offers, sorted by name -- the counterpart of [ArtifactScanner.scanJar]. */
-    fun scanKlib(klib: File, includePrefixes: List<String>): List<ArtifactCallable> =
-        scan(klib, includePrefixes).mapNotNull { it.callable }.sortedBy { it.name }
+    fun scanKlib(klib: File, includePrefixes: List<String>, excludePackages: List<String> = emptyList()): List<ArtifactCallable> =
+        scan(klib, includePrefixes, excludePackages).mapNotNull { it.callable }.sortedBy { it.name }
 
     /** The same walk with the other half of its result kept -- the counterpart of
      * [ArtifactScanner.scanDeclarations], sorted the same way so the two producers' models can be
      * concatenated without either one's order depending on which walked first. */
-    fun scanKlibDeclarations(klib: File, includePrefixes: List<String>): List<DeclarationModel> =
-        scan(klib, includePrefixes)
+    fun scanKlibDeclarations(klib: File, includePrefixes: List<String>, excludePackages: List<String> = emptyList()): List<DeclarationModel> =
+        scan(klib, includePrefixes, excludePackages)
             .map { it.declaration }
             .sortedWith(compareBy({ it.owner }, { it.simpleName }, { it.bindingName ?: "" }))
 
-    private fun scan(klib: File, includePrefixes: List<String>): List<Candidate> {
+    private fun scan(klib: File, includePrefixes: List<String>, excludePackages: List<String>): List<Candidate> {
         val abi = try {
             LibraryAbiReader.readAbiInfo(klib)
         } catch (e: Exception) {
@@ -185,6 +185,8 @@ internal object KlibScanner {
         val candidates = abi.topLevelDeclarations.declarations
             .filterIsInstance<AbiFunction>()
             .filter { matchesInclude(it.qualifiedName, includePrefixes) }
+            // SPEC B-11: `internal` packages and `excludePackages`, as on the JVM producer.
+            .filter { !ArtifactScanner.isExcludedPackage(it.qualifiedName.packageName.toString(), excludePackages) }
             // Not a *top-level* shape at all: a constructor belongs to an `AbiClass` and needs a
             // receiver handle to be worth anything. Filtered rather than declined for the same reason
             // `ArtifactScanner` never builds a model for `<init>`.

python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/PythonArtifactBindingsTask.kt

@@ -61,6 +61,10 @@ abstract class PythonArtifactBindingsTask : DefaultTask() {
     @get:Input
     abstract val includePrefixes: ListProperty<String>
 
+    /** Packages to leave out (`pythonBindings.excludePackages`), on top of every `internal` package; SPEC B-11. */
+    @get:Input
+    abstract val excludePackages: ListProperty<String>
+
     /**
      * The whole classpath of the isolated worker that runs [KlibScanner], bar this plugin's own jar:
      * the klib reader plus what [ArtifactScanner] needs, resolved by the consumer's project. See
@@ -105,6 +109,7 @@ abstract class PythonArtifactBindingsTask : DefaultTask() {
         thunkDestination.mkdirs()
 
         val includes = includePrefixes.get()
+        val excludes = excludePackages.get()
         val coordinates = coordinatesByFileName.get()
         val objectNames = mutableListOf<String>()
 
@@ -119,7 +124,7 @@ abstract class PythonArtifactBindingsTask : DefaultTask() {
             .sortedBy { it.name }
             .forEach { file ->
                 val entries = if (file.name.endsWith(".jar")) {
-                    ArtifactScanner.scanJar(file, includes, classpath = resolvedJars)
+                    ArtifactScanner.scanJar(file, includes, classpath = resolvedJars, excludePackages = excludes)
                 } else {
                     // The klib path takes no classpath yet, and whether it needs one is open
                     // rather than overlooked: the jar path needs other artefacts because an
@@ -128,7 +133,7 @@ abstract class PythonArtifactBindingsTask : DefaultTask() {
                     //
                     // Not a direct `KlibScanner.scanKlib` call: see this class's KDoc, "The klib
                     // branch runs behind a worker classloader boundary".
-                    workerExecutor.scanKlibIsolated(file, includes, temporaryDir, klibReaderClasspath.files).callables
+                    workerExecutor.scanKlibIsolated(file, includes, excludes, temporaryDir, klibReaderClasspath.files).callables
                 }
                 // An artefact that contributed nothing gets no fragment: a fragment with no entries
                 // would still take a `moduleName` in `UpcallTable`'s installed set, which is the one

python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PythonStubsTask.kt

@@ -66,6 +66,10 @@ abstract class PythonStubsTask : DefaultTask() {
     @get:Input
     abstract val includePrefixes: ListProperty<String>
 
+    /** Packages to leave out, the same set the binder gets; SPEC B-11. */
+    @get:Input
+    abstract val excludePackages: ListProperty<String>
+
     /**
      * The klib reader for the isolated worker, exactly as
      * [python.multiplatform.gradle.artifact.PythonArtifactBindingsTask.klibReaderClasspath] -- including
@@ -86,9 +90,10 @@ abstract class PythonStubsTask : DefaultTask() {
         destination.mkdirs()
 
         val includes = includePrefixes.get()
+        val excludes = excludePackages.get()
         val jars = artifacts.files.filter { it.isFile && it.name.endsWith(".jar") }
         val declarations = jars.sortedBy { it.name }.flatMap { jar ->
-            ArtifactScanner.scanDeclarations(jar, includes, classpath = jars)
+            ArtifactScanner.scanDeclarations(jar, includes, classpath = jars, excludePackages = excludes)
         } + artifacts.files.filter { it.isFile && it.name.endsWith(".klib") }.sortedBy { it.name }.flatMap { klib ->
             // A Kotlin/Native target resolves klibs where a JVM target resolves jars, and both
             // producers now build the same `DeclarationModel` -- so a stub is not a JVM-only product.
@@ -97,7 +102,7 @@ abstract class PythonStubsTask : DefaultTask() {
             //
             // Not a direct `KlibScanner.scanKlibDeclarations` call: same classloader-scope collision
             // `PythonArtifactBindingsTask`'s KDoc documents, worked around the same way.
-            workerExecutor.scanKlibIsolated(klib, includes, temporaryDir, klibReaderClasspath.files).declarations
+            workerExecutor.scanKlibIsolated(klib, includes, excludes, temporaryDir, klibReaderClasspath.files).declarations
         }
 
         val files = renderKotlinFqnStubs(declarations)

python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/InternalPackageExclusionTest.kt

@@ -0,0 +1,104 @@
+package python.multiplatform.gradle.artifact
+
+import python.multiplatform.gradle.stubs.renderKotlinFqnStubs
+import java.io.File
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+/**
+ * Issue #192 over `internalpackagefixture/`: the walker keeps public declarations of an `internal`
+ * package out of the bindings and the stubs, and honours `excludePackages` like the KSP side (SPEC B-11).
+ *
+ * ### Red before the implementation
+ *
+ * Every test here that passes `excludePackages =` fails to compile before the change (the parameter does
+ * not exist), which is the expected red. Once it compiles, [aPublicDeclarationInAnInternalPackageIsNotBound]
+ * and [theStubsOfAnInternalPackageAreNotEmitted] fail with the `.internal` names present, and the guards
+ * ([aNormalPackageStaysBound], [aPackageThatOnlyLooksInternalStaysBound], [withoutTheOptionAnExcludedPackageIsBound])
+ * stay green.
+ */
+class InternalPackageExclusionTest {
+
+    private val pkg = "fixture.artifactinternal"
+
+    private val fixtureClasses: File
+        get() {
+            val location = Class.forName("$pkg.Kept").protectionDomain.codeSource.location
+            return File(location.toURI()).also { assertTrue(it.isDirectory, "expected a directory of .class files: $it") }
+        }
+
+    private fun names(exclude: List<String> = emptyList()): Set<String> =
+        ArtifactScanner.scanJar(fixtureClasses, includePrefixes = listOf(pkg), excludePackages = exclude)
+            .map { it.name }.toSet()
+
+    @Test
+    fun aNormalPackageStaysBound() {
+        val bound = names()
+        assertTrue("$pkg.Kept" in bound, "bound: $bound")
+        assertTrue("$pkg.keptFunction" in bound, "bound: $bound")
+    }
+
+    @Test
+    fun aPublicDeclarationInAnInternalPackageIsNotBound() {
+        val bound = names()
+        assertEquals(emptyList(), bound.filter { ".internal." in it || it.endsWith(".internal") }.sorted(), "bound: $bound")
+        // Below an `internal` segment as well: `*.internal.*`.
+        assertEquals(emptyList(), bound.filter { "HiddenBelowInternal" in it || "hiddenBelowInternal" in it }, "bound: $bound")
+    }
+
+    @Test
+    fun aPackageThatOnlyLooksInternalStaysBound() {
+        val bound = names()
+        assertTrue("$pkg.internals.LooksInternal" in bound, "bound: $bound")
+        assertTrue("$pkg.internals.looksInternalFunction" in bound, "bound: $bound")
+    }
+
+    @Test
+    fun withoutTheOptionAnExcludedPackageIsBound() {
+        val bound = names()
+        assertTrue("$pkg.excluded.ExcludedByOption" in bound, "bound: $bound")
+        assertTrue("$pkg.excluded.excludedByOptionFunction" in bound, "bound: $bound")
+    }
+
+    @Test
+    fun excludePackagesKeepsAPackageAndItsSubpackagesOut() {
+        val bound = names(exclude = listOf("$pkg.excluded"))
+        assertEquals(emptyList(), bound.filter { ".excluded." in it }, "bound: $bound")
+        assertTrue("$pkg.Kept" in bound, "the control must still bind; bound: $bound")
+        assertTrue("$pkg.internals.LooksInternal" in bound, "bound: $bound")
+    }
+
+    @Test
+    fun excludePackagesMatchesANamespaceNotAPrefixOfText() {
+        // `fixture.artifactinternal.intern` is a text prefix of `...internals` and no package.
+        val bound = names(exclude = listOf("$pkg.intern"))
+        assertTrue("$pkg.internals.LooksInternal" in bound, "bound: $bound")
+        // An entry that names the package itself excludes it.
+        assertEquals(emptyList(), names(exclude = listOf("$pkg.internals")).filter { ".internals." in it })
+    }
+
+    @Test
+    fun noDeclarationModelIsKeptForAnInternalOrExcludedPackage() {
+        val owners = ArtifactScanner.scanDeclarations(
+            fixtureClasses, includePrefixes = listOf(pkg), excludePackages = listOf("$pkg.excluded"),
+        ).map { it.owner }.toSet()
+        assertEquals(emptyList(), owners.filter { owner -> owner.split('.').any { it == "internal" || it == "excluded" } }, "owners: $owners")
+        assertTrue(owners.any { it == pkg || it.startsWith("$pkg.Kept") }, "the control must still be modelled; owners: $owners")
+    }
+
+    @Test
+    fun theStubsOfAnInternalPackageAreNotEmitted() {
+        val files = renderKotlinFqnStubs(
+            ArtifactScanner.scanDeclarations(
+                fixtureClasses, includePrefixes = listOf(pkg), excludePackages = listOf("$pkg.excluded"),
+            ),
+        )
+        val paths = files.keys
+        assertTrue(paths.any { it.startsWith("fixture/artifactinternal/") }, "the control must be stubbed; paths: $paths")
+        assertEquals(emptyList(), paths.filter { "/internal/" in it || "/excluded/" in it }, "paths: $paths")
+        val text = files.values.joinToString("\n")
+        assertTrue("HiddenByInternal" !in text && "HiddenBelowInternal" !in text && "ExcludedByOption" !in text, "stub text leaks a skipped declaration")
+        assertTrue(paths.any { "/internals/" in it }, "paths: $paths")
+    }
+}

python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/KlibScannerTest.kt

@@ -151,12 +151,12 @@ class KlibScannerTest {
     @Test
     fun aPublishedApiInternalDeclarationIsDeclinedWithItsOwnReason() {
         val declarations = KlibScanner.scanKlibDeclarations(coroutinesKlib, includePrefixes = emptyList())
-        val checkIndexOverflow = declarations.single { it.simpleName == "checkIndexOverflow" }
-        assertEquals(null, checkIndexOverflow.bindingName)
-        assertTrue(
-            checkIndexOverflow.declineReason?.contains("PublishedApi") == true,
-            "expected a @PublishedApi decline reason, got ${checkIndexOverflow.declineReason}",
-        )
+        // checkIndexOverflow lives in kotlinx.coroutines.internal, which B-11 (#192) skips outright:
+        // it gets no model at all, so the @PublishedApi reason is checked on declarations elsewhere.
+        assertTrue(declarations.none { it.simpleName == "checkIndexOverflow" })
+        val publishedApi = declarations.filter { it.declineReason?.contains("PublishedApi") == true }
+        assertTrue(publishedApi.isNotEmpty(), "expected a @PublishedApi decline outside internal packages")
+        assertTrue(publishedApi.all { it.bindingName == null })
     }
 
     /**