Implement module accessibility checks differently in compiler and IDE

Essentially, the logic that was previously in
JvmModuleAccessibilityChecker.diagnosticFor, is moved into a new
abstract method JavaModuleResolver.checkAccessibility, which is
implemented differently in the compiler and in the IDE. In the compiler,
we use our JavaModuleInfo and JavaModuleGraph, as previously. In the
IDE, we use intellij's PsiJavaModule and JavaModuleGraphUtil.

This fixes strange behavior in IDE where some modules could be observed
in an invalid state. The cause of that was the JavaModuleGraph instance
caching modules in IdeJavaModuleResolver, which is a project component.

Moreover, this will allow to report an error "named module does not read
unnamed module" in the compiler, and avoid reporting it in the IDE (see
the comment in IdeJavaModuleResolver about that)

compiler/cli/src/org/jetbrains/kotlin/cli/jvm/modules/CliJavaModuleResolver.kt

@@ -21,12 +21,13 @@ 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.name.FqName
 import org.jetbrains.kotlin.resolve.jvm.modules.JavaModule
 import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleGraph
 import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleResolver
 
 class CliJavaModuleResolver(
-        override val moduleGraph: JavaModuleGraph,
+        private val moduleGraph: JavaModuleGraph,
         private val javaModules: List<JavaModule>
 ) : JavaModuleResolver {
     init {
@@ -37,7 +38,7 @@ class CliJavaModuleResolver(
 
     private val sourceModule = javaModules.firstOrNull { !it.isBinary }
 
-    override fun findJavaModule(file: VirtualFile): JavaModule? =
+    private fun findJavaModule(file: VirtualFile): JavaModule? =
             when (file.fileType) {
                 KotlinFileType.INSTANCE, JavaFileType.INSTANCE -> sourceModule
                 JavaClassFileType.INSTANCE -> javaModules.firstOrNull { module ->
@@ -45,4 +46,28 @@ class CliJavaModuleResolver(
                 }
                 else -> null
             }
+
+    override fun checkAccessibility(
+            fileFromOurModule: VirtualFile?, referencedFile: VirtualFile, referencedPackage: FqName?
+    ): JavaModuleResolver.AccessError? {
+        val ourModule = fileFromOurModule?.let(this::findJavaModule)
+        val theirModule = this.findJavaModule(referencedFile)
+
+        if (ourModule?.name == theirModule?.name) return null
+
+        if (theirModule == null) {
+            return JavaModuleResolver.AccessError.ModuleDoesNotReadUnnamedModule
+        }
+
+        if (ourModule != null && !moduleGraph.reads(ourModule.name, theirModule.name)) {
+            return JavaModuleResolver.AccessError.ModuleDoesNotReadModule(theirModule.name)
+        }
+
+        val fqName = referencedPackage ?: return null
+        if (!theirModule.exports(fqName) && (ourModule == null || !theirModule.exportsTo(fqName, ourModule.name))) {
+            return JavaModuleResolver.AccessError.ModuleDoesNotExportPackage(theirModule.name)
+        }
+
+        return null
+    }
 }

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmModuleAccessibilityChecker.kt

@@ -34,8 +34,9 @@ import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker
 import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext
 import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
 import org.jetbrains.kotlin.resolve.checkers.ClassifierUsageChecker
-import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm
+import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm.*
 import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleResolver
+import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleResolver.AccessError.*
 import org.jetbrains.kotlin.resolve.source.getPsi
 import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedMemberDescriptor
 
@@ -61,33 +62,21 @@ class JvmModuleAccessibilityChecker(project: Project) : CallChecker {
     ): Diagnostic? {
         val referencedFile = findVirtualFile(targetClassOrPackage, originalDescriptor) ?: return null
 
-        // TODO: do not resolve our JavaModule every time, invent a way to obtain it from ModuleDescriptor and do it once in constructor
+        val referencedPackageFqName =
-        val ourModule = fileFromOurModule?.let(moduleResolver::findJavaModule)
+                DescriptorUtils.getParentOfType(targetClassOrPackage, PackageFragmentDescriptor::class.java, false)?.fqName
-        val theirModule = moduleResolver.findJavaModule(referencedFile)
+        val diagnostic = moduleResolver.checkAccessibility(fileFromOurModule, referencedFile, referencedPackageFqName)
 
-        // If we're both in the unnamed module, it's OK, no error should be reported
+        return when (diagnostic) {
-        if (ourModule == null && theirModule == null) return null
+            is ModuleDoesNotReadUnnamedModule ->
-
+                // TODO: report this error as soon as module path is used instead of class path for modular compilation
-        if (theirModule == null) {
+                // JAVA_MODULE_DOES_NOT_READ_UNNAMED_MODULE.on(reportOn)
-            // We should probably prohibit this usage according to JPMS (named module cannot use types from unnamed module),
+                null
-            // but we cannot be sure that a module without module-info.java is going to be actually used as an unnamed module.
+            is ModuleDoesNotReadModule ->
-            // It could also be an automatic module, in which case it would be read by every module.
+                JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE.on(reportOn, diagnostic.dependencyModuleName)
-            return null
+            is ModuleDoesNotExportPackage ->
+                JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE.on(reportOn, diagnostic.dependencyModuleName, referencedPackageFqName!!.asString())
+            else -> null
         }
-
-        if (ourModule?.name == theirModule.name) return null
-
-        if (ourModule != null && !moduleResolver.moduleGraph.reads(ourModule.name, theirModule.name)) {
-            return ErrorsJvm.JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE.on(reportOn, theirModule.name)
-        }
-
-        val containingPackage = DescriptorUtils.getParentOfType(targetClassOrPackage, PackageFragmentDescriptor::class.java, false)
-        val fqName = containingPackage?.fqName ?: return null
-        if (!theirModule.exports(fqName) && (ourModule == null || !theirModule.exportsTo(fqName, ourModule.name))) {
-            return ErrorsJvm.JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE.on(reportOn, theirModule.name, fqName.asString())
-        }
-
-        return null
     }
 
     private fun findVirtualFile(

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java

@@ -115,6 +115,7 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension {
         MAP.put(OBSOLETE_SUSPEND_INLINE_FUNCTIONS_ABI, "Cannot inline suspend function built with compiler version less than 1.1.4/1.2-M1");
 
         MAP.put(JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE, "Symbol is declared in module ''{0}'' which current module does not depend on", STRING);
+        MAP.put(JAVA_MODULE_DOES_NOT_READ_UNNAMED_MODULE, "Symbol is declared in unnamed module which is not read by current module");
         MAP.put(JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE, "Symbol is declared in module ''{0}'' which does not export package ''{1}''", STRING, STRING);
     }
 

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java

@@ -96,6 +96,7 @@ public interface ErrorsJvm {
     DiagnosticFactory0<PsiElement> OBSOLETE_SUSPEND_INLINE_FUNCTIONS_ABI = DiagnosticFactory0.create(ERROR);
 
     DiagnosticFactory1<PsiElement, String> JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE = DiagnosticFactory1.create(ERROR);
+    DiagnosticFactory0<PsiElement> JAVA_MODULE_DOES_NOT_READ_UNNAMED_MODULE = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory2<PsiElement, String, String> JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE = DiagnosticFactory2.create(ERROR);
 
     @SuppressWarnings("UnusedDeclaration")

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

@@ -19,12 +19,16 @@ package org.jetbrains.kotlin.resolve.jvm.modules
 import com.intellij.openapi.components.ServiceManager
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.vfs.VirtualFile
+import org.jetbrains.kotlin.name.FqName
 
 interface JavaModuleResolver {
-    val moduleGraph: JavaModuleGraph
+    fun checkAccessibility(fileFromOurModule: VirtualFile?, referencedFile: VirtualFile, referencedPackage: FqName?): AccessError?
 
-    // For any .java, .kt or .class file in the project, returns the corresponding module or null if there's none
+    sealed class AccessError {
-    fun findJavaModule(file: VirtualFile): JavaModule?
+        object ModuleDoesNotReadUnnamedModule : AccessError()
+        data class ModuleDoesNotReadModule(val dependencyModuleName: String) : AccessError()
+        data class ModuleDoesNotExportPackage(val dependencyModuleName: String) : AccessError()
+    }
 
     companion object SERVICE {
         fun getInstance(project: Project): JavaModuleResolver =

idea/idea-analysis/src/org/jetbrains/kotlin/idea/modules/IdeJavaModuleResolver.kt

@@ -19,38 +19,62 @@ package org.jetbrains.kotlin.idea.modules
 import com.intellij.codeInsight.daemon.impl.analysis.JavaModuleGraphUtil
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.vfs.VirtualFile
-import com.intellij.psi.PsiCompiledElement
 import com.intellij.psi.PsiJavaModule
 import com.intellij.psi.PsiManager
-import com.intellij.psi.impl.file.impl.JavaFileManager
 import com.intellij.psi.impl.light.LightJavaModule
-import com.intellij.psi.search.GlobalSearchScope
+import org.jetbrains.annotations.NotNull
-import org.jetbrains.kotlin.resolve.jvm.modules.*
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.resolve.jvm.modules.JavaModuleResolver
 
 class IdeJavaModuleResolver(project: Project) : JavaModuleResolver {
     private val psiManager = PsiManager.getInstance(project)
-    private val fileManager = JavaFileManager.SERVICE.getInstance(project)
-    private val allScope = GlobalSearchScope.allScope(project)
 
-    override val moduleGraph: JavaModuleGraph = JavaModuleGraph(
+    private fun findJavaModule(file: VirtualFile): PsiJavaModule? {
-            object : JavaModuleFinder {
+        return psiManager.findFile(file)?.let(JavaModuleGraphUtil::findDescriptorByElement)
-                override fun findModule(name: String): JavaModule? {
-                    return fileManager.findModules(name, allScope).singleOrNull()?.toJavaModule()
-                }
-            }
-    )
-
-    override fun findJavaModule(file: VirtualFile): JavaModule? {
-        val psiFile = psiManager.findFile(file) ?: return null
-        return JavaModuleGraphUtil.findDescriptorByElement(psiFile)?.toJavaModule()
     }
 
-    private fun PsiJavaModule.toJavaModule(): JavaModule {
+    override fun checkAccessibility(
-        if (this is LightJavaModule) {
+            fileFromOurModule: VirtualFile?, referencedFile: VirtualFile, referencedPackage: FqName?
-            return JavaModule.Automatic(name, rootVirtualFile)
+    ): JavaModuleResolver.AccessError? {
+        val ourModule = fileFromOurModule?.let(this::findJavaModule)
+        val theirModule = this.findJavaModule(referencedFile)
+
+        // If we're both in the unnamed module, it's OK, no error should be reported
+        if (ourModule == null && theirModule == null) return null
+
+        if (theirModule == null) {
+            // We should probably prohibit this usage according to JPMS (named module cannot use types from unnamed module),
+            // but we cannot be sure that a module without module-info.java is going to be actually used as an unnamed module.
+            // It could also be an automatic module, in which case it would be read by every module.
+            return null
         }
 
-        val virtualFile = containingFile?.virtualFile ?: error("No VirtualFile found for module $this ($javaClass)")
+        if (ourModule?.name == theirModule.name) return null
-        return JavaModule.Explicit(JavaModuleInfo.create(this), virtualFile.parent, virtualFile, this is PsiCompiledElement)
+
+        if (ourModule != null && !JavaModuleGraphUtil.reads(ourModule, theirModule)) {
+            return JavaModuleResolver.AccessError.ModuleDoesNotReadModule(theirModule.name)
+        }
+
+        val fqName = referencedPackage?.asString() ?: return null
+        if (!exports(theirModule, fqName, ourModule)) {
+            return JavaModuleResolver.AccessError.ModuleDoesNotExportPackage(theirModule.name)
+        }
+
+        return null
+    }
+
+    // Returns whether or not [source] exports [packageName] to [target]
+    private fun exports(source: PsiJavaModule, packageName: String, target: PsiJavaModule?): Boolean {
+        if (source is LightJavaModule) {
+            return true
+        }
+
+        // TODO: simply call JavaModuleGraphUtil.exports as soon as its 'target' parameter is nullable
+        if (target != null) {
+            return JavaModuleGraphUtil.exports(source, packageName, target)
+        }
+        return source.exports.any { statement ->
+            statement.moduleNames.isEmpty() && statement.packageName == packageName
+        }
     }
 }