Skip to content

Commit

Permalink
Fix false positive for const val with non-public marker (#245)
Browse files Browse the repository at this point in the history
Closes #90
  • Loading branch information
martinbonnin authored Jun 28, 2024
1 parent 7474660 commit 0dd6e41
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,30 @@ class NonPublicMarkersTest : BaseKotlinGradleTest() {
Assertions.assertThat(rootProjectApiDump.readText()).isEqualToIgnoringNewLines(expected)
}
}

@Test
fun testIgnoredMarkersOnConstProperties() {
val runner = test {
buildGradleKts {
resolve("/examples/gradle/base/withPlugin.gradle.kts")
resolve("/examples/gradle/configuration/nonPublicMarkers/markers.gradle.kts")
}

kotlin("ConstProperty.kt") {
resolve("/examples/classes/ConstProperty.kt")
}

apiFile(projectName = rootProjectDir.name) {
resolve("/examples/classes/ConstProperty.dump")
}

runner {
arguments.add(":apiCheck")
}
}

runner.withDebug(true).build().apply {
assertTaskSuccess(":apiCheck")
}
}
}
11 changes: 11 additions & 0 deletions src/functionalTest/resources/examples/classes/ConstProperty.dump
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
public final class foo/Foo {
public static final field Companion Lfoo/Foo$Companion;
public fun <init> ()V
}

public final class foo/Foo$Companion {
}

public abstract interface annotation class foo/HiddenProperty : java/lang/annotation/Annotation {
}

15 changes: 15 additions & 0 deletions src/functionalTest/resources/examples/classes/ConstProperty.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright 2016-2022 JetBrains s.r.o.
* Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
*/
package foo

@Target(AnnotationTarget.PROPERTY)
annotation class HiddenProperty

public class Foo {
companion object {
@HiddenProperty
const val bar = "barValue"
}
}
8 changes: 6 additions & 2 deletions src/main/kotlin/api/KotlinMetadataSignature.kt
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ internal fun FieldNode.isCompanionField(outerClassMetadata: KotlinClassMetadata?
return metadata.toKmClass().companionObject == name
}

internal fun ClassNode.companionName(outerClassMetadata: KotlinClassMetadata?): String {
val outerKClass = (outerClassMetadata as KotlinClassMetadata.Class).toKmClass()
internal fun ClassNode.companionName(outerClassMetadata: KotlinClassMetadata?): String? {
if (outerClassMetadata !is KotlinClassMetadata.Class) {
// Happens when outerClassMetadata == KotlinClassMetadata$FileFacade for an example
return null
}
val outerKClass = outerClassMetadata.toKmClass()
return name + "$" + outerKClass.companionObject
}
37 changes: 34 additions & 3 deletions src/main/kotlin/api/KotlinSignaturesLoading.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

package kotlinx.validation.api

import kotlinx.metadata.Flag
import kotlinx.metadata.KmProperty
import kotlinx.metadata.internal.metadata.jvm.deserialization.JvmFlags
import kotlinx.metadata.jvm.*
import kotlinx.validation.*
import org.objectweb.asm.*
Expand Down Expand Up @@ -119,10 +122,26 @@ private fun FieldNode.buildFieldSignature(
* as non-public API using some annotation, then we won't be able to filter out
* the companion field.
*/
val companionName = ownerClass.companionName(ownerClass.kotlinMetadata)
val companionName = ownerClass.companionName(ownerClass.kotlinMetadata)!!
companionClass = classes[companionName]
foundAnnotations.addAll(companionClass?.visibleAnnotations.orEmpty())
foundAnnotations.addAll(companionClass?.invisibleAnnotations.orEmpty())
} else if (isStatic(access) && isFinal(access)) {
companionClass = ownerClass.companionName(ownerClass.kotlinMetadata)?.let {
classes[it]
}

val property = companionClass?.kmProperty(name)

if (property != null && JvmFlag.Property.IS_MOVED_FROM_INTERFACE_COMPANION(property.flags)) {
/*
* The property was moved from the companion object. Take all the annotations from there
* to be able to filter out the non-public markers.
*
* See https://github.com/Kotlin/binary-compatibility-validator/issues/90
*/
foundAnnotations.addAll(companionClass!!.methods.annotationsFor(property.syntheticMethodForAnnotations))
}
}

val fieldSignature = toFieldBinarySignature(foundAnnotations)
Expand All @@ -133,6 +152,18 @@ private fun FieldNode.buildFieldSignature(
}
}

private fun ClassNode.kmProperty(name: String?): KmProperty? {
val metadata = kotlinMetadata ?: return null

if (metadata !is KotlinClassMetadata.Class) {
return null
}

return metadata.toKmClass().properties.firstOrNull {
it.name == name
}
}

private fun MethodNode.buildMethodSignature(
ownerVisibility: ClassVisibility?,
ownerClass: ClassNode
Expand Down Expand Up @@ -236,8 +267,8 @@ public fun List<ClassBinarySignature>.extractAnnotatedPackages(targetAnnotations
// package-info classes are private synthetic abstract interfaces since 2005 (JDK-6232928).
it.access.isInterface && it.access.isSynthetic && it.access.isAbstract
}.filter {
it.annotations.any {
ann -> targetAnnotations.any { ann.refersToName(it) }
it.annotations.any { ann ->
targetAnnotations.any { ann.refersToName(it) }
}
}.map {
val res = it.name.substring(0, it.name.length - "/package-info".length)
Expand Down
24 changes: 24 additions & 0 deletions src/test/kotlin/cases/companions/companions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,27 @@ class FilteredNamedCompanionObjectHolder private constructor() {
val F: Int = 42
}
}

class FilteredCompanionProperties private constructor() {
companion object {
public val F1: Int = 1
public const val F2: Int = 2
private val F3: Int = 3
private const val F4: Int = 4
@PrivateApi
val F5: Int = 5
@PrivateApi
const val F6: Int = 6
@PrivateApi
const val F7: Int = 7
}
}

class FilteredCompanionFunctions private constructor() {
companion object {
public fun f1(): Int = 1
private fun f2(): Int = 2
@PrivateApi
public fun f3(): Int = 3
}
}
17 changes: 17 additions & 0 deletions src/test/kotlin/cases/companions/companions.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
public final class cases/companions/FilteredCompanionFunctions {
public static final field Companion Lcases/companions/FilteredCompanionFunctions$Companion;
}

public final class cases/companions/FilteredCompanionFunctions$Companion {
public final fun f1 ()I
}

public final class cases/companions/FilteredCompanionObjectHolder {
}

public final class cases/companions/FilteredCompanionProperties {
public static final field Companion Lcases/companions/FilteredCompanionProperties$Companion;
public static final field F2 I
}

public final class cases/companions/FilteredCompanionProperties$Companion {
public final fun getF1 ()I
}

public final class cases/companions/FilteredNamedCompanionObjectHolder {
}

Expand Down

0 comments on commit 0dd6e41

Please sign in to comment.