Report error on incompatible .kotlin_module files in classpath

Also remove obsolete test wrongAbiVersionNoErrors #KT-25973 Fixed #KT-26266 Open
2018-08-17 19:01:26 +02:00
parent 4f11812668
commit 852760b3b0
19 changed files with 105 additions and 90 deletions
@@ -201,7 +201,9 @@ public class ClassFileFactory implements OutputFileCollection {
                case "kotlin_module": {
                    ModuleMapping mapping = ModuleMappingUtilKt.loadModuleMapping(
                            ModuleMapping.Companion, file.asByteArray(), relativePath.getPath(),
-                            CompilerDeserializationConfiguration.Default.INSTANCE
+                            CompilerDeserializationConfiguration.Default.INSTANCE, version -> {
+                                throw new IllegalStateException("Version of the generated module cannot be incompatible: " + version);
+                            }
                    );
                    for (Map.Entry<String, PackageParts> entry : mapping.getPackageFqName2Parts().entrySet()) {
                        FqName packageFqName = new FqName(entry.getKey());
@@ -38,7 +38,9 @@ private fun Iterable<PackageParts>.addCompiledParts(state: GenerationState): Lis
    val incrementalCache = state.incrementalCacheForThisTarget ?: return this.toList()
    val moduleMappingData = incrementalCache.getModuleMappingData() ?: return this.toList()

-    val mapping = ModuleMapping.loadModuleMapping(moduleMappingData, "<incremental>", state.deserializationConfiguration)
+    val mapping = ModuleMapping.loadModuleMapping(moduleMappingData, "<incremental>", state.deserializationConfiguration) { version ->
+        throw IllegalStateException("Version of the generated module cannot be incompatible: $version")
+    }

    incrementalCache.getObsoletePackageParts().forEach { internalName ->
        val qualifier = JvmClassName.byInternalName(internalName).packageFqName.asString()
@@ -26,6 +26,8 @@ import org.jetbrains.kotlin.cli.common.messages.MessageCollector
 import org.jetbrains.kotlin.cli.jvm.index.JavaRoot
 import org.jetbrains.kotlin.config.LanguageVersionSettings
 import org.jetbrains.kotlin.load.kotlin.PackagePartProvider
+import org.jetbrains.kotlin.load.kotlin.loadModuleMapping
+import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmMetadataVersion
 import org.jetbrains.kotlin.metadata.jvm.deserialization.ModuleMapping
 import org.jetbrains.kotlin.metadata.jvm.deserialization.PackageParts
 import org.jetbrains.kotlin.name.ClassId
@@ -94,7 +96,14 @@ class JvmPackagePartProvider(
                try {
                    val mapping = ModuleMapping.loadModuleMapping(
                        moduleFile.contentsToByteArray(), moduleFile.toString(), deserializationConfiguration
-                    )
+                    ) { incompatibleVersion ->
+                        messageCollector.report(
+                            ERROR,
+                            "Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is " +
+                                    "$incompatibleVersion, expected version is ${JvmMetadataVersion.INSTANCE}.",
+                            CompilerMessageLocation.create(moduleFile.path)
+                        )
+                    }
                    loadedModules.add(ModuleMappingInfo(root, mapping, moduleFile.nameWithoutExtension))
                } catch (e: EOFException) {
                    messageCollector.report(
@@ -33,7 +33,10 @@ class IncrementalPackagePartProvider(

    private val moduleMappings = storageManager.createLazyValue {
        incrementalCaches.map { cache ->
-            ModuleMapping.loadModuleMapping(cache.getModuleMappingData(), "<incremental>", deserializationConfiguration)
+            ModuleMapping.loadModuleMapping(cache.getModuleMappingData(), "<incremental>", deserializationConfiguration) { version ->
+                // Incremental compilation should fall back to full rebuild if the minor component of the metadata version has changed
+                throw IllegalStateException("Version of the generated module should not be incompatible: $version")
+            }
        }
    }

@@ -1,4 +1,5 @@
 error: incompatible classes were found in dependencies. Remove them from the classpath or use '-Xskip-metadata-version-check' to suppress errors
+compiler/testData/cli/jvm/wrongAbiVersionLib/bin/META-INF/main.kotlin_module: error: module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 0.30.0, expected version is $ABI_VERSION$.
 compiler/testData/cli/jvm/wrongAbiVersion.kt:3:12: error: class 'ClassWithWrongAbiVersion' was compiled with an incompatible version of Kotlin. The binary version of its metadata is 0.30.0, expected version is $ABI_VERSION$.
 The class is loaded from $TESTDATA_DIR$/wrongAbiVersionLib/bin/ClassWithWrongAbiVersion.class
 fun foo(x: ClassWithWrongAbiVersion) {
@@ -10,4 +11,4 @@ compiler/testData/cli/jvm/wrongAbiVersion.kt:6:7: error: unresolved reference. N
 public fun String.replaceIndent(newIndent: String = ...): String defined in kotlin.text
    1.replaceIndent(2, 3)
      ^
-COMPILATION_ERROR
+COMPILATION_ERROR
@@ -1,5 +0,0 @@
-$TESTDATA_DIR$/wrongAbiVersionNoErrors.kt
-classpath
-$TESTDATA_DIR$/wrongAbiVersionLib/bin
-d
-$TEMP_DIR$
@@ -1,7 +0,0 @@
-// This should not compile because there are usages of symbols with the wrong ABI version!
-
-import wrong.ClassWithInnerLambda
-
-fun happy(): Int {
-    return 2 + 2
-}
@@ -1,6 +0,0 @@
-error: incompatible classes were found in dependencies. Remove them from the classpath or use '-Xskip-metadata-version-check' to suppress errors
-compiler/testData/cli/jvm/wrongAbiVersionNoErrors.kt:3:14: error: class 'wrong.ClassWithInnerLambda' was compiled with an incompatible version of Kotlin. The binary version of its metadata is 0.30.0, expected version is $ABI_VERSION$.
-The class is loaded from $TESTDATA_DIR$/wrongAbiVersionLib/bin/wrong/ClassWithInnerLambda.class
-import wrong.ClassWithInnerLambda
-             ^
-COMPILATION_ERROR
@@ -1,4 +1,5 @@
 error: incompatible classes were found in dependencies. Remove them from the classpath or use '-Xskip-metadata-version-check' to suppress errors
+$TMP_DIR$/library-after.jar!/META-INF/main.kotlin_module: error: module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 42.0.0, expected version is $ABI_VERSION$.
 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersion/source.kt:5:16: error: class 'a.A' was compiled with an incompatible version of Kotlin. The binary version of its metadata is 42.0.0, expected version is $ABI_VERSION$.
 The class is loaded from $TMP_DIR$/library-after.jar!/a/A.class
 fun baz(param: A, nested: A.Nested) {
@@ -1,3 +1,4 @@
+$TMP_DIR$/library-after.jar!/META-INF/main.kotlin_module: error: module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 42.0.0, expected version is $ABI_VERSION$.
 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:12:13: error: unresolved reference: foo
    val x = foo()
            ^
@@ -10,4 +11,4 @@ compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMeta
 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata/source.kt:15:12: error: unresolved reference: TA
    val z: TA = ""
           ^
-COMPILATION_ERROR
+COMPILATION_ERROR
@@ -1,3 +1,4 @@
+$TMP_DIR$/library-after.jar!/META-INF/main.kotlin_module: error: module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 42.0.0, expected version is $ABI_VERSION$.
 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:12:13: error: unresolved reference: foo
    val x = foo()
            ^
@@ -10,4 +11,4 @@ compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMeta
 compiler/testData/compileKotlinAgainstCustomBinaries/wrongMetadataVersionBadMetadata2/source.kt:15:12: error: unresolved reference: TA
    val z: TA = ""
           ^
-COMPILATION_ERROR
+COMPILATION_ERROR
@@ -586,11 +586,6 @@ public class CliTestGenerated extends AbstractCliTest {
            runTest("compiler/testData/cli/jvm/wrongAbiVersion.args");
        }

-        @TestMetadata("wrongAbiVersionNoErrors.args")
-        public void testWrongAbiVersionNoErrors() throws Exception {
-            runTest("compiler/testData/cli/jvm/wrongAbiVersionNoErrors.args");
-        }
-
        @TestMetadata("wrongArgument.args")
        public void testWrongArgument() throws Exception {
            runTest("compiler/testData/cli/jvm/wrongArgument.args");
@@ -50,7 +50,8 @@ class JvmModuleProtoBufTest : KtUsefulTestCase() {
                File(tmpdir, "META-INF/$moduleName.${ModuleMapping.MAPPING_FILE_EXT}").readBytes(), "test",
                CompilerDeserializationConfiguration(
                        LanguageVersionSettingsImpl(loadWith, ApiVersion.createByLanguageVersion(loadWith))
-                )
+                ),
+                ::error
        )
        val result = buildString {
            for (annotationClassId in mapping.moduleData.annotations) {
@@ -5,17 +5,20 @@

 package org.jetbrains.kotlin.load.kotlin

+import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmMetadataVersion
 import org.jetbrains.kotlin.metadata.jvm.deserialization.ModuleMapping
 import org.jetbrains.kotlin.serialization.deserialization.DeserializationConfiguration

 fun ModuleMapping.Companion.loadModuleMapping(
    bytes: ByteArray?,
    debugName: String,
-    configuration: DeserializationConfiguration
+    configuration: DeserializationConfiguration,
+    reportIncompatibleVersionError: (JvmMetadataVersion) -> Unit
 ): ModuleMapping =
    loadModuleMapping(
        bytes,
        debugName,
        configuration.skipMetadataVersionCheck,
-        configuration.isJvmPackageNameSupported
+        configuration.isJvmPackageNameSupported,
+        reportIncompatibleVersionError
    )
@@ -18,6 +18,7 @@ package kotlin.reflect.jvm.internal.components

 import org.jetbrains.kotlin.load.kotlin.PackagePartProvider
 import org.jetbrains.kotlin.load.kotlin.loadModuleMapping
+import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmMetadataVersion
 import org.jetbrains.kotlin.metadata.jvm.deserialization.ModuleMapping
 import org.jetbrains.kotlin.name.ClassId
 import org.jetbrains.kotlin.serialization.deserialization.DeserializationConfiguration
@@ -45,11 +46,20 @@ class RuntimePackagePartProvider(private val classLoader: ClassLoader) : Package
        for (resource in resources) {
            try {
                resource.openStream()?.use { stream ->
-                    val mapping = ModuleMapping.loadModuleMapping(stream.readBytes(), resourcePath, DeserializationConfiguration.Default)
+                    val mapping = ModuleMapping.loadModuleMapping(
+                        stream.readBytes(), resourcePath, DeserializationConfiguration.Default
+                    ) { version ->
+                        throw UnsupportedOperationException(
+                            "Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is $version, " +
+                                    "expected version is ${JvmMetadataVersion.INSTANCE}. Please update Kotlin to the latest version"
+                        )
+                    }
                    for ((packageFqName, parts) in mapping.packageFqName2Parts) {
                        packageParts.getOrPut(packageFqName) { linkedSetOf() }.addAll(parts.parts)
                    }
                }
+            } catch (e: UnsupportedOperationException) {
+                throw e
            } catch (e: Exception) {
                // TODO: do not swallow this exception?
            }
@@ -33,7 +33,8 @@ class ModuleMapping private constructor(
            bytes: ByteArray?,
            debugName: String,
            skipMetadataVersionCheck: Boolean,
-            isJvmPackageNameSupported: Boolean
+            isJvmPackageNameSupported: Boolean,
+            reportIncompatibleVersionError: (JvmMetadataVersion) -> Unit
        ): ModuleMapping {
            if (bytes == null) {
                return EMPTY
@@ -47,47 +48,47 @@ class ModuleMapping private constructor(
                return CORRUPTED
            }

-            if (skipMetadataVersionCheck || JvmMetadataVersion(*versionNumber).isCompatible()) {
-                val moduleProto = JvmModuleProtoBuf.Module.parseFrom(stream) ?: return EMPTY
-                val result = linkedMapOf<String, PackageParts>()
-
-                for (proto in moduleProto.packagePartsList) {
-                    val packageFqName = proto.packageFqName
-                    val packageParts = result.getOrPut(packageFqName) { PackageParts(packageFqName) }
-
-                    for ((index, partShortName) in proto.shortClassNameList.withIndex()) {
-                        val multifileFacadeId = proto.multifileFacadeShortNameIdList.getOrNull(index)?.minus(1)
-                        val facadeShortName = multifileFacadeId?.let(proto.multifileFacadeShortNameList::getOrNull)
-                        val facadeInternalName = facadeShortName?.let { internalNameOf(packageFqName, it) }
-                        packageParts.addPart(internalNameOf(packageFqName, partShortName), facadeInternalName)
-                    }
-
-                    if (isJvmPackageNameSupported) {
-                        for ((index, partShortName) in proto.classWithJvmPackageNameShortNameList.withIndex()) {
-                            val packageId = proto.classWithJvmPackageNamePackageIdList.getOrNull(index)
-                                ?: proto.classWithJvmPackageNamePackageIdList.lastOrNull()
-                                ?: continue
-                            val jvmPackageName = moduleProto.jvmPackageNameList.getOrNull(packageId) ?: continue
-                            packageParts.addPart(internalNameOf(jvmPackageName, partShortName), null)
-                        }
-                    }
-                }
-
-                for (proto in moduleProto.metadataPartsList) {
-                    val packageParts = result.getOrPut(proto.packageFqName) { PackageParts(proto.packageFqName) }
-                    proto.shortClassNameList.forEach(packageParts::addMetadataPart)
-                }
-
-                // TODO: read arguments of module annotations
-                val nameResolver = NameResolverImpl(moduleProto.stringTable, moduleProto.qualifiedNameTable)
-                val annotations = moduleProto.annotationList.map { proto -> nameResolver.getQualifiedClassName(proto.id) }
-
-                return ModuleMapping(result, BinaryModuleData(annotations), debugName)
-            } else {
-                // TODO: consider reporting "incompatible ABI version" error for package parts
+            val version = JvmMetadataVersion(*versionNumber)
+            if (!skipMetadataVersionCheck && !version.isCompatible()) {
+                reportIncompatibleVersionError(version)
+                return EMPTY
            }

-            return EMPTY
+            val moduleProto = JvmModuleProtoBuf.Module.parseFrom(stream) ?: return EMPTY
+            val result = linkedMapOf<String, PackageParts>()
+
+            for (proto in moduleProto.packagePartsList) {
+                val packageFqName = proto.packageFqName
+                val packageParts = result.getOrPut(packageFqName) { PackageParts(packageFqName) }
+
+                for ((index, partShortName) in proto.shortClassNameList.withIndex()) {
+                    val multifileFacadeId = proto.multifileFacadeShortNameIdList.getOrNull(index)?.minus(1)
+                    val facadeShortName = multifileFacadeId?.let(proto.multifileFacadeShortNameList::getOrNull)
+                    val facadeInternalName = facadeShortName?.let { internalNameOf(packageFqName, it) }
+                    packageParts.addPart(internalNameOf(packageFqName, partShortName), facadeInternalName)
+                }
+
+                if (isJvmPackageNameSupported) {
+                    for ((index, partShortName) in proto.classWithJvmPackageNameShortNameList.withIndex()) {
+                        val packageId = proto.classWithJvmPackageNamePackageIdList.getOrNull(index)
+                            ?: proto.classWithJvmPackageNamePackageIdList.lastOrNull()
+                            ?: continue
+                        val jvmPackageName = moduleProto.jvmPackageNameList.getOrNull(packageId) ?: continue
+                        packageParts.addPart(internalNameOf(jvmPackageName, partShortName), null)
+                    }
+                }
+            }
+
+            for (proto in moduleProto.metadataPartsList) {
+                val packageParts = result.getOrPut(proto.packageFqName) { PackageParts(proto.packageFqName) }
+                proto.shortClassNameList.forEach(packageParts::addMetadataPart)
+            }
+
+            // TODO: read arguments of module annotations
+            val nameResolver = NameResolverImpl(moduleProto.stringTable, moduleProto.qualifiedNameTable)
+            val annotations = moduleProto.annotationList.map { proto -> nameResolver.getQualifiedClassName(proto.id) }
+
+            return ModuleMapping(result, BinaryModuleData(annotations), debugName)
        }
    }
 }
@@ -45,11 +45,13 @@ object KotlinJvmModuleAnnotationsIndex : FileBasedIndexExtension<String, List<Cl
    override fun getIndexer(): DataIndexer<String, List<ClassId>, FileContent> = DataIndexer { inputData ->
        val file = inputData.file
        try {
-            val moduleMapping = ModuleMapping.loadModuleMapping(inputData.content, file.toString(), DeserializationConfiguration.Default)
-            return@DataIndexer mapOf(file.nameWithoutExtension to moduleMapping.moduleData.annotations.map(ClassId::fromString))
+            val moduleMapping = ModuleMapping.loadModuleMapping(inputData.content, file.toString(), DeserializationConfiguration.Default) {}
+            if (moduleMapping !== ModuleMapping.EMPTY) {
+                return@DataIndexer mapOf(file.nameWithoutExtension to moduleMapping.moduleData.annotations.map(ClassId::fromString))
+            }
        } catch (e: Exception) {
            // Exceptions are already reported in KotlinModuleMappingIndex
-            emptyMap()
        }
+        emptyMap()
    }
 }
@@ -73,20 +73,19 @@ object KotlinModuleMappingIndex : FileBasedIndexExtension<String, PackageParts>(

    override fun getVersion(): Int = 5

-    override fun getIndexer(): DataIndexer<String, PackageParts, FileContent> {
-        return DataIndexer<String, PackageParts, FileContent> { inputData ->
-            val content = inputData.content
-            val file = inputData.file
-            try {
-                val moduleMapping = ModuleMapping.loadModuleMapping(content, file.toString(), DeserializationConfiguration.Default)
-                if (moduleMapping === ModuleMapping.CORRUPTED) {
-                    file.refresh(true, false)
-                }
-                return@DataIndexer moduleMapping.packageFqName2Parts
+    override fun getIndexer(): DataIndexer<String, PackageParts, FileContent> = DataIndexer { inputData ->
+        val content = inputData.content
+        val file = inputData.file
+        try {
+            val moduleMapping = ModuleMapping.loadModuleMapping(content, file.toString(), DeserializationConfiguration.Default) {
+                // Do nothing; it's OK for an IDE index to just ignore incompatible module files
            }
-            catch(e: Exception) {
-                throw RuntimeException("Error on indexing $file", e)
+            if (moduleMapping === ModuleMapping.CORRUPTED) {
+                file.refresh(true, false)
            }
+            return@DataIndexer moduleMapping.packageFqName2Parts
+        } catch (e: Exception) {
+            throw RuntimeException("Error on indexing $file", e)
        }
    }
 }
@@ -23,7 +23,9 @@ import org.jetbrains.kotlin.metadata.jvm.deserialization.serializeToByteArray
 class KotlinModuleMetadata(@Suppress("CanBeParameter", "MemberVisibilityCanBePrivate") val bytes: ByteArray) {
    internal val data: ModuleMapping = ModuleMapping.loadModuleMapping(
        bytes, javaClass.name, skipMetadataVersionCheck = false, isJvmPackageNameSupported = true
-    )
+    ) {
+        // TODO: report incorrect versions of modules
+    }

    /**
     * A [KmModuleVisitor] that generates the metadata of a Kotlin JVM module file.