Fix incorrect behavior and refactor JvmModuleAccessibilityChecker
Previously we assumed that a symbol is accessible if its containing package is exported by module-info.java. Which was obviously wrong and could lead to a situation where a symbol would be incorrectly accessible if a usage module has a dependency on the symbol's module in IDEA project terms, but does not require it in its module-info.java #KT-18598 In Progress
This commit is contained in:
+10
-13
@@ -34,7 +34,6 @@ import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker
|
|||||||
import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext
|
import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext
|
||||||
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
|
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
|
||||||
import org.jetbrains.kotlin.resolve.checkers.ClassifierUsageChecker
|
import org.jetbrains.kotlin.resolve.checkers.ClassifierUsageChecker
|
||||||
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe
|
|
||||||
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
|
||||||
import org.jetbrains.kotlin.resolve.source.getPsi
|
import org.jetbrains.kotlin.resolve.source.getPsi
|
||||||
@@ -76,21 +75,19 @@ class JvmModuleAccessibilityChecker(project: Project) : CallChecker {
|
|||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|
||||||
val containingPackage = DescriptorUtils.getParentOfType(targetClassOrPackage, PackageFragmentDescriptor::class.java, false)
|
if (ourModule?.name == theirModule.name) return null
|
||||||
val fqName = containingPackage?.fqNameUnsafe?.toSafe() ?: return null
|
|
||||||
if (theirModule.exports(fqName)) return null
|
|
||||||
|
|
||||||
if (ourModule != null) {
|
if (ourModule != null && !moduleResolver.moduleGraph.reads(ourModule.name, theirModule.name)) {
|
||||||
if (ourModule.name == theirModule.name) return null
|
return ErrorsJvm.JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE.on(reportOn, theirModule.name)
|
||||||
|
|
||||||
if (!moduleResolver.moduleGraph.reads(ourModule.name, theirModule.name)) {
|
|
||||||
return ErrorsJvm.JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE.on(reportOn, theirModule.name)
|
|
||||||
}
|
|
||||||
|
|
||||||
if (theirModule.exportsTo(fqName, ourModule.name)) return null
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return ErrorsJvm.JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE.on(reportOn, theirModule.name, fqName.asString())
|
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(
|
private fun findVirtualFile(
|
||||||
|
|||||||
+3
@@ -0,0 +1,3 @@
|
|||||||
|
package foo;
|
||||||
|
|
||||||
|
public class Foo {}
|
||||||
+3
@@ -0,0 +1,3 @@
|
|||||||
|
module dependency {
|
||||||
|
exports foo;
|
||||||
|
}
|
||||||
+3
@@ -0,0 +1,3 @@
|
|||||||
|
module main {
|
||||||
|
// does not require dependency
|
||||||
|
}
|
||||||
+5
@@ -0,0 +1,5 @@
|
|||||||
|
import foo.<error>Foo</error>
|
||||||
|
|
||||||
|
fun usage() {
|
||||||
|
<error>Foo</error>()
|
||||||
|
}
|
||||||
+5
@@ -73,4 +73,9 @@ class Java9MultiModuleHighlightingTest : AbstractMultiModuleHighlightingTest() {
|
|||||||
module("unnamed").addDependency(d)
|
module("unnamed").addDependency(d)
|
||||||
checkHighlightingInAllFiles()
|
checkHighlightingInAllFiles()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fun testExportedPackageIsInaccessibleWithoutRequires() = doTest {
|
||||||
|
module("main").addDependency(module("dependency"))
|
||||||
|
checkHighlightingInAllFiles()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user