Merge pull request #1903 from 333fred/sort-orders

Ensure unimported things are sorted after imported things
OmniSharp · Aug 20, 2020 · ec1a442 · ec1a442
2 parents 282c44a + 36f4c0f
commit ec1a442
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 8 deletions.
diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs
@@ -157,16 +157,15 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
             // that completion provider is still creating the cache. We'll mark this completion list as not completed, and the
             // editor will ask again when the user types more. By then, hopefully the cache will have populated and we can mark
             // the completion as done.
-            bool isIncomplete = expandedItemsAvailable &&
-                                _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true;
+            bool seenUnimportedCompletions = false;
+            bool expectingImportedItems = expandedItemsAvailable && _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true;
 
             for (int i = 0; i < completions.Items.Length; i++)
             {
                 var completion = completions.Items[i];
-                var commitCharacters = buildCommitCharacters(completions, completion.Rules.CommitCharacterRules, triggerCharactersBuilder);
-
                 var insertTextFormat = InsertTextFormat.PlainText;
                 ImmutableArray<LinePositionSpanTextChange>? additionalTextEdits = null;
+                char sortTextPrepend = '0';
 
                 if (!completion.TryGetInsertionText(out string insertText))
                 {
@@ -254,7 +253,8 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
                             // This is technically slightly incorrect: extension method completion can provide
                             // partial results. However, this should only affect the first completion session or
                             // two and isn't a big problem in practice.
-                            isIncomplete = false;
+                            seenUnimportedCompletions = true;
+                            sortTextPrepend = '1';
                             goto default;
 
                         default:
@@ -263,13 +263,16 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
                     }
                 }
 
+                var commitCharacters = buildCommitCharacters(completions, completion.Rules.CommitCharacterRules, triggerCharactersBuilder);
+
                 completionsBuilder.Add(new CompletionItem
                 {
                     Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix,
                     InsertText = insertText,
                     InsertTextFormat = insertTextFormat,
                     AdditionalTextEdits = additionalTextEdits,
-                    SortText = completion.SortText,
+                    // Ensure that unimported items are sorted after things already imported.
+                    SortText = expectingImportedItems ? sortTextPrepend + completion.SortText : completion.SortText,
                     FilterText = completion.FilterText,
                     Kind = getCompletionItemKind(completion.Tags),
                     Detail = completion.InlineDescription,
@@ -281,7 +284,7 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
 
             return new CompletionResponse
             {
-                IsIncomplete = isIncomplete,
+                IsIncomplete = !seenUnimportedCompletions && expectingImportedItems,
                 Items = completionsBuilder.ToImmutableArray()
             };
 

diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -178,7 +179,12 @@ public Class1()
 
             using var host = GetImportCompletionHost();
             var completions = await FindCompletionsWithImportedAsync(filename, input, host);
-            Assert.True(completions.Items.First(c => c.InsertText == "guid").Data < completions.Items.First(c => c.InsertText == "Guid").Data);
+            CompletionItem localCompletion = completions.Items.First(c => c.InsertText == "guid");
+            CompletionItem typeCompletion = completions.Items.First(c => c.InsertText == "Guid");
+            Assert.True(localCompletion.Data < typeCompletion.Data);
+            Assert.StartsWith("0", localCompletion.SortText);
+            Assert.StartsWith("1", typeCompletion.SortText);
+            VerifySortOrders(completions.Items);
         }
 
         [Theory]
@@ -210,6 +216,7 @@ public static void Test(this object o)
             using var host = GetImportCompletionHost();
             var completions = await FindCompletionsWithImportedAsync(filename, input, host);
             Assert.Contains("Test", completions.Items.Select(c => c.InsertText));
+            VerifySortOrders(completions.Items);
         }
 
         [Theory]
@@ -250,6 +257,7 @@ public static void Test(this object o)
             Assert.Equal(0, additionalEdit.StartColumn);
             Assert.Equal(6, additionalEdit.EndLine);
             Assert.Equal(13, additionalEdit.EndColumn);
+            VerifySortOrders(completions.Items);
         }
 
         [Theory]
@@ -290,6 +298,7 @@ public static void Test(this object o)
             Assert.Equal(0, additionalEdit.StartColumn);
             Assert.Equal(6, additionalEdit.EndLine);
             Assert.Equal(19, additionalEdit.EndColumn);
+            VerifySortOrders(completions.Items);
         }
 
         [Theory]
@@ -336,6 +345,7 @@ public class C3
             Assert.Equal(6, additionalEdit.StartColumn);
             Assert.Equal(8, additionalEdit.EndLine);
             Assert.Equal(11, additionalEdit.EndColumn);
+            VerifySortOrders(completions.Items);
         }
 
         [Theory]
@@ -1278,6 +1288,14 @@ private OmniSharpTestHost GetImportCompletionHost()
 
         private static string NormalizeNewlines(string str)
             => str.Replace("\r\n", Environment.NewLine);
+
+        private static void VerifySortOrders(ImmutableArray<CompletionItem> items)
+        {
+            Assert.All(items, c =>
+            {
+                Assert.True(c.SortText.StartsWith("0") || c.SortText.StartsWith("1"));
+            });
+        }
     }
 
     internal static class CompletionResponseExtensions