Skip to content

Commit 8b19a7d

Browse files
authored
Merge pull request #49 from thisisthepy/work/stub-case
Fix: Stub paths are case-unique; Python-keyword Kotlin names left out
2 parents eb45de8 + 2d32062 commit 8b19a7d

5 files changed

Lines changed: 262 additions & 9 deletions

File tree

‎.github/workflows/stubs.yml‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ concurrency:
3232
jobs:
3333
stubs:
3434
name: Generate stubs
35-
# ubuntu, not macos: two Kotlin names can differ only by case (`graphics.Shadow` the class module and
36-
# `graphics.shadow` the package), which are two paths here and one on a case-insensitive filesystem.
3735
runs-on: ubuntu-latest
3836
permissions:
3937
contents: write # attaching the stubs to a tag's release
@@ -108,3 +106,44 @@ jobs:
108106
gh release view "$GITHUB_REF_NAME" >/dev/null 2>&1 \
109107
|| gh release create "$GITHUB_REF_NAME" --title "$GITHUB_REF_NAME" --notes "Kotlin-name stubs for the Compose version this tag builds against."
110108
gh release upload "$GITHUB_REF_NAME" build/kotlin-stubs.zip --clobber
109+
110+
# Issue #44: the stub layout is case-unique, so the stubs generate, extract and type-check on a
111+
# case-insensitive filesystem too (`graphics.Shadow` the class and `graphics.shadow` the package).
112+
stubs-macos:
113+
name: Generate stubs (macOS, case-insensitive)
114+
runs-on: macos-latest
115+
steps:
116+
- name: Checkout
117+
uses: actions/checkout@v4
118+
119+
- name: Setup Java
120+
uses: actions/setup-java@v4
121+
with:
122+
java-version: '21'
123+
distribution: 'zulu'
124+
cache: 'gradle'
125+
126+
- name: Setup Python (for mypy)
127+
uses: actions/setup-python@v5
128+
with:
129+
python-version: '3.13'
130+
131+
- name: Generate the stubs over the Compose jars
132+
run: ./gradlew :ksp-fixtures:compose:generatePythonStubs --console=plain
133+
134+
- name: No two stub paths are equal under case folding
135+
run: |
136+
python3 - ksp-fixtures/compose/build/generated/pythonStubs/desktopMain <<'PY'
137+
import os, sys
138+
seen = {}
139+
for root, dirs, files in os.walk(sys.argv[1]):
140+
for name in dirs + files:
141+
rel = os.path.relpath(os.path.join(root, name), sys.argv[1])
142+
seen.setdefault(rel.casefold(), []).append(rel)
143+
clashes = [v for v in seen.values() if len(v) > 1]
144+
print("clashes:", clashes)
145+
sys.exit(1 if clashes else 0)
146+
PY
147+
148+
- name: Type-check a consumer against them (mypy)
149+
run: tools/stubs/check-stubs.sh ksp-fixtures/compose/build/generated/pythonStubs/desktopMain

‎docs/design/pyi-generation-design.md‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,18 @@ turns `PaddingValues`, `TextStyle`, `Color`, `AnnotatedString` and `Dp` itself i
346346
collapses to its `float`). It is the honest loss of this design, not a bug to chase: the alternatives
347347
are a differently named class (a renamed spelling the binder does not have) or a private side module.
348348

349-
**Case-colliding module paths** (`androidx.compose.ui.graphics.Shadow`, a class module, and
350-
`...graphics.shadow`, a package) are two files on Linux and one on macOS and Windows.
351-
`PythonStubsTask` warns (`caseCollidingPaths`); generate and publish from a case-sensitive
352-
filesystem, which is why the CI job (`.github/workflows/stubs.yml`) runs on ubuntu.
349+
**Stub paths are case-unique** (issue #44). `androidx.compose.ui.graphics.Shadow` (a class the runtime
350+
exposes as a module) and `...graphics.shadow` (a package) would be one directory on macOS and Windows.
351+
The rule: a class module whose path equals a sibling's under case folding (directories above a module
352+
count) is emitted in its parent package's `__init__.pyi` -- as the class `Shadow`, its functions as
353+
`@staticmethod`s, nested class modules as nested classes -- and the package keeps `shadow/__init__.pyi`.
354+
In each colliding group the all-lower-case spelling is the package and keeps its path. A class module
355+
without a collision still gets its own `<Class>/__init__.pyi`. The runtime import path is unchanged; only
356+
the stub layout moves, and `from androidx.compose.ui.graphics import Shadow` resolves to the class.
357+
If the parent package already defines a function of the same name the class cannot sit beside it, and
358+
nothing is emitted for that module (the runtime resolves the name to the function too).
359+
`StubCaseLayoutTest` asserts no two emitted paths are equal under `lowercase()`;
360+
`PythonStubsTask` still warns (`caseCollidingPaths`) as a backstop, and the CI job runs on macOS as well.
353361

354362
### 3.2 Java declarations have no types to map
355363

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

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,32 @@ private class StubRenderer(private val bound: List<DeclarationModel>) {
159159
entries.flatMap { d -> leafOf(d).let { leaf -> if (isSuffixed(leaf)) listOf(leaf, baseOf(leaf)) else listOf(leaf) } }.toSet()
160160
}
161161

162-
private val moduleNames: Set<String> = byModule.keys
162+
/**
163+
* Module prefixes (every module and every directory above it) that name one directory on a
164+
* case-insensitive filesystem together with a sibling (issue #44). In each such group the
165+
* all-lower-case spelling keeps its path -- a Kotlin package -- and every other spelling is a
166+
* class module that is emitted inside its parent package's `__init__.pyi` instead.
167+
*/
168+
private val foldedPrefixes: Set<String> = run {
169+
val prefixes = byModule.keys.flatMap { module ->
170+
val parts = module.split('.')
171+
parts.indices.map { parts.take(it + 1).joinToString(".") }
172+
}.toSet()
173+
prefixes.groupBy { it.lowercase() }.values
174+
.filter { it.size > 1 }
175+
.flatten()
176+
.filter { '.' in it && it.substringAfterLast('.') != it.substringAfterLast('.').lowercase() }
177+
.toSet()
178+
}
179+
180+
/** The shortest folded prefix of [module] (the module itself included), or `null` if it keeps its own path. */
181+
private fun foldedPrefixOf(module: String): String? {
182+
val parts = module.split('.')
183+
return parts.indices.map { parts.take(it + 1).joinToString(".") }.firstOrNull { it in foldedPrefixes }
184+
}
185+
186+
/** The modules that are still files: a folded class module is a class, not a module, for `classReference`. */
187+
private val moduleNames: Set<String> = byModule.keys.filter { foldedPrefixOf(it) == null }.toSet()
163188

164189
private val supertypes: Map<String, List<String>> = bound
165190
.filter { it.returnSupertypes.isNotEmpty() }
@@ -283,6 +308,12 @@ private class StubRenderer(private val bound: List<DeclarationModel>) {
283308

284309
private fun renderDef(d: DeclarationModel, ctx: ModuleOut): String {
285310
val leaf = leafOf(d)
311+
// A Kotlin name that is a Python keyword (Compose's `Shadow.None`) cannot be written as an
312+
// attribute or a def -- `None: Shadow` makes the whole file a syntax error. Leave it out and
313+
// say how to reach it; the runtime still serves it under its Kotlin name.
314+
if (leaf in PYTHON_KEYWORDS) {
315+
return "# Kotlin: ${kotlinSignatureOf(d)} -- '$leaf' is a Python keyword; reach it with getattr(..., '$leaf')"
316+
}
286317
if (d.kind == "STATIC_GETTER") {
287318
return buildString {
288319
appendLine("$leaf: ${result(d, ctx)}")
@@ -297,7 +328,8 @@ private class StubRenderer(private val bound: List<DeclarationModel>) {
297328
}
298329

299330
private fun overloadDefs(name: String, group: List<DeclarationModel>, ctx: ModuleOut): String =
300-
group.withIndex().joinToString("\n") { (index, d) ->
331+
if (name in PYTHON_KEYWORDS) "# '$name' is a Python keyword; reach it with getattr(..., '$name')"
332+
else group.withIndex().joinToString("\n") { (index, d) ->
301333
val signature = "def $name(${parameters(d, ctx, method = false)}) -> ${result(d, ctx)}: ..." + shadowed(index)
302334
if (group.size > 1) "@$TYPING.overload\n$signature" else signature
303335
}
@@ -313,6 +345,11 @@ private class StubRenderer(private val bound: List<DeclarationModel>) {
313345

314346
private fun renderModuleDefs() {
315347
byModule.forEach { (module, entries) ->
348+
val folded = foldedPrefixOf(module)
349+
if (folded != null) {
350+
renderFoldedModule(module, folded, entries)
351+
return@forEach
352+
}
316353
val ctx = out(module)
317354
val sorted = entries.sortedBy { it.bindingName }
318355
sorted.forEach { ctx.defs += renderDef(it, ctx) }
@@ -327,6 +364,32 @@ private class StubRenderer(private val bound: List<DeclarationModel>) {
327364
}
328365
}
329366

367+
/**
368+
* A class module that collides case-insensitively with a sibling path (issue #44): its functions
369+
* become static members of the class of that name in the parent package's module, nested for the
370+
* parts of [module] below [folded]. The runtime import path is unchanged; only the stub layout is.
371+
* If the parent already defines a function of that name, the class cannot be declared beside it
372+
* (the runtime resolves the name to the function, as for `classReference`), so nothing is emitted.
373+
*/
374+
private fun renderFoldedModule(module: String, folded: String, entries: List<DeclarationModel>) {
375+
val parentModule = folded.substringBeforeLast('.')
376+
val className = folded.substringAfterLast('.')
377+
if (className in takenNames[parentModule].orEmpty()) return
378+
val ctx = out(parentModule)
379+
val path = listOf(className) + module.removePrefix(folded).split('.').filter { it.isNotEmpty() }
380+
val node = ctx.node(path)
381+
val sorted = entries.sortedBy { it.bindingName }
382+
val defs = sorted.map { renderDef(it, ctx) }
383+
val unsuffixedLeaves = sorted.filter { !isSuffixed(leafOf(it)) }.map { leafOf(it) }.toSet()
384+
val overloads = sorted.filter { it.kind != "STATIC_GETTER" && isSuffixed(leafOf(it)) }
385+
.groupBy { baseOf(leafOf(it)) }
386+
.toSortedMap()
387+
.mapNotNull { (base, group) -> if (base !in unsuffixedLeaves) overloadDefs(base, group, ctx) else null }
388+
(defs + overloads).forEach { text ->
389+
node.members += text.lines().joinToString("\n") { if (it.startsWith("def ")) "@staticmethod\n$it" else it }
390+
}
391+
}
392+
330393
// --------------------------------------------------------------------------- classes
331394

332395
/** 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<DeclarationModel>) {
398461
node.bases?.let { into.append('(').append(it).append(')') }
399462
into.appendLine(":")
400463
into.append(indent).append(" \"\"\"").append(node.doc ?: "Kotlin: container of nested classes").appendLine("\"\"\"")
401-
node.members.forEach { into.append(indent).append(" ").appendLine(it) }
464+
node.members.forEach { member ->
465+
member.lines().forEach { into.append(indent).append(" ").appendLine(it) }
466+
}
402467
node.children.values.forEach { renderNode(it, "$indent ", into) }
403468
}
404469

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package python.multiplatform.gradle.stubs
2+
3+
import python.multiplatform.gradle.model.DeclarationModel
4+
import python.multiplatform.gradle.model.DeclaredParameter
5+
import python.multiplatform.gradle.model.KotlinTypeModel
6+
import kotlin.test.Test
7+
import kotlin.test.assertEquals
8+
import kotlin.test.assertFalse
9+
import kotlin.test.assertTrue
10+
11+
/**
12+
* Issue #44: no two emitted stub paths may be equal under case folding. `androidx.compose.ui.graphics`
13+
* has a class module `Shadow` and a package `shadow`; on macOS and Windows those are one directory.
14+
*
15+
* The rule: a class module whose path collides case-insensitively with a sibling (or an ancestor of
16+
* one) is emitted inside its parent package's `__init__.pyi`, as the class, with its functions as
17+
* static methods. Modules that do not collide keep their own `<Class>/__init__.pyi`.
18+
*/
19+
class StubCaseLayoutTest {
20+
21+
private val graphics = "androidx.compose.ui.graphics"
22+
private val unit = KotlinTypeModel("kotlin.Unit")
23+
private val float = KotlinTypeModel("kotlin.Float")
24+
25+
private fun fn(binding: String, vararg params: DeclaredParameter) = DeclarationModel(
26+
simpleName = binding.substringAfterLast('.'),
27+
owner = binding.substringBeforeLast('.'),
28+
ownerIsClass = false,
29+
receiver = null,
30+
parameters = params.toList(),
31+
returnType = unit,
32+
returnBoundaryTag = "UNIT",
33+
bindingName = binding,
34+
)
35+
36+
private val blur = DeclaredParameter("radius", float, false, "FLOAT")
37+
38+
/** The Compose shape: a class module `Shadow` and a package `shadow`, plus an unrelated class module. */
39+
private val declarations = listOf(
40+
fn("$graphics.Shadow.create", blur),
41+
fn("$graphics.Shadow.Companion.default"),
42+
fn("$graphics.shadow.drawShadow", blur),
43+
fn("$graphics.Color.fromArgb", blur),
44+
fn("$graphics.draw"),
45+
)
46+
47+
private val files get() = renderKotlinFqnStubs(declarations)
48+
49+
/** Red against the renderer before #44: `Shadow/__init__.pyi` and `shadow/__init__.pyi` are both emitted. */
50+
@Test
51+
fun noTwoEmittedPathsAreEqualUnderCasefold() {
52+
val paths = files.keys
53+
val clashes = paths.groupBy { it.lowercase() }.values.filter { it.size > 1 }
54+
assertEquals(emptyList(), clashes, paths.sorted().toString())
55+
// Directories too: `Shadow/` without an `__init__.pyi` would still collide with `shadow/`.
56+
val directories = paths.flatMap { path ->
57+
val parts = path.split('/').dropLast(1)
58+
parts.indices.map { parts.take(it + 1).joinToString("/") }
59+
}.toSet()
60+
assertEquals(emptyList(), directories.groupBy { it.lowercase() }.values.filter { it.size > 1 }, directories.toString())
61+
}
62+
63+
@Test
64+
fun theCollidingClassModuleIsTheClassInItsParentsInit() {
65+
val parent = files.getValue("androidx/compose/ui/graphics/__init__.pyi")
66+
assertTrue("class Shadow:" in parent, parent)
67+
assertTrue(" @staticmethod\n def create(radius: float) -> None:" in parent, parent)
68+
// A nested class module goes into the nested class.
69+
assertTrue(" class Companion:" in parent, parent)
70+
assertTrue(" @staticmethod\n def default() -> None:" in parent, parent)
71+
assertTrue("def draw() -> None:" in parent, parent)
72+
}
73+
74+
@Test
75+
fun thePackageKeepsItsOwnPath() {
76+
val shadow = files.getValue("androidx/compose/ui/graphics/shadow/__init__.pyi")
77+
assertTrue("def drawShadow(radius: float) -> None:" in shadow, shadow)
78+
assertFalse("create" in shadow, shadow)
79+
}
80+
81+
@Test
82+
fun aClassModuleThatDoesNotCollideKeepsItsOwnPath() {
83+
val color = files.getValue("androidx/compose/ui/graphics/Color/__init__.pyi")
84+
assertTrue("def fromArgb(radius: float) -> None:" in color, color)
85+
assertFalse("class Color:" in files.getValue("androidx/compose/ui/graphics/__init__.pyi"))
86+
}
87+
88+
@Test
89+
fun withoutACollisionNothingMoves() {
90+
val noPackage = renderKotlinFqnStubs(declarations.filter { ".shadow." !in it.bindingName!! })
91+
assertTrue("androidx/compose/ui/graphics/Shadow/__init__.pyi" in noPackage.keys, noPackage.keys.toString())
92+
}
93+
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package python.multiplatform.gradle.stubs
2+
3+
import python.multiplatform.gradle.model.DeclarationModel
4+
import python.multiplatform.gradle.model.KotlinTypeModel
5+
import kotlin.test.Test
6+
import kotlin.test.assertFalse
7+
import kotlin.test.assertTrue
8+
9+
/**
10+
* A Kotlin declaration may be named like a Python keyword: Compose's `Shadow.None`. Written as
11+
* `None: Shadow` it makes the whole `.pyi` a syntax error, and mypy then checks nothing in that
12+
* module. Such a name cannot be written as an attribute in Python at all (`Shadow.None` does not
13+
* parse), so the stub leaves it out and says how to reach it.
14+
*/
15+
class StubKeywordNameTest {
16+
17+
private val graphics = "androidx.compose.ui.graphics"
18+
private val shadow = KotlinTypeModel("$graphics.Shadow")
19+
20+
private fun decl(binding: String, kind: String) = DeclarationModel(
21+
simpleName = binding.substringAfterLast('.'),
22+
owner = binding.substringBeforeLast('.'),
23+
ownerIsClass = false,
24+
receiver = null,
25+
parameters = emptyList(),
26+
returnType = shadow,
27+
returnBoundaryTag = "OBJECT",
28+
bindingName = binding,
29+
kind = kind,
30+
)
31+
32+
private val keywordLine = Regex("""^\s*(None|True|False|class|def|from|import|lambda|pass)\s*[:(]""", RegexOption.MULTILINE)
33+
34+
@Test
35+
fun aConstantNamedLikeAPythonKeywordIsNotWrittenAsAnAttribute() {
36+
val files = renderKotlinFqnStubs(listOf(decl("$graphics.Shadow.None", "STATIC_GETTER")))
37+
val text = files.values.joinToString("\n")
38+
assertFalse(keywordLine.containsMatchIn(text), text)
39+
assertTrue("getattr" in text && "None" in text, "the stub says how to reach it:\n$text")
40+
}
41+
42+
@Test
43+
fun aFunctionNamedLikeAPythonKeywordIsNotWrittenAsADef() {
44+
val files = renderKotlinFqnStubs(listOf(decl("$graphics.Shadow.from", "FUNCTION")))
45+
val text = files.values.joinToString("\n")
46+
assertFalse(Regex("""^\s*def from\(""", RegexOption.MULTILINE).containsMatchIn(text), text)
47+
}
48+
}

0 commit comments

Comments
 (0)