diff --git a/docs/SPEC.md b/docs/SPEC.md index 1458c2275..f356d2444 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -252,6 +252,24 @@ All in `PM/commonTest`, so they run wherever the interpreter loads. statics, abstract classes (no constructor), nested classes. Not exposed: annotation classes, generic declarations, data-class synthetics. `Status: implemented` on desktop, `GeneratedDeclarationKindsTest.kt`. + **A companion object is a value** (#322): the public companion object of a public class is the value + `Owner.Companion` (`Owner.` for a named companion), read as an attribute of the owner's + proxy class or of its module. When the companion lists the owner as a direct supertype in its Kotlin + metadata (`Modifier.Companion : Modifier`) the value is an `Owner` proxy instance (`isinstance` holds), + otherwise a proxy of the companion's own type. Each read returns a new handle to the same Kotlin object. Two proxies are `==` when they hold the same + Kotlin object (reference identity) and hash by its identity hash; `is` is not guaranteed; Kotlin + `equals` and `hashCode` are not used (value equality is not provided). On wasmJs, which has no + identity hash, the hash is the object's `hashCode()`, so there a mutable object that overrides it + should not be used as a dict key or set member. A non-public companion + binds nothing; a generic owner is named without type arguments. A class-level constant of the + companion is reachable through the proxy class (`Owner.Default`). `register_empty(type, factory)` + takes the type as its Kotlin name or its proxy class, and the factory as a Kotlin function name (string) + or a Python callable returning an owned proxy (`register_empty(Modifier, lambda: Modifier.Companion)`); + both forms work. `Status: implemented` on desktop, + `GP/artifact/CompanionHandleBindingTest.kt` (walker), `GP/stubs/CompanionHandleStubTest.kt` (stub), + `PM/desktopTest/.../pythonx/CompanionHandleTest.kt` (runtime), + `python-multiplatform-gradle-plugin/fixtures/artifact/.../WalkedArtifactCompanionHandleTest.kt`, + `python-multiplatform-compose/fixtures/compose/.../ProxyClassArgumentRenderTest.kt`. - **U-5** `suspend` functions are awaitable from Python (`await g.greetNow(1)`); a function that never suspends completes without a Future; cancelling the Python future cancels the Kotlin coroutine. `Status: implemented` on desktop and Kotlin/Native, `PM/desktopTest/.../upcall/AsyncUpcallDeliveryTest.kt`, diff --git a/python-multiplatform-compose/fixtures/compose/src/desktopTest/kotlin/fixture/compose/ProxyClassArgumentRenderTest.kt b/python-multiplatform-compose/fixtures/compose/src/desktopTest/kotlin/fixture/compose/ProxyClassArgumentRenderTest.kt index 051d0a553..12a7e6442 100644 --- a/python-multiplatform-compose/fixtures/compose/src/desktopTest/kotlin/fixture/compose/ProxyClassArgumentRenderTest.kt +++ b/python-multiplatform-compose/fixtures/compose/src/desktopTest/kotlin/fixture/compose/ProxyClassArgumentRenderTest.kt @@ -57,6 +57,25 @@ class ProxyClassArgumentRenderTest { assertEquals(viaInstance, viaClass, "the class spelling must draw what the instance spelling draws") } + /** + * Issue #322: the empty value is `Modifier.Companion` itself, read from Python (no Kotlin + * `emptyModifier` helper): `register_empty(Modifier, lambda: Modifier.Companion)`. + */ + @Test + fun aModifierClassPassedToCardIsTheCompanionRegisteredFromPython() { + val setup = """ + import python_multiplatform.binding as _pm_binding + from androidx.compose.ui import Modifier + from androidx.compose.material3 import Card, Text + _pm_binding.register_empty(Modifier, lambda: Modifier.Companion) + """.trimIndent() + val viaClass = inkOf(setup + "\nCard(modifier=Modifier, content=lambda scope: Text('hi'))") + val withoutModifier = inkOf(setup + "\nCard(content=lambda scope: Text('hi'))") + println("compose render: Card(modifier=Modifier) -> $viaClass px, Card() -> $withoutModifier px") + assertTrue(viaClass > 0, "Card(modifier=Modifier) drew nothing") + assertEquals(withoutModifier, viaClass, "the class spelling must draw what Card() draws") + } + private fun inkOf(body: String): Int { val scene = ImageComposeScene(width = 200, height = 80, density = Density(1f)) { PythonComposition(body) diff --git a/python-multiplatform-gradle-plugin/fixtures/artifact/build.gradle.kts b/python-multiplatform-gradle-plugin/fixtures/artifact/build.gradle.kts index 3a3a84b18..b65538238 100644 --- a/python-multiplatform-gradle-plugin/fixtures/artifact/build.gradle.kts +++ b/python-multiplatform-gradle-plugin/fixtures/artifact/build.gradle.kts @@ -131,6 +131,9 @@ pythonBindings { // so it can only ever be held in Python as a boxed handle (issue #168). "androidx.compose.ui.unit.TextUnitKt", "androidx.compose.ui.unit.TextUnitType", + // `Modifier` is an interface whose companion is itself a `Modifier` (issue #322): the + // companion handle `Modifier.Companion` is what `WalkedArtifactCompanionHandleTest` reads. + "androidx.compose.ui.Modifier", ), ) } diff --git a/python-multiplatform-gradle-plugin/fixtures/artifact/src/desktopTest/kotlin/fixture/artifact/WalkedArtifactCompanionHandleTest.kt b/python-multiplatform-gradle-plugin/fixtures/artifact/src/desktopTest/kotlin/fixture/artifact/WalkedArtifactCompanionHandleTest.kt new file mode 100644 index 000000000..e2ed568ac --- /dev/null +++ b/python-multiplatform-gradle-plugin/fixtures/artifact/src/desktopTest/kotlin/fixture/artifact/WalkedArtifactCompanionHandleTest.kt @@ -0,0 +1,60 @@ +package fixture.artifact + +import python.multiplatform.ffi.Python3 +import python.multiplatform.ffi.pythonx.PythonxAdapter +import python.multiplatform.ffi.upcall.PythonProxySource +import python.multiplatform.ffi.upcall.UpcallBootstrap +import python.multiplatform.generated.FunctionTable +import python.multiplatform.generated.artifacts.ArtifactTable +import python.multiplatform.reflection.UpcallTable +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertTrue + +/** + * Issue #322 (SPEC U-4, B-1): `Modifier.Companion` from the real Compose jars is a `Modifier` from + * Python. The walker binds the companion object itself (`androidx.compose.ui.Modifier.Companion`, + * generated Kotlin `Modifier.Companion`); the proxy class serves it as an attribute. + * + * Red before the change: no `androidx.compose.ui.Modifier.Companion` key is bound, so the attribute + * read raises AttributeError. + */ +class WalkedArtifactCompanionHandleTest { + + @BeforeTest + fun install() { + Python3.initialize(silent = true) + UpcallTable.clear() + UpcallTable.install(FunctionTable.fragments + ArtifactTable.fragments) + check(UpcallBootstrap.publishToGlobals()) { "UpcallBootstrap.publishToGlobals() failed" } + PythonProxySource.install() + PythonxAdapter.install(PYTHONX_RAW_VALUE_CLASSES) + } + + @AfterTest + fun cleanup() { + UpcallTable.clear() + } + + @Test + fun theCompanionKeyIsBound() { + assertTrue(UpcallTable.resolve("androidx.compose.ui.Modifier.Companion").isValid) + } + + @Test + fun modifierCompanionIsAModifierAndTwoReadsCompareEqual() { + Python3.exec( + """ + from androidx.compose.ui import Modifier + from fixture.artifact import isTheEmptyModifier + _a = Modifier.Companion + _b = Modifier.Companion + assert isinstance(_a, Modifier), type(_a) + assert _a == _b, 'two reads of Modifier.Companion are not ==' + assert isTheEmptyModifier(_a), 'Modifier.Companion is not Compose\'s empty Modifier' + assert not hasattr(Modifier, 'companion'), 'a lower-case companion alias must not exist' + """.trimIndent(), + ) + } +} diff --git a/python-multiplatform-gradle-plugin/fixtures/artifact/src/desktopTest/kotlin/fixture/artifact/WalkedArtifactStubTest.kt b/python-multiplatform-gradle-plugin/fixtures/artifact/src/desktopTest/kotlin/fixture/artifact/WalkedArtifactStubTest.kt index ee91ab5e8..b74b971cc 100644 --- a/python-multiplatform-gradle-plugin/fixtures/artifact/src/desktopTest/kotlin/fixture/artifact/WalkedArtifactStubTest.kt +++ b/python-multiplatform-gradle-plugin/fixtures/artifact/src/desktopTest/kotlin/fixture/artifact/WalkedArtifactStubTest.kt @@ -249,12 +249,23 @@ class WalkedArtifactStubTest { assertEquals(emptySet(), members - memberKeys, "a member stub names no member key") assertEquals(emptySet(), memberKeys - members, "a member key has no stub") + // A sixth shape (issue #322): a companion handle (`pkg.Owner.Companion`) is a class attribute of its + // owner's stub class, or a comment saying why not; either way it carries its key after + // `Kotlin companion: `. Matched by key, like a property. + val companions = generatedPaths() + .filter { it.endsWith(".pyi") } + .flatMap { path -> + Regex("""Kotlin companion: ([^\s"]+)""").findAll(stub(path)).map { it.groupValues[1] }.toList() + } + .toSet() + assertEquals(emptySet(), companions - installed, "a companion stub names no table key") + // The base name of an overload set (`padding` for `padding__Dp`, ...) is served by the binding // layer's dispatcher and is stubbed as `@overload`s; it is not itself a table key. val overloadBases = installed.map { it.substringBeforeLast('.') + "." + it.substringAfterLast('.').substringBefore("__") } .filter { it !in installed }.toSet() assertEquals(emptySet(), stubbed - installed - overloadBases, "stubbed but not callable") - assertEquals(emptySet(), installed - propertyKeys - memberKeys - stubbed - constructors, "callable but not stubbed") + assertEquals(emptySet(), installed - propertyKeys - memberKeys - stubbed - constructors - companions, "callable but not stubbed") } /** diff --git a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/ArtifactScanner.kt b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/ArtifactScanner.kt index 8984d5cd2..70babf976 100644 --- a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/ArtifactScanner.kt +++ b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/artifact/ArtifactScanner.kt @@ -163,6 +163,12 @@ internal data class Candidate( */ val plan: ArtifactScanner.DefaultOmissionPlan? = null, val isConstructor: Boolean = false, + /** + * True only for the candidate that publishes a companion object itself (`Owner.Companion`). It + * shares the owner's name as a prefix, which the constructor-collision rule would otherwise + * read as a constant hiding the constructor; the owner stays constructible beside its companion. + */ + val isCompanionHandle: Boolean = false, ) internal object ArtifactScanner { @@ -342,7 +348,8 @@ internal object ArtifactScanner { for (candidate in bound) { if (!candidate.isConstructor) continue val name = candidate.callable!!.name - if (boundNames.any { it != name && it.startsWith("$name.") }) { + val others = bound.filter { !it.isCompanionHandle }.map { it.callable!!.name } + if (others.any { it != name && it.startsWith("$name.") }) { dropReasons.putIfAbsent(candidate, "constructor name collides with a constant this class publishes under its own name") } } @@ -767,10 +774,12 @@ internal object ArtifactScanner { // never read as an entry. It is loaded from here instead, by the name the outer class's own // metadata gives it -- which is also why the bound name can drop `Companion` while the // generated call keeps it. + var companionHandle: Candidate? = null val companionProperties = kmClass.companionObject?.let { companionName -> val companionNode = classpath.classNode(ownerNode.name + "\$" + companionName) ?: return@let null val companionMetadata = kotlinClassMetadataOf(companionNode) as? KotlinClassMetadata.Class ?: return@let null if (companionMetadata.kmClass.visibility != Visibility.PUBLIC) return@let null + companionHandle = companionHandleCandidate(qualified, ownerNode, companionName, companionMetadata.kmClass, classpath) constantsOf( companionMetadata.kmClass, companionNode, @@ -780,7 +789,71 @@ internal object ArtifactScanner { ) }.orEmpty() - return ownProperties + companionProperties + return ownProperties + companionProperties + listOfNotNull(companionHandle) + } + + /** + * The companion object itself, bound as `Owner.` (a zero-argument getter). + * + * The generated body names the companion in Kotlin source (`Owner.Companion`), which kotlinc + * compiles to a field read; no JVM member is looked up by name. The value is typed as the owner + * when the companion lists the owner as a direct supertype (`Modifier.Companion : Modifier`), so + * it is an instance of the owner's proxy; otherwise as the companion's own type. A generic owner + * is named without type arguments. + */ + private fun companionHandleCandidate( + qualified: String, + ownerNode: ClassNode, + companionName: String, + companionClass: kotlin.metadata.KmClass, + classpath: ArtifactClasspath, + ): Candidate { + val ownerMetadataName = ownerNode.name.replace('$', '.') + val companionMetadataName = "$ownerMetadataName.$companionName" + val ownerIsSupertype = companionClass.supertypes.any { + (it.classifier as? KmClassifier.Class)?.name == ownerMetadataName + } + val typeName = (if (ownerIsSupertype) ownerMetadataName else companionMetadataName).replace('/', '.') + val name = "$qualified.$companionName" + val boundary = BoundaryType("OBJECT", "(%s as $typeName)", "(%s)") + val supertypes = classpath.nameablePublicSupertypesOf( + if (ownerIsSupertype) ownerMetadataName else companionMetadataName, + ) + return Candidate( + callable = ArtifactCallable( + name = name, + arity = 0, + paramTags = emptyList(), + returnTag = boundary.tag, + lambdaBody = "{ ${boundary.wrapReturn(name)} }", + returnTypeName = typeName, + returnSupertypes = supertypes, + paramNames = emptyList(), + paramTypeNames = emptyList(), + paramHasDefault = emptyList(), + kind = "STATIC_GETTER", + ), + declaration = DeclarationModel( + simpleName = companionName, + owner = qualified, + ownerIsClass = true, + receiver = null, + parameters = emptyList(), + returnType = python.multiplatform.gradle.model.KotlinTypeModel( + qualifiedName = typeName, + isNullable = false, + arguments = emptyList(), + valueClass = null, + ), + isComposable = false, + isSuspend = false, + kind = "STATIC_GETTER", + bindingName = name, + returnBoundaryTag = boundary.tag, + isCompanionHandle = true, + ), + isCompanionHandle = true, + ) } /** diff --git a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/model/StubModel.kt b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/model/StubModel.kt index 8c1db58ed..bdd1e4b92 100644 --- a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/model/StubModel.kt +++ b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/model/StubModel.kt @@ -103,6 +103,13 @@ internal data class DeclarationModel( * the `Any` rule. */ val isConstructor: Boolean = false, + /** + * The declaration is a companion object itself (`Owner.Companion`): [owner] is the class and + * [simpleName] the companion's name. A stub renders it as a class attribute of the owner's stub + * class rather than as a module attribute, which would turn the owner into a module and drop + * its class stub. + */ + val isCompanionHandle: Boolean = false, ) : Serializable /** @param name `null` when the producer could not read one (ยง3.2); never a synthesised `arg0`. */ 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 93082c4a1..d161568c1 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 @@ -121,6 +121,12 @@ internal const val CONSTRUCTOR_MARKER = "Kotlin constructor: " */ internal const val METHOD_MARKER = "Kotlin member: " +/** + * What precedes a companion handle's table key (`pkg.Owner.Companion`, issue #322) in the docstring of + * the class attribute it is stubbed as, or in the comment that says why there is none. + */ +internal const val COMPANION_MARKER = "Kotlin companion: " + private val PROPERTY_KINDS = setOf("GETTER", "SETTER") private const val METHOD_KIND = "METHOD" @@ -201,7 +207,13 @@ private enum class Role { PARAM, RESULT } private class StubRenderer(declarations: List) { /** Everything that is a module attribute; properties and member functions are attributes of a proxy instead. */ - private val bound: List = declarations.filter { it.kind !in PROPERTY_KINDS && it.kind != METHOD_KIND } + private val bound: List = + declarations.filter { it.kind !in PROPERTY_KINDS && it.kind != METHOD_KIND && !it.isCompanionHandle } + + /** owner class -> its companion object handles (`Owner.Companion`), each a class attribute of the owner's stub. */ + private val companionHandles: Map> = declarations + .filter { it.isCompanionHandle && it.bindingName != null } + .groupBy { it.owner } /** receiver type -> its bound member functions (issue #215), each a method of the receiver's stub class. */ private val methods: Map> = declarations @@ -694,6 +706,17 @@ private class StubRenderer(declarations: List) { } stubBasesOf(qualifiedName, ctx).takeIf { it.isNotEmpty() }?.let { node.bases = it.joinToString(", ") } + // The companion object, a class attribute (`Modifier.Companion`). Typed as the owner when the + // companion is one (`Companion: Modifier`), else as the companion's own type. + companionHandles[qualifiedName].orEmpty().forEach { d -> + val name = d.simpleName + if (!isIdentifier(name) || name in node.children) { + node.members += "# $COMPANION_MARKER${d.bindingName} is not stubbed: '$name' cannot be written as an attribute here" + return@forEach + } + node.members += "$name: ${result(d, ctx)}\n\"\"\"$COMPANION_MARKER${d.bindingName} (${kotlinSignatureOf(d)})\"\"\"" + } + constructorsByClass[qualifiedName]?.let { group -> group.forEachIndexed { index, d -> node.members += if (group.size == 1) { @@ -849,6 +872,26 @@ private class StubRenderer(declarations: List) { } } + /** + * Every owner with a companion handle gets its stub class. One whose name is a module (it also + * publishes companion constants) cannot have a class, so the handle is a module attribute there, + * as the constants are; one whose class is taken by a function gets a comment instead. + */ + private fun registerCompanionOwners() { + companionHandles.forEach { (owner, handles) -> + val ref = classRefOf(owner) + val home = ref?.pkg ?: owner.substringBeforeLast('.', "") + val ctx = out(home) + if (ref != null && classReference(owner, null, ctx) != ANY) return@forEach + if (owner in moduleNames) { + val moduleCtx = out(owner) + handles.forEach { moduleCtx.defs += renderDef(it, moduleCtx) } + } else { + handles.forEach { ctx.defs += "# $COMPANION_MARKER${it.bindingName} is not stubbed: ${owner.substringAfterLast('.')} has no stub class" } + } + } + } + private fun renderClasses() { val done = mutableSetOf() while (true) { @@ -875,6 +918,7 @@ private class StubRenderer(declarations: List) { renderModuleDefs() registerPropertyOwners() registerMethodOwners() + registerCompanionOwners() renderClasses() return modules.filterValues { it.defs.isNotEmpty() || it.classes.isNotEmpty() } .mapValues { (_, ctx) -> diff --git a/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/CompanionHandleBindingTest.kt b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/CompanionHandleBindingTest.kt new file mode 100644 index 000000000..7fe64a10a --- /dev/null +++ b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/CompanionHandleBindingTest.kt @@ -0,0 +1,86 @@ +package python.multiplatform.gradle.artifact + +import java.io.File +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * Issue #322 (SPEC U-4, B-1): a public companion object of a public class is bound as a value, + * `Owner.Companion` (or `Owner.`), by a zero-argument STATIC_GETTER whose generated + * Kotlin source names the companion. Before the change the walker bound a companion's members only. + */ +class CompanionHandleBindingTest { + + private val fixtureClasses: File + get() = File(Class.forName("fixture.artifactcompanion.Widget").protectionDomain.codeSource.location.toURI()) + + private val pkg = "fixture.artifactcompanion" + + private fun candidates() = ArtifactScanner.scanJar(fixtureClasses, includePrefixes = listOf(pkg)) + + private fun named(name: String) = candidates().singleOrNull { it.name == name } + + @Test + fun aCompanionThatIsTheOwnersSubtypeIsBoundAsTheOwner() { + val handle = assertNotNull(named("$pkg.Marker.Companion")) + assertEquals("STATIC_GETTER", handle.kind) + assertEquals(0, handle.arity) + assertEquals("OBJECT", handle.returnTag) + assertEquals("{ ($pkg.Marker.Companion) }", handle.lambdaBody) + assertEquals("$pkg.Marker", handle.returnTypeName) + } + + @Test + fun aNamedCompanionIsBoundUnderItsName() { + val handle = assertNotNull(named("$pkg.Widget.Factory")) + assertEquals("STATIC_GETTER", handle.kind) + assertEquals("{ ($pkg.Widget.Factory) }", handle.lambdaBody) + assertNull(named("$pkg.Widget.Companion")) + } + + @Test + fun aCompanionThatIsNotTheOwnersSubtypeKeepsItsOwnType() { + val handle = assertNotNull(named("$pkg.Widget.Factory")) + assertEquals("$pkg.Widget.Factory", handle.returnTypeName) + val holder = assertNotNull(named("$pkg.Holder.Companion")) + assertEquals("$pkg.Holder.Companion", holder.returnTypeName) + } + + @Test + fun aGenericOwnersCompanionIsNamedWithoutTypeArguments() { + val handle = assertNotNull(named("$pkg.Box.Companion")) + assertEquals("{ ($pkg.Box.Companion) }", handle.lambdaBody) + assertEquals("$pkg.Box", handle.returnTypeName) + } + + @Test + fun aNonPublicCompanionAndAClassWithoutOneBindNothing() { + assertNull(named("$pkg.Hidden.Companion")) + assertNull(named("$pkg.Plain.Companion")) + } + + @Test + fun theCompanionsConstantsAreStillBoundBesideTheHandle() { + val default = assertNotNull(named("$pkg.Holder.Default")) + assertEquals("{ ($pkg.Holder.Companion.Default) }", default.lambdaBody) + } + + @Test + fun theOwnersConstructorIsStillBoundBesideItsCompanionHandle() { + // `Widget.Factory` starts with `Widget.`, which the constructor-collision rule would read as a + // constant hiding the constructor. The handle is exempt, so the constructor survives. + val constructor = assertNotNull(named("$pkg.Widget")) + assertEquals(1, constructor.arity) + assertTrue(named("$pkg.Widget.Factory") != null) + } + + @Test + fun onlyTheHandleIsFlaggedAsACompanionHandle() { + val model = ArtifactScanner.scanDeclarations(fixtureClasses, includePrefixes = listOf(pkg)) + assertTrue(model.single { it.bindingName == "$pkg.Holder.Companion" }.isCompanionHandle) + assertTrue(!model.single { it.bindingName == "$pkg.Holder.Default" }.isCompanionHandle) + } +} diff --git a/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/companionfixture/CompanionFixtures.kt b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/companionfixture/CompanionFixtures.kt new file mode 100644 index 000000000..da6786926 --- /dev/null +++ b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/artifact/companionfixture/CompanionFixtures.kt @@ -0,0 +1,37 @@ +// Fixture, not production source: compiled into this module's own test classes and walked as +// bytecode by `CompanionHandleBindingTest`. A package of its own so no other test's exact list moves. +package fixture.artifactcompanion + +/** The `Modifier` shape: the companion is itself a `Modifier`. */ +interface Marker { + companion object : Marker +} + +/** A named companion that is not a `Marker`; the owner is also constructible. */ +class Widget(val size: Int) { + companion object Factory { + fun make(): Widget = Widget(1) + } +} + +/** A companion of its own type with a constant, so the owner publishes more than the handle. */ +class Holder(val size: Int) { + companion object { + val Default: Holder = Holder(0) + } +} + +/** A generic owner whose companion is a subtype of it. */ +interface Box { + companion object : Box +} + +/** A class whose companion is not public. */ +class Hidden { + private companion object { + val value: Int = 1 + } +} + +/** A class with no companion. */ +class Plain diff --git a/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/CompanionHandleStubTest.kt b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/CompanionHandleStubTest.kt new file mode 100644 index 000000000..a2b11a193 --- /dev/null +++ b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/CompanionHandleStubTest.kt @@ -0,0 +1,63 @@ +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 + +/** + * Issue #322: a companion handle (`Owner.Companion`) is stubbed as a class attribute of the owner's + * stub class. A module-level getter would make `Owner` a module of its own, and a module takes the + * name away from the class: the stub then types every `Owner` as `Any`. + * + * Red before the change: the handle is a module attribute of `...ui.Modifier`, the class `Modifier` + * is dropped and `padding(self: Modifier)` is annotated `Any`. + */ +class CompanionHandleStubTest { + + private val pkg = "androidx.compose.ui" + private val modifier = "$pkg.Modifier" + + private val handle = DeclarationModel( + simpleName = "Companion", + owner = modifier, + ownerIsClass = true, + receiver = null, + parameters = emptyList(), + returnType = KotlinTypeModel(modifier), + returnBoundaryTag = "OBJECT", + kind = "STATIC_GETTER", + bindingName = "$modifier.Companion", + isCompanionHandle = true, + ) + + private val extension = DeclarationModel( + simpleName = "tag", + owner = pkg, + ownerIsClass = false, + receiver = KotlinTypeModel(modifier), + parameters = emptyList(), + returnType = KotlinTypeModel(modifier), + returnBoundaryTag = "OBJECT", + receiverBoundaryTag = "OBJECT", + kind = "FUNCTION", + bindingName = "$pkg.tag", + ) + + private val stubs get() = renderKotlinFqnStubs(listOf(handle, extension)) + + @Test + fun theOwnerClassStubKeepsItsNameAndCarriesTheCompanionAttribute() { + val body = stubs.getValue("androidx/compose/ui/__init__.pyi") + assertTrue("class Modifier:" in body, body) + assertTrue(" Companion: Modifier" in body, body) + } + + @Test + fun theOwnerIsNotAModuleSoItsClassIsNotDropped() { + assertFalse(stubs.keys.any { it.startsWith("androidx/compose/ui/Modifier/") }, stubs.keys.toString()) + val body = stubs.getValue("androidx/compose/ui/__init__.pyi") + assertFalse("receiver: _t.Any" in body, body) + } +} diff --git a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonCallables.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonCallables.kt index a2e7eac7a..262fabfe7 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonCallables.kt +++ b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonCallables.kt @@ -368,6 +368,28 @@ object PythonCallables { paramHasDefault = listOf(false), callable = { args -> unboxScalar(args[0] as Long) }, ), + // SPEC U-4: two proxies are `==` when they hold the same Kotlin object (identity), and hash by + // its identity hash; Kotlin `equals`/`hashCode` are not used. Both take raw handle numbers (INT) and release nothing. + ExposedCallable( + name = SAME_OBJECT, + arity = 2, + paramTypes = listOf(TypeTag.INT, TypeTag.INT), + returnType = TypeTag.BOOLEAN, + paramNames = listOf("left", "right"), + paramTypeNames = listOf("kotlin.Long", "kotlin.Long"), + paramHasDefault = listOf(false, false), + callable = { args -> sameObject(args[0] as Long, args[1] as Long) }, + ), + ExposedCallable( + name = HASH_OF, + arity = 1, + paramTypes = listOf(TypeTag.INT), + returnType = TypeTag.INT, + paramNames = listOf("handle"), + paramTypeNames = listOf("kotlin.Long"), + paramHasDefault = listOf(false), + callable = { args -> hashOf(args[0] as Long) }, + ), ) private fun boxEntry(name: String, tag: TypeTag, typeName: String, box: (Any?) -> Any): ExposedCallable = @@ -421,6 +443,17 @@ object PythonCallables { return python.multiplatform.ffi.upcall.ScalarResult(held) } + /** Reference identity (`===`) of the objects two live handles name; a stale or null handle is nothing. */ + internal fun sameObject(left: Long, right: Long): Boolean { + val a = HandleTable.resolveRaw(left) ?: return false + val b = HandleTable.resolveRaw(right) ?: return false + return a === b + } + + /** Identity hash of the object a live handle names; a stale handle hashes to 0. Kotlin `hashCode` is not used. */ + internal fun hashOf(raw: Long): Long = + (HandleTable.resolveRaw(raw)?.let { python.multiplatform.reflection.identityHashOf(it) } ?: 0).toLong() + /** The name the binding layer resolves. Shared so the Python source and the entry cannot drift. */ internal const val NEW_FUNCTION: String = "python.multiplatform.ffi.pythonx.PythonCallables.newFunction" @@ -442,6 +475,12 @@ object PythonCallables { /** @see boxScalar */ internal const val BOX_STRING: String = "python.multiplatform.ffi.pythonx.PythonCallables.boxString" + /** @see sameObject */ + internal const val SAME_OBJECT: String = "python.multiplatform.ffi.pythonx.PythonCallables.sameObject" + + /** @see hashOf */ + internal const val HASH_OF: String = "python.multiplatform.ffi.pythonx.PythonCallables.hashOf" + /** @see unboxScalar */ internal const val UNBOX_SCALAR: String = "python.multiplatform.ffi.pythonx.PythonCallables.unboxScalar" } diff --git a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonxAdapter.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonxAdapter.kt index baf6e08e4..eef800ae0 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonxAdapter.kt +++ b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/pythonx/PythonxAdapter.kt @@ -1049,7 +1049,9 @@ object PythonxAdapter { one. Registering the factory by name is the seam; a proxy type with none refuses the class-object spelling with a message that says so rather than guessing. ''' - _EMPTY_FACTORIES[kotlin_type_name] = kotlin_factory_name + # The type may be given as its proxy class (`Modifier`), and the factory as a Python callable + # (`lambda: Modifier.Companion`) that answers an owned proxy, instead of a Kotlin function name. + _EMPTY_FACTORIES[getattr(kotlin_type_name, '_kotlin_type_name', kotlin_type_name)] = kotlin_factory_name _MEMBER_RESOLVERS = [] @@ -1227,6 +1229,13 @@ object PythonxAdapter { if resolved is None: kotlin = _member_aliases(cls._kotlin_type_name).get(name) if kotlin is None: + # A Kotlin class and the module of the same name share one name: what the + # module holds (a companion object, `Owner.Companion`, or a companion's + # constants) is also reachable on the class. Answered as `_adapt` answers + # it; a constant is read on every access, never cached on the class. + statics = _BY_PACKAGE.get(cls._kotlin_type_name, {}).get(name) + if statics: + return _adapt(cls._kotlin_type_name, name)[0] raise AttributeError( 'no Kotlin extension named ' + name + ' on ' + cls._kotlin_type_name ) @@ -1427,6 +1436,24 @@ object PythonxAdapter { def __repr__(self): return '<' + _simple_name(kotlin_type_name) + ' handle=' + repr(self._pm_handle) + '>' + def __eq__(self, other): + # Reference identity of the two Kotlin objects the handles name (SPEC U-4), never Kotlin `equals`: + # a companion read twice is two handles to one object, so identity of the wrappers cannot answer. + if other is self: + return True + mine = self.__dict__.get('_pm_handle') + theirs = getattr(other, '__dict__', {}).get('_pm_handle') + # Only two live object handles reach Kotlin; anything else (a non-proxy, a handle-less + # wrapper) is not this proxy's equal and is left to the other side. + if not isinstance(mine, int) or not isinstance(theirs, int) or mine <= 0 or theirs <= 0: + return NotImplemented + if mine == theirs: + return True + return bool(_boundary()['invoke'](_resolve('${PythonCallables.SAME_OBJECT}'), (mine, theirs))) + + def __hash__(self): + return hash(_boundary()['invoke'](_resolve('${PythonCallables.HASH_OF}'), (int(self._pm_handle),))) + @classmethod def empty(cls): factory = _EMPTY_FACTORIES.get(kotlin_type_name) @@ -1436,6 +1463,8 @@ object PythonxAdapter { "python_multiplatform.binding.register_empty('" + kotlin_type_name + "', '') " 'or start the chain from a value Kotlin handed you' ) + if callable(factory): + return factory() return _wrap(_boundary()['invoke'](_resolve(factory), ()), kotlin_type_name) body = { @@ -1444,6 +1473,8 @@ object PythonxAdapter { '__getattr__': __getattr__, '__setattr__': __setattr__, '__repr__': __repr__, + '__eq__': __eq__, + '__hash__': __hash__, 'empty': empty, '_kotlin_type_name': kotlin_type_name, '_kotlin_attached': [], @@ -2315,6 +2346,28 @@ object PythonxAdapter { module = _sys.modules.get(name) if module is not None: _adapt_module(module, name) + # Issue #322: one Python name cannot be both a module and a class (PythonProxySource's + # rule), and the class wins. A companion bound as `pkg.Owner.Companion` makes + # `pkg.Owner` a module too; the proxy layer binds that module onto `pkg` by name, which + # shadowed the proxy class `pkg.Owner`. The class goes back, and its `__getattr__` + # serves the module's statics (`Owner.Companion`). + for name in list(_PACKAGES_SEEN): + if class_wins_over_module(name): + parent, _, leaf = name.rpartition('.') + holder = _sys.modules.get(parent) + if holder is not None and isinstance(holder.__dict__.get(leaf), _types.ModuleType): + setattr(holder, leaf, _proxy_type(name)) + + + def class_wins_over_module(qualified): + '''Whether the parent package's attribute [qualified] must be the proxy class rather than a + module of that name: exactly when `_adapt` would answer the class. A declaration bound under + the same name in the parent wins first, as in `_adapt` -- issue #78's callable module + (`TextRange(2)` beside `TextRange.Zero`) stays the module that answers both.''' + parent, _, leaf = qualified.rpartition('.') + if _BY_PACKAGE.get(parent, {}).get(leaf): + return False + return leaf[:1].isupper() and (qualified in _BY_RECEIVER or qualified in _PROXY_TYPES) class _Loader: diff --git a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt index b32203d30..0e8d0dd9a 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt +++ b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt @@ -362,7 +362,15 @@ object PythonProxySource { _pm_sys.modules[_name] = _m if '.' in _name: _parent, _, _leaf = _name.rpartition('.') - setattr(_pm_module(_parent), _leaf, _m) + # Issue #322: a name the binding layer serves as a proxy class stays that class on + # its parent; this module is still in sys.modules and the class's __getattr__ + # reaches its statics. One Python name is not both a module and a class. + _binding = _pm_sys.modules.get('python_multiplatform.binding') + _is_type = getattr(_binding, 'class_wins_over_module', None) + if _is_type is None or not _is_type(_name): + setattr(_pm_module(_parent), _leaf, _m) + else: + _pm_module(_parent) return _m diff --git a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/reflection/IdentityHash.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/reflection/IdentityHash.kt new file mode 100644 index 000000000..6922d2a09 --- /dev/null +++ b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/reflection/IdentityHash.kt @@ -0,0 +1,7 @@ +package python.multiplatform.reflection + +/** + * The identity hash of [value]: equal for the same object, whatever its own `hashCode` says. + * Backs a proxy's `__hash__` (SPEC U-4), whose `==` is reference identity. + */ +internal expect fun identityHashOf(value: Any): Int diff --git a/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/pythonx/CompanionHandleTest.kt b/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/pythonx/CompanionHandleTest.kt new file mode 100644 index 000000000..67f44b3ba --- /dev/null +++ b/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/pythonx/CompanionHandleTest.kt @@ -0,0 +1,234 @@ +package python.multiplatform.ffi.pythonx + +import python.multiplatform.ffi.Python3 +import python.multiplatform.ffi.PythonTestFixture +import python.multiplatform.reflection.CallableKind +import python.multiplatform.reflection.ExposedCallable +import python.multiplatform.reflection.FunctionTableFragment +import python.multiplatform.reflection.HandleTable +import python.multiplatform.reflection.TypeTag +import python.multiplatform.reflection.UpcallTable +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * Issue #322 (SPEC U-4): a public companion object is a value, `Owner.Companion`, read as an attribute + * of the owner's proxy class. When the companion is a subtype of the owner the value is an instance of + * the owner's proxy. Two reads are two handles to one Kotlin object: they compare `==` (reference + * identity, never Kotlin `equals`) and `is` is not promised. + * + * Red before the change: `Modifier.Companion` raises AttributeError on the proxy class (the table key + * lives in the module of the same name, which the class never consulted), and two proxies never + * compare equal. + */ +class CompanionHandleTest { + + private object TheCompanion + + private object Fragment : FunctionTableFragment { + override val moduleName: String = "test_companion_handle" + + private const val UI = "androidx.compose.ui" + private const val MODIFIER = "$UI.Modifier" + + override fun entries(): List = listOf( + ExposedCallable( + name = "$MODIFIER.Companion", + arity = 0, + paramTypes = emptyList(), + returnType = TypeTag.OBJECT, + kind = CallableKind.STATIC_GETTER, + paramNames = emptyList(), + paramTypeNames = emptyList(), + returnTypeName = MODIFIER, + ) { TheCompanion }, + // A class-level constant through the proxy class, beside the companion handle. + ExposedCallable( + name = "$MODIFIER.Label", + arity = 0, + paramTypes = emptyList(), + returnType = TypeTag.STRING, + kind = CallableKind.STATIC_GETTER, + paramNames = emptyList(), + paramTypeNames = emptyList(), + returnTypeName = "kotlin.String", + ) { "label" }, + ExposedCallable( + name = "$UI.emptyModifier", + arity = 0, + paramTypes = emptyList(), + returnType = TypeTag.OBJECT, + paramNames = emptyList(), + paramTypeNames = emptyList(), + returnTypeName = MODIFIER, + ) { TheCompanion }, + // Two distinct objects that Kotlin `equals` calls equal: `==` must still be False. + ExposedCallable( + name = "$MODIFIER.FreshA", + arity = 0, + paramTypes = emptyList(), + returnType = TypeTag.OBJECT, + kind = CallableKind.STATIC_GETTER, + paramNames = emptyList(), + paramTypeNames = emptyList(), + returnTypeName = MODIFIER, + ) { ArrayList(listOf(1)) }, + ExposedCallable( + name = "$MODIFIER.FreshB", + arity = 0, + paramTypes = emptyList(), + returnType = TypeTag.OBJECT, + kind = CallableKind.STATIC_GETTER, + paramNames = emptyList(), + paramTypeNames = emptyList(), + returnTypeName = MODIFIER, + ) { ArrayList(listOf(1)) }, + // An extension makes Modifier a receiver type, so `Modifier` is a proxy class. + ExposedCallable( + name = "$UI.tag", + arity = 1, + paramTypes = listOf(TypeTag.OBJECT), + returnType = TypeTag.STRING, + paramNames = listOf(""), + paramTypeNames = listOf(MODIFIER), + returnTypeName = "kotlin.String", + isExtension = true, + receiverTypeName = MODIFIER, + ) { args -> if (args[0] === TheCompanion) "companion" else "other" }, + ) + } + + @BeforeTest + fun install() { + check(PythonTestFixture.available) { + "CPython could not be initialized (${PythonTestFixture.failureReason})" + } + UpcallTable.install(listOf(Fragment)) + } + + @AfterTest + fun cleanup() { + UpcallTable.clear() + HandleTable.releaseAll() + } + + private fun fresh(block: () -> Unit) = PythonTestFixture.withInterpreter { + Python3.exec( + """ + import sys as _cf_sys + for _cf_n in [_n for _n in _cf_sys.modules if _n == 'androidx' or _n.startswith('androidx.')]: + del _cf_sys.modules[_cf_n] + """.trimIndent(), + ) + PythonxAdapter.install() + block() + } + + private fun eval(expression: String): String = PythonTestFixture.eval(expression).toString() + + @Test + fun theCompanionIsAnInstanceOfTheOwnersProxy() = fresh { + Python3.exec( + """ + from androidx.compose.ui import Modifier + _cf = isinstance(Modifier.Companion, Modifier) + _cf_tag = Modifier.Companion.tag() + """.trimIndent(), + ) + assertEquals("True", eval("_cf")) + assertEquals("companion", eval("_cf_tag")) + } + + @Test + fun twoReadsCompareEqualAndAreNotPromisedIdentical() = fresh { + Python3.exec( + """ + from androidx.compose.ui import Modifier + _a = Modifier.Companion + _b = Modifier.Companion + _cf = (_a == _b, hash(_a) == hash(_b), _a != 3) + """.trimIndent(), + ) + assertEquals("(True, True, True)", eval("_cf")) + } + + @Test + fun twoDistinctObjectsThatKotlinEqualsCallEqualAreNotEqual() = fresh { + Python3.exec( + """ + from androidx.compose.ui import Modifier + _a = Modifier.FreshA + _b = Modifier.FreshB + _cf = (_a == _b, _a != _b, _a == _a) + """.trimIndent(), + ) + assertEquals("(False, True, True)", eval("_cf")) + } + + @Test + fun thereIsNoLowerCaseCompanionAlias() = fresh { + Python3.exec( + """ + from androidx.compose.ui import Modifier + _cf = hasattr(Modifier, 'companion') + """.trimIndent(), + ) + assertEquals("False", eval("_cf")) + } + + @Test + fun aClassLevelConstantIsReadThroughTheProxyClass() = fresh { + Python3.exec( + """ + from androidx.compose.ui import Modifier + _cf = Modifier.Label + """.trimIndent(), + ) + assertEquals("label", eval("_cf")) + } + + @Test + fun anUnknownNameOnTheClassStillRaisesAttributeError() = fresh { + Python3.exec( + """ + from androidx.compose.ui import Modifier + try: + Modifier.Nothing + _cf = 'no error' + except AttributeError: + _cf = 'AttributeError' + """.trimIndent(), + ) + assertEquals("AttributeError", eval("_cf")) + } + + @Test + fun aPythonCallableCanBeTheEmptyFactory() = fresh { + Python3.exec( + """ + import python_multiplatform.binding as _pm_binding + from androidx.compose.ui import Modifier + _pm_binding.register_empty(Modifier, lambda: Modifier.Companion) + _cf = Modifier.empty() == Modifier.Companion + """.trimIndent(), + ) + assertEquals("True", eval("_cf")) + } + + @Test + fun theStringFactoryFormStillWorksWithATypeNameAndWithTheClass() = fresh { + Python3.exec( + """ + import python_multiplatform.binding as _pm_binding + from androidx.compose.ui import Modifier + _pm_binding.register_empty('androidx.compose.ui.Modifier', 'androidx.compose.ui.emptyModifier') + _one = Modifier.empty() == Modifier.Companion + _pm_binding.register_empty(Modifier, 'androidx.compose.ui.emptyModifier') + _cf = (_one, Modifier.empty() == Modifier.Companion) + """.trimIndent(), + ) + assertEquals("(True, True)", eval("_cf")) + } +} diff --git a/python-multiplatform/src/jvmMain/kotlin/python/multiplatform/reflection/IdentityHash.jvm.kt b/python-multiplatform/src/jvmMain/kotlin/python/multiplatform/reflection/IdentityHash.jvm.kt new file mode 100644 index 000000000..90d54f644 --- /dev/null +++ b/python-multiplatform/src/jvmMain/kotlin/python/multiplatform/reflection/IdentityHash.jvm.kt @@ -0,0 +1,3 @@ +package python.multiplatform.reflection + +internal actual fun identityHashOf(value: Any): Int = System.identityHashCode(value) diff --git a/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/reflection/IdentityHash.native.kt b/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/reflection/IdentityHash.native.kt new file mode 100644 index 000000000..6b2298ad8 --- /dev/null +++ b/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/reflection/IdentityHash.native.kt @@ -0,0 +1,7 @@ +@file:OptIn(kotlin.experimental.ExperimentalNativeApi::class) + +package python.multiplatform.reflection + +import kotlin.native.identityHashCode + +internal actual fun identityHashOf(value: Any): Int = value.identityHashCode() diff --git a/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/reflection/IdentityHash.wasmJs.kt b/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/reflection/IdentityHash.wasmJs.kt new file mode 100644 index 000000000..bb2e894de --- /dev/null +++ b/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/reflection/IdentityHash.wasmJs.kt @@ -0,0 +1,6 @@ +package python.multiplatform.reflection + +// Kotlin/Wasm exposes no identity hash. The object's own `hashCode` is still equal for the same object, +// which is all `__hash__` needs next to an identity `__eq__`; it only costs spread for a type that +// overrides `hashCode`. +internal actual fun identityHashOf(value: Any): Int = value.hashCode()