diff --git a/src/main/kotlin/api/AsmMetadataLoading.kt b/src/main/kotlin/api/AsmMetadataLoading.kt index fc7149af..c3731684 100644 --- a/src/main/kotlin/api/AsmMetadataLoading.kt +++ b/src/main/kotlin/api/AsmMetadataLoading.kt @@ -47,9 +47,7 @@ val ClassNode.outerClassName: String? get() = innerClassNode?.outerName const val publishedApiAnnotationName = "kotlin/PublishedApi" fun ClassNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null -fun MethodNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null -fun FieldNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null - +fun List.isPublishedApi() = firstOrNull { it.refersToName(publishedApiAnnotationName) } != null fun ClassNode.isDefaultImpls(metadata: KotlinClassMetadata?) = isInner() && name.endsWith("\$DefaultImpls") && metadata.isSyntheticClass() diff --git a/src/main/kotlin/api/KotlinMetadataSignature.kt b/src/main/kotlin/api/KotlinMetadataSignature.kt index fff11b66..29a7a311 100644 --- a/src/main/kotlin/api/KotlinMetadataSignature.kt +++ b/src/main/kotlin/api/KotlinMetadataSignature.kt @@ -7,6 +7,7 @@ package kotlinx.validation.api import kotlinx.metadata.jvm.* import kotlinx.validation.* +import org.objectweb.asm.Opcodes import org.objectweb.asm.tree.* @ExternalApi // Only name is part of the API, nothing else is used by stdlib @@ -48,7 +49,8 @@ data class MethodBinarySignature( override val jvmMember: JvmMethodSignature, override val isPublishedApi: Boolean, override val access: AccessFlags, - override val annotations: List + override val annotations: List, + private val alternateDefaultSignature: JvmMethodSignature? ) : MemberBinarySignature { override val signature: String get() = "${access.getModifierString()} fun $name $desc" @@ -59,60 +61,51 @@ data class MethodBinarySignature( && !isDummyDefaultConstructor() override fun findMemberVisibility(classVisibility: ClassVisibility?): MemberVisibility? { - return super.findMemberVisibility(classVisibility) ?: classVisibility?.let { alternateDefaultSignature(it.name)?.let(it::findMember) } + return super.findMemberVisibility(classVisibility) + ?: classVisibility?.let { alternateDefaultSignature?.let(it::findMember) } } - /** - * Checks whether the method is a $default counterpart of internal @PublishedApi method - */ - public fun isPublishedApiWithDefaultArguments( - classVisibility: ClassVisibility?, - publishedApiSignatures: Set - ): Boolean { - // Fast-path - findMemberVisibility(classVisibility)?.isInternal() ?: return false - val name = jvmMember.name - if (!name.endsWith("\$default")) return false - // Leverage the knowledge about modified signature - val expectedPublishedApiCounterPart = JvmMethodSignature( - name.removeSuffix("\$default"), - jvmMember.desc.replace( ";ILjava/lang/Object;)", ";)")) - return expectedPublishedApiCounterPart in publishedApiSignatures - } + private fun isAccessOrAnnotationsMethod() = + access.isSynthetic && (name.startsWith("access\$") || name.endsWith("\$annotations")) - private fun isAccessOrAnnotationsMethod() = access.isSynthetic && (name.startsWith("access\$") || name.endsWith("\$annotations")) - - private fun isDummyDefaultConstructor() = access.isSynthetic && name == "" && desc == "(Lkotlin/jvm/internal/DefaultConstructorMarker;)V" - - /** - * Calculates the signature of this method without default parameters - * - * Returns `null` if this method isn't an entry point of a function - * or a constructor with default parameters. - * Returns an incorrect result, if there are more than 31 default parameters. - */ - private fun alternateDefaultSignature(className: String): JvmMethodSignature? { - return when { - !access.isSynthetic -> null - name == "" && "ILkotlin/jvm/internal/DefaultConstructorMarker;" in desc -> - JvmMethodSignature(name, desc.replace("ILkotlin/jvm/internal/DefaultConstructorMarker;", "")) - name.endsWith("\$default") && "ILjava/lang/Object;)" in desc -> - JvmMethodSignature( - name.removeSuffix("\$default"), - desc.replace("ILjava/lang/Object;)", ")").replace("(L$className;", "(") - ) - else -> null - } + private fun isDummyDefaultConstructor() = + access.isSynthetic && name == "" && desc == "(Lkotlin/jvm/internal/DefaultConstructorMarker;)V" +} + +/** + * Calculates the signature of this method without default parameters + * + * Returns `null` if this method isn't an entry point of a function + * or a constructor with default parameters. + * Returns an incorrect result, if there are more than 31 default parameters. + */ +internal fun MethodNode.alternateDefaultSignature(className: String): JvmMethodSignature? { + return when { + access and Opcodes.ACC_SYNTHETIC == 0 -> null + name == "" && "ILkotlin/jvm/internal/DefaultConstructorMarker;" in desc -> + JvmMethodSignature(name, desc.replace("ILkotlin/jvm/internal/DefaultConstructorMarker;", "")) + name.endsWith("\$default") && "ILjava/lang/Object;)" in desc -> + JvmMethodSignature( + name.removeSuffix("\$default"), + desc.replace("ILjava/lang/Object;)", ")").replace("(L$className;", "(") + ) + else -> null } } -fun MethodNode.toMethodBinarySignature(propertyAnnotations: List) = - MethodBinarySignature( +fun MethodNode.toMethodBinarySignature( + extraAnnotations: List, + alternateDefaultSignature: JvmMethodSignature? +): MethodBinarySignature { + val allAnnotations = visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + extraAnnotations + return MethodBinarySignature( JvmMethodSignature(name, desc), - isPublishedApi(), + allAnnotations.isPublishedApi(), AccessFlags(access), - visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + propertyAnnotations + allAnnotations, + alternateDefaultSignature ) +} data class FieldBinarySignature( override val jvmMember: JvmFieldSignature, @@ -129,13 +122,15 @@ data class FieldBinarySignature( } } -fun FieldNode.toFieldBinarySignature() = - FieldBinarySignature( +fun FieldNode.toFieldBinarySignature(extraAnnotations: List): FieldBinarySignature { + val allAnnotations = visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + extraAnnotations + return FieldBinarySignature( JvmFieldSignature(name, desc), - isPublishedApi(), + allAnnotations.isPublishedApi(), AccessFlags(access), - annotations(visibleAnnotations, invisibleAnnotations)) - + allAnnotations + ) +} private val MemberBinarySignature.kind: Int get() = when (this) { is FieldBinarySignature -> 1 @@ -156,7 +151,9 @@ data class AccessFlags(val access: Int) { val isFinal: Boolean get() = isFinal(access) val isSynthetic: Boolean get() = isSynthetic(access) - fun getModifiers(): List = ACCESS_NAMES.entries.mapNotNull { if (access and it.key != 0) it.value else null } + fun getModifiers(): List = + ACCESS_NAMES.entries.mapNotNull { if (access and it.key != 0) it.value else null } + fun getModifierString(): String = getModifiers().joinToString(" ") } diff --git a/src/main/kotlin/api/KotlinMetadataVisibilities.kt b/src/main/kotlin/api/KotlinMetadataVisibilities.kt index 79ebe80a..2fe76397 100644 --- a/src/main/kotlin/api/KotlinMetadataVisibilities.kt +++ b/src/main/kotlin/api/KotlinMetadataVisibilities.kt @@ -87,7 +87,7 @@ fun KotlinClassMetadata?.isSyntheticClass() = this is KotlinClassMetadata.Synthe // Auxiliary class that stores signatures of corresponding field and method for a property. class PropertyAnnotationHolders( - val field: JvmMemberSignature?, + val field: JvmFieldSignature?, val method: JvmMethodSignature?, ) @@ -143,7 +143,7 @@ fun KotlinClassMetadata.toClassVisibility(classNode: ClassNode): ClassVisibility property.getterSignature == null && property.setterSignature == null -> property.flags // JvmField or const case else -> flagsOf(Flag.IS_PRIVATE) } - addMember(property.fieldSignature, fieldVisibility, isReified = false) + addMember(property.fieldSignature, fieldVisibility, isReified = false, propertyAnnotation = propertyAnnotations) } } diff --git a/src/main/kotlin/api/KotlinSignaturesLoading.kt b/src/main/kotlin/api/KotlinSignaturesLoading.kt index 81d0a52b..17d1dc01 100644 --- a/src/main/kotlin/api/KotlinSignaturesLoading.kt +++ b/src/main/kotlin/api/KotlinSignaturesLoading.kt @@ -43,8 +43,12 @@ public fun Sequence.loadApiFromJvmClasses(visibilityFilter: (String val supertypes = listOf(superName) - "java/lang/Object" + interfaces.sorted() val fieldSignatures = fields - .map { it.toFieldBinarySignature() } - .filter { + .map { + val annotationHolders = + mVisibility?.members?.get(JvmFieldSignature(it.name, it.desc))?.propertyAnnotation + val foundAnnotations = methods.annotationsFor(annotationHolders?.method) + it.toFieldBinarySignature(foundAnnotations) + }.filter { it.isEffectivelyPublic(classAccess, mVisibility) }.filter { /* @@ -62,7 +66,7 @@ public fun Sequence.loadApiFromJvmClasses(visibilityFilter: (String } // NB: this 'map' is O(methods + properties * methods) which may accidentally be quadratic - val allMethods = methods.map { + val methodSignatures = methods.map { /** * For getters/setters, pull the annotations from the property * This is either on the field if any or in a '$annotations' synthetic function. @@ -70,32 +74,21 @@ public fun Sequence.loadApiFromJvmClasses(visibilityFilter: (String val annotationHolders = mVisibility?.members?.get(JvmMethodSignature(it.name, it.desc))?.propertyAnnotation val foundAnnotations = ArrayList() - // lookup annotations from $annotations() - val syntheticPropertyMethod = annotationHolders?.method - if (syntheticPropertyMethod != null) { - foundAnnotations += methods - .firstOrNull { it.name == syntheticPropertyMethod.name && it.desc == syntheticPropertyMethod.desc } - ?.visibleAnnotations ?: emptyList() - } + foundAnnotations += fields.annotationsFor(annotationHolders?.field) + foundAnnotations += methods.annotationsFor(annotationHolders?.method) - val backingField = annotationHolders?.field - if (backingField != null) { - foundAnnotations += fields - .firstOrNull { it.name == backingField.name && it.desc == backingField.desc } - ?.visibleAnnotations ?: emptyList() + /** + * For synthetic $default methods, pull the annotations from the corresponding method + */ + val alternateDefaultSignature = mVisibility?.name?.let { className -> + it.alternateDefaultSignature(className) } + foundAnnotations += methods.annotationsFor(alternateDefaultSignature) - it.toMethodBinarySignature(foundAnnotations) + it.toMethodBinarySignature(foundAnnotations, alternateDefaultSignature) + }.filter { + it.isEffectivelyPublic(classAccess, mVisibility) } - // Signatures marked with @PublishedApi - val publishedApiSignatures = allMethods.filter { - it.isPublishedApi - }.map { it.jvmMember }.toSet() - val methodSignatures = allMethods - .filter { - it.isEffectivelyPublic(classAccess, mVisibility) || - it.isPublishedApiWithDefaultArguments(mVisibility, publishedApiSignatures) - } ClassBinarySignature( name, superName, outerClassName, supertypes, fieldSignatures + methodSignatures, classAccess, @@ -107,6 +100,24 @@ public fun Sequence.loadApiFromJvmClasses(visibilityFilter: (String } } +private fun List.annotationsFor(methodSignature: JvmMethodSignature?): List { + if (methodSignature == null) return emptyList() + + return firstOrNull { it.name == methodSignature.name && it.desc == methodSignature.desc } + ?.run { + visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + } ?: emptyList() +} + +private fun List.annotationsFor(fieldSignature: JvmFieldSignature?): List { + if (fieldSignature == null) return emptyList() + + return firstOrNull { it.name == fieldSignature.name && it.desc == fieldSignature.desc } + ?.run { + visibleAnnotations.orEmpty() + invisibleAnnotations.orEmpty() + } ?: emptyList() +} + @ExternalApi public fun List.filterOutAnnotated(targetAnnotations: Set): List { if (targetAnnotations.isEmpty()) return this diff --git a/src/test/kotlin/cases/inline/inline.txt b/src/test/kotlin/cases/inline/inline.txt index 4961dbcf..4a29e9b0 100644 --- a/src/test/kotlin/cases/inline/inline.txt +++ b/src/test/kotlin/cases/inline/inline.txt @@ -3,7 +3,10 @@ public final class cases/inline/InlineExposedKt { } public final class cases/inline/InternalClassExposed { + public field fieldExposed Ljava/lang/String; public fun ()V public final fun funExposed ()V + public final fun getPropertyExposed ()Ljava/lang/String; + public final fun setPropertyExposed (Ljava/lang/String;)V } diff --git a/src/test/kotlin/cases/inline/inlineExposed.kt b/src/test/kotlin/cases/inline/inlineExposed.kt index 5aee74a8..79030161 100644 --- a/src/test/kotlin/cases/inline/inlineExposed.kt +++ b/src/test/kotlin/cases/inline/inlineExposed.kt @@ -11,14 +11,10 @@ internal class InternalClassExposed @PublishedApi internal fun funExposed() {} - // TODO: Cover unsupported cases: requires correctly reflecting annotations from properties - /* @PublishedApi internal var propertyExposed: String? = null @JvmField @PublishedApi internal var fieldExposed: String? = null - */ - } diff --git a/src/test/kotlin/cases/marker/marker.kt b/src/test/kotlin/cases/marker/marker.kt index e0c907a9..f7b5989e 100644 --- a/src/test/kotlin/cases/marker/marker.kt +++ b/src/test/kotlin/cases/marker/marker.kt @@ -6,6 +6,8 @@ annotation class HiddenField @Target(AnnotationTarget.PROPERTY) annotation class HiddenProperty +annotation class HiddenMethod + public class Foo { // HiddenField will be on the field @HiddenField @@ -14,5 +16,9 @@ public class Foo { // HiddenField will be on a synthetic `$annotations()` method @HiddenProperty var bar2 = 42 + + @HiddenMethod + fun hiddenMethod(bar: Int = 42) { + } } diff --git a/src/test/kotlin/cases/marker/marker.txt b/src/test/kotlin/cases/marker/marker.txt index cd931920..31054470 100644 --- a/src/test/kotlin/cases/marker/marker.txt +++ b/src/test/kotlin/cases/marker/marker.txt @@ -5,6 +5,9 @@ public final class cases/marker/Foo { public abstract interface annotation class cases/marker/HiddenField : java/lang/annotation/Annotation { } +public abstract interface annotation class cases/marker/HiddenMethod : java/lang/annotation/Annotation { +} + public abstract interface annotation class cases/marker/HiddenProperty : java/lang/annotation/Annotation { } diff --git a/src/test/kotlin/tests/CasesPublicAPITest.kt b/src/test/kotlin/tests/CasesPublicAPITest.kt index 7b6cf896..33f0f02f 100644 --- a/src/test/kotlin/tests/CasesPublicAPITest.kt +++ b/src/test/kotlin/tests/CasesPublicAPITest.kt @@ -41,7 +41,7 @@ class CasesPublicAPITest { @Test fun localClasses() { snapshotAPIAndCompare(testName.methodName) } - @Test fun marker() { snapshotAPIAndCompare(testName.methodName, setOf("cases/marker/HiddenField", "cases/marker/HiddenProperty")) } + @Test fun marker() { snapshotAPIAndCompare(testName.methodName, setOf("cases/marker/HiddenField", "cases/marker/HiddenProperty", "cases/marker/HiddenMethod")) } @Test fun nestedClasses() { snapshotAPIAndCompare(testName.methodName) }