Skip to content
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.

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

Fix false positive for const val with non-public marker #245

Merged
merged 4 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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() {
fzhinkin marked this conversation as resolved.
Show resolved Hide resolved
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 (access.and(Opcodes.ACC_STATIC) != 0 && access.and(Opcodes.ACC_FINAL) != 0) {
fzhinkin marked this conversation as resolved.
Show resolved Hide resolved
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