From ef8be9560a15fbcf1954381fc55db3869fdf4318 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Mon, 26 Jan 2015 14:54:28 +0300 Subject: [PATCH] Import insertion helper is not confused by inaccessible classes + check visibility of imported declarations through PackageViewDescriptor instead of PackageFragmentDescriptor --- .../jetbrains/kotlin/resolve/lazy/LazyFileScope.kt | 12 ++++++------ .../kotlin/resolve/lazy/LazyImportScope.kt | 3 +-- .../kotlin/idea/quickfix/ImportInsertHelperImpl.kt | 14 +++++++++++++- ...flictingNameForInaccessibleClass1.dependency.kt | 3 +++ .../NoConflictingNameForInaccessibleClass1.kt | 6 ++++++ ...NoConflictingNameForInaccessibleClass1.kt.after | 7 +++++++ ...flictingNameForInaccessibleClass2.dependency.kt | 5 +++++ .../NoConflictingNameForInaccessibleClass2.kt | 6 ++++++ ...NoConflictingNameForInaccessibleClass2.kt.after | 7 +++++++ .../kotlin/addImport/AddImportTestGenerated.java | 13 ++++++++++++- 10 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 idea/testData/addImport/NoConflictingNameForInaccessibleClass1.dependency.kt create mode 100644 idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt create mode 100644 idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt.after create mode 100644 idea/testData/addImport/NoConflictingNameForInaccessibleClass2.dependency.kt create mode 100644 idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt create mode 100644 idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt.after diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt index 97ab7fcd40e..ebc6b0195ee 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt @@ -82,20 +82,20 @@ class LazyFileScope private( val scopeChain = ArrayList() - scopeChain.add(LazyImportScope(aliasImportResolver, packageFragment, LazyImportScope.FilteringKind.ALL, "Alias imports in $debugName")) + scopeChain.add(LazyImportScope(aliasImportResolver, LazyImportScope.FilteringKind.ALL, "Alias imports in $debugName")) scopeChain.add(NoSubpackagesInPackageScope(packageView)) //TODO: problems with visibility too scopeChain.add(JetModuleUtil.getSubpackagesOfRootScope(resolveSession.getModuleDescriptor())) - scopeChain.add(LazyImportScope(defaultAliasImportResolver, packageFragment, LazyImportScope.FilteringKind.ALL, "Default alias imports in $debugName")) + scopeChain.add(LazyImportScope(defaultAliasImportResolver, LazyImportScope.FilteringKind.ALL, "Default alias imports in $debugName")) - scopeChain.add(LazyImportScope(defaultAllUnderImportResolver, packageFragment, LazyImportScope.FilteringKind.VISIBLE_CLASSES, "Default all under imports in $debugName (visible classes)")) - scopeChain.add(LazyImportScope(allUnderImportResolver, packageFragment, LazyImportScope.FilteringKind.VISIBLE_CLASSES, "All under imports in $debugName (visible classes)")) + scopeChain.add(LazyImportScope(defaultAllUnderImportResolver, LazyImportScope.FilteringKind.VISIBLE_CLASSES, "Default all under imports in $debugName (visible classes)")) + scopeChain.add(LazyImportScope(allUnderImportResolver, LazyImportScope.FilteringKind.VISIBLE_CLASSES, "All under imports in $debugName (visible classes)")) scopeChain.addAll(additionalScopes) - scopeChain.add(LazyImportScope(defaultAllUnderImportResolver, packageFragment, LazyImportScope.FilteringKind.INVISIBLE_CLASSES, "Default all under imports in $debugName (invisible classes only)")) - scopeChain.add(LazyImportScope(allUnderImportResolver, packageFragment, LazyImportScope.FilteringKind.INVISIBLE_CLASSES, "All under imports in $debugName (invisible classes only)")) + scopeChain.add(LazyImportScope(defaultAllUnderImportResolver, LazyImportScope.FilteringKind.INVISIBLE_CLASSES, "Default all under imports in $debugName (invisible classes only)")) + scopeChain.add(LazyImportScope(allUnderImportResolver, LazyImportScope.FilteringKind.INVISIBLE_CLASSES, "All under imports in $debugName (invisible classes only)")) return LazyFileScope(scopeChain, aliasImportResolver, allUnderImportResolver, packageFragment, debugName) } 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 b6a55261d25..764d2a80e98 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyImportScope.kt @@ -226,7 +226,6 @@ class LazyImportResolver( class LazyImportScope( private val importResolver: LazyImportResolver, - private val packageFragment: PackageFragmentDescriptor, private val filteringKind: LazyImportScope.FilteringKind, private val debugName: String ) : JetScope { @@ -248,7 +247,7 @@ class LazyImportScope( val visibility = descriptor.getVisibility() val includeVisible = filteringKind == FilteringKind.VISIBLE_CLASSES if (!visibility.mustCheckInImports()) return includeVisible - return Visibilities.isVisible(ReceiverValue.IRRELEVANT_RECEIVER, descriptor, packageFragment) == includeVisible + return Visibilities.isVisible(ReceiverValue.IRRELEVANT_RECEIVER, descriptor, importResolver.packageView) == includeVisible } } diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/ImportInsertHelperImpl.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/ImportInsertHelperImpl.kt index ab067457574..bbfb4f62960 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/ImportInsertHelperImpl.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/ImportInsertHelperImpl.kt @@ -46,6 +46,9 @@ import java.util.ArrayList import org.jetbrains.kotlin.utils.addIfNotNull import org.jetbrains.kotlin.resolve.scopes.JetScope import org.jetbrains.kotlin.descriptors.ModuleDescriptor +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue +import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility public class ImportInsertHelperImpl : ImportInsertHelper { /** @@ -227,17 +230,26 @@ public class ImportInsertHelperImpl : ImportInsertHelper { } .filterNotNull() + val filePackage = moduleDescriptor.getPackage(file.getPackageFqName())!! + + fun isVisible(descriptor: DeclarationDescriptor): Boolean { + if (descriptor !is DeclarationDescriptorWithVisibility) return true + val visibility = descriptor.getVisibility() + return !visibility.mustCheckInImports() || Visibilities.isVisible(ReceiverValue.IRRELEVANT_RECEIVER, descriptor, filePackage) + } + val classNamesToImport = scopeToImport .getDescriptorsFiltered(DescriptorKindFilter.CLASSIFIERS, { true }) + .filter(::isVisible) .map { it.getName() } val aliasNames = imports.map { it.getImportedName() }.filterNotNull().toSet() - //TODO: check for visibility var conflictCandidates: List = classNamesToImport .filter { it.asString() !in aliasNames } .flatMap { importedScopes.map { scope -> scope.getClassifier(it) }.filterNotNull() } + .filter(::isVisible) val conflicts = detectNeededImports(conflictCandidates) val addedImport = addImport(parentFqName, true) diff --git a/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.dependency.kt b/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.dependency.kt new file mode 100644 index 00000000000..18825d653ed --- /dev/null +++ b/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.dependency.kt @@ -0,0 +1,3 @@ +package dependency; + +private class Date {} diff --git a/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt b/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt new file mode 100644 index 00000000000..854530337b9 --- /dev/null +++ b/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt @@ -0,0 +1,6 @@ +// IMPORT: java.util.ArrayList +package p + +import dependency.* + +val d = Date(1, 2) diff --git a/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt.after b/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt.after new file mode 100644 index 00000000000..e21716fdd94 --- /dev/null +++ b/idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt.after @@ -0,0 +1,7 @@ +// IMPORT: java.util.ArrayList +package p + +import dependency.* +import java.util.* + +val d = Date(1, 2) diff --git a/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.dependency.kt b/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.dependency.kt new file mode 100644 index 00000000000..55696c3e6df --- /dev/null +++ b/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.dependency.kt @@ -0,0 +1,5 @@ +package dependency; + +private class Date {} + +class X diff --git a/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt b/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt new file mode 100644 index 00000000000..a81956e949a --- /dev/null +++ b/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt @@ -0,0 +1,6 @@ +// IMPORT: dependency.X +package p + +import java.util.* + +val d = Date(1, 2) diff --git a/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt.after b/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt.after new file mode 100644 index 00000000000..c7435f6df30 --- /dev/null +++ b/idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt.after @@ -0,0 +1,7 @@ +// IMPORT: dependency.X +package p + +import java.util.* +import dependency.* + +val d = Date(1, 2) diff --git a/idea/tests/org/jetbrains/kotlin/addImport/AddImportTestGenerated.java b/idea/tests/org/jetbrains/kotlin/addImport/AddImportTestGenerated.java index d4aa75bd23d..925ced083a0 100644 --- a/idea/tests/org/jetbrains/kotlin/addImport/AddImportTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/addImport/AddImportTestGenerated.java @@ -17,7 +17,6 @@ package org.jetbrains.kotlin.addImport; import com.intellij.testFramework.TestDataPath; -import org.jetbrains.kotlin.test.InnerTestClasses; import org.jetbrains.kotlin.test.JUnit3RunnerWithInners; import org.jetbrains.kotlin.test.JetTestUtils; import org.jetbrains.kotlin.test.TestMetadata; @@ -180,6 +179,18 @@ public class AddImportTestGenerated extends AbstractAddImportTest { doTest(fileName); } + @TestMetadata("NoConflictingNameForInaccessibleClass1.kt") + public void testNoConflictingNameForInaccessibleClass1() throws Exception { + String fileName = JetTestUtils.navigationMetadata("idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt"); + doTest(fileName); + } + + @TestMetadata("NoConflictingNameForInaccessibleClass2.kt") + public void testNoConflictingNameForInaccessibleClass2() throws Exception { + String fileName = JetTestUtils.navigationMetadata("idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt"); + doTest(fileName); + } + @TestMetadata("NoNeedToImportStandardClass.kt") public void testNoNeedToImportStandardClass() throws Exception { String fileName = JetTestUtils.navigationMetadata("idea/testData/addImport/NoNeedToImportStandardClass.kt");