- KT-1756 Filter invisible members from the first completion list

- Analyze is reference variant is suitable for completion before creating LookupElement
- Show visibility icon in completion list for kotlin declarations

 #KT-1756 fixed

idea/src/org/jetbrains/jet/plugin/JetDescriptorIconProvider.java

@@ -17,6 +17,8 @@
 package org.jetbrains.jet.plugin;
 
 import com.intellij.openapi.diagnostic.Logger;
+import com.intellij.openapi.util.Iconable;
+import com.intellij.ui.RowIcon;
 import com.intellij.util.PlatformIcons;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.jet.lang.descriptors.*;
@@ -33,7 +35,43 @@ public final class JetDescriptorIconProvider {
     private JetDescriptorIconProvider() {
     }
 
-    public static Icon getIcon(@NotNull DeclarationDescriptor descriptor) {
+    public static Icon getIcon(@NotNull DeclarationDescriptor descriptor, @Iconable.IconFlags int flags) {
+        Icon result = getBaseIcon(descriptor);
+        if ((flags & Iconable.ICON_FLAG_VISIBILITY) > 0) {
+            RowIcon rowIcon = new RowIcon(2);
+            rowIcon.setIcon(result, 0);
+            rowIcon.setIcon(getVisibilityIcon(descriptor), 1);
+            result = rowIcon;
+        }
+
+        return result;
+    }
+
+    public static Icon getVisibilityIcon(@NotNull DeclarationDescriptor descriptor) {
+        if (descriptor instanceof DeclarationDescriptorWithVisibility) {
+            DeclarationDescriptorWithVisibility descriptorWithVisibility = (DeclarationDescriptorWithVisibility)descriptor;
+            Visibility visibility = descriptorWithVisibility.getVisibility();
+            if (visibility == Visibilities.PUBLIC) {
+                return PlatformIcons.PUBLIC_ICON;
+            }
+
+            if (visibility == Visibilities.PROTECTED || visibility == Visibilities.INTERNAL_PROTECTED) {
+                return PlatformIcons.PROTECTED_ICON;
+            }
+
+            if (visibility == Visibilities.PRIVATE) {
+                return PlatformIcons.PRIVATE_ICON;
+            }
+
+            if (visibility == Visibilities.INTERNAL) {
+                return PlatformIcons.PACKAGE_LOCAL_ICON;
+            }
+        }
+
+        return null;
+    }
+
+    public static Icon getBaseIcon(@NotNull DeclarationDescriptor descriptor) {
         if (descriptor instanceof NamespaceDescriptor) {
             return PlatformIcons.PACKAGE_OPEN_ICON;
         }

idea/src/org/jetbrains/jet/plugin/completion/DescriptorLookupConverter.java

@@ -104,13 +104,7 @@ public final class DescriptorLookupConverter {
         }
 
         element = element.setTailText(tailText, tailTextGrayed).setTypeText(typeText);
-
+        element = element.setIcon(JetDescriptorIconProvider.getIcon(descriptor, Iconable.ICON_FLAG_VISIBILITY));
-        if (declaration != null) {
-            element = element.setIcon(declaration.getIcon(Iconable.ICON_FLAG_OPEN | Iconable.ICON_FLAG_VISIBILITY));
-        }
-        else {
-            element = element.setIcon(JetDescriptorIconProvider.getIcon(descriptor));
-        }
 
         return element;
     }

idea/src/org/jetbrains/jet/plugin/completion/JetCompletionContributor.java

@@ -16,9 +16,10 @@
 
 package org.jetbrains.jet.plugin.completion;
 
+import com.google.common.base.Predicate;
+import com.google.common.collect.Collections2;
 import com.intellij.codeInsight.completion.*;
 import com.intellij.codeInsight.lookup.LookupElement;
-import com.intellij.codeInsight.lookup.LookupElementBuilder;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.Condition;
 import com.intellij.patterns.PlatformPatterns;
@@ -30,9 +31,11 @@ import com.intellij.util.Consumer;
 import com.intellij.util.ProcessingContext;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.jet.compiler.TipsManager;
 import org.jetbrains.jet.lang.descriptors.*;
 import org.jetbrains.jet.lang.psi.*;
 import org.jetbrains.jet.lang.resolve.BindingContext;
+import org.jetbrains.jet.lang.resolve.scopes.JetScope;
 import org.jetbrains.jet.lexer.JetTokens;
 import org.jetbrains.jet.plugin.caches.JetCacheManager;
 import org.jetbrains.jet.plugin.caches.JetShortNamesCache;
@@ -49,6 +52,9 @@ public class JetCompletionContributor extends CompletionContributor {
     private static class CompletionSession {
         public boolean isSomethingAdded = false;
         public int customInvocationCount = 0;
+
+        @Nullable
+        public DeclarationDescriptor inDescriptor = null;
     }
 
     public JetCompletionContributor() {
@@ -56,7 +62,7 @@ public class JetCompletionContributor extends CompletionContributor {
                new CompletionProvider<CompletionParameters>() {
                    @Override
                    protected void addCompletions(@NotNull CompletionParameters parameters, ProcessingContext context,
-                                                 @NotNull CompletionResultSet result) {
+                           @NotNull CompletionResultSet result) {
                        result.restartCompletionWhenNothingMatches();
 
                        CompletionSession session = new CompletionSession();
@@ -69,6 +75,14 @@ public class JetCompletionContributor extends CompletionContributor {
 
                        JetSimpleNameReference jetReference = getJetReference(parameters);
                        if (jetReference != null) {
+
+                           BindingContext jetContext =
+                                   WholeProjectAnalyzerFacade.analyzeProjectWithCacheOnAFile((JetFile)position.getContainingFile())
+                                           .getBindingContext();
+
+                           JetScope scope = jetContext.get(BindingContext.RESOLUTION_SCOPE, jetReference.getExpression());
+                           session.inDescriptor = scope != null ? scope.getContainingDeclaration() : null;
+
                            completeForReference(parameters, result, position, jetReference, session);
 
                            if (!session.isSomethingAdded && session.customInvocationCount == 0) {
@@ -85,11 +99,11 @@ public class JetCompletionContributor extends CompletionContributor {
     }
 
     private static void completeForReference(
-        @NotNull CompletionParameters parameters,
+            @NotNull CompletionParameters parameters,
-        @NotNull CompletionResultSet result,
+            @NotNull CompletionResultSet result,
-        @NotNull PsiElement position,
+            @NotNull PsiElement position,
-        @NotNull JetSimpleNameReference jetReference,
+            @NotNull JetSimpleNameReference jetReference,
-        @NotNull CompletionSession session
+            @NotNull CompletionSession session
     ) {
         if (isOnlyKeywordCompletion(position)) {
             return;
@@ -100,9 +114,9 @@ public class JetCompletionContributor extends CompletionContributor {
                 addClasses(parameters, result, session);
             }
             else {
-                for (Object variant : jetReference.getVariants()) {
+                for (LookupElement variant : getReferenceVariants(jetReference, result, session)) {
                     if (isTypeDeclaration(variant)) {
-                        addReferenceVariant(result, variant, session);
+                        addCheckedCompletionToResult(result, variant, session);
                     }
                 }
             }
@@ -110,8 +124,8 @@ public class JetCompletionContributor extends CompletionContributor {
             return;
         }
 
-        for (Object variant : jetReference.getVariants()) {
+        for (LookupElement variant : getReferenceVariants(jetReference, result, session)) {
-            addReferenceVariant(result, variant, session);
+            addCheckedCompletionToResult(result, variant, session);
         }
 
         String prefix = result.getPrefixMatcher().getPrefix();
@@ -154,7 +168,7 @@ public class JetCompletionContributor extends CompletionContributor {
         Collection<DeclarationDescriptor> jetCallableExtensions = namesCache.getJetCallableExtensions(
                 matchPrefixCondition, expression, GlobalSearchScope.allScope(position.getProject()));
 
-        BindingContext context = WholeProjectAnalyzerFacade.analyzeProjectWithCacheOnAFile((JetFile) position.getContainingFile())
+        BindingContext context = WholeProjectAnalyzerFacade.analyzeProjectWithCacheOnAFile((JetFile)position.getContainingFile())
                 .getBindingContext();
 
         for (DeclarationDescriptor jetCallableExtension : jetCallableExtensions) {
@@ -162,24 +176,11 @@ public class JetCompletionContributor extends CompletionContributor {
         }
     }
 
-    private static void addReferenceVariant(
-            @NotNull CompletionResultSet result,
-            @NotNull Object variant,
-            @NotNull CompletionSession session) {
-
-        if (variant instanceof LookupElement) {
-            addCompletionToResult(result, (LookupElement) variant, session);
-        }
-        else {
-            addCompletionToResult(result, LookupElementBuilder.create(variant.toString()), session);
-        }
-    }
-
     public static boolean isTypeDeclaration(@NotNull Object variant) {
         if (variant instanceof LookupElement) {
             Object object = ((LookupElement)variant).getObject();
             if (object instanceof JetLookupObject) {
-                DeclarationDescriptor descriptor = ((JetLookupObject) object).getDescriptor();
+                DeclarationDescriptor descriptor = ((JetLookupObject)object).getDescriptor();
                 return (descriptor instanceof ClassDescriptor) ||
                        (descriptor instanceof NamespaceDescriptor) ||
                        (descriptor instanceof TypeParameterDescriptor);
@@ -189,8 +190,10 @@ public class JetCompletionContributor extends CompletionContributor {
         return false;
     }
 
-    private static void addJetTopLevelFunctions(JetSimpleNameExpression expression, @NotNull CompletionResultSet result, @NotNull PsiElement position,
+    private static void addJetTopLevelFunctions(JetSimpleNameExpression expression,
-                                                @NotNull CompletionSession session) {
+            @NotNull CompletionResultSet result,
+            @NotNull PsiElement position,
+            @NotNull CompletionSession session) {
 
         String actualPrefix = result.getPrefixMatcher().getPrefix();
 
@@ -200,7 +203,8 @@ public class JetCompletionContributor extends CompletionContributor {
         GlobalSearchScope scope = GlobalSearchScope.allScope(project);
         Collection<String> functionNames = namesCache.getAllTopLevelFunctionNames();
 
-        BindingContext resolutionContext = WholeProjectAnalyzerFacade.analyzeProjectWithCacheOnAFile((JetFile)position.getContainingFile()).getBindingContext();
+        BindingContext resolutionContext =
+                WholeProjectAnalyzerFacade.analyzeProjectWithCacheOnAFile((JetFile)position.getContainingFile()).getBindingContext();
 
         for (String name : functionNames) {
             if (name.contains(actualPrefix)) {
@@ -247,7 +251,7 @@ public class JetCompletionContributor extends CompletionContributor {
         PsiElement element = parameters.getPosition();
         if (element.getNode().getElementType() == JetTokens.IDENTIFIER) {
             if (element.getParent() instanceof JetSimpleNameExpression) {
-                JetSimpleNameExpression nameExpression = (JetSimpleNameExpression) element.getParent();
+                JetSimpleNameExpression nameExpression = (JetSimpleNameExpression)element.getParent();
 
                 // Top level completion should be executed for simple which is not in qualified expression
                 return (PsiTreeUtil.getParentOfType(nameExpression, JetQualifiedExpression.class) == null);
@@ -276,7 +280,7 @@ public class JetCompletionContributor extends CompletionContributor {
             if (references.length != 0) {
                 for (PsiReference reference : references) {
                     if (reference instanceof JetSimpleNameReference) {
-                        return (JetSimpleNameReference) reference;
+                        return (JetSimpleNameReference)reference;
                     }
                 }
             }
@@ -288,16 +292,72 @@ public class JetCompletionContributor extends CompletionContributor {
     private static void addCompletionToResult(
             @NotNull CompletionResultSet result,
             @NotNull LookupElement element,
-            @NotNull CompletionSession session) {
+            @NotNull CompletionSession session
-
+    ) {
-        if (result.getPrefixMatcher().prefixMatches(element)) {
+        if (!result.getPrefixMatcher().prefixMatches(element) || !isVisibleElement(element, session)) {
-            result.addElement(element);
+            return;
-            session.isSomethingAdded = true;
         }
+
+        addCheckedCompletionToResult(result, element, session);
     }
 
-    @Override
+    private static void addCheckedCompletionToResult(
-    public void beforeCompletion(@NotNull CompletionInitializationContext context) {
+            @NotNull CompletionResultSet result,
-        super.beforeCompletion(context);    //To change body of overridden methods use File | Settings | File Templates.
+            @NotNull LookupElement element,
+            @NotNull CompletionSession session
+    ) {
+        result.addElement(element);
+        session.isSomethingAdded = true;
+    }
+
+    private static boolean isVisibleElement(LookupElement element, CompletionSession session) {
+        if (session.inDescriptor != null) {
+            if (element.getObject() instanceof JetLookupObject) {
+                JetLookupObject jetObject = (JetLookupObject)element.getObject();
+                DeclarationDescriptor descriptor = jetObject.getDescriptor();
+                return isVisibleDescriptor(descriptor, session);
+            }
+        }
+
+        return true;
+    }
+
+    private static boolean isVisibleDescriptor(DeclarationDescriptor descriptor, CompletionSession session) {
+        if (session.customInvocationCount >= 2) {
+            // Show everything if user insist on showing completion list
+            return true;
+        }
+
+        if (descriptor instanceof DeclarationDescriptorWithVisibility) {
+            return Visibilities.isVisible((DeclarationDescriptorWithVisibility)descriptor, session.inDescriptor);
+        }
+
+        return true;
+    }
+
+    @NotNull
+    private static LookupElement[] getReferenceVariants(
+            @NotNull JetSimpleNameReference reference,
+            @NotNull final CompletionResultSet result,
+            @NotNull final CompletionSession session
+    ) {
+        BindingContext bindingContext = WholeProjectAnalyzerFacade.analyzeProjectWithCacheOnAFile(
+                (JetFile)reference.getExpression().getContainingFile())
+                .getBindingContext();
+
+        Collection<DeclarationDescriptor> descriptors = TipsManager.getReferenceVariants(reference.getExpression(), bindingContext);
+
+        Collection<DeclarationDescriptor> checkedDescriptors = Collections2.filter(descriptors, new Predicate<DeclarationDescriptor>() {
+            @Override
+            public boolean apply(@Nullable DeclarationDescriptor descriptor) {
+                if (descriptor == null) {
+                    return false;
+                }
+
+                return result.getPrefixMatcher().prefixMatches(descriptor.getName()) && isVisibleDescriptor(descriptor, session);
+            }
+        });
+
+        return DescriptorLookupConverter.collectLookupElements(bindingContext, checkedDescriptors);
     }
 }

idea/testData/completion/basic/VisibilityClassMembersFromExternal.kt

@@ -0,0 +1,15 @@
+class Some() {
+    public val testPublic = 12
+    protected val testProtected = 12
+    private val testPrivate = 12
+    val testPackage = 12
+}
+
+fun test() {
+    Some().<caret>
+}
+
+// TIME: 1
+// EXIST: testPublic, testPackage
+// ABSENT: testPrivate, testProtected
+

idea/testData/completion/basic/VisibilityClassMembersFromExternalForce.kt

@@ -0,0 +1,13 @@
+class Some() {
+    public val testPublic = 12
+    protected val testProtected = 12
+    private val testPrivate = 12
+    val testPackage = 12
+}
+
+fun test() {
+    Some().<caret>
+}
+
+// TIME: 2
+// EXIST: testPublic, testProtected, testPrivate, testPackage

idea/testData/completion/basic/VisibilityInSubclass.kt

@@ -0,0 +1,16 @@
+class Some() {
+    public val testPublic = 12
+    protected val testProtected = 12
+    private val testPrivate = 12
+    val testPackage = 12
+}
+
+class SomeSubclass : Some() {
+    fun test() {
+        <caret>
+    }
+}
+
+// TIME: 1
+// EXIST: testPublic, testProtected, testPackage
+// ABSENT: testPrivate

idea/testData/completion/basic/VisibilityInSubclassForce.kt

@@ -0,0 +1,18 @@
+class Some() {
+    public val testPublic = 12
+    protected val testProtected = 12
+    private val testPrivate = 12
+    val testPackage = 12
+}
+
+class SomeSubclass : Some() {
+    fun test() {
+        <caret>
+    }
+}
+
+// TIME: 2
+// EXIST: testPublic, testProtected, testPackage
+
+// Should exist after KT-1805 Better diagnostic for access to private field of parent class
+// ABSENT: testPrivate

idea/tests/org/jetbrains/jet/completion/JetBasicCompletionTest.java

@@ -153,6 +153,22 @@ public class JetBasicCompletionTest extends JetCompletionTestBase {
         doTest();
     }
 
+    public void testVisibilityClassMembersFromExternal() {
+        doTest();
+    }
+
+    public void testVisibilityClassMembersFromExternalForce() {
+        doTest();
+    }
+
+    public void testVisibilityInSubclass() {
+        doTest();
+    }
+
+    public void testVisibilityInSubclassForce() {
+        doTest();
+    }
+
     @Override
     protected String getTestDataPath() {
         return new File(PluginTestCaseBase.getTestDataPathBase(), "/completion/basic").getPath() +