Completion presentation for provate variables (#1158)

Author: Mikhail Arkhipov

src/Analysis/Ast/Impl/Extensions/PythonClassExtensions.cs

@@ -17,22 +17,47 @@
 using Microsoft.Python.Analysis.Analyzer;
 using Microsoft.Python.Analysis.Specializations.Typing;
 using Microsoft.Python.Analysis.Types;
+using Microsoft.Python.Core;
 
 namespace Microsoft.Python.Analysis {
     public static class PythonClassExtensions {
-        public static bool IsGeneric(this IPythonClassType cls) 
+        public static bool IsGeneric(this IPythonClassType cls)
             => cls.Bases != null && cls.Bases.Any(b => b is IGenericType || b is IGenericClassParameter);
 
         public static void AddMemberReference(this IPythonType type, string name, IExpressionEvaluator eval, Location location) {
             var m = type.GetMember(name);
             if (m is LocatedMember lm) {
                 lm.AddReference(location);
-            } else if(type is IPythonClassType cls) {
+            } else if (type is IPythonClassType cls) {
                 using (eval.OpenScope(cls)) {
                     eval.LookupNameInScopes(name, out _, out var v, LookupOptions.Local);
                     v?.AddReference(location);
                 }
             }
         }
+
+        /// <summary>
+        /// Converts mangled name of a private member to its original form,
+        /// by removing private prefix, such as '_ClassName__x' to '__x'.
+        /// </summary>
+        public static string UnmangleMemberName(this IPythonClassType cls, string memberName) {
+            if (memberName.Length == 0 || (memberName.Length > 0 && memberName[0] != '_')) {
+                return memberName;
+            }
+
+            var prefix = $"_{cls.Name}__";
+            return memberName.StartsWithOrdinal(prefix)
+                ? memberName.Substring(cls.Name.Length + 1)
+                : memberName;
+        }
+
+        /// <summary>
+        /// Determines if particular member is private by checking its name to mangled form
+        /// of Python class private members, such as '_ClassName__memberName'.
+        /// </summary>
+        public static bool IsPrivateMember(this IPythonClassType cls, string memberName) {
+            var unmangledName = cls.UnmangleMemberName(memberName);
+            return unmangledName.StartsWithOrdinal("__") && memberName.EqualsOrdinal($"_{cls.Name}{unmangledName}");
+        }
     }
 }

src/LanguageServer/Impl/Completion/CompletionSource.cs

@@ -16,6 +16,7 @@
 using System.Linq;
 using Microsoft.Python.Analysis;
 using Microsoft.Python.Analysis.Analyzer.Expressions;
+using Microsoft.Python.Core;
 using Microsoft.Python.Core.Text;
 using Microsoft.Python.Parsing;
 using Microsoft.Python.Parsing.Ast;
@@ -88,7 +89,7 @@ public CompletionResult GetCompletions(IDocumentAnalysis analysis, SourceLocatio
                             return CompletionResult.Empty;
                         }
                         return result == CompletionResult.Empty ? TopLevelCompletion.GetCompletions(statement, scope, context) : result;
-                }
+                    }
             }
         }
     }

src/LanguageServer/Impl/Completion/ExpressionCompletion.cs

@@ -16,12 +16,15 @@
 using System.Collections.Generic;
 using System.Linq;
 using Microsoft.Python.Analysis;
+using Microsoft.Python.Analysis.Analyzer;
+using Microsoft.Python.Analysis.Types;
 using Microsoft.Python.Analysis.Values;
+using Microsoft.Python.Core;
 using Microsoft.Python.LanguageServer.Protocol;
 using Microsoft.Python.Parsing.Ast;
 
 namespace Microsoft.Python.LanguageServer.Completion {
-    internal static  class ExpressionCompletion {
+    internal static class ExpressionCompletion {
         public static IEnumerable<CompletionItem> GetCompletionsFromMembers(Expression e, IScope scope, CompletionContext context) {
             using (context.Analysis.ExpressionEvaluator.OpenScope(scope)) {
                 return GetItemsFromExpression(e, context);
@@ -35,14 +38,19 @@ public static IEnumerable<CompletionItem> GetCompletionsFromMembers(Expression e
         }
 
         private static IEnumerable<CompletionItem> GetItemsFromExpression(Expression e, CompletionContext context) {
-            var value = context.Analysis.ExpressionEvaluator.GetValueFromExpression(e);
+            var eval = context.Analysis.ExpressionEvaluator;
+            var value = eval.GetValueFromExpression(e);
             if (!value.IsUnknown()) {
-                var items = new List<CompletionItem>();
+
                 var type = value.GetPythonType();
-                var names = type.GetMemberNames().ToArray();
-                foreach (var t in names) {
+                if(type is IPythonClassType cls) {
+                    return GetClassItems(cls, e, context);
+                }
+
+                var items = new List<CompletionItem>();
+                foreach (var t in type.GetMemberNames().ToArray()) {
                     var m = type.GetMember(t);
-                    if(m is IVariable v && v.Source != VariableSource.Declaration) {
+                    if (m is IVariable v && v.Source != VariableSource.Declaration) {
                         continue;
                     }
                     items.Add(context.ItemSource.CreateCompletionItem(t, m, type));
@@ -51,5 +59,36 @@ private static IEnumerable<CompletionItem> GetItemsFromExpression(Expression e,
             }
             return Enumerable.Empty<CompletionItem>();
         }
+
+        private static IEnumerable<CompletionItem> GetClassItems(IPythonClassType cls, Expression e, CompletionContext context) {
+            var eval = context.Analysis.ExpressionEvaluator;
+            // See if we are completing on self. Note that we may be inside inner function
+            // that does not necessarily have 'self' argument so we are looking beyond local
+            // scope. We then check that variable type matches the class type, if any.
+            var selfVariable = eval.LookupNameInScopes("self");
+            var completingOnSelf = cls.Equals(selfVariable?.GetPythonType()) && e is NameExpression nex && nex.Name == "self";
+
+            var items = new List<CompletionItem>();
+            var names = cls.GetMemberNames().ToArray();
+
+            foreach (var t in names) {
+                var m = cls.GetMember(t);
+                if (m is IVariable v && v.Source != VariableSource.Declaration) {
+                    continue;
+                }
+                // If this is class member completion, unmangle private member names.
+                var unmangledName = cls.UnmangleMemberName(t);
+                if (!string.IsNullOrEmpty(unmangledName)) {
+                    // Hide private variables outside of the class scope.
+                    if (!completingOnSelf && cls.IsPrivateMember(t)) {
+                        continue;
+                    }
+                    items.Add(context.ItemSource.CreateCompletionItem(unmangledName, m, cls));
+                } else {
+                    items.Add(context.ItemSource.CreateCompletionItem(t, m, cls));
+                }
+            }
+            return items;
+        }
     }
 }

src/LanguageServer/Test/CompletionTests.cs

@@ -1150,5 +1150,27 @@ def func():
             var result = cs.GetCompletions(analysis, new SourceLocation(4, 2));
             result.Completions.Select(c => c.label).Should().NotContain("aaa");
         }
+
+        [TestMethod, Priority(0)]
+        public async Task PrivateMembers() {
+            const string code = @"
+class A:
+    def __init__(self):
+        self.__x = 123
+
+    def func(self):
+        self.
+
+A().
+";
+            var analysis = await GetAnalysisAsync(code);
+            var cs = new CompletionSource(new PlainTextDocumentationSource(), ServerSettings.completion);
+
+            var result = cs.GetCompletions(analysis, new SourceLocation(7, 14));
+            result.Completions.Select(c => c.label).Should().Contain("__x").And.NotContain("_A__x");
+
+            result = cs.GetCompletions(analysis, new SourceLocation(9, 5));
+            result.Completions.Select(c => c.label).Should().NotContain("_A__x").And.NotContain("__x");
+        }
     }
 }