From 1360f3cd43fcceca00239e07fe703833382ac3ce Mon Sep 17 00:00:00 2001 From: Stanislav Erokhin Date: Fri, 18 Sep 2015 17:16:41 +0300 Subject: [PATCH] Fix visibility checker for import. --- .../resolve/QualifiedExpressionResolver.kt | 12 +-- .../resolve/lazy/FileScopeProviderImpl.kt | 11 ++- .../kotlin/resolve/lazy/LazyImportScope.kt | 6 +- .../tests/imports/CheckJavaVisibility.kt | 73 ++++++++++++++++++ .../tests/imports/CheckJavaVisibility.txt | 52 +++++++++++++ .../tests/imports/CheckVisibility.kt | 28 +++++++ .../tests/imports/CheckVisibility.txt | 74 +++++++++++++++++++ .../checkers/JetDiagnosticsTestGenerated.java | 12 +++ .../idea/caches/resolve/resolutionApi.kt | 3 +- 9 files changed, 257 insertions(+), 14 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.kt create mode 100644 compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.txt create mode 100644 compiler/testData/diagnostics/tests/imports/CheckVisibility.kt create mode 100644 compiler/testData/diagnostics/tests/imports/CheckVisibility.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt index ea2babd6a25..8b66b78e153 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt @@ -117,14 +117,14 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa importDirective: JetImportDirective, moduleDescriptor: ModuleDescriptor, trace: BindingTrace, - shouldBeVisibleFrom: DeclarationDescriptor // todo + packageFragmentForVisibilityCheck: PackageFragmentDescriptor? ): JetScope { val importedReference = importDirective.importedReference ?: return JetScope.Empty val path = importedReference.asQualifierPartList(trace) val lastPart = path.lastOrNull() ?: return JetScope.Empty if (importDirective.isAllUnder) { - val packageOrClassDescriptor = resolveToPackageOrClass(path, moduleDescriptor, trace, shouldBeVisibleFrom, + val packageOrClassDescriptor = resolveToPackageOrClass(path, moduleDescriptor, trace, packageFragmentForVisibilityCheck, scopeForFirstPart = null, inImport = true) ?: return JetScope.Empty if (packageOrClassDescriptor is ClassDescriptor && packageOrClassDescriptor.kind.isSingleton) { trace.report(Errors.CANNOT_IMPORT_MEMBERS_FROM_SINGLETON.on(lastPart.expression, packageOrClassDescriptor)) // todo report on star @@ -136,12 +136,12 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa else { val aliasName = JetPsiUtil.getAliasName(importDirective) if (aliasName == null) { // import kotlin. - resolveToPackageOrClass(path, moduleDescriptor, trace, shouldBeVisibleFrom, scopeForFirstPart = null, inImport = true) + resolveToPackageOrClass(path, moduleDescriptor, trace, packageFragmentForVisibilityCheck, scopeForFirstPart = null, inImport = true) return JetScope.Empty } val packageOrClassDescriptor = resolveToPackageOrClass(path.subList(0, path.size() - 1), moduleDescriptor, - trace, shouldBeVisibleFrom, scopeForFirstPart = null, inImport = true) + trace, packageFragmentForVisibilityCheck, scopeForFirstPart = null, inImport = true) ?: return JetScope.Empty val descriptors = SmartList() @@ -167,7 +167,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa else -> throw IllegalStateException("Should be class or package: $packageOrClassDescriptor") } if (descriptors.isNotEmpty()) { - storageResult(trace, lastPart.expression, descriptors, shouldBeVisibleFrom, inImport = true, isQualifier = false) + storageResult(trace, lastPart.expression, descriptors, packageFragmentForVisibilityCheck, inImport = true, isQualifier = false) } else { tryResolveDescriptorsWhichCannotBeImported(trace, moduleDescriptor, packageOrClassDescriptor, lastPart) @@ -249,7 +249,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa path: List, moduleDescriptor: ModuleDescriptor, trace: BindingTrace, - shouldBeVisibleFrom: DeclarationDescriptor, + shouldBeVisibleFrom: DeclarationDescriptor?, scopeForFirstPart: LexicalScope?, inImport: Boolean ): DeclarationDescriptor? { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/FileScopeProviderImpl.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/FileScopeProviderImpl.kt index d85136bcc68..05c32c415dd 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/FileScopeProviderImpl.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/FileScopeProviderImpl.kt @@ -60,10 +60,13 @@ public class FileScopeProviderImpl( val packageFragment = topLevelDescriptorProvider.getPackageFragment(file.getPackageFqName()) .sure { "Could not find fragment ${file.getPackageFqName()} for file ${file.getName()}" } - val aliasImportResolver = LazyImportResolver(storageManager, qualifiedExpressionResolver, this, moduleDescriptor, AliasImportsIndexed(imports), bindingTrace) - val allUnderImportResolver = LazyImportResolver(storageManager, qualifiedExpressionResolver, this, moduleDescriptor, AllUnderImportsIndexed(imports), bindingTrace) - val defaultAliasImportResolver = LazyImportResolver(storageManager, qualifiedExpressionResolver, this, moduleDescriptor, AliasImportsIndexed(defaultImports), tempTrace) - val defaultAllUnderImportResolver = LazyImportResolver(storageManager, qualifiedExpressionResolver, this, moduleDescriptor, AllUnderImportsIndexed(defaultImports), tempTrace) + fun createImportResolver(indexedImports: IndexedImports, trace: BindingTrace) + = LazyImportResolver(storageManager, qualifiedExpressionResolver, this, moduleDescriptor, indexedImports, trace, packageFragment) + + val aliasImportResolver = createImportResolver(AliasImportsIndexed(imports), bindingTrace) + val allUnderImportResolver = createImportResolver(AllUnderImportsIndexed(imports), bindingTrace) + val defaultAliasImportResolver = createImportResolver(AliasImportsIndexed(defaultImports), tempTrace) + val defaultAllUnderImportResolver = createImportResolver(AllUnderImportsIndexed(defaultImports), tempTrace) val scopeChain = ArrayList() diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt index 60cc5b0bd33..4aecf68791e 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt @@ -70,7 +70,8 @@ class LazyImportResolver( val fileScopeProvider: FileScopeProvider, val moduleDescriptor: ModuleDescriptor, val indexedImports: IndexedImports, - private val traceForImportResolve: BindingTrace + private val traceForImportResolve: BindingTrace, + private val packageFragment: PackageFragmentDescriptor ) { private val importedScopesProvider = storageManager.createMemoizedFunction { directive: JetImportDirective -> ImportDirectiveResolveCache(directive) @@ -99,9 +100,8 @@ class LazyImportResolver( directiveUnderResolve = directive try { - // todo use packageViewFragment for visibility val directiveImportScope = qualifiedExpressionResolver.processImportReference( - directive, moduleDescriptor, traceForImportResolve, moduleDescriptor) + directive, moduleDescriptor, traceForImportResolve, packageFragment) val descriptors = if (directive.isAllUnder()) emptyList() else directiveImportScope.getAllDescriptors() PlatformTypesMappedToKotlinChecker.checkPlatformTypesMappedToKotlin(moduleDescriptor, traceForImportResolve, directive, descriptors) diff --git a/compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.kt b/compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.kt new file mode 100644 index 00000000000..59645abeaf0 --- /dev/null +++ b/compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.kt @@ -0,0 +1,73 @@ +// FILE: j/JavaPublic.java +package j; + +public class JavaPublic { + public static void javaM() {} + public static int javaP = 4; + static int javaPackageLocal = 5; +} + +// FILE: j/JavaPackageLocal.java +package j; + +class JavaPackageLocal { + static void javaMPackage() {} + static int javaPPackage = 4; +} + +// FILE: j/JavaProtected.java +package j; + +public class JavaProtected { + protected static void javaMProtected() {} + protected static int javaPProtected = 4; +} + +// FILE: j/JavaPrivate.java +package j; + +public class JavaPrivate { + private static void javaMPrivate() {} + private static int javaPPrivate = 4; +} + + +// FILE: k1.kt +package k + +import j.JavaPublic +import j.JavaPublic.javaM +import j.JavaPublic.javaP +import j.JavaPublic.javaPackageLocal + +import j.JavaPackageLocal +import j.JavaPackageLocal.javaMPackage +import j.JavaPackageLocal.javaPPackage + +import j.JavaProtected +import j.JavaProtected.javaMProtected +import j.JavaProtected.javaPProtected + +import j.JavaPrivate +import j.JavaPrivate.javaMPrivate +import j.JavaPrivate.javaPPrivate + +// FILE: k2.kt +package j + +import j.JavaPublic +import j.JavaPublic.javaM +import j.JavaPublic.javaP +import j.JavaPublic.javaPackageLocal + +import j.JavaPackageLocal +import j.JavaPackageLocal.javaMPackage +import j.JavaPackageLocal.javaPPackage + +import j.JavaProtected +import j.JavaProtected.javaMProtected +import j.JavaProtected.javaPProtected + +import j.JavaPrivate +import j.JavaPrivate.javaMPrivate +import j.JavaPrivate.javaPPrivate \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.txt b/compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.txt new file mode 100644 index 00000000000..6e50bdaafef --- /dev/null +++ b/compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.txt @@ -0,0 +1,52 @@ +package + +package j { + + public/*package*/ open class JavaPackageLocal { + public/*package*/ constructor JavaPackageLocal() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public/*package*/ final var javaPPackage: kotlin.Int + public/*package*/ open fun javaMPackage(): kotlin.Unit + } + + public open class JavaPrivate { + public constructor JavaPrivate() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + private final var javaPPrivate: kotlin.Int + private open fun javaMPrivate(): kotlin.Unit + } + + public open class JavaProtected { + public constructor JavaProtected() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + protected/*protected static*/ final var javaPProtected: kotlin.Int + protected/*protected static*/ open fun javaMProtected(): kotlin.Unit + } + + public open class JavaPublic { + public constructor JavaPublic() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final var javaP: kotlin.Int + public/*package*/ final var javaPackageLocal: kotlin.Int + public open fun javaM(): kotlin.Unit + } +} + +package k { +} diff --git a/compiler/testData/diagnostics/tests/imports/CheckVisibility.kt b/compiler/testData/diagnostics/tests/imports/CheckVisibility.kt new file mode 100644 index 00000000000..8bc6f6885d3 --- /dev/null +++ b/compiler/testData/diagnostics/tests/imports/CheckVisibility.kt @@ -0,0 +1,28 @@ +// MODULE: m1 +// FILE: k1.kt +package k + +private class KPrivate +internal class KInternal +public class KPublic + +class A { + protected class KProtected +} + +// FILE: k2.kt +package k2 + +import k.KPrivate +import k.KInternal +import k.KPublic +import k.A.KProtected + +// MODULE: m2(m1) +// FILE: k3.kt +package k3 + +import k.KPrivate +import k.KInternal +import k.KPublic +import k.A.KProtected \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/imports/CheckVisibility.txt b/compiler/testData/diagnostics/tests/imports/CheckVisibility.txt new file mode 100644 index 00000000000..426a19aed45 --- /dev/null +++ b/compiler/testData/diagnostics/tests/imports/CheckVisibility.txt @@ -0,0 +1,74 @@ +// -- Module: -- +package + +package k { + + public final class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + protected final class KProtected { + public constructor KProtected() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + } + + internal final class KInternal { + public constructor KInternal() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + + private final class KPrivate { + public constructor KPrivate() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + + public final class KPublic { + public constructor KPublic() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} + +package k2 { +} + + +// -- Module: -- +package + +package k { + + public final class A { + // -- Module: -- + } + + internal final class KInternal { + // -- Module: -- + } + + private final class KPrivate { + // -- Module: -- + } + + public final class KPublic { + // -- Module: -- + } +} + +package k2 { +} + +package k3 { +} + + diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index d2c331ced02..f0fc93ba86e 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -6537,6 +6537,18 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("CheckJavaVisibility.kt") + public void testCheckJavaVisibility() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/imports/CheckJavaVisibility.kt"); + doTest(fileName); + } + + @TestMetadata("CheckVisibility.kt") + public void testCheckVisibility() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/imports/CheckVisibility.kt"); + doTest(fileName); + } + @TestMetadata("ClassClash.kt") public void testClassClash() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/imports/ClassClash.kt"); diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/resolutionApi.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/resolutionApi.kt index 690081ae5df..1fe239d3c64 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/resolutionApi.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/resolve/resolutionApi.kt @@ -74,6 +74,7 @@ public fun JetElement.analyzeFullyAndGetResult(vararg extraFiles: JetFile): Anal return KotlinCacheService.getInstance(getProject()).getResolutionFacade(listOf(this) + extraFiles.toList()).analyzeFullyAndGetResult(listOf(this)) } +// this method don't check visibility and collect all descriptors with given fqName public fun ResolutionFacade.resolveImportReference( moduleDescriptor: ModuleDescriptor, fqName: FqName @@ -81,7 +82,7 @@ public fun ResolutionFacade.resolveImportReference( val importDirective = JetPsiFactory(project).createImportDirective(ImportPath(fqName, false)) val qualifiedExpressionResolver = this.getFrontendService(moduleDescriptor, QualifiedExpressionResolver::class.java) return qualifiedExpressionResolver.processImportReference( - importDirective, moduleDescriptor, BindingTraceContext(), moduleDescriptor).getAllDescriptors() + importDirective, moduleDescriptor, BindingTraceContext(), packageFragmentForVisibilityCheck = null).getAllDescriptors() } //NOTE: idea default API returns module search scope for file under module but not in source or production source (for example, test data )