De-duplicate completions (#602)

Mikhail Arkhipov · web-flow · commit 75e14ab158b5 · 2019-02-14T11:46:28.000-08:00
* Part 7 * Buildable * PR feedback * Merge conflict * Fix #446 * Fix #446 * Part 8 * Part 9 * Buildable * Part 10 * Part 11 * Part 12 * Buildable * Part 14 * First passing test * Simplify configuration * Style * Fix test and move code to folders * Builtins import * Fix #470 * Fluents * Add search path * Import analysis, part I * Simplify builtins handling * Remove IMember * Handle import specific * More tests * Add typeshed * Renames * Make sure lazy modules are loaded * Renames * Move/rename * Rework importing * Derivation rework * Part 2 * Part 3 * Buildable * Module members * Async walk * Imports test pass * Remove lazy types * Fix from import * Stubs * Double overloads * Fix datetime test * Couple more tests + fluents * Few more tests * Additionl test + union type * Built-in scrape tests * Full stdlib scrape test * Complete async AST walker * Conditional defines test + variable loc cleanup * More stub tests Fix stub loading for packages (port from DDG) Split walker into multiple files * Add some (broken mostly) tests from DDG * Move document tests * Function arg eval, part I * Instance/factory * Builds * Test fixes * Fix static and instance call eval * More tests * More ported tests * Specialize builtin functions * Make walkers common and handle nested functions * Moar tests * Parser fixes + more tests * Handle negative numbers * Fix null ref * Basic list support * Few more list tests * Basic iterators * Support __iter__ * Iterators * Fix couple of tests * Add decorator test * Generics, part I * Generics, part 2 * Generics, part 3 * Basic TypeVar test * Typings, part 4 * Fix test * Generics, part 6 * Generics, part 7 * More tests (failing) * Forward ref fixes * Reorg * Improve symbol resolution + test fixes * Test fixes * Dictionary, part I * Part 11 * Fix test * Tests * Tests * More dict work * List ctor * Skip some tests for now * Fix iterators * Tuple slicing * Polish type comparo in return types * Add Mapping and tests * Add Iterable * Fix typo * Add Iterator[T] + test * Simplify typing types * Class reduction * Fix tests * Test fix * Handle 'with' statement * Handle try-except * Class method inheritance + NewType * Container types * Containers test * Tests * Handle generic type alias * Named tuple * Global/non-local * Handle tuples in for Handle custom iterators * Basic generator * Any/AnyStr * Test fixes * Type/Optional/etc handling * Proper doc population * Tests + range * Argument match * Basic argset and diagnostics * Argset tests * Exclude WIP * Exclude WIP * Arg eval * Arg match, part 2 * Tests and generic arg comparisons * Function eval with arguments * Baselines * Fix test * Undo AST formatting change and update baseline * LS cleanup 1 * Fix list ctor argument unpacking * Cleanup 2 * Builds * Partial completions * Partial * Partial * Simple test * Tests * Basic startup * Clean up a bit * Remove debug code * Port formatter tests * Fix tokenizer crash * Async fixes * Hover * Basic hover * Adjust expression options * Basic signature help * Fix class/instance * Update test * Fix builtin creation exception * Fix tests * Actually provide declared module * Completion test (partial) * Undo * Fix null await Fix override completions + test * Exports filtering Prevent augmenting imported types * Filter variables & exports * Ported tests * Test fixes * More ported tests * Fix exception completions * Import completions * Scope completions * With completions * Test fixes * WIP * Test fix * Better arg match * Temp disable WIP * First cut * Fix type leak * WIP * Remove ConfigureAwait and handle canceled and failed in the analysis notifications * WIP * Generic class base * Generic forward reference resolution * Suppress completion in strings + test * Prevent recursion on generic resolution Better match arguments * Handle call expression in generics * Relax condition as it happens in tensorflow * Fix typeshed version search Make writing cached modules async Fix module doc fetching * Hover tests * Fix prom import hover * Hover tests * Synchronize test cache writing * First cut * Test * Fixes * Add tests for os.path Null ref fix * Fix cache check * Improve resolution of builtins and typing in stubs * Merge tests * Add ntst for requests * Handle typeshed better * Fix custom stub handling * Better sync * Move files * Fix parameter locations * Hover improvement * PEP hints * One more test for PEP hints * Better handle hover over import as * Text based generic constraints * Handle with better with generic stubs * Undo debug * Handle non-binary open() Temporary fix 'with' handler since we haven't specialized IO/TextIO/BinaryIO yet. * Output syntax errors * Properly clear * - Fix async issue with analysis completion - Clean up diagnostics service interface - Use real DS in tests * Use proper scope when analyzing module * Severity mapping and reporting * Add publishing test Add NSubstitute Move all services to the same namespace. * Unused var * Test forced publish on close * Fix typo * Update test framework * Import location * Remove incorrect reference * Diagnostic severity mapping test Fix post-mortem earlier PR comment * Minor fixes * Move interface to the main class part * Flicker reduction * De-duplicate completions * Fix os.path in typeshed
diff --git a/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs b/src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs
@@ -42,12 +42,8 @@ public async Task<bool> HandleFromImportAsync(FromImportStatement node, Cancella
 
             var imports = ModuleResolution.CurrentPathResolver.FindImports(Module.FilePath, node);
             switch (imports) {
-                case ModuleImport moduleImport when moduleImport.FullName == Module.Name && Module.ModuleType == ModuleType.Stub:
-                    // If we are processing stub, ignore imports of the original module.
-                    // For example, typeshed stub for 'sys' imports sys.
-                    break;
                 case ModuleImport moduleImport when moduleImport.FullName == Module.Name:
-                    ImportMembersFromSelf(node);
+                    await ImportMembersFromSelfAsync(node, cancellationToken);
                     break;
                 case ModuleImport moduleImport:
                     await ImportMembersFromModuleAsync(node, moduleImport.FullName, cancellationToken);
@@ -65,7 +61,7 @@ public async Task<bool> HandleFromImportAsync(FromImportStatement node, Cancella
             return false;
         }
 
-        private void ImportMembersFromSelf(FromImportStatement node) {
+        private async Task ImportMembersFromSelfAsync(FromImportStatement node, CancellationToken cancellationToken = default) {
             var names = node.Names;
             var asNames = node.AsNames;
 
@@ -84,6 +80,14 @@ private void ImportMembersFromSelf(FromImportStatement node) {
                 var memberName = memberReference.Name;
 
                 var member = Module.GetMember(importName);
+                if (member == null && Eval.Module == Module) {
+                    // We are still evaluating this module so members are not complete yet.
+                    // Consider 'from . import path as path' in os.pyi in typeshed.
+                    var import = ModuleResolution.CurrentPathResolver.GetModuleImportFromModuleName($"{Module.Name}.{importName}");
+                    if (!string.IsNullOrEmpty(import?.FullName)) {
+                        member = await ModuleResolution.ImportModuleAsync(import.FullName, cancellationToken);
+                    }
+                }
                 Eval.DeclareVariable(memberName, member ?? Eval.UnknownType, VariableSource.Declaration, Eval.GetLoc(names[i]));
             }
         }
diff --git a/src/Analysis/Core/Impl/DependencyResolution/PathResolverSnapshot.cs b/src/Analysis/Core/Impl/DependencyResolution/PathResolverSnapshot.cs
@@ -71,12 +71,14 @@ private PathResolverSnapshot(PythonLanguageVersion pythonLanguageVersion, string
         public IEnumerable<string> GetAllModuleNames() => GetModuleNames(_roots.Prepend(_nonRooted).Append(_builtins));
         public IEnumerable<string> GetInterpreterModuleNames() => GetModuleNames(_roots.Skip(_userRootsCount).Append(_builtins));
 
-        private static IEnumerable<string> GetModuleNames(IEnumerable<Node> roots) => roots
-            .SelectMany(r => r.TraverseBreadthFirst(n => n.IsModule ? Enumerable.Empty<Node>() : n.Children))
-            .Where(n => n.IsModule)
+        private IEnumerable<string> GetModuleNames(IEnumerable<Node> roots) {
+            var builtins =  new HashSet<Node>(_builtins.Children);
+            return roots.SelectMany(r => r.TraverseBreadthFirst(n => n.IsModule? Enumerable.Empty<Node>() : n.Children))
+            .Where(n => n.IsModule || builtins.Contains(n))
             .Select(n => n.FullModuleName);
+        }
 
-        public ModuleImport GetModuleImportFromModuleName(in string fullModuleName) {
+    public ModuleImport GetModuleImportFromModuleName(in string fullModuleName) {
             foreach (var root in _roots) {
                 var node = root;
                 var matched = true;
diff --git a/src/LanguageServer/Impl/Completion/CompletionSource.cs b/src/LanguageServer/Impl/Completion/CompletionSource.cs
@@ -38,14 +38,11 @@ public async Task<CompletionResult> GetCompletionsAsync(IDocumentAnalysis analys
             switch (expression) {
                 case MemberExpression me when me.Target != null && me.DotIndex > me.StartIndex && context.Position > me.DotIndex:
                     return new CompletionResult(await ExpressionCompletion.GetCompletionsFromMembersAsync(me.Target, scope, context, cancellationToken));
-                case ConstantExpression ce when ce.Value is double || ce.Value is float:
+                case ConstantExpression ce1 when ce1.Value is double || ce1.Value is float:
                     // no completions on integer ., the user is typing a float
-                    return CompletionResult.Empty;
-                case ConstantExpression ce when ce.Value is string:
+                case ConstantExpression ce2 when ce2.Value is string: 
                     // no completions in strings
-                    return CompletionResult.Empty;
                 case null when context.Ast.IsInsideComment(context.Location):
-                    return CompletionResult.Empty;
                 case null when context.Ast.IsInsideString(context.Location):
                     return CompletionResult.Empty;
             }
diff --git a/src/LanguageServer/Impl/Completion/ImportCompletion.cs b/src/LanguageServer/Impl/Completion/ImportCompletion.cs
@@ -116,16 +116,8 @@ public static CompletionResult GetCompletionsInFromImport(FromImportStatement fr
         }
 
         private static IEnumerable<CompletionItem> GetImportsFromModuleName(IEnumerable<NameExpression> nameExpressions, CompletionContext context) {
-            IReadOnlyList<CompletionItem> items;
             var names = nameExpressions.TakeWhile(n => n.StartIndex <= context.Position).Select(n => n.Name).ToArray();
-            if (names.Length <= 1) {
-                var mres = context.Analysis.Document.Interpreter.ModuleResolution;
-                var importable = mres.CurrentPathResolver.GetAllModuleNames();
-                items = importable.Select(m => CompletionItemSource.CreateCompletionItem(m, CompletionItemKind.Module)).ToArray();
-            } else {
-                items = GetChildModules(names, context);
-            }
-            return items;
+            return names.Length <= 1 ? GetAllImportableModules(context) : GetChildModules(names, context);
         }
 
         private static IEnumerable<CompletionItem> GetModuleMembers(IEnumerable<NameExpression> nameExpressions, CompletionContext context) {
@@ -140,7 +132,7 @@ private static IEnumerable<CompletionItem> GetModuleMembers(IEnumerable<NameExpr
 
         private static IEnumerable<CompletionItem> GetAllImportableModules(CompletionContext context) {
             var mres = context.Analysis.Document.Interpreter.ModuleResolution;
-            var modules = mres.CurrentPathResolver.GetAllModuleNames();
+            var modules = mres.CurrentPathResolver.GetAllModuleNames().Distinct();
             return modules.Select(n => CompletionItemSource.CreateCompletionItem(n, CompletionItemKind.Module));
         }
 
diff --git a/src/LanguageServer/Impl/Implementation/Server.Editor.cs b/src/LanguageServer/Impl/Implementation/Server.Editor.cs
@@ -44,11 +44,6 @@ public async Task<CompletionList> Completion(CompletionParams @params, Cancellat
             return res;
         }
 
-        public Task<CompletionItem> CompletionItemResolve(CompletionItem item, CancellationToken token) {
-            // TODO: Fill out missing values in item
-            return Task.FromResult(item);
-        }
-
         public async Task<Hover> Hover(TextDocumentPositionParams @params, CancellationToken cancellationToken) {
             var uri = @params.textDocument.uri;
             _log?.Log(TraceEventType.Verbose, $"Hover in {uri} at {@params.position}");
diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs
@@ -238,10 +238,6 @@ public async Task<CompletionList> Completion(JToken token, CancellationToken can
             return await _server.Completion(ToObject<CompletionParams>(token), GetToken(cancellationToken));
         }
 
-        [JsonRpcMethod("completionItem/resolve")]
-        public Task<CompletionItem> CompletionItemResolve(JToken token, CancellationToken cancellationToken)
-           => _server.CompletionItemResolve(ToObject<CompletionItem>(token), GetToken(cancellationToken));
-
         [JsonRpcMethod("textDocument/hover")]
         public async Task<Hover> Hover(JToken token, CancellationToken cancellationToken) {
             await _prioritizer.DefaultPriorityAsync(cancellationToken);
diff --git a/src/LanguageServer/Test/CompletionTests.cs b/src/LanguageServer/Test/CompletionTests.cs
@@ -17,7 +17,9 @@
 using System.Linq;
 using System.Threading.Tasks;
 using FluentAssertions;
+using FluentAssertions.Common;
 using Microsoft.Python.Analysis.Types;
+using Microsoft.Python.Core;
 using Microsoft.Python.Core.Text;
 using Microsoft.Python.LanguageServer.Completion;
 using Microsoft.Python.LanguageServer.Protocol;
@@ -791,7 +793,7 @@ import os
             result.Should().HaveLabels("path", @"devnull", "SEEK_SET", @"curdir");
         }
 
-        [DataRow(false), Ignore("https://github.com/Microsoft/python-language-server/issues/574")]
+        [DataRow(false)]
         [DataRow(true)]
         [DataTestMethod, Priority(0)]
         public async Task OsPathMembers(bool is3x) {
@@ -805,5 +807,16 @@ import os
             var result = await cs.GetCompletionsAsync(analysis, new SourceLocation(3, 9));
             result.Should().HaveLabels("split", @"getsize", @"islink", @"abspath");
         }
+
+        [TestMethod, Priority(0)]
+        public async Task NoDuplicateMembers() {
+            const string code = @"import sy";
+            var analysis = await GetAnalysisAsync(code);
+            var cs = new CompletionSource(new PlainTextDocumentationSource(), ServerSettings.completion);
+
+            var result = await cs.GetCompletionsAsync(analysis, new SourceLocation(1, 10));
+            result.Completions.Count(c => c.label.EqualsOrdinal(@"sys")).Should().Be(1);
+            result.Completions.Count(c => c.label.EqualsOrdinal(@"sysconfig")).Should().Be(1);
+        }
     }
 }
