Skip to content

Commit 9bb0e4d

Browse files
fzhinkinshanshin
fzhinkin
authored and
shanshin
committed
[ABI Validation] Add all Companion class annotations to the corresponding Companion field.
* Add all Companion class annotations to the corresponding Companion field. * Extract logic for building field and method signatures to separate methods Fixes Kotlin/binary-compatibility-validator#157 Pull request Kotlin/binary-compatibility-validator#162
1 parent 1f16136 commit 9bb0e4d

File tree

5 files changed

+117
-42
lines changed

5 files changed

+117
-42
lines changed

‎libraries/tools/abi-validation/src/main/kotlin/api/KotlinMetadataSignature.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,11 +164,16 @@ internal data class AccessFlags(val access: Int) {
164164
fun getModifierString(): String = getModifiers().joinToString(" ")
165165
}
166166

167-
internal fun FieldBinarySignature.isCompanionField(outerClassMetadata: KotlinClassMetadata?): Boolean {
167+
internal fun FieldNode.isCompanionField(outerClassMetadata: KotlinClassMetadata?): Boolean {
168+
val access = AccessFlags(access)
168169
if (!access.isFinal || !access.isStatic) return false
169170
val metadata = outerClassMetadata ?: return false
170171
// Non-classes are not affected by the problem
171172
if (metadata !is KotlinClassMetadata.Class) return false
172173
return metadata.toKmClass().companionObject == name
173174
}
174175

176+
internal fun ClassNode.companionName(outerClassMetadata: KotlinClassMetadata?): String {
177+
val outerKClass = (outerClassMetadata as KotlinClassMetadata.Class).toKmClass()
178+
return name + "$" + outerKClass.companionObject
179+
}

‎libraries/tools/abi-validation/src/main/kotlin/api/KotlinSignaturesLoading.kt

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

4545
val fieldSignatures = fields
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 {
52-
it.isEffectivelyPublic(classAccess, mVisibility)
53-
}.filter {
46+
.map { it.buildFieldSignature(mVisibility, this, classNodeMap) }
47+
.filter { it.field.isEffectivelyPublic(classAccess, mVisibility) }
48+
.filter {
5449
/*
5550
* Filter out 'public static final Companion' field that doesn't constitute public API.
5651
* For that we first check if field corresponds to the 'Companion' class and then
5752
* if companion is effectively public by itself, so the 'Companion' field has the same visibility.
5853
*/
59-
if (!it.isCompanionField(classNode.kotlinMetadata)) return@filter true
60-
val outerKClass = (classNode.kotlinMetadata as KotlinClassMetadata.Class).toKmClass()
61-
val companionName = name + "$" + outerKClass.companionObject
62-
// False positive is better than the crash here
63-
val companionClass = classNodeMap[companionName] ?: return@filter true
64-
val visibility = visibilityMap[companionName] ?: return@filter true
54+
val companionClass = when (it) {
55+
is BasicFieldBinarySignature -> return@filter true
56+
is CompanionFieldBinarySignature -> it.companion
57+
}
58+
val visibility = visibilityMap[companionClass.name] ?: return@filter true
6559
companionClass.isEffectivelyPublic(visibility)
66-
}
60+
}.map { it.field }
6761

6862
// NB: this 'map' is O(methods + properties * methods) which may accidentally be quadratic
69-
val methodSignatures = methods.map {
70-
/**
71-
* For getters/setters, pull the annotations from the property
72-
* This is either on the field if any or in a '$annotations' synthetic function.
73-
*/
74-
val annotationHolders =
75-
mVisibility?.members?.get(JvmMethodSignature(it.name, it.desc))?.propertyAnnotation
76-
val foundAnnotations = ArrayList<AnnotationNode>()
77-
if (annotationHolders != null) {
78-
foundAnnotations += fields.annotationsFor(annotationHolders.field)
79-
foundAnnotations += methods.annotationsFor(annotationHolders.method)
80-
}
81-
82-
/**
83-
* For synthetic $default methods, pull the annotations from the corresponding method
84-
*/
85-
val alternateDefaultSignature = mVisibility?.name?.let { className ->
86-
it.alternateDefaultSignature(className)
87-
}
88-
foundAnnotations += methods.annotationsFor(alternateDefaultSignature)
89-
90-
it.toMethodBinarySignature(foundAnnotations, alternateDefaultSignature)
91-
}.filter {
92-
it.isEffectivelyPublic(classAccess, mVisibility)
93-
}
63+
val methodSignatures = methods.map { it.buildMethodSignature(mVisibility, this) }
64+
.filter { it.isEffectivelyPublic(classAccess, mVisibility) }
9465

9566
ClassBinarySignature(
9667
name, superName, outerClassName, supertypes, fieldSignatures + methodSignatures, classAccess,
@@ -102,6 +73,82 @@ public fun Sequence<InputStream>.loadApiFromJvmClasses(visibilityFilter: (String
10273
}
10374
}
10475

76+
/**
77+
* Wraps a [FieldBinarySignature] along with additional information.
78+
*/
79+
private sealed class FieldBinarySignatureWrapper(val field: FieldBinarySignature)
80+
81+
/**
82+
* Wraps a regular field's binary signature.
83+
*/
84+
private class BasicFieldBinarySignature(field: FieldBinarySignature) : FieldBinarySignatureWrapper(field)
85+
86+
/**
87+
* Wraps a binary signature for a field referencing a companion object.
88+
*/
89+
private class CompanionFieldBinarySignature(field: FieldBinarySignature, val companion: ClassNode) :
90+
FieldBinarySignatureWrapper(field)
91+
92+
private fun FieldNode.buildFieldSignature(
93+
ownerVisibility: ClassVisibility?,
94+
ownerClass: ClassNode,
95+
classes: TreeMap<String, ClassNode>
96+
): FieldBinarySignatureWrapper {
97+
val annotationHolders =
98+
ownerVisibility?.members?.get(JvmFieldSignature(name, desc))?.propertyAnnotation
99+
val foundAnnotations = mutableListOf<AnnotationNode>()
100+
foundAnnotations.addAll(ownerClass.methods.annotationsFor(annotationHolders?.method))
101+
102+
var companionClass: ClassNode? = null
103+
if (isCompanionField(ownerClass.kotlinMetadata)) {
104+
/*
105+
* If the field was generated to hold the reference to a companion class's instance,
106+
* then we have to also take all annotations from the companion class an associate it with
107+
* the field. Otherwise, all these annotations will be lost and if the class was marked
108+
* as non-public API using some annotation, then we won't be able to filter out
109+
* the companion field.
110+
*/
111+
val companionName = ownerClass.companionName(ownerClass.kotlinMetadata)
112+
companionClass = classes[companionName]
113+
foundAnnotations.addAll(companionClass?.visibleAnnotations.orEmpty())
114+
foundAnnotations.addAll(companionClass?.invisibleAnnotations.orEmpty())
115+
}
116+
117+
val fieldSignature = toFieldBinarySignature(foundAnnotations)
118+
return if (companionClass != null) {
119+
CompanionFieldBinarySignature(fieldSignature, companionClass)
120+
} else {
121+
BasicFieldBinarySignature(fieldSignature)
122+
}
123+
}
124+
125+
private fun MethodNode.buildMethodSignature(
126+
ownerVisibility: ClassVisibility?,
127+
ownerClass: ClassNode
128+
): MethodBinarySignature {
129+
/**
130+
* For getters/setters, pull the annotations from the property
131+
* This is either on the field if any or in a '$annotations' synthetic function.
132+
*/
133+
val annotationHolders =
134+
ownerVisibility?.members?.get(JvmMethodSignature(name, desc))?.propertyAnnotation
135+
val foundAnnotations = ArrayList<AnnotationNode>()
136+
if (annotationHolders != null) {
137+
foundAnnotations += ownerClass.fields.annotationsFor(annotationHolders.field)
138+
foundAnnotations += ownerClass.methods.annotationsFor(annotationHolders.method)
139+
}
140+
141+
/**
142+
* For synthetic $default methods, pull the annotations from the corresponding method
143+
*/
144+
val alternateDefaultSignature = ownerVisibility?.name?.let { className ->
145+
alternateDefaultSignature(className)
146+
}
147+
foundAnnotations += ownerClass.methods.annotationsFor(alternateDefaultSignature)
148+
149+
return toMethodBinarySignature(foundAnnotations, alternateDefaultSignature)
150+
}
151+
105152
private fun List<MethodNode>.annotationsFor(methodSignature: JvmMethodSignature?): List<AnnotationNode> {
106153
if (methodSignature == null) return emptyList()
107154

‎libraries/tools/abi-validation/src/test/kotlin/cases/companions/companions.kt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,20 @@ object PrivateInterfaces {
140140
}
141141
}
142142

143+
@PrivateApi
144+
annotation class PrivateApi
145+
146+
147+
class FilteredCompanionObjectHolder private constructor() {
148+
@PrivateApi
149+
companion object {
150+
val F: Int = 42
151+
}
152+
}
153+
154+
class FilteredNamedCompanionObjectHolder private constructor() {
155+
@PrivateApi
156+
companion object Named {
157+
val F: Int = 42
158+
}
159+
}

‎libraries/tools/abi-validation/src/test/kotlin/cases/companions/companions.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
public final class cases/companions/FilteredCompanionObjectHolder {
2+
}
3+
4+
public final class cases/companions/FilteredNamedCompanionObjectHolder {
5+
}
6+
17
public final class cases/companions/InternalClasses {
28
public static final field INSTANCE Lcases/companions/InternalClasses;
39
}

‎libraries/tools/abi-validation/src/test/kotlin/tests/CasesPublicAPITest.kt

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

2828
@Test fun annotations() { snapshotAPIAndCompare(testName.methodName) }
2929

30-
@Test fun companions() { snapshotAPIAndCompare(testName.methodName) }
30+
@Test fun companions() { snapshotAPIAndCompare(testName.methodName, setOf("cases/companions/PrivateApi")) }
3131

3232
@Test fun default() { snapshotAPIAndCompare(testName.methodName) }
3333

0 commit comments

Comments
 (0)