Skip to content

Commit 66031d9

Browse files
martinbonninmartinbonnin
authored
For synthetic $default methods, lookup the annotations on their corresponding non-synthetic method (#85)
Fixes #58
1 parent 902ff60 commit 66031d9

File tree

9 files changed

+101
-87
lines changed

9 files changed

+101
-87
lines changed

‎src/main/kotlin/api/AsmMetadataLoading.kt‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ val ClassNode.outerClassName: String? get() = innerClassNode?.outerName
4747

4848
const val publishedApiAnnotationName = "kotlin/PublishedApi"
4949
fun ClassNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null
50-
fun MethodNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null
51-
fun FieldNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null
52-
50+
fun List<AnnotationNode>.isPublishedApi() = firstOrNull { it.refersToName(publishedApiAnnotationName) } != null
5351

5452
fun ClassNode.isDefaultImpls(metadata: KotlinClassMetadata?) = isInner() && name.endsWith("\$DefaultImpls") && metadata.isSyntheticClass()
5553

‎src/main/kotlin/api/KotlinMetadataSignature.kt‎

Lines changed: 49 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package kotlinx.validation.api
77

88
import kotlinx.metadata.jvm.*
99
import kotlinx.validation.*
10+
import org.objectweb.asm.Opcodes
1011
import org.objectweb.asm.tree.*
1112

1213
@ExternalApi // Only name is part of the API, nothing else is used by stdlib
@@ -48,7 +49,8 @@ data class MethodBinarySignature(
4849
override val jvmMember: JvmMethodSignature,
4950
override val isPublishedApi: Boolean,
5051
override val access: AccessFlags,
51-
override val annotations: List<AnnotationNode>
52+
override val annotations: List<AnnotationNode>,
53+
private val alternateDefaultSignature: JvmMethodSignature?
5254
) : MemberBinarySignature {
5355
override val signature: String
5456
get() = "${access.getModifierString()} fun $name $desc"
@@ -59,60 +61,51 @@ data class MethodBinarySignature(
5961
&& !isDummyDefaultConstructor()
6062

6163
override fun findMemberVisibility(classVisibility: ClassVisibility?): MemberVisibility? {
62-
return super.findMemberVisibility(classVisibility) ?: classVisibility?.let { alternateDefaultSignature(it.name)?.let(it::findMember) }
64+
return super.findMemberVisibility(classVisibility)
65+
?: classVisibility?.let { alternateDefaultSignature?.let(it::findMember) }
6366
}
6467

65-
/**
66-
* Checks whether the method is a $default counterpart of internal @PublishedApi method
67-
*/
68-
public fun isPublishedApiWithDefaultArguments(
69-
classVisibility: ClassVisibility?,
70-
publishedApiSignatures: Set<JvmMethodSignature>
71-
): Boolean {
72-
// Fast-path
73-
findMemberVisibility(classVisibility)?.isInternal() ?: return false
74-
val name = jvmMember.name
75-
if (!name.endsWith("\$default")) return false
76-
// Leverage the knowledge about modified signature
77-
val expectedPublishedApiCounterPart = JvmMethodSignature(
78-
name.removeSuffix("\$default"),
79-
jvmMember.desc.replace( ";ILjava/lang/Object;)", ";)"))
80-
return expectedPublishedApiCounterPart in publishedApiSignatures
81-
}
68+
private fun isAccessOrAnnotationsMethod() =
69+
access.isSynthetic && (name.startsWith("access\$") || name.endsWith("\$annotations"))
8270

83-
private fun isAccessOrAnnotationsMethod() = access.isSynthetic && (name.startsWith("access\$") || name.endsWith("\$annotations"))
84-
85-
private fun isDummyDefaultConstructor() = access.isSynthetic && name == "<init>" && desc == "(Lkotlin/jvm/internal/DefaultConstructorMarker;)V"
86-
87-
/**
88-
* Calculates the signature of this method without default parameters
89-
*
90-
* Returns `null` if this method isn't an entry point of a function
91-
* or a constructor with default parameters.
92-
* Returns an incorrect result, if there are more than 31 default parameters.
93-
*/
94-
private fun alternateDefaultSignature(className: String): JvmMethodSignature? {
95-
return when {
96-
!access.isSynthetic -> null
97-
name == "<init>" && "ILkotlin/jvm/internal/DefaultConstructorMarker;" in desc ->
98-
JvmMethodSignature(name, desc.replace("ILkotlin/jvm/internal/DefaultConstructorMarker;", ""))
99-
name.endsWith("\$default") && "ILjava/lang/Object;)" in desc ->
100-
JvmMethodSignature(
101-
name.removeSuffix("\$default"),
102-
desc.replace("ILjava/lang/Object;)", ")").replace("(L$className;", "(")
103-
)
104-
else -> null
105-
}
71+
private fun isDummyDefaultConstructor() =
72+
access.isSynthetic && name == "<init>" && desc == "(Lkotlin/jvm/internal/DefaultConstructorMarker;)V"
73+
}
74+
75+
/**
76+
* Calculates the signature of this method without default parameters
77+
*
78+
* Returns `null` if this method isn't an entry point of a function
79+
* or a constructor with default parameters.
80+
* Returns an incorrect result, if there are more than 31 default parameters.
81+
*/
82+
internal fun MethodNode.alternateDefaultSignature(className: String): JvmMethodSignature? {
83+
return when {
84+
access and Opcodes.ACC_SYNTHETIC == 0 -> null
85+
name == "<init>" && "ILkotlin/jvm/internal/DefaultConstructorMarker;" in desc ->
86+
JvmMethodSignature(name, desc.replace("ILkotlin/jvm/internal/DefaultConstructorMarker;", ""))
87+
name.endsWith("\$default") && "ILjava/lang/Object;)" in desc ->
88+
JvmMethodSignature(
89+
name.removeSuffix("\$default"),
90+
desc.replace("ILjava/lang/Object;)", ")").replace("(L$className;", "(")
91+
)
92+
else -> null
10693
}
10794
}
10895

109-
fun MethodNode.toMethodBinarySignature(propertyAnnotations: List<AnnotationNode>) =
110-
MethodBinarySignature(
96+
fun MethodNode.toMethodBinarySignature(
97+
extraAnnotations: List<AnnotationNode>,
98+
alternateDefaultSignature: JvmMethodSignature?
99+
): MethodBinarySignature {
100+
val allAnnotations = visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + extraAnnotations
101+
return MethodBinarySignature(
111102
JvmMethodSignature(name, desc),
112-
isPublishedApi(),
103+
allAnnotations.isPublishedApi(),
113104
AccessFlags(access),
114-
visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + propertyAnnotations
105+
allAnnotations,
106+
alternateDefaultSignature
115107
)
108+
}
116109

117110
data class FieldBinarySignature(
118111
override val jvmMember: JvmFieldSignature,
@@ -129,13 +122,15 @@ data class FieldBinarySignature(
129122
}
130123
}
131124

132-
fun FieldNode.toFieldBinarySignature() =
133-
FieldBinarySignature(
125+
fun FieldNode.toFieldBinarySignature(extraAnnotations: List<AnnotationNode>): FieldBinarySignature {
126+
val allAnnotations = visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + extraAnnotations
127+
return FieldBinarySignature(
134128
JvmFieldSignature(name, desc),
135-
isPublishedApi(),
129+
allAnnotations.isPublishedApi(),
136130
AccessFlags(access),
137-
annotations(visibleAnnotations, invisibleAnnotations))
138-
131+
allAnnotations
132+
)
133+
}
139134
private val MemberBinarySignature.kind: Int
140135
get() = when (this) {
141136
is FieldBinarySignature -> 1
@@ -156,7 +151,9 @@ data class AccessFlags(val access: Int) {
156151
val isFinal: Boolean get() = isFinal(access)
157152
val isSynthetic: Boolean get() = isSynthetic(access)
158153

159-
fun getModifiers(): List<String> = ACCESS_NAMES.entries.mapNotNull { if (access and it.key != 0) it.value else null }
154+
fun getModifiers(): List<String> =
155+
ACCESS_NAMES.entries.mapNotNull { if (access and it.key != 0) it.value else null }
156+
160157
fun getModifierString(): String = getModifiers().joinToString(" ")
161158
}
162159

‎src/main/kotlin/api/KotlinMetadataVisibilities.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ fun KotlinClassMetadata?.isSyntheticClass() = this is KotlinClassMetadata.Synthe
8787

8888
// Auxiliary class that stores signatures of corresponding field and method for a property.
8989
class PropertyAnnotationHolders(
90-
val field: JvmMemberSignature?,
90+
val field: JvmFieldSignature?,
9191
val method: JvmMethodSignature?,
9292
)
9393

@@ -143,7 +143,7 @@ fun KotlinClassMetadata.toClassVisibility(classNode: ClassNode): ClassVisibility
143143
property.getterSignature == null && property.setterSignature == null -> property.flags // JvmField or const case
144144
else -> flagsOf(Flag.IS_PRIVATE)
145145
}
146-
addMember(property.fieldSignature, fieldVisibility, isReified = false)
146+
addMember(property.fieldSignature, fieldVisibility, isReified = false, propertyAnnotation = propertyAnnotations)
147147
}
148148
}
149149

‎src/main/kotlin/api/KotlinSignaturesLoading.kt‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,12 @@ public fun Sequence<InputStream>.loadApiFromJvmClasses(visibilityFilter: (String
4343
val supertypes = listOf(superName) - "java/lang/Object" + interfaces.sorted()
4444

4545
val fieldSignatures = fields
46-
.map { it.toFieldBinarySignature() }
47-
.filter {
46+
.map {
47+
val annotationHolders =
48+
mVisibility?.members?.get(JvmFieldSignature(it.name, it.desc))?.propertyAnnotation
49+
val foundAnnotations = methods.annotationsFor(annotationHolders?.method)
50+
it.toFieldBinarySignature(foundAnnotations)
51+
}.filter {
4852
it.isEffectivelyPublic(classAccess, mVisibility)
4953
}.filter {
5054
/*
@@ -62,40 +66,29 @@ public fun Sequence<InputStream>.loadApiFromJvmClasses(visibilityFilter: (String
6266
}
6367

6468
// NB: this 'map' is O(methods + properties * methods) which may accidentally be quadratic
65-
val allMethods = methods.map {
69+
val methodSignatures = methods.map {
6670
/**
6771
* For getters/setters, pull the annotations from the property
6872
* This is either on the field if any or in a '$annotations' synthetic function.
6973
*/
7074
val annotationHolders =
7175
mVisibility?.members?.get(JvmMethodSignature(it.name, it.desc))?.propertyAnnotation
7276
val foundAnnotations = ArrayList<AnnotationNode>()
73-
// lookup annotations from $annotations()
74-
val syntheticPropertyMethod = annotationHolders?.method
75-
if (syntheticPropertyMethod != null) {
76-
foundAnnotations += methods
77-
.firstOrNull { it.name == syntheticPropertyMethod.name && it.desc == syntheticPropertyMethod.desc }
78-
?.visibleAnnotations ?: emptyList()
79-
}
77+
foundAnnotations += fields.annotationsFor(annotationHolders?.field)
78+
foundAnnotations += methods.annotationsFor(annotationHolders?.method)
8079

81-
val backingField = annotationHolders?.field
82-
if (backingField != null) {
83-
foundAnnotations += fields
84-
.firstOrNull { it.name == backingField.name && it.desc == backingField.desc }
85-
?.visibleAnnotations ?: emptyList()
80+
/**
81+
* For synthetic $default methods, pull the annotations from the corresponding method
82+
*/
83+
val alternateDefaultSignature = mVisibility?.name?.let { className ->
84+
it.alternateDefaultSignature(className)
8685
}
86+
foundAnnotations += methods.annotationsFor(alternateDefaultSignature)
8787

88-
it.toMethodBinarySignature(foundAnnotations)
88+
it.toMethodBinarySignature(foundAnnotations, alternateDefaultSignature)
89+
}.filter {
90+
it.isEffectivelyPublic(classAccess, mVisibility)
8991
}
90-
// Signatures marked with @PublishedApi
91-
val publishedApiSignatures = allMethods.filter {
92-
it.isPublishedApi
93-
}.map { it.jvmMember }.toSet()
94-
val methodSignatures = allMethods
95-
.filter {
96-
it.isEffectivelyPublic(classAccess, mVisibility) ||
97-
it.isPublishedApiWithDefaultArguments(mVisibility, publishedApiSignatures)
98-
}
9992

10093
ClassBinarySignature(
10194
name, superName, outerClassName, supertypes, fieldSignatures + methodSignatures, classAccess,
@@ -107,6 +100,24 @@ public fun Sequence<InputStream>.loadApiFromJvmClasses(visibilityFilter: (String
107100
}
108101
}
109102

103+
private fun List<MethodNode>.annotationsFor(methodSignature: JvmMethodSignature?): List<AnnotationNode> {
104+
if (methodSignature == null) return emptyList()
105+
106+
return firstOrNull { it.name == methodSignature.name && it.desc == methodSignature.desc }
107+
?.run {
108+
visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty()
109+
} ?: emptyList()
110+
}
111+
112+
private fun List<FieldNode>.annotationsFor(fieldSignature: JvmFieldSignature?): List<AnnotationNode> {
113+
if (fieldSignature == null) return emptyList()
114+
115+
return firstOrNull { it.name == fieldSignature.name && it.desc == fieldSignature.desc }
116+
?.run {
117+
visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty()
118+
} ?: emptyList()
119+
}
120+
110121
@ExternalApi
111122
public fun List<ClassBinarySignature>.filterOutAnnotated(targetAnnotations: Set<String>): List<ClassBinarySignature> {
112123
if (targetAnnotations.isEmpty()) return this

‎src/test/kotlin/cases/inline/inline.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ public final class cases/inline/InlineExposedKt {
33
}
44

55
public final class cases/inline/InternalClassExposed {
6+
public field fieldExposed Ljava/lang/String;
67
public fun <init> ()V
78
public final fun funExposed ()V
9+
public final fun getPropertyExposed ()Ljava/lang/String;
10+
public final fun setPropertyExposed (Ljava/lang/String;)V
811
}
912

‎src/test/kotlin/cases/inline/inlineExposed.kt‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,10 @@ internal class InternalClassExposed
1111
@PublishedApi
1212
internal fun funExposed() {}
1313

14-
// TODO: Cover unsupported cases: requires correctly reflecting annotations from properties
15-
/*
1614
@PublishedApi
1715
internal var propertyExposed: String? = null
1816

1917
@JvmField
2018
@PublishedApi
2119
internal var fieldExposed: String? = null
22-
*/
23-
2420
}

‎src/test/kotlin/cases/marker/marker.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ annotation class HiddenField
66
@Target(AnnotationTarget.PROPERTY)
77
annotation class HiddenProperty
88

9+
annotation class HiddenMethod
10+
911
public class Foo {
1012
// HiddenField will be on the field
1113
@HiddenField
@@ -14,5 +16,9 @@ public class Foo {
1416
// HiddenField will be on a synthetic `$annotations()` method
1517
@HiddenProperty
1618
var bar2 = 42
19+
20+
@HiddenMethod
21+
fun hiddenMethod(bar: Int = 42) {
22+
}
1723
}
1824

‎src/test/kotlin/cases/marker/marker.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ public final class cases/marker/Foo {
55
public abstract interface annotation class cases/marker/HiddenField : java/lang/annotation/Annotation {
66
}
77

8+
public abstract interface annotation class cases/marker/HiddenMethod : java/lang/annotation/Annotation {
9+
}
10+
811
public abstract interface annotation class cases/marker/HiddenProperty : java/lang/annotation/Annotation {
912
}
1013

‎src/test/kotlin/tests/CasesPublicAPITest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class CasesPublicAPITest {
4141

4242
@Test fun localClasses() { snapshotAPIAndCompare(testName.methodName) }
4343

44-
@Test fun marker() { snapshotAPIAndCompare(testName.methodName, setOf("cases/marker/HiddenField", "cases/marker/HiddenProperty")) }
44+
@Test fun marker() { snapshotAPIAndCompare(testName.methodName, setOf("cases/marker/HiddenField", "cases/marker/HiddenProperty", "cases/marker/HiddenMethod")) }
4545

4646
@Test fun nestedClasses() { snapshotAPIAndCompare(testName.methodName) }
4747

0 commit comments

Comments
 (0)