Skip to content

Commit 724c5d5

Browse files
martinbonninshanshin
authored andcommitted
[ABI Validation] Fix false positive for const val with non-public marker
Closes Kotlin/binary-compatibility-validator#90 Pull request Kotlin/binary-compatibility-validator#245
1 parent a27354c commit 724c5d5

File tree

7 files changed

+133
-5
lines changed

7 files changed

+133
-5
lines changed

libraries/tools/abi-validation/src/functionalTest/kotlin/kotlinx/validation/test/NonPublicMarkersTest.kt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,4 +104,30 @@ class NonPublicMarkersTest : BaseKotlinGradleTest() {
104104
Assertions.assertThat(rootProjectApiDump.readText()).isEqualToIgnoringNewLines(expected)
105105
}
106106
}
107+
108+
@Test
109+
fun testIgnoredMarkersOnConstProperties() {
110+
val runner = test {
111+
buildGradleKts {
112+
resolve("/examples/gradle/base/withPlugin.gradle.kts")
113+
resolve("/examples/gradle/configuration/nonPublicMarkers/markers.gradle.kts")
114+
}
115+
116+
kotlin("ConstProperty.kt") {
117+
resolve("/examples/classes/ConstProperty.kt")
118+
}
119+
120+
apiFile(projectName = rootProjectDir.name) {
121+
resolve("/examples/classes/ConstProperty.dump")
122+
}
123+
124+
runner {
125+
arguments.add(":apiCheck")
126+
}
127+
}
128+
129+
runner.withDebug(true).build().apply {
130+
assertTaskSuccess(":apiCheck")
131+
}
132+
}
107133
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
public final class foo/Foo {
2+
public static final field Companion Lfoo/Foo$Companion;
3+
public fun <init> ()V
4+
}
5+
6+
public final class foo/Foo$Companion {
7+
}
8+
9+
public abstract interface annotation class foo/HiddenProperty : java/lang/annotation/Annotation {
10+
}
11+
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright 2016-2022 JetBrains s.r.o.
3+
* Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
4+
*/
5+
package foo
6+
7+
@Target(AnnotationTarget.PROPERTY)
8+
annotation class HiddenProperty
9+
10+
public class Foo {
11+
companion object {
12+
@HiddenProperty
13+
const val bar = "barValue"
14+
}
15+
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,11 @@ internal fun FieldNode.isCompanionField(outerClassMetadata: KotlinClassMetadata?
175175
return metadata.toKmClass().companionObject == name
176176
}
177177

178-
internal fun ClassNode.companionName(outerClassMetadata: KotlinClassMetadata?): String {
179-
val outerKClass = (outerClassMetadata as KotlinClassMetadata.Class).toKmClass()
178+
internal fun ClassNode.companionName(outerClassMetadata: KotlinClassMetadata?): String? {
179+
if (outerClassMetadata !is KotlinClassMetadata.Class) {
180+
// Happens when outerClassMetadata == KotlinClassMetadata$FileFacade for an example
181+
return null
182+
}
183+
val outerKClass = outerClassMetadata.toKmClass()
180184
return name + "$" + outerKClass.companionObject
181185
}

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

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55

66
package kotlinx.validation.api
77

8+
import kotlinx.metadata.Flag
9+
import kotlinx.metadata.KmProperty
10+
import kotlinx.metadata.internal.metadata.jvm.deserialization.JvmFlags
811
import kotlinx.metadata.jvm.*
912
import kotlinx.validation.*
1013
import org.objectweb.asm.*
@@ -119,10 +122,26 @@ private fun FieldNode.buildFieldSignature(
119122
* as non-public API using some annotation, then we won't be able to filter out
120123
* the companion field.
121124
*/
122-
val companionName = ownerClass.companionName(ownerClass.kotlinMetadata)
125+
val companionName = ownerClass.companionName(ownerClass.kotlinMetadata)!!
123126
companionClass = classes[companionName]
124127
foundAnnotations.addAll(companionClass?.visibleAnnotations.orEmpty())
125128
foundAnnotations.addAll(companionClass?.invisibleAnnotations.orEmpty())
129+
} else if (isStatic(access) && isFinal(access)) {
130+
companionClass = ownerClass.companionName(ownerClass.kotlinMetadata)?.let {
131+
classes[it]
132+
}
133+
134+
val property = companionClass?.kmProperty(name)
135+
136+
if (property != null && JvmFlag.Property.IS_MOVED_FROM_INTERFACE_COMPANION(property.flags)) {
137+
/*
138+
* The property was moved from the companion object. Take all the annotations from there
139+
* to be able to filter out the non-public markers.
140+
*
141+
* See https://github.com/Kotlin/binary-compatibility-validator/issues/90
142+
*/
143+
foundAnnotations.addAll(companionClass!!.methods.annotationsFor(property.syntheticMethodForAnnotations))
144+
}
126145
}
127146

128147
val fieldSignature = toFieldBinarySignature(foundAnnotations)
@@ -133,6 +152,18 @@ private fun FieldNode.buildFieldSignature(
133152
}
134153
}
135154

155+
private fun ClassNode.kmProperty(name: String?): KmProperty? {
156+
val metadata = kotlinMetadata ?: return null
157+
158+
if (metadata !is KotlinClassMetadata.Class) {
159+
return null
160+
}
161+
162+
return metadata.toKmClass().properties.firstOrNull {
163+
it.name == name
164+
}
165+
}
166+
136167
private fun MethodNode.buildMethodSignature(
137168
ownerVisibility: ClassVisibility?,
138169
ownerClass: ClassNode
@@ -236,8 +267,8 @@ public fun List<ClassBinarySignature>.extractAnnotatedPackages(targetAnnotations
236267
// package-info classes are private synthetic abstract interfaces since 2005 (JDK-6232928).
237268
it.access.isInterface && it.access.isSynthetic && it.access.isAbstract
238269
}.filter {
239-
it.annotations.any {
240-
ann -> targetAnnotations.any { ann.refersToName(it) }
270+
it.annotations.any { ann ->
271+
targetAnnotations.any { ann.refersToName(it) }
241272
}
242273
}.map {
243274
val res = it.name.substring(0, it.name.length - "/package-info".length)

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,27 @@ class FilteredNamedCompanionObjectHolder private constructor() {
157157
val F: Int = 42
158158
}
159159
}
160+
161+
class FilteredCompanionProperties private constructor() {
162+
companion object {
163+
public val F1: Int = 1
164+
public const val F2: Int = 2
165+
private val F3: Int = 3
166+
private const val F4: Int = 4
167+
@PrivateApi
168+
val F5: Int = 5
169+
@PrivateApi
170+
const val F6: Int = 6
171+
@PrivateApi
172+
const val F7: Int = 7
173+
}
174+
}
175+
176+
class FilteredCompanionFunctions private constructor() {
177+
companion object {
178+
public fun f1(): Int = 1
179+
private fun f2(): Int = 2
180+
@PrivateApi
181+
public fun f3(): Int = 3
182+
}
183+
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,23 @@
1+
public final class cases/companions/FilteredCompanionFunctions {
2+
public static final field Companion Lcases/companions/FilteredCompanionFunctions$Companion;
3+
}
4+
5+
public final class cases/companions/FilteredCompanionFunctions$Companion {
6+
public final fun f1 ()I
7+
}
8+
19
public final class cases/companions/FilteredCompanionObjectHolder {
210
}
311

12+
public final class cases/companions/FilteredCompanionProperties {
13+
public static final field Companion Lcases/companions/FilteredCompanionProperties$Companion;
14+
public static final field F2 I
15+
}
16+
17+
public final class cases/companions/FilteredCompanionProperties$Companion {
18+
public final fun getF1 ()I
19+
}
20+
421
public final class cases/companions/FilteredNamedCompanionObjectHolder {
522
}
623

0 commit comments

Comments
 (0)