From a8bb8f10b59b651d5fb301400d6b085081bd2b29 Mon Sep 17 00:00:00 2001 From: BREW <81044362+b-re-w@users.noreply.github.com> Date: Sat, 3 Oct 2026 09:00:43 +0900 Subject: [PATCH] Fix: Stub paths are case-unique, and Kotlin names that are Python keywords are left out A class module whose path collides case-insensitively with a sibling (Compose's graphics.Shadow vs the graphics.shadow package) is emitted as a class inside its parent's __init__.pyi; non-colliding class modules keep their own path. macOS and Windows can now extract and type-check the stub set. That exposed a second defect: Shadow.None was written as 'None: Shadow', a syntax error that made mypy skip the whole module. A Kotlin name that is a Python keyword cannot be an attribute or def in Python at all; the stub now leaves it out with a comment naming getattr(). A stubs-macos CI job checks both. --- .github/workflows/stubs.yml | 43 ++++++++- docs/design/pyi-generation-design.md | 16 +++- .../gradle/stubs/PyiRendering.kt | 71 +++++++++++++- .../gradle/stubs/StubCaseLayoutTest.kt | 93 +++++++++++++++++++ .../gradle/stubs/StubKeywordNameTest.kt | 48 ++++++++++ 5 files changed, 262 insertions(+), 9 deletions(-) create mode 100644 python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubCaseLayoutTest.kt create mode 100644 python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubKeywordNameTest.kt diff --git a/.github/workflows/stubs.yml b/.github/workflows/stubs.yml index fa96d92da..b6a7a785d 100644 --- a/.github/workflows/stubs.yml +++ b/.github/workflows/stubs.yml @@ -32,8 +32,6 @@ concurrency: jobs: stubs: name: Generate stubs - # ubuntu, not macos: two Kotlin names can differ only by case (`graphics.Shadow` the class module and - # `graphics.shadow` the package), which are two paths here and one on a case-insensitive filesystem. runs-on: ubuntu-latest permissions: contents: write # attaching the stubs to a tag's release @@ -108,3 +106,44 @@ jobs: gh release view "$GITHUB_REF_NAME" >/dev/null 2>&1 \ || gh release create "$GITHUB_REF_NAME" --title "$GITHUB_REF_NAME" --notes "Kotlin-name stubs for the Compose version this tag builds against." gh release upload "$GITHUB_REF_NAME" build/kotlin-stubs.zip --clobber + + # Issue #44: the stub layout is case-unique, so the stubs generate, extract and type-check on a + # case-insensitive filesystem too (`graphics.Shadow` the class and `graphics.shadow` the package). + stubs-macos: + name: Generate stubs (macOS, case-insensitive) + runs-on: macos-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Java + uses: actions/setup-java@v4 + with: + java-version: '21' + distribution: 'zulu' + cache: 'gradle' + + - name: Setup Python (for mypy) + uses: actions/setup-python@v5 + with: + python-version: '3.13' + + - name: Generate the stubs over the Compose jars + run: ./gradlew :ksp-fixtures:compose:generatePythonStubs --console=plain + + - name: No two stub paths are equal under case folding + run: | + python3 - ksp-fixtures/compose/build/generated/pythonStubs/desktopMain <<'PY' + import os, sys + seen = {} + for root, dirs, files in os.walk(sys.argv[1]): + for name in dirs + files: + rel = os.path.relpath(os.path.join(root, name), sys.argv[1]) + seen.setdefault(rel.casefold(), []).append(rel) + clashes = [v for v in seen.values() if len(v) > 1] + print("clashes:", clashes) + sys.exit(1 if clashes else 0) + PY + + - name: Type-check a consumer against them (mypy) + run: tools/stubs/check-stubs.sh ksp-fixtures/compose/build/generated/pythonStubs/desktopMain diff --git a/docs/design/pyi-generation-design.md b/docs/design/pyi-generation-design.md index bd618aba2..59eb1732c 100644 --- a/docs/design/pyi-generation-design.md +++ b/docs/design/pyi-generation-design.md @@ -346,10 +346,18 @@ turns `PaddingValues`, `TextStyle`, `Color`, `AnnotatedString` and `Dp` itself i collapses to its `float`). It is the honest loss of this design, not a bug to chase: the alternatives are a differently named class (a renamed spelling the binder does not have) or a private side module. -**Case-colliding module paths** (`androidx.compose.ui.graphics.Shadow`, a class module, and -`...graphics.shadow`, a package) are two files on Linux and one on macOS and Windows. -`PythonStubsTask` warns (`caseCollidingPaths`); generate and publish from a case-sensitive -filesystem, which is why the CI job (`.github/workflows/stubs.yml`) runs on ubuntu. +**Stub paths are case-unique** (issue #44). `androidx.compose.ui.graphics.Shadow` (a class the runtime +exposes as a module) and `...graphics.shadow` (a package) would be one directory on macOS and Windows. +The rule: a class module whose path equals a sibling's under case folding (directories above a module +count) is emitted in its parent package's `__init__.pyi` -- as the class `Shadow`, its functions as +`@staticmethod`s, nested class modules as nested classes -- and the package keeps `shadow/__init__.pyi`. +In each colliding group the all-lower-case spelling is the package and keeps its path. A class module +without a collision still gets its own `/__init__.pyi`. The runtime import path is unchanged; only +the stub layout moves, and `from androidx.compose.ui.graphics import Shadow` resolves to the class. +If the parent package already defines a function of the same name the class cannot sit beside it, and +nothing is emitted for that module (the runtime resolves the name to the function too). +`StubCaseLayoutTest` asserts no two emitted paths are equal under `lowercase()`; +`PythonStubsTask` still warns (`caseCollidingPaths`) as a backstop, and the CI job runs on macOS as well. ### 3.2 Java declarations have no types to map diff --git a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt index 3bfd0e88d..3c3c27dc0 100644 --- a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt +++ b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt @@ -159,7 +159,32 @@ private class StubRenderer(private val bound: List) { entries.flatMap { d -> leafOf(d).let { leaf -> if (isSuffixed(leaf)) listOf(leaf, baseOf(leaf)) else listOf(leaf) } }.toSet() } - private val moduleNames: Set = byModule.keys + /** + * Module prefixes (every module and every directory above it) that name one directory on a + * case-insensitive filesystem together with a sibling (issue #44). In each such group the + * all-lower-case spelling keeps its path -- a Kotlin package -- and every other spelling is a + * class module that is emitted inside its parent package's `__init__.pyi` instead. + */ + private val foldedPrefixes: Set = run { + val prefixes = byModule.keys.flatMap { module -> + val parts = module.split('.') + parts.indices.map { parts.take(it + 1).joinToString(".") } + }.toSet() + prefixes.groupBy { it.lowercase() }.values + .filter { it.size > 1 } + .flatten() + .filter { '.' in it && it.substringAfterLast('.') != it.substringAfterLast('.').lowercase() } + .toSet() + } + + /** The shortest folded prefix of [module] (the module itself included), or `null` if it keeps its own path. */ + private fun foldedPrefixOf(module: String): String? { + val parts = module.split('.') + return parts.indices.map { parts.take(it + 1).joinToString(".") }.firstOrNull { it in foldedPrefixes } + } + + /** The modules that are still files: a folded class module is a class, not a module, for `classReference`. */ + private val moduleNames: Set = byModule.keys.filter { foldedPrefixOf(it) == null }.toSet() private val supertypes: Map> = bound .filter { it.returnSupertypes.isNotEmpty() } @@ -283,6 +308,12 @@ private class StubRenderer(private val bound: List) { private fun renderDef(d: DeclarationModel, ctx: ModuleOut): String { val leaf = leafOf(d) + // A Kotlin name that is a Python keyword (Compose's `Shadow.None`) cannot be written as an + // attribute or a def -- `None: Shadow` makes the whole file a syntax error. Leave it out and + // say how to reach it; the runtime still serves it under its Kotlin name. + if (leaf in PYTHON_KEYWORDS) { + return "# Kotlin: ${kotlinSignatureOf(d)} -- '$leaf' is a Python keyword; reach it with getattr(..., '$leaf')" + } if (d.kind == "STATIC_GETTER") { return buildString { appendLine("$leaf: ${result(d, ctx)}") @@ -297,7 +328,8 @@ private class StubRenderer(private val bound: List) { } private fun overloadDefs(name: String, group: List, ctx: ModuleOut): String = - group.withIndex().joinToString("\n") { (index, d) -> + if (name in PYTHON_KEYWORDS) "# '$name' is a Python keyword; reach it with getattr(..., '$name')" + else group.withIndex().joinToString("\n") { (index, d) -> val signature = "def $name(${parameters(d, ctx, method = false)}) -> ${result(d, ctx)}: ..." + shadowed(index) if (group.size > 1) "@$TYPING.overload\n$signature" else signature } @@ -313,6 +345,11 @@ private class StubRenderer(private val bound: List) { private fun renderModuleDefs() { byModule.forEach { (module, entries) -> + val folded = foldedPrefixOf(module) + if (folded != null) { + renderFoldedModule(module, folded, entries) + return@forEach + } val ctx = out(module) val sorted = entries.sortedBy { it.bindingName } sorted.forEach { ctx.defs += renderDef(it, ctx) } @@ -327,6 +364,32 @@ private class StubRenderer(private val bound: List) { } } + /** + * A class module that collides case-insensitively with a sibling path (issue #44): its functions + * become static members of the class of that name in the parent package's module, nested for the + * parts of [module] below [folded]. The runtime import path is unchanged; only the stub layout is. + * If the parent already defines a function of that name, the class cannot be declared beside it + * (the runtime resolves the name to the function, as for `classReference`), so nothing is emitted. + */ + private fun renderFoldedModule(module: String, folded: String, entries: List) { + val parentModule = folded.substringBeforeLast('.') + val className = folded.substringAfterLast('.') + if (className in takenNames[parentModule].orEmpty()) return + val ctx = out(parentModule) + val path = listOf(className) + module.removePrefix(folded).split('.').filter { it.isNotEmpty() } + val node = ctx.node(path) + val sorted = entries.sortedBy { it.bindingName } + val defs = sorted.map { renderDef(it, ctx) } + val unsuffixedLeaves = sorted.filter { !isSuffixed(leafOf(it)) }.map { leafOf(it) }.toSet() + val overloads = sorted.filter { it.kind != "STATIC_GETTER" && isSuffixed(leafOf(it)) } + .groupBy { baseOf(leafOf(it)) } + .toSortedMap() + .mapNotNull { (base, group) -> if (base !in unsuffixedLeaves) overloadDefs(base, group, ctx) else null } + (defs + overloads).forEach { text -> + node.members += text.lines().joinToString("\n") { if (it.startsWith("def ")) "@staticmethod\n$it" else it } + } + } + // --------------------------------------------------------------------------- classes /** The class a declaration's extension receiver names, when the runtime attaches it as a method. */ @@ -398,7 +461,9 @@ private class StubRenderer(private val bound: List) { node.bases?.let { into.append('(').append(it).append(')') } into.appendLine(":") into.append(indent).append(" \"\"\"").append(node.doc ?: "Kotlin: container of nested classes").appendLine("\"\"\"") - node.members.forEach { into.append(indent).append(" ").appendLine(it) } + node.members.forEach { member -> + member.lines().forEach { into.append(indent).append(" ").appendLine(it) } + } node.children.values.forEach { renderNode(it, "$indent ", into) } } diff --git a/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubCaseLayoutTest.kt b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubCaseLayoutTest.kt new file mode 100644 index 000000000..0ffc91cf7 --- /dev/null +++ b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubCaseLayoutTest.kt @@ -0,0 +1,93 @@ +package python.multiplatform.gradle.stubs + +import python.multiplatform.gradle.model.DeclarationModel +import python.multiplatform.gradle.model.DeclaredParameter +import python.multiplatform.gradle.model.KotlinTypeModel +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Issue #44: no two emitted stub paths may be equal under case folding. `androidx.compose.ui.graphics` + * has a class module `Shadow` and a package `shadow`; on macOS and Windows those are one directory. + * + * The rule: a class module whose path collides case-insensitively with a sibling (or an ancestor of + * one) is emitted inside its parent package's `__init__.pyi`, as the class, with its functions as + * static methods. Modules that do not collide keep their own `/__init__.pyi`. + */ +class StubCaseLayoutTest { + + private val graphics = "androidx.compose.ui.graphics" + private val unit = KotlinTypeModel("kotlin.Unit") + private val float = KotlinTypeModel("kotlin.Float") + + private fun fn(binding: String, vararg params: DeclaredParameter) = DeclarationModel( + simpleName = binding.substringAfterLast('.'), + owner = binding.substringBeforeLast('.'), + ownerIsClass = false, + receiver = null, + parameters = params.toList(), + returnType = unit, + returnBoundaryTag = "UNIT", + bindingName = binding, + ) + + private val blur = DeclaredParameter("radius", float, false, "FLOAT") + + /** The Compose shape: a class module `Shadow` and a package `shadow`, plus an unrelated class module. */ + private val declarations = listOf( + fn("$graphics.Shadow.create", blur), + fn("$graphics.Shadow.Companion.default"), + fn("$graphics.shadow.drawShadow", blur), + fn("$graphics.Color.fromArgb", blur), + fn("$graphics.draw"), + ) + + private val files get() = renderKotlinFqnStubs(declarations) + + /** Red against the renderer before #44: `Shadow/__init__.pyi` and `shadow/__init__.pyi` are both emitted. */ + @Test + fun noTwoEmittedPathsAreEqualUnderCasefold() { + val paths = files.keys + val clashes = paths.groupBy { it.lowercase() }.values.filter { it.size > 1 } + assertEquals(emptyList(), clashes, paths.sorted().toString()) + // Directories too: `Shadow/` without an `__init__.pyi` would still collide with `shadow/`. + val directories = paths.flatMap { path -> + val parts = path.split('/').dropLast(1) + parts.indices.map { parts.take(it + 1).joinToString("/") } + }.toSet() + assertEquals(emptyList(), directories.groupBy { it.lowercase() }.values.filter { it.size > 1 }, directories.toString()) + } + + @Test + fun theCollidingClassModuleIsTheClassInItsParentsInit() { + val parent = files.getValue("androidx/compose/ui/graphics/__init__.pyi") + assertTrue("class Shadow:" in parent, parent) + assertTrue(" @staticmethod\n def create(radius: float) -> None:" in parent, parent) + // A nested class module goes into the nested class. + assertTrue(" class Companion:" in parent, parent) + assertTrue(" @staticmethod\n def default() -> None:" in parent, parent) + assertTrue("def draw() -> None:" in parent, parent) + } + + @Test + fun thePackageKeepsItsOwnPath() { + val shadow = files.getValue("androidx/compose/ui/graphics/shadow/__init__.pyi") + assertTrue("def drawShadow(radius: float) -> None:" in shadow, shadow) + assertFalse("create" in shadow, shadow) + } + + @Test + fun aClassModuleThatDoesNotCollideKeepsItsOwnPath() { + val color = files.getValue("androidx/compose/ui/graphics/Color/__init__.pyi") + assertTrue("def fromArgb(radius: float) -> None:" in color, color) + assertFalse("class Color:" in files.getValue("androidx/compose/ui/graphics/__init__.pyi")) + } + + @Test + fun withoutACollisionNothingMoves() { + val noPackage = renderKotlinFqnStubs(declarations.filter { ".shadow." !in it.bindingName!! }) + assertTrue("androidx/compose/ui/graphics/Shadow/__init__.pyi" in noPackage.keys, noPackage.keys.toString()) + } +} diff --git a/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubKeywordNameTest.kt b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubKeywordNameTest.kt new file mode 100644 index 000000000..9f9addcb2 --- /dev/null +++ b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/StubKeywordNameTest.kt @@ -0,0 +1,48 @@ +package python.multiplatform.gradle.stubs + +import python.multiplatform.gradle.model.DeclarationModel +import python.multiplatform.gradle.model.KotlinTypeModel +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * A Kotlin declaration may be named like a Python keyword: Compose's `Shadow.None`. Written as + * `None: Shadow` it makes the whole `.pyi` a syntax error, and mypy then checks nothing in that + * module. Such a name cannot be written as an attribute in Python at all (`Shadow.None` does not + * parse), so the stub leaves it out and says how to reach it. + */ +class StubKeywordNameTest { + + private val graphics = "androidx.compose.ui.graphics" + private val shadow = KotlinTypeModel("$graphics.Shadow") + + private fun decl(binding: String, kind: String) = DeclarationModel( + simpleName = binding.substringAfterLast('.'), + owner = binding.substringBeforeLast('.'), + ownerIsClass = false, + receiver = null, + parameters = emptyList(), + returnType = shadow, + returnBoundaryTag = "OBJECT", + bindingName = binding, + kind = kind, + ) + + private val keywordLine = Regex("""^\s*(None|True|False|class|def|from|import|lambda|pass)\s*[:(]""", RegexOption.MULTILINE) + + @Test + fun aConstantNamedLikeAPythonKeywordIsNotWrittenAsAnAttribute() { + val files = renderKotlinFqnStubs(listOf(decl("$graphics.Shadow.None", "STATIC_GETTER"))) + val text = files.values.joinToString("\n") + assertFalse(keywordLine.containsMatchIn(text), text) + assertTrue("getattr" in text && "None" in text, "the stub says how to reach it:\n$text") + } + + @Test + fun aFunctionNamedLikeAPythonKeywordIsNotWrittenAsADef() { + val files = renderKotlinFqnStubs(listOf(decl("$graphics.Shadow.from", "FUNCTION"))) + val text = files.values.joinToString("\n") + assertFalse(Regex("""^\s*def from\(""", RegexOption.MULTILINE).containsMatchIn(text), text) + } +}