Skip to content

feat: special support for @Composable when doing binary compatibility checks. #1310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: special support for @Composable when doing binary compatibility checks. #1310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45d99a8
feat: special support for @Composable when doing binary compatibility…
autonomousapps Nov 15, 2024
360251e
chore: log if there are errors deleting test fixtures.
autonomousapps Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/functionalTest/groovy/com/autonomousapps/AbstractFunctionalSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,13 @@ abstract class AbstractFunctionalSpec extends Specification {
def cleanup() {
// Delete fixtures on CI to prevent disk space growing out of bounds
if (gradleProject != null && isCi) {
def f = gradleProject.rootDir
try {
println("Deleting $f")
gradleProject.rootDir.deleteDir()
} catch (Throwable t) {
System.err.println("Error deleting gradleProject $f: ${t.localizedMessage}")
t.printStackTrace()
}
}
}
Expand Down
60 changes: 50 additions & 10 deletions src/main/kotlin/com/autonomousapps/internal/asm.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import com.autonomousapps.internal.kotlin.AccessFlags
import com.autonomousapps.internal.utils.METHOD_DESCRIPTOR_REGEX
import com.autonomousapps.internal.utils.efficient
import com.autonomousapps.internal.utils.genericTypes
import com.autonomousapps.model.internal.intermediates.producer.Member
import com.autonomousapps.model.internal.intermediates.consumer.MemberAccess
import com.autonomousapps.model.internal.intermediates.producer.Member
import kotlinx.metadata.jvm.Metadata
import org.gradle.api.logging.Logger
import java.util.SortedSet
Expand All @@ -19,7 +19,10 @@ import java.util.concurrent.atomic.AtomicReference
private val logDebug: Boolean get() = Flags.logBytecodeDebug()
private const val ASM_VERSION = Opcodes.ASM9

/** This will collect the class name and information about annotations. */
/**
* This will collect the class name and information about annotations. It is used on the "producer" side to create
* the [ExplodedJar][com.autonomousapps.model.internal.intermediates.producer.ExplodedJar] model, via [AnalyzedClass].
*/
internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : ClassVisitor(ASM_VERSION) {

private lateinit var className: String
Expand Down Expand Up @@ -98,7 +101,7 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas

override fun visitMethod(
access: Int, name: String, descriptor: String, signature: String?, exceptions: Array<out String>?
): MethodVisitor? {
): MethodVisitor {
log { "- visitMethod: ${Access.fromInt(access)} descriptor=$descriptor name=$name signature=$signature" }

if (!("()V" == descriptor && ("<init>" == name || "<clinit>" == name))) {
Expand All @@ -107,17 +110,17 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
methods.add(Method(descriptor))
}

var method: Member.Method? = null
if (isEffectivelyPublic(access)) {
effectivelyPublicMethods.add(
Member.Method(
access = access,
name = name,
descriptor = descriptor,
)
method = Member.Method(
access = access,
name = name,
descriptor = descriptor,
)
effectivelyPublicMethods.add(method)
}

return null
return MethodAnalyzer(logger, effectivelyPublicMethods, method)
}

override fun visitField(
Expand Down Expand Up @@ -189,6 +192,43 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
}
}
}

private class MethodAnalyzer(
private val logger: Logger,
private val effectivelyPublicMethods: MutableSet<Member.Method>,
private val method: Member.Method?,
) : MethodVisitor(ASM_VERSION) {

private fun log(msgProvider: () -> String) {
if (!logDebug) {
logger.quiet(msgProvider())
}
}

override fun visitAnnotation(descriptor: String?, visible: Boolean): AnnotationVisitor? {
log { "MethodAnalyzer#visitAnnotation: descriptor=$descriptor visible=$visible" }

// For @Composable functions, we also add an artificial version that matches what the compose Kotlin compiler
// plugin generates. For example:
//
// * In source, the `isSystemInDarkTheme()` function seems to have the descriptor `()Z` (takes no arguments,
// returns a boolean).
// * In the generated bytecode, the descriptor is `(Landroidx/compose/runtime/Composer;I)Z` (takes a Composer and
// an int, returns a boolean).
if (descriptor == "Landroidx/compose/runtime/Composable;") {
effectivelyPublicMethods
.find { method == it }
?.let { m ->
// remove the original
effectivelyPublicMethods.remove(m)
// and add a new version with the special property set
effectivelyPublicMethods.add(m.copy(special = Member.SPECIAL_COMPOSABLE))
}
}

return null
}
}
}

internal data class ClassRef(
Expand Down
6 changes: 3 additions & 3 deletions src/main/kotlin/com/autonomousapps/model/internal/Capability.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ internal data class BinaryClassCapability(
/**
* Partitions and returns artificial pair of [BinaryClasses][BinaryClass]. Non-null elements indicate relevant (to
* [memberAccess] matching and non-matching members of this `BinaryClass`. Matching members are binary-compatible; and
* non-matching members have the same [name][com.autonomousapps.model.intermediates.producer.Member.name] but
* incompatible [descriptors][com.autonomousapps.model.intermediates.producer.Member.descriptor], and are therefore
* binary-incompatible.
* non-matching members have the same [name][com.autonomousapps.model.internal.intermediates.producer.Member.name] but
* incompatible [descriptors][com.autonomousapps.model.internal.intermediates.producer.Member.descriptor], and are
* therefore binary-incompatible.
*
* nb: We don't want this as a method directly in BinaryClass because it can't safely assert the prerequisite that
* it's only called on "relevant" classes. THIS class, however, can, via findRelevantBinaryClasses.
Expand Down
28 changes: 23 additions & 5 deletions src/main/kotlin/com/autonomousapps/model/internal/intermediates/producer/Member.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,20 @@ import dev.zacsweers.moshix.sealed.annotations.TypeLabel
* Represents a member of a [class][BinaryClass].
*
* nb: Borrowing heavily from `asmUtils.kt` and similar but substantially different from
* [MemberAccess][com.autonomousapps.model.intermediates.consumer.MemberAccess] on the consumer side.
* [MemberAccess][com.autonomousapps.model.internal.intermediates.consumer.MemberAccess] on the consumer side.
*/
@JsonClass(generateAdapter = false, generator = "sealed:type")
internal sealed class Member(
open val access: Int,
open val name: String,
open val descriptor: String,
open val special: String?,
) : Comparable<Member> {

companion object {
const val SPECIAL_COMPOSABLE = "@Composable"
}

internal class Printable(
val className: String,
val memberName: String,
Expand Down Expand Up @@ -51,14 +56,23 @@ internal sealed class Member(
.compare(this, other)
}

/** Returns true for matching name and descriptor. */
/**
* Returns true for matching name and descriptor, unless [special] == [SPECIAL_COMPOSABLE], in which case we relax the
* requirement that the descriptors match. `@Composable` functions get special attention from a Kotlin compiler
* plugin, such that the source and bytecode do not match.
*/
fun matches(memberAccess: MemberAccess): Boolean {
return name == memberAccess.name && descriptor == memberAccess.descriptor
return name == memberAccess.name
&& (descriptor == memberAccess.descriptor || special == SPECIAL_COMPOSABLE)
}

/** Returns true for matching name and non-matching descriptor. */
/**
* Returns true for matching name and non-matching descriptor, unless [special] == [SPECIAL_COMPOSABLE], in which case
* we can't say that [memberAccess] doesn't match.
*/
fun doesNotMatch(memberAccess: MemberAccess): Boolean {
return name == memberAccess.name && descriptor != memberAccess.descriptor
return name == memberAccess.name
&& (descriptor != memberAccess.descriptor && special != SPECIAL_COMPOSABLE)
}

protected val accessFlags get() = AccessFlags(access)
Expand All @@ -78,10 +92,12 @@ internal sealed class Member(
override val access: Int,
override val name: String,
override val descriptor: String,
override val special: String? = null,
) : Member(
access = access,
name = name,
descriptor = descriptor,
special = special,
) {
override val signature: String
get() = "${accessFlags.getModifierString()} fun $name $descriptor"
Expand All @@ -93,10 +109,12 @@ internal sealed class Member(
override val access: Int,
override val name: String,
override val descriptor: String,
override val special: String? = null,
) : Member(
access = access,
name = name,
descriptor = descriptor,
special = special,
) {
override val signature: String
get() = "${accessFlags.getModifierString()} field $name $descriptor"
Expand Down
29 changes: 6 additions & 23 deletions src/main/kotlin/com/autonomousapps/tasks/ComputeUsagesTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,11 @@
package com.autonomousapps.tasks

import com.autonomousapps.internal.utils.*
import com.autonomousapps.model.*
import com.autonomousapps.model.Coordinates
import com.autonomousapps.model.DuplicateClass
import com.autonomousapps.model.declaration.internal.Bucket
import com.autonomousapps.model.declaration.internal.Declaration
import com.autonomousapps.model.internal.AndroidAssetCapability
import com.autonomousapps.model.internal.AndroidLinterCapability
import com.autonomousapps.model.internal.AndroidManifestCapability
import com.autonomousapps.model.internal.AndroidResCapability
import com.autonomousapps.model.internal.AndroidResSource
import com.autonomousapps.model.internal.AnnotationProcessorCapability
import com.autonomousapps.model.internal.BinaryClassCapability
import com.autonomousapps.model.internal.ClassCapability
import com.autonomousapps.model.internal.ConstantCapability
import com.autonomousapps.model.internal.Dependency
import com.autonomousapps.model.internal.DependencyGraphView
import com.autonomousapps.model.internal.InferredCapability
import com.autonomousapps.model.internal.InlineMemberCapability
import com.autonomousapps.model.internal.NativeLibCapability
import com.autonomousapps.model.internal.ProjectVariant
import com.autonomousapps.model.internal.SecurityProviderCapability
import com.autonomousapps.model.internal.ServiceLoaderCapability
import com.autonomousapps.model.internal.TypealiasCapability
import com.autonomousapps.model.internal.*
import com.autonomousapps.model.internal.intermediates.DependencyTraceReport
import com.autonomousapps.model.internal.intermediates.DependencyTraceReport.Kind
import com.autonomousapps.model.internal.intermediates.Reason
Expand Down Expand Up @@ -422,7 +406,6 @@ private class GraphVisitor(
// Can't be incompatible if the code compiles in the context of no duplication
if (context.duplicateClasses.isEmpty()) return

// TODO(tsr): special handling for @Composable
val memberAccessOwners = context.project.memberAccesses.mapToSet { it.owner }
val relevantDuplicates = context.duplicateClasses
.filter { duplicate -> coordinates in duplicate.dependencies && duplicate.className in memberAccessOwners }
Expand All @@ -435,9 +418,9 @@ private class GraphVisitor(
val relevantMemberAccesses = context.project.memberAccesses
.filterToOrderedSet { access -> access.owner in relevantDuplicateClassNames }

val partitionResult = relevantMemberAccesses.mapToSet { access ->
binaryClassCapability.findMatchingClasses(access)
}.reduce()
val partitionResult = relevantMemberAccesses
.mapToSet { access -> binaryClassCapability.findMatchingClasses(access) }
.reduce()
val matchingBinaryClasses = partitionResult.matchingClasses
val nonMatchingBinaryClasses = partitionResult.nonMatchingClasses

Expand Down
Loading