fix(metadata-generator): read Kotlin metadata leniently

Classes compiled by a Kotlin newer than the one the generator jar was built
against were silently dropped from the generated metadata, so their members
were missing at runtime. Reported against Kotlin 2.4.x, where an app failed
to start with "Unable to create application".

KotlinClassMetadata.readStrict rejects any metadata version newer than the
bundled kotlin-metadata-jvm, and the resulting IllegalArgumentException
surfaced lazily inside Builder.build's catch-all, which just logged
"Skip <class>" and moved on. readLenient accepts any version from 1.1.0 up;
the only thing it gives up is writing metadata back, which this tool never
does. A version bump alone cannot fix this, since the jar ships prebuilt
while apps pick their own Kotlin version.

Unreadable metadata now falls back to bytecode-only parsing instead of
dropping the class, and an extension function whose metadata signature has
no matching bytecode method is skipped rather than throwing.

Kotlin and kotlin-metadata-jvm go to 2.4.10. Both have to move together:
kotlin-metadata-jvm pulls in a matching kotlin-stdlib, which an older
compiler then refuses to read.

Verified that the metadata generated for the test app is unchanged.

Author: edusperoni

test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/parsing/kotlin/classes/KotlinClassDescriptor.kt

@@ -263,7 +263,19 @@ class KotlinClassDescriptor(nativeClass: JavaClass, private val metadataAnnotati
                 metadataAnnotation.packageName,
                 metadataAnnotation.extraInt)
 
-        KotlinClassMetadata.readStrict(metadata)
+        // readStrict rejects any class whose metadata version is newer than the bundled
+        // kotlin-metadata-jvm, which happens whenever an app or one of its dependencies is
+        // compiled with a Kotlin release newer than the one this jar was built against.
+        // Lenient reading only gives up the ability to write metadata back, which nothing here does.
+        try {
+            KotlinClassMetadata.readLenient(metadata)
+        } catch (e: Exception) {
+            // Bytecode-only parsing still yields usable members, so never let an unreadable
+            // annotation drop the whole class from the generated metadata.
+            println("Warning: could not read Kotlin metadata for $className; falling back to bytecode-only parsing")
+            println("\tError: $e")
+            null
+        }
     }
 
 

test-app/build-tools/android-metadata-generator/src/src/com/telerik/metadata/parsing/kotlin/extensions/bytecode/BytecodeExtensionFunctionsCollector.kt

@@ -26,11 +26,13 @@ class BytecodeExtensionFunctionsCollector(private val kotlinClassMetadataParser:
                 val functionName = signature.name
                 val functionSignature = signature.descriptor
 
+                // Metadata written by a newer Kotlin compiler can describe functions this jar's
+                // bytecode view does not match; skip those rather than dropping the whole class.
                 val extensionFunctionDescriptor: KotlinMethodDescriptor = Arrays
                         .stream(kotlinClassDescriptor.methods)
                         .filter { x -> x.name == functionName && x.signature == functionSignature }
                         .findFirst()
-                        .get()
+                        .orElse(null) ?: continue
 
                 if (extensionFunctionDescriptor.isStatic) {
                     val receiverType = extensionFunctionDescriptor.argumentTypes[0] // kotlin extension functions' first argument is the receiver type

test-app/build-tools/android-metadata-generator/src/test/java/com/telerik/metadata/parsing/kotlin/classes/KotlinClassDescriptorMetadataVersionTest.kt

@@ -0,0 +1,104 @@
+package com.telerik.metadata.parsing.kotlin.classes
+
+import com.telerik.metadata.parsing.kotlin.metadata.MetadataAnnotation
+import org.apache.bcel.classfile.ClassParser
+import org.apache.bcel.classfile.JavaClass
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Assert.fail
+import org.junit.Test
+import kotlin.metadata.jvm.KotlinClassMetadata
+import kotlin.metadata.jvm.Metadata
+
+class KotlinClassDescriptorMetadataVersionTest {
+
+    companion object {
+        // Newer than any Kotlin release the bundled kotlin-metadata-jvm knows about, so the test
+        // stays meaningful after the library is upgraded.
+        private val FUTURE_METADATA_VERSION = intArrayOf(2, 9, 0)
+    }
+
+    private val fixture = KotlinMetadataVersionFixture::class.java
+
+    @Test
+    fun `reads class metadata written by a newer Kotlin compiler`() {
+        val metadata = descriptorFor(fixture, FUTURE_METADATA_VERSION).kotlinMetadata
+
+        assertTrue("Expected readable class metadata, got $metadata", metadata is KotlinClassMetadata.Class)
+        assertTrue((metadata as KotlinClassMetadata.Class).kmClass.name.endsWith("KotlinMetadataVersionFixture"))
+    }
+
+    @Test
+    fun `exposes members of a class compiled with a newer Kotlin compiler`() {
+        val descriptor = descriptorFor(fixture, FUTURE_METADATA_VERSION)
+
+        assertTrue("Class visibility should come from Kotlin metadata", descriptor.isPublic)
+        assertTrue("Expected the 'counter' property", descriptor.properties.any { it.name == "counter" })
+        assertTrue("Expected the 'greet' method", descriptor.methods.any { it.name == "greet" })
+    }
+
+    @Test
+    fun `falls back to bytecode parsing when metadata cannot be read at all`() {
+        val descriptor = KotlinClassDescriptor(bcelClass(fixture), corruptAnnotation(), false)
+
+        assertEquals(null, descriptor.kotlinMetadata)
+        assertTrue("Bytecode-derived visibility should still be available", descriptor.isPublic)
+        assertTrue("Bytecode-derived methods should still be available", descriptor.methods.any { it.name == "greet" })
+    }
+
+    @Test
+    fun `strict reading is what rejects newer metadata`() {
+        val annotation = annotationWithVersion(fixture, FUTURE_METADATA_VERSION)
+
+        try {
+            KotlinClassMetadata.readStrict(toMetadata(annotation))
+            fail("readStrict was expected to reject metadata version 2.9.0")
+        } catch (e: IllegalArgumentException) {
+            assertTrue(e.message!!.contains("version"))
+        }
+    }
+
+    private fun descriptorFor(clazz: Class<*>, metadataVersion: IntArray) =
+            KotlinClassDescriptor(bcelClass(clazz), annotationWithVersion(clazz, metadataVersion), false)
+
+    private fun bcelClass(clazz: Class<*>): JavaClass {
+        val resource = clazz.name.replace('.', '/') + ".class"
+        return clazz.classLoader.getResourceAsStream(resource).use {
+            ClassParser(it, resource).parse()
+        }
+    }
+
+    private fun annotationWithVersion(clazz: Class<*>, metadataVersion: IntArray): MetadataAnnotation {
+        val real = clazz.getAnnotation(kotlin.Metadata::class.java)
+        return object : MetadataAnnotation {
+            override val kind = real.kind
+            override val metadataVersion = metadataVersion
+            override val bytecodeVersion = intArrayOf()
+            override val data1 = real.data1
+            override val data2 = real.data2
+            override val extraString = real.extraString
+            override val packageName = real.packageName
+            override val extraInt = real.extraInt
+        }
+    }
+
+    private fun corruptAnnotation(): MetadataAnnotation = object : MetadataAnnotation {
+        override val kind = 1
+        override val metadataVersion = intArrayOf()
+        override val bytecodeVersion = intArrayOf()
+        override val data1 = emptyArray<String>()
+        override val data2 = emptyArray<String>()
+        override val extraString = ""
+        override val packageName = ""
+        override val extraInt = 0
+    }
+
+    private fun toMetadata(annotation: MetadataAnnotation) = Metadata(
+            annotation.kind,
+            annotation.metadataVersion,
+            annotation.data1,
+            annotation.data2,
+            annotation.extraString,
+            annotation.packageName,
+            annotation.extraInt)
+}

test-app/build-tools/android-metadata-generator/src/test/java/com/telerik/metadata/parsing/kotlin/classes/KotlinMetadataVersionFixture.kt

@@ -0,0 +1,8 @@
+package com.telerik.metadata.parsing.kotlin.classes
+
+class KotlinMetadataVersionFixture(val greeting: String) {
+
+    var counter: Int = 0
+
+    fun greet(name: String): String = "$greeting, $name"
+}

test-app/gradle.properties

@@ -39,7 +39,7 @@ ns_default_google_java_format_version = 1.6
 ns_default_gson_version = 2.10.1
 ns_default_json_version = 20180813
 ns_default_junit_version = 4.13.2
-ns_default_kotlin_version = 2.2.20
-ns_default_kotlinx_metadata_jvm_version = 2.2.20
+ns_default_kotlin_version = 2.4.10
+ns_default_kotlinx_metadata_jvm_version = 2.4.10
 ns_default_mockito_core_version = 3.0.0
 ns_default_spotbugs_version = 3.1.12