Import insertion helper is not confused by inaccessible classes + check visibility of imported declarations through PackageViewDescriptor instead of PackageFragmentDescriptor

compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/LazyFileScope.kt

@@ -82,20 +82,20 @@ class LazyFileScope private(
 
             val scopeChain = ArrayList<JetScope>()
 
-            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)
         }

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
         }
     }
 

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<ClassifierDescriptor> = 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)

idea/testData/addImport/NoConflictingNameForInaccessibleClass1.dependency.kt

@@ -0,0 +1,3 @@
+package dependency;
+
+private class Date {}

idea/testData/addImport/NoConflictingNameForInaccessibleClass1.kt

@@ -0,0 +1,6 @@
+// IMPORT: java.util.ArrayList
+package p
+
+import dependency.*
+
+val d = Date(1, 2)

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)

idea/testData/addImport/NoConflictingNameForInaccessibleClass2.dependency.kt

@@ -0,0 +1,5 @@
+package dependency;
+
+private class Date {}
+
+class X

idea/testData/addImport/NoConflictingNameForInaccessibleClass2.kt

@@ -0,0 +1,6 @@
+// IMPORT: dependency.X
+package p
+
+import java.util.*
+
+val d = Date(1, 2)

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)

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");