Load automatic module names correctly

Load the Automatic-Module-Name manifest entry value if it's present (see
http://mail.openjdk.java.net/pipermail/jpms-spec-observers/2017-May/000877.html),
otherwise sanitize the name of the .jar file, throwing out all chars
except alphanumeric, duplicate dots and a version qualifier

compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/ClasspathRootsResolver.kt

@@ -17,10 +17,12 @@
 package org.jetbrains.kotlin.cli.jvm.compiler
 
 import com.intellij.openapi.vfs.StandardFileSystems
+import com.intellij.openapi.vfs.VfsUtilCore
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.openapi.vfs.VirtualFileManager
 import com.intellij.psi.PsiJavaModule
 import com.intellij.psi.PsiManager
+import com.intellij.psi.impl.light.LightJavaModule
 import org.jetbrains.kotlin.cli.common.messages.CompilerMessageLocation
 import org.jetbrains.kotlin.cli.common.messages.CompilerMessageSeverity
 import org.jetbrains.kotlin.cli.common.messages.CompilerMessageSeverity.*
@@ -38,6 +40,9 @@ import org.jetbrains.kotlin.name.isValidJavaFqName
 import org.jetbrains.kotlin.resolve.jvm.modules.JavaModule
 import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleGraph
 import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleInfo
+import java.io.File
+import java.io.IOException
+import java.util.jar.Manifest
 
 internal class ClasspathRootsResolver(
         private val psiManager: PsiManager,
@@ -78,11 +83,8 @@ internal class ClasspathRootsResolver(
                     result += JavaRoot(root, JavaRoot.RootType.BINARY)
                 }
                 is JvmModulePathRoot -> {
-                    // TODO: sanitize the automatic module name exactly as in javac
+                    val module = modularBinaryRoot(root, contentRoot.file)
-                    // TODO: read Automatic-Module-Name manifest entry
-                    val module = modularBinaryRoot(root, automaticModuleName = { contentRoot.file.name })
                     if (module != null) {
-                        // TODO: report something in case of several modules with the same name?
                         modules += module
                     }
                 }
@@ -108,18 +110,38 @@ internal class ClasspathRootsResolver(
         return JavaModule.Explicit(JavaModuleInfo.create(psiJavaModule), root, moduleInfoFile, isBinary = false)
     }
 
-    private fun modularBinaryRoot(root: VirtualFile, automaticModuleName: () -> String): JavaModule? {
+    private fun modularBinaryRoot(root: VirtualFile, originalFile: File): JavaModule? {
         val moduleInfoFile =
                 root.findChild(PsiJavaModule.MODULE_INFO_CLS_FILE)
                 ?: root.takeIf { it.fileSystem.protocol == StandardFileSystems.JAR_PROTOCOL }
                         ?.findFileByRelativePath(MULTI_RELEASE_MODULE_INFO_CLS_FILE)
-        return if (moduleInfoFile != null) {
+        if (moduleInfoFile != null) {
             val moduleInfo = JavaModuleInfo.read(moduleInfoFile) ?: return null
-            JavaModule.Explicit(moduleInfo, root, moduleInfoFile, isBinary = true)
+            return JavaModule.Explicit(moduleInfo, root, moduleInfoFile, isBinary = true)
         }
-        else {
+
-            JavaModule.Automatic(automaticModuleName(), root)
+        // Only .jar files can be automatic modules
+        if (originalFile.extension == StandardFileSystems.JAR_PROTOCOL) {
+            val manifestFile = root.findChild("META-INF")?.findChild("MANIFEST.MF")
+            if (manifestFile != null) {
+                try {
+                    val moduleName = Manifest(manifestFile.inputStream).mainAttributes.getValue(AUTOMATIC_MODULE_NAME)
+                    return JavaModule.Automatic(moduleName, root)
+                }
+                catch (e: IOException) {
+                    return null
+                }
+            }
+
+            val moduleName = LightJavaModule.moduleName(originalFile.nameWithoutExtension)
+            if (moduleName.isEmpty()) {
+                report(ERROR, "Cannot infer automatic module name for the file", VfsUtilCore.getVirtualFileForJar(root) ?: root)
+                return null
+            }
+            return JavaModule.Automatic(moduleName, root)
         }
+
+        return null
     }
 
     private fun addModularRoots(modules: List<JavaModule>, result: MutableList<JavaRoot>) {
@@ -215,5 +237,6 @@ internal class ClasspathRootsResolver(
 
     private companion object {
         const val MULTI_RELEASE_MODULE_INFO_CLS_FILE = "META-INF/versions/9/${PsiJavaModule.MODULE_INFO_CLS_FILE}"
+        const val AUTOMATIC_MODULE_NAME = "Automatic-Module-Name"
     }
 }

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/modules/JavaModuleInfo.kt

@@ -44,11 +44,15 @@ class JavaModuleInfo(
         fun create(psiJavaModule: PsiJavaModule): JavaModuleInfo {
             return JavaModuleInfo(
                     psiJavaModule.name,
-                    psiJavaModule.requires.map { statement ->
+                    psiJavaModule.requires.mapNotNull { statement ->
-                        JavaModuleInfo.Requires(statement.moduleName!!, statement.hasModifierProperty(PsiModifier.TRANSITIVE))
+                        statement.moduleName?.let { moduleName ->
+                            JavaModuleInfo.Requires(moduleName, statement.hasModifierProperty(PsiModifier.TRANSITIVE))
+                        }
                     },
-                    psiJavaModule.exports.map { statement ->
+                    psiJavaModule.exports.mapNotNull { statement ->
-                        JavaModuleInfo.Exports(FqName(statement.packageName!!), statement.moduleNames)
+                        statement.packageName?.let { packageName ->
+                            JavaModuleInfo.Exports(FqName(packageName), statement.moduleNames)
+                        }
                     }
             )
         }

compiler/testData/javaModules/automaticModuleNames/automatic-module1/m1/A.java

@@ -0,0 +1,3 @@
+package m1;
+
+public class A {}

compiler/testData/javaModules/automaticModuleNames/automatic-module2/m2/B.java

@@ -0,0 +1,3 @@
+package m2;
+
+public class B {}

compiler/testData/javaModules/automaticModuleNames/main.txt

@@ -0,0 +1 @@
+OK

compiler/testData/javaModules/automaticModuleNames/main/module-info.java

@@ -0,0 +1,4 @@
+module main {
+    requires auto.mat1c.m0d.ule;
+    requires automodule2;
+}

compiler/testData/javaModules/automaticModuleNames/main/usage.kt

@@ -0,0 +1,6 @@
+import m1.A
+import m2.B
+
+fun usage(): String {
+    return "${A()}${B()}"
+}

compiler/tests/org/jetbrains/kotlin/jvm/compiler/AbstractKotlinCompilerIntegrationTest.kt

@@ -26,8 +26,9 @@ import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler
 import org.jetbrains.kotlin.test.KotlinTestUtils
 import org.jetbrains.kotlin.test.TestCaseWithTmpdir
 import java.io.File
+import java.util.jar.JarOutputStream
+import java.util.jar.Manifest
 import java.util.regex.Pattern
-import java.util.zip.ZipOutputStream
 
 abstract class AbstractKotlinCompilerIntegrationTest : TestCaseWithTmpdir() {
     protected abstract val testDataPath: String
@@ -56,6 +57,7 @@ abstract class AbstractKotlinCompilerIntegrationTest : TestCaseWithTmpdir() {
                 KotlinTestUtils.compileJavaFiles(javaFiles, listOf("-d", outputDir.path))
             },
             checkKotlinOutput: (String) -> Unit = { actual -> assertEquals(normalizeOutput("" to ExitCode.OK), actual) },
+            manifest: Manifest? = null,
             vararg extraClassPath: File
     ): File {
         val sourceDir = File(testDataDirectory, libraryName)
@@ -80,10 +82,14 @@ abstract class AbstractKotlinCompilerIntegrationTest : TestCaseWithTmpdir() {
 
         if (isJar) {
             destination.delete()
-            ZipOutputStream(destination.outputStream()).use { zip ->
+            val stream =
-                ZipUtil.addDirToZipRecursively(zip, destination, outputDir, "", null, null)
+                    if (manifest != null) JarOutputStream(destination.outputStream(), manifest)
+                    else JarOutputStream(destination.outputStream())
+            stream.use { jar ->
+                ZipUtil.addDirToZipRecursively(jar, destination, outputDir, "", null, null)
             }
         }
+        else assertNull("Manifest is ignored if destination is not a .jar file", manifest)
 
         return destination
     }

compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt

@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.jvm.compiler
 import com.intellij.openapi.util.io.FileUtil
 import org.jetbrains.kotlin.test.KotlinTestUtils
 import java.io.File
+import java.util.jar.Manifest
 
 class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() {
     override val testDataPath: String
@@ -27,7 +28,8 @@ class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() {
     private fun module(
             name: String,
             modulePath: List<File> = emptyList(),
-            addModules: List<String> = emptyList()
+            addModules: List<String> = emptyList(),
+            manifest: Manifest? = null
     ): File {
         val jdk9Home = KotlinTestUtils.getJdk9HomeIfPossible() ?: return File("<test-skipped>")
 
@@ -55,7 +57,8 @@ class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() {
                     }
                     KotlinTestUtils.compileJavaFilesExternallyWithJava9(javaFiles, javaOptions)
                 },
-                checkKotlinOutput = checkKotlinOutput(name)
+                checkKotlinOutput = checkKotlinOutput(name),
+                manifest = manifest
         )
     }
 
@@ -182,4 +185,17 @@ class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() {
 
         module("main", listOf(libraryJar))
     }
+
+    fun testAutomaticModuleNames() {
+        // This name should be sanitized to just "auto.mat1c.m0d.ule"
+        val m1 = File(tmpdir, ".auto--mat1c-_-!@#\$%^&*()m0d_ule--1.0..0-release..jar")
+        module("automatic-module1").renameTo(m1)
+
+        val m2 = module("automatic-module2", manifest = Manifest().apply {
+            mainAttributes.putValue("Manifest-Version", "1.0")
+            mainAttributes.putValue("Automatic-Module-Name", "automodule2")
+        })
+
+        module("main", listOf(m1, m2))
+    }
 }