fix KT-9299 In a project with circular dependencies between modules, IDE reports error on use of internal class from another module, but the corresponding code still compiles and runs.

#KT-9299 Fixed
2015-10-06 16:23:37 +03:00
parent 17b275af23
commit 6ebe0c30ec
23 changed files with 270 additions and 21 deletions
@@ -24,6 +24,7 @@ import org.jetbrains.kotlin.load.kotlin.ModuleVisibilityManager
 import org.jetbrains.kotlin.load.kotlin.getSourceElement
 import org.jetbrains.kotlin.load.kotlin.isContainedByCompiledPartOfOurModule
 import org.jetbrains.kotlin.modules.Module
+import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.resolve.lazy.descriptors.LazyPackageDescriptor
 import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
 import org.jetbrains.kotlin.util.ModuleVisibilityHelper
@@ -44,8 +45,18 @@ class ModuleVisibilityHelperImpl : ModuleVisibilityHelper {

        val moduleVisibilityManager = ModuleVisibilityManager.SERVICE.getInstance(project)

+        fun findModule(kotlinFile: KtFile): Module? = moduleVisibilityManager.chunk.firstOrNull { it.getSourceFiles().containsRaw(kotlinFile.virtualFile.path) }
+
        val whatSource = getSourceElement(what)
-        if (whatSource is KotlinSourceElement) return true
+        if (whatSource is KotlinSourceElement) {
+            if (moduleVisibilityManager.chunk.size > 1 && fromSource is KotlinSourceElement) {
+                val fromSourceKotlinFile = fromSource.psi.getContainingJetFile()
+                val whatSourceKotlinFile = whatSource.psi.getContainingJetFile()
+                return findModule(whatSourceKotlinFile) === findModule(fromSourceKotlinFile)
+            }
+
+            return true
+        }

        moduleVisibilityManager.friendPaths.forEach {
            if (isContainedByCompiledPartOfOurModule(what, File(it))) return true
@@ -29,6 +29,7 @@ import org.jetbrains.kotlin.resolve.scopes.LexicalScope
 import org.jetbrains.kotlin.resolve.scopes.receivers.QualifierReceiver
 import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue
 import org.jetbrains.kotlin.resolve.scopes.utils.findClassifier
+import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
 import org.jetbrains.kotlin.resolve.validation.SymbolUsageValidator
 import org.jetbrains.kotlin.utils.addIfNotNull
 import org.jetbrains.kotlin.utils.addToStdlib.check
@@ -122,11 +123,19 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa
        val importedReference = importDirective.importedReference ?: return null
        val path = importedReference.asQualifierPartList(trace)
        val lastPart = path.lastOrNull() ?: return null
+        val packageFragmentForCheck =
+                if (packageFragmentForVisibilityCheck is DeclarationDescriptorWithSource && packageFragmentForVisibilityCheck.source == SourceElement.NO_SOURCE) {
+                    PackageFragmentWithCustomSource(packageFragmentForVisibilityCheck, KotlinSourceElement(importDirective.getContainingJetFile()))
+                }
+
+                else {
+                    packageFragmentForVisibilityCheck
+                }

        if (!importDirective.isAllUnder) {
-            return processSingleImport(moduleDescriptor, trace, importDirective, path, lastPart, packageFragmentForVisibilityCheck)
+            return processSingleImport(moduleDescriptor, trace, importDirective, path, lastPart, packageFragmentForCheck)
        }
-        val packageOrClassDescriptor = resolveToPackageOrClass(path, moduleDescriptor, trace, packageFragmentForVisibilityCheck,
+        val packageOrClassDescriptor = resolveToPackageOrClass(path, moduleDescriptor, trace, packageFragmentForCheck,
                                                               scopeForFirstPart = null, inImport = true) ?: return null
        if (packageOrClassDescriptor is ClassDescriptor && packageOrClassDescriptor.kind.isSingleton) {
            trace.report(Errors.CANNOT_ALL_UNDER_IMPORT_FROM_SINGLETON.on(lastPart.expression, packageOrClassDescriptor)) // todo report on star
@@ -398,8 +407,17 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa
            symbolUsageValidator.validateTypeUsage(descriptor, trace, referenceExpression)
        }

-        if (descriptor is DeclarationDescriptorWithVisibility && !isVisible(descriptor, shouldBeVisibleFrom, inImport)) {
-            trace.report(Errors.INVISIBLE_REFERENCE.on(referenceExpression, descriptor, descriptor.visibility, descriptor))
+        if (descriptor is DeclarationDescriptorWithVisibility) {
+            val fromToCheck =
+                if (shouldBeVisibleFrom is PackageFragmentDescriptor && shouldBeVisibleFrom.source == SourceElement.NO_SOURCE) {
+                    PackageFragmentWithCustomSource(shouldBeVisibleFrom, KotlinSourceElement(referenceExpression.getContainingJetFile()))
+                }
+                else {
+                    shouldBeVisibleFrom
+                }
+            if (!isVisible(descriptor, fromToCheck, inImport)) {
+                trace.report(Errors.INVISIBLE_REFERENCE.on(referenceExpression, descriptor, descriptor.visibility, descriptor))
+            }
        }

        if (isQualifier) {
@@ -429,3 +447,11 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa
        return Visibilities.isVisible(ReceiverValue.IRRELEVANT_RECEIVER, descriptor, shouldBeVisibleFrom)
    }
 }
+
+/*
+    This purpose of this class is to pass information about source file for current package fragment in order for check visibilities between modules
+    (see ModuleVisibilityHelperImpl.isInFriendModule).
+ */
+private class PackageFragmentWithCustomSource(private val original: PackageFragmentDescriptor, private val source: SourceElement) : PackageFragmentDescriptor by original {
+    override fun getSource(): SourceElement = source
+}
@@ -79,6 +79,18 @@ public class IncrementalJpsTestGenerated extends AbstractIncrementalJpsTest {
            doTest(fileName);
        }

+        @TestMetadata("simpleDependencyErrorOnAccessToInternal1")
+        public void testSimpleDependencyErrorOnAccessToInternal1() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("jps-plugin/testData/incremental/multiModule/simpleDependencyErrorOnAccessToInternal1/");
+            doTest(fileName);
+        }
+
+        @TestMetadata("simpleDependencyErrorOnAccessToInternal2")
+        public void testSimpleDependencyErrorOnAccessToInternal2() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("jps-plugin/testData/incremental/multiModule/simpleDependencyErrorOnAccessToInternal2/");
+            doTest(fileName);
+        }
+
        @TestMetadata("simpleDependencyUnchanged")
        public void testSimpleDependencyUnchanged() throws Exception {
            String fileName = JetTestUtils.navigationMetadata("jps-plugin/testData/incremental/multiModule/simpleDependencyUnchanged/");
@@ -494,20 +494,14 @@ public class KotlinJpsBuildTest : AbstractKotlinJpsBuildTestCase() {
        initProject()
        val result = makeAll()
        result.assertFailed()
-
-        val actualErrors = result.getMessages(BuildMessage.Kind.ERROR)
-                .map { it as CompilerMessage }
-                .map { "${it.messageText} at line ${it.line}, column ${it.column}" }.sorted().joinToString("\n")
-        val projectRoot = File(AbstractKotlinJpsBuildTestCase.TEST_DATA_PATH + "general/" + getTestName(false))
-        val expectedFile = File(projectRoot, "errors.txt")
-        JetTestUtils.assertEqualsToFile(expectedFile, actualErrors)
+        result.checkErrors()
    }

-    // TODO See KT-9299 In a project with circular dependencies between modules, IDE reports error on use of internal class from another module, but the corresponding code still compiles and runs.
    public fun testCircularDependenciesInternalFromAnotherModule() {
        initProject()
        val result = makeAll()
-        result.assertSuccessful()
+        result.assertFailed()
+        result.checkErrors()
    }

    public fun testCircularDependencyWithReferenceToOldVersionLib() {
@@ -679,6 +673,15 @@ public class KotlinJpsBuildTest : AbstractKotlinJpsBuildTestCase() {
        assertFalse(File(storageRoot, "targets/java-production/module2/kotlin").exists())
    }

+    private fun BuildResult.checkErrors() {
+        val actualErrors = getMessages(BuildMessage.Kind.ERROR)
+                .map { it as CompilerMessage }
+                .map { "${it.messageText} at line ${it.line}, column ${it.column}" }.sorted().joinToString("\n")
+        val projectRoot = File(AbstractKotlinJpsBuildTestCase.TEST_DATA_PATH + "general/" + getTestName(false))
+        val expectedFile = File(projectRoot, "errors.txt")
+        JetTestUtils.assertEqualsToFile(expectedFile, actualErrors)
+    }
+
    private fun buildCustom(canceledStatus: CanceledStatus, logger: TestProjectBuilderLogger,buildResult: BuildResult) {
        val scopeBuilder = CompileScopeTestBuilder.make().all()
        val descriptor = this.createProjectDescriptor(BuildLoggingManager(logger))
@@ -0,0 +1,7 @@
+'internal open val member: kotlin.Int defined in test.ClassBB1' has no access to 'internal abstract val member: kotlin.Int defined in test.ClassB1', so it cannot override it at line 14, column 14
+Cannot access 'InternalClass1': it is 'internal' in 'test' at line 5, column 13
+Cannot access 'InternalClass1': it is 'internal' in 'test' at line 8, column 36
+Cannot access 'InternalClass2': it is 'internal' in 'test' at line 19, column 15
+Cannot access 'InternalClassAnnotation': it is 'internal' in 'test' at line 10, column 2
+Cannot access 'InternalFileAnnotation': it is 'internal' in 'test' at line 1, column 7
+Cannot access 'member': it is 'invisible_fake' in 'ClassAA1' at line 27, column 25
@@ -2,9 +2,18 @@ package test

 internal open class InternalClass1

+@Target(AnnotationTarget.FILE)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalFileAnnotation()
+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalClassAnnotation()
+
 abstract class ClassA1(internal val member: Int)

 abstract class ClassB1 {
    internal abstract val member: Int
 }

+class ClassD: InternalClass2()
@@ -1,11 +1,13 @@
-// ERROR: 'internal open val member: kotlin.Int defined in test.ClassBB1' has no access to 'internal abstract val member: kotlin.Int defined in test.ClassB1', so it cannot override it
-// ERROR: Cannot access 'InternalClass1': it is 'internal' in 'test'
-// ERROR: Cannot access 'member': it is 'invisible_fake' in 'ClassAA1'
+@file:InternalFileAnnotation
+
 package test

+import test.InternalClass1
+
 // InternalClass1, ClassA1, ClassB1 are in module1
 class ClassInheritedFromInternal1: InternalClass1()

+@InternalClassAnnotation
 class ClassAA1 : ClassA1(10)

 class ClassBB1 : ClassB1() {
@@ -1,4 +1,6 @@
-'internal open val member: kotlin.Int defined in test.ClassBB1' has no access to 'internal abstract val member: kotlin.Int defined in test.ClassB1', so it cannot override it at line 11, column 14
-Cannot access 'InternalClass1': it is 'internal' in 'test' at line 3, column 13
-Cannot access 'InternalClass1': it is 'internal' in 'test' at line 6, column 36
-Cannot access 'member': it is 'invisible_fake' in 'ClassAA1' at line 24, column 25
+'internal open val member: kotlin.Int defined in test.ClassBB1' has no access to 'internal abstract val member: kotlin.Int defined in test.ClassB1', so it cannot override it at line 14, column 14
+Cannot access 'InternalClass1': it is 'internal' in 'test' at line 5, column 13
+Cannot access 'InternalClass1': it is 'internal' in 'test' at line 8, column 36
+Cannot access 'InternalClassAnnotation': it is 'internal' in 'test' at line 10, column 2
+Cannot access 'InternalTestAnnotation': it is 'internal' in 'test' at line 1, column 7
+Cannot access 'member': it is 'invisible_fake' in 'ClassAA1' at line 27, column 25
@@ -1,4 +1,13 @@
 package test
+@Target(AnnotationTarget.FILE)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalTestAnnotation()
+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalClassAnnotation()
+
+private class PrivateClass1

 internal open class InternalClass1

@@ -1,3 +1,5 @@
+@file:InternalTestAnnotation
+
 package test

 import test.InternalClass1
@@ -5,6 +7,7 @@ import test.InternalClass1
 // InternalClass1, ClassA1, ClassB1 are in module1
 class ClassInheritedFromInternal1: InternalClass1()

+@InternalClassAnnotation
 class ClassAA1 : ClassA1(10)

 class ClassBB1 : ClassB1() {
@@ -0,0 +1,26 @@
+Cleaning output files:
+out/production/module1/META-INF/module1.kotlin_module
+out/production/module1/a/A.class
+out/production/module1/a/ClassAnnotation.class
+out/production/module1/a/FileAnnotation.class
+out/production/module1/a/Module1_aKt.class
+End of files
+Compiling files:
+module1/src/module1_a.kt
+End of files
+Cleaning output files:
+out/production/module2/META-INF/module2.kotlin_module
+out/production/module2/b/B.class
+out/production/module2/b/Module2_bKt.class
+End of files
+Compiling files:
+module2/src/module2_b.kt
+End of files
+COMPILATION FAILED
+Cannot access 'FileAnnotation': it is 'internal' in 'a'
+Cannot access 'A': it is 'internal' in 'a'
+Cannot access 'FileAnnotation': it is 'internal' in 'a'
+Cannot access 'ClassAnnotation': it is 'internal' in 'a'
+Cannot access 'ClassAnnotation': it is 'internal' in 'a'
+Cannot access 'A': it is 'internal' in 'a'
+Cannot access 'a': it is 'internal' in 'a'
@@ -0,0 +1,2 @@
+module1->
+module2->module1
@@ -0,0 +1,14 @@
+package a
+
+@Target(AnnotationTarget.FILE)
+@Retention(AnnotationRetention.SOURCE)
+annotation class FileAnnotation()
+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.SOURCE)
+annotation class ClassAnnotation()
+
+class A
+
+fun a() {
+}
@@ -0,0 +1,15 @@
+package a
+
+@Target(AnnotationTarget.FILE)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class FileAnnotation()
+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class ClassAnnotation()
+
+internal class A
+
+internal fun a(): String {
+    return ":)"
+}
@@ -0,0 +1,13 @@
+@file:FileAnnotation
+package b
+
+import a.A
+import a.FileAnnotation
+import a.ClassAnnotation
+
+@ClassAnnotation
+class B
+
+fun b(param: a.A) {
+    a.a()
+}
@@ -0,0 +1,5 @@
+package c
+
+fun c() {
+    // This file doesn't use anything from module1, so it won't be recompiled after change
+}
@@ -0,0 +1,26 @@
+Cleaning output files:
+out/production/module1/META-INF/module1.kotlin_module
+out/production/module1/a/A.class
+out/production/module1/a/InternalA.class
+out/production/module1/a/InternalClassAnnotation.class
+out/production/module1/a/InternalFileAnnotation.class
+out/production/module1/a/Module1_aKt.class
+End of files
+Compiling files:
+module1/src/module1_a.kt
+End of files
+Cleaning output files:
+out/production/module2/META-INF/module2.kotlin_module
+out/production/module2/b/B.class
+out/production/module2/b/Module2_bKt.class
+End of files
+Compiling files:
+module2/src/module2_b.kt
+End of files
+COMPILATION FAILED
+Cannot access 'InternalFileAnnotation': it is 'internal' in 'a'
+Cannot access 'InternalFileAnnotation': it is 'internal' in 'a'
+Cannot access 'InternalClassAnnotation': it is 'internal' in 'a'
+Cannot access 'InternalClassAnnotation': it is 'internal' in 'a'
+Unresolved reference: InternalA
+Unresolved reference: internalA
@@ -0,0 +1,2 @@
+module1->
+module2->module1
@@ -0,0 +1,19 @@
+package a
+
+@Target(AnnotationTarget.FILE)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalFileAnnotation()
+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalClassAnnotation()
+
+class A
+
+internal class InternalA
+
+fun a() {
+}
+
+internal fun internalA() {
+}
@@ -0,0 +1,19 @@
+package a
+
+@Target(AnnotationTarget.FILE)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalFileAnnotation()
+
+@Target(AnnotationTarget.CLASS)
+@Retention(AnnotationRetention.SOURCE)
+internal annotation class InternalClassAnnotation()
+
+class A
+
+internal class AA
+
+fun a() {
+}
+
+internal fun aa() {
+}
@@ -0,0 +1,7 @@
+package b
+
+class B
+
+fun b(param: a.A) {
+    a.a()
+}
@@ -0,0 +1,12 @@
+@file:InternalFileAnnotation
+package b
+
+import a.InternalFileAnnotation
+import a.InternalClassAnnotation
+
+@InternalClassAnnotation
+class B
+
+fun b(param: a.InternalA) {
+    a.internalA()
+}
@@ -0,0 +1,5 @@
+package c
+
+fun c() {
+    // This file doesn't use anything from module1, so it won't be recompiled after change
+}