From 8c21ff66a4908001a7acc471f90f24e84e579489 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 28 Jun 2017 19:03:43 +0300 Subject: [PATCH] Always treat all sources as parts of the module being compiled If a path to the module-info.java file is passed as an argument, we should treat all other source files passed as arguments (either as individual source files or inside a source directory) as members of that module. Previously we treated other source files as members of the unnamed module, and this resulted in incorrect errors when using a member exported with a qualification from another named module, for example #KT-18598 In Fixed --- .../cli/jvm/modules/CliJavaModuleResolver.kt | 20 ++++++++++++--- .../kotlin/resolve/jvm/modules/JavaModule.kt | 5 ++-- .../moduleA/foo/Foo.java | 3 +++ .../moduleA/module-info.java | 3 +++ .../moduleA/unexported/Unexported.java | 3 +++ .../moduleB.txt | 10 ++++++++ .../moduleB/usage.kt | 14 +++++++++++ .../Unrelated.java | 1 + .../module-info.java | 3 +++ .../compiler/Java9ModulesIntegrationTest.kt | 25 ++++++++++++++++--- 10 files changed, 78 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/foo/Foo.java create mode 100644 compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/module-info.java create mode 100644 compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/unexported/Unexported.java create mode 100644 compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB.txt create mode 100644 compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB/usage.kt create mode 100644 compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/Unrelated.java create mode 100644 compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/module-info.java diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/modules/CliJavaModuleResolver.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/modules/CliJavaModuleResolver.kt index 8bb5221a6c1..7b4d0c52ef1 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/modules/CliJavaModuleResolver.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/modules/CliJavaModuleResolver.kt @@ -17,8 +17,10 @@ package org.jetbrains.kotlin.cli.jvm.modules import com.intellij.ide.highlighter.JavaClassFileType +import com.intellij.ide.highlighter.JavaFileType import com.intellij.openapi.vfs.VfsUtilCore import com.intellij.openapi.vfs.VirtualFile +import org.jetbrains.kotlin.idea.KotlinFileType import org.jetbrains.kotlin.resolve.jvm.modules.JavaModule import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleGraph import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleResolver @@ -27,10 +29,20 @@ class CliJavaModuleResolver( override val moduleGraph: JavaModuleGraph, private val javaModules: List ) : JavaModuleResolver { - override fun findJavaModule(file: VirtualFile): JavaModule? { - val isBinary = file.fileType == JavaClassFileType.INSTANCE - return javaModules.firstOrNull { module -> - module.isBinary == isBinary && VfsUtilCore.isAncestor(module.moduleRoot, file, false) + init { + assert(javaModules.count { !it.isBinary } <= 1) { + "Modules computed by ClasspathRootsResolver cannot have more than one source module: $javaModules" } } + + private val sourceModule = javaModules.firstOrNull { !it.isBinary } + + override fun findJavaModule(file: VirtualFile): JavaModule? = + when (file.fileType) { + KotlinFileType.INSTANCE, JavaFileType.INSTANCE -> sourceModule + JavaClassFileType.INSTANCE -> javaModules.firstOrNull { module -> + module.isBinary && VfsUtilCore.isAncestor(module.moduleRoot, file, false) + } + else -> null + } } diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/modules/JavaModule.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/modules/JavaModule.kt index f837677cb61..3d183abb2aa 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/modules/JavaModule.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/modules/JavaModule.kt @@ -28,8 +28,9 @@ interface JavaModule { val name: String /** - * A directory or the root of a .jar file on the module path. In case of an explicit module, - * module-info.class file can be found in its children. + * A directory or the root of a .jar file on the module path. Can also be the path to the `module-info.java` file if that path + * was passed as an explicit argument to the compiler. + * In case of an explicit module, module-info.class file can be found in its children. */ val moduleRoot: VirtualFile diff --git a/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/foo/Foo.java b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/foo/Foo.java new file mode 100644 index 00000000000..7f566bafd45 --- /dev/null +++ b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/foo/Foo.java @@ -0,0 +1,3 @@ +package foo; + +public class Foo {} diff --git a/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/module-info.java b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/module-info.java new file mode 100644 index 00000000000..3afc03276f3 --- /dev/null +++ b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/module-info.java @@ -0,0 +1,3 @@ +module moduleA { + exports foo to moduleB; +} diff --git a/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/unexported/Unexported.java b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/unexported/Unexported.java new file mode 100644 index 00000000000..f8e04583f11 --- /dev/null +++ b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleA/unexported/Unexported.java @@ -0,0 +1,3 @@ +package unexported; + +public class Unexported {} diff --git a/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB.txt b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB.txt new file mode 100644 index 00000000000..1e784e235b8 --- /dev/null +++ b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB.txt @@ -0,0 +1,10 @@ +compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB/usage.kt:2:19: error: symbol is declared in module 'moduleA' which does not export package 'unexported' +import unexported.Unexported + ^ +compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB/usage.kt:9:5: error: symbol is declared in module 'moduleA' which does not export package 'unexported' + Unexported() + ^ +compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB/usage.kt:13:5: error: unresolved reference: Unrelated + Unrelated() + ^ +COMPILATION_ERROR diff --git a/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB/usage.kt b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB/usage.kt new file mode 100644 index 00000000000..05501bce3f7 --- /dev/null +++ b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/moduleB/usage.kt @@ -0,0 +1,14 @@ +import foo.Foo +import unexported.Unexported + +fun usage() { + // Should be OK + Foo() + + // Should be error + Unexported() + + // Should be error: if we're passing the path to a particular module-info.java to the compiler, + // we should not treat its location as a root + Unrelated() +} diff --git a/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/Unrelated.java b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/Unrelated.java new file mode 100644 index 00000000000..2a6c1d179a2 --- /dev/null +++ b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/Unrelated.java @@ -0,0 +1 @@ +public class Unrelated {} diff --git a/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/module-info.java b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/module-info.java new file mode 100644 index 00000000000..b8f934c3a2d --- /dev/null +++ b/compiler/testData/javaModules/specifyPathToModuleInfoInArguments/someOtherDirectoryWithTheActualModuleInfo/module-info.java @@ -0,0 +1,3 @@ +module moduleB { + requires moduleA; +} diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt index ff1ad2446ef..6f616406d74 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/Java9ModulesIntegrationTest.kt @@ -54,12 +54,14 @@ class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() { } KotlinTestUtils.compileJavaFilesExternallyWithJava9(javaFiles, javaOptions) }, - checkKotlinOutput = { actual -> - KotlinTestUtils.assertEqualsToFile(File(testDataDirectory, "$name.txt"), actual) - } + checkKotlinOutput = checkKotlinOutput(name) ) } + private fun checkKotlinOutput(moduleName: String): (String) -> Unit = { actual -> + KotlinTestUtils.assertEqualsToFile(File(testDataDirectory, "$moduleName.txt"), actual) + } + fun testSimple() { val a = module("moduleA") module("moduleB", listOf(a)) @@ -130,4 +132,21 @@ class Java9ModulesIntegrationTest : AbstractKotlinCompilerIntegrationTest() { val c = module("moduleC", listOf(a, b)) module("moduleD", listOf(c, b, a)) } + + fun testSpecifyPathToModuleInfoInArguments() { + val a = module("moduleA") + + val jdk9Home = KotlinTestUtils.getJdk9HomeIfPossible() ?: return + val kotlinOptions = mutableListOf( + "$testDataDirectory/someOtherDirectoryWithTheActualModuleInfo/module-info.java", + "-jdk-home", jdk9Home.path, + "-Xmodule-path=${a.path}" + ) + compileLibrary( + "moduleB", + additionalOptions = kotlinOptions, + compileJava = { _, _, _ -> error("No .java files in moduleB in this test") }, + checkKotlinOutput = checkKotlinOutput("moduleB") + ) + } }