From 85895bec62039d527f7d8d8bac13ab03ea95607d Mon Sep 17 00:00:00 2001 From: Stanislav Erokhin Date: Mon, 21 Sep 2015 17:35:42 +0300 Subject: [PATCH] Improved error reporting, when we trying import invisible descriptor. --- .../resolve/QualifiedExpressionResolver.kt | 96 ++++++++++++------- .../tests/imports/ImportOverloadFunctions.kt | 44 +++++++++ .../tests/imports/ImportOverloadFunctions.txt | 18 ++++ .../checkers/JetDiagnosticsTestGenerated.java | 6 ++ 4 files changed, 129 insertions(+), 35 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.kt create mode 100644 compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt index 8b66b78e153..e0dbcd7c93d 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/QualifiedExpressionResolver.kt @@ -43,7 +43,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa ) { val packageNames = packageDirective.packageNames for ((index, nameExpression) in packageNames.withIndex()) { - storageResult(trace, nameExpression, listOf(module.getPackage(packageDirective.getFqName(nameExpression))), + storageResult(trace, nameExpression, module.getPackage(packageDirective.getFqName(nameExpression)), shouldBeVisibleFrom = null, inImport = false, isQualifier = index != packageNames.lastIndex) } } @@ -56,7 +56,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa if (userType.qualifier == null && !userType.startWithPackage) { // optimization for non-qualified types return userType.referenceExpression?.let { val classifier = scope.getClassifier(it.getReferencedNameAsName(), KotlinLookupLocation(it)) - storageResult(trace, it, listOfNotNull(classifier), scope.ownerDescriptor, inImport = false, isQualifier = false) + storageResult(trace, it, classifier, scope.ownerDescriptor, inImport = false, isQualifier = false) classifier } } @@ -82,7 +82,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa is ClassDescriptor -> qualifier.unsubstitutedInnerClassesScope.getClassifier(lastPart.name, lastPart.location) else -> null } - storageResult(trace, lastPart.expression, listOfNotNull(classifier), scope.ownerDescriptor, inImport = false, isQualifier = false) + storageResult(trace, lastPart.expression, classifier, scope.ownerDescriptor, inImport = false, isQualifier = false) return classifier } @@ -173,7 +173,10 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa tryResolveDescriptorsWhichCannotBeImported(trace, moduleDescriptor, packageOrClassDescriptor, lastPart) } - return SingleImportScope(aliasName, descriptors) + val importedDescriptors = descriptors.filter { isVisible(it, packageFragmentForVisibilityCheck, inImport = true) }. + check { it.isNotEmpty() } ?: descriptors + + return SingleImportScope(aliasName, importedDescriptors) } } @@ -260,7 +263,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa val firstDescriptor = scopeForFirstPart?.let { val firstPart = path.first() it.getClassifier(firstPart.name, firstPart.location)?.apply { - storageResult(trace, firstPart.expression, listOf(this), shouldBeVisibleFrom, inImport) + storageResult(trace, firstPart.expression, this, shouldBeVisibleFrom, inImport) } } @@ -290,7 +293,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa } else -> null } - storageResult(trace, qualifierPart.expression, listOfNotNull(nextDescriptor), shouldBeVisibleFrom, inImport) + storageResult(trace, qualifierPart.expression, nextDescriptor, shouldBeVisibleFrom, inImport) nextDescriptor } } @@ -324,7 +327,7 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa inImport: Boolean ) { path.foldRight(packageView) { qualifierPart, currentView -> - storageResult(trace, qualifierPart.expression, listOfNotNull(currentView), shouldBeVisibleFrom = null, inImport = inImport) + storageResult(trace, qualifierPart.expression, currentView, shouldBeVisibleFrom = null, inImport = inImport) val parentView = currentView.containingDeclaration assert(parentView != null) { "Containing Declaration must be not null for package with fqName: ${currentView.fqName}, " + @@ -342,25 +345,49 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa inImport: Boolean, isQualifier: Boolean = true ) { - if (descriptors.isEmpty()) { - trace.report(Errors.UNRESOLVED_REFERENCE.on(referenceExpression, referenceExpression)) - } - else if(descriptors.size() > 1) { - // todo all descriptors invisible - report specific error - trace.record(BindingContext.AMBIGUOUS_REFERENCE_TARGET, referenceExpression, descriptors) + if (descriptors.size() > 1) { + val visibleDescriptors = descriptors.filter { isVisible(it, shouldBeVisibleFrom, inImport) } + if (visibleDescriptors.isEmpty()) { + val descriptor = descriptors.first() as DeclarationDescriptorWithVisibility + trace.report(Errors.INVISIBLE_REFERENCE.on(referenceExpression, descriptor, descriptor.visibility, descriptor.containingDeclaration!!)) + } + else if (visibleDescriptors.size() > 1) { + trace.record(BindingContext.AMBIGUOUS_REFERENCE_TARGET, referenceExpression, visibleDescriptors) + } + else { + storageResult(trace, referenceExpression, visibleDescriptors.single(), null, inImport, isQualifier) + } } else { - val descriptor = descriptors.single() - trace.record(BindingContext.REFERENCE_TARGET, referenceExpression, descriptor) - if (descriptor is ClassifierDescriptor) { - symbolUsageValidator.validateTypeUsage(descriptor, trace, referenceExpression) - } - if (descriptor is DeclarationDescriptorWithVisibility && shouldBeVisibleFrom != null) { - checkVisibility(descriptor, trace, referenceExpression, shouldBeVisibleFrom, inImport) - } - if (isQualifier) { - storageQualifier(trace, referenceExpression, descriptor) - } + storageResult(trace, referenceExpression, descriptors.singleOrNull(), shouldBeVisibleFrom, inImport, isQualifier) + } + } + + private fun storageResult( + trace: BindingTrace, + referenceExpression: JetSimpleNameExpression, + descriptor: DeclarationDescriptor?, + shouldBeVisibleFrom: DeclarationDescriptor?, + inImport: Boolean, + isQualifier: Boolean = true + ) { + if (descriptor == null) { + trace.report(Errors.UNRESOLVED_REFERENCE.on(referenceExpression, referenceExpression)) + return + } + + trace.record(BindingContext.REFERENCE_TARGET, referenceExpression, descriptor) + + if (descriptor is ClassifierDescriptor) { + symbolUsageValidator.validateTypeUsage(descriptor, trace, referenceExpression) + } + + if (descriptor is DeclarationDescriptorWithVisibility && !isVisible(descriptor, shouldBeVisibleFrom, inImport)) { + trace.report(Errors.INVISIBLE_REFERENCE.on(referenceExpression, descriptor, descriptor.visibility, descriptor.containingDeclaration!!)) + } + + if (isQualifier) { + storageQualifier(trace, referenceExpression, descriptor) } } @@ -371,19 +398,18 @@ public class QualifiedExpressionResolver(val symbolUsageValidator: SymbolUsageVa } } - private fun checkVisibility( - descriptor: DeclarationDescriptorWithVisibility, - trace: BindingTrace, - referenceExpression: JetSimpleNameExpression, - shouldBeVisibleFrom: DeclarationDescriptor, + private fun isVisible( + descriptor: DeclarationDescriptor, + shouldBeVisibleFrom: DeclarationDescriptor?, inImport: Boolean - ) { + ): Boolean { + if (descriptor !is DeclarationDescriptorWithVisibility || shouldBeVisibleFrom == null) return true + val visibility = descriptor.visibility - if (inImport && !visibility.mustCheckInImports()) { - return - } - if (!Visibilities.isVisible(ReceiverValue.IRRELEVANT_RECEIVER, descriptor, shouldBeVisibleFrom)) { - trace.report(Errors.INVISIBLE_REFERENCE.on(referenceExpression, descriptor, visibility, descriptor.containingDeclaration!!)) + if (inImport) { + if (Visibilities.isPrivate(visibility)) return false + if (!visibility.mustCheckInImports()) return true } + return Visibilities.isVisible(ReceiverValue.IRRELEVANT_RECEIVER, descriptor, shouldBeVisibleFrom) } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.kt b/compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.kt new file mode 100644 index 00000000000..ff9b736309a --- /dev/null +++ b/compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.kt @@ -0,0 +1,44 @@ +// !DIAGNOSTICS: -UNUSED_EXPRESSION -UNUSED_PARAMETER -UNUSED_VARIABLE +// FILE: 1.kt +package k + +private fun zero() {} +private fun zero(a: Int) {} +private fun zero(a: String) {} + +fun one() {} +private fun one(a: Int) {} +private fun one(a: String) {} + +fun two() {} +fun two(a: Int) {} +private fun two(a: String) {} + +fun all() {} +fun all(a: Int) {} +fun all(a: String) {} + +// FILE: 2.kt + +import k.zero +import k.one +import k.two +import k.all + +fun test() { + zero() + zero(1) + zero("") + + one() + one(1) + one("") + + two() + two(1) + two("") + + all() + all(1) + all("") +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.txt b/compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.txt new file mode 100644 index 00000000000..3d3a026eeab --- /dev/null +++ b/compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.txt @@ -0,0 +1,18 @@ +package + +public fun test(): kotlin.Unit + +package k { + public fun all(): kotlin.Unit + public fun all(/*0*/ a: kotlin.Int): kotlin.Unit + public fun all(/*0*/ a: kotlin.String): kotlin.Unit + public fun one(): kotlin.Unit + private fun one(/*0*/ a: kotlin.Int): kotlin.Unit + private fun one(/*0*/ a: kotlin.String): kotlin.Unit + public fun two(): kotlin.Unit + public fun two(/*0*/ a: kotlin.Int): kotlin.Unit + private fun two(/*0*/ a: kotlin.String): kotlin.Unit + private fun zero(): kotlin.Unit + private fun zero(/*0*/ a: kotlin.Int): kotlin.Unit + private fun zero(/*0*/ a: kotlin.String): kotlin.Unit +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index f0fc93ba86e..940d534bcb8 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -6651,6 +6651,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("ImportOverloadFunctions.kt") + public void testImportOverloadFunctions() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/imports/ImportOverloadFunctions.kt"); + doTest(fileName); + } + @TestMetadata("ImportProtectedClass.kt") public void testImportProtectedClass() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/imports/ImportProtectedClass.kt");