Merge pull request #75655 from dibarbet/completion_soft_selection_lsp

Ensure discards are initially soft selected in VSCode

src/EditorFeatures/Core/IntelliSense/AsyncCompletion/ItemManager.CompletionListUpdater.cs

@@ -754,9 +754,7 @@ private static bool TryGetInitialTriggerLocation(AsyncCompletionSessionDataSnaps
             return false;
         }
 
-        private bool IsHardSelection(
-            RoslynCompletionItem item,
-            bool matchedFilterText)
+        private bool IsHardSelection(RoslynCompletionItem item, bool matchedFilterText)
         {
             if (_hasSuggestedItemOptions)
             {
@@ -779,7 +777,7 @@ private bool IsHardSelection(
             // It's possible the user is just typing language punctuation and selecting
             // anything in the list will interfere.  We only allow this if the filter text
             // exactly matches something in the list already. 
-            if (_filterText.Length > 0 && IsAllPunctuation(_filterText) && _filterText != item.DisplayText)
+            if (_filterText.Length > 0 && CompletionService.IsAllPunctuation(_filterText) && _filterText != item.DisplayText)
             {
                 return false;
             }
@@ -819,19 +817,6 @@ private bool IsHardSelection(
             return true;
         }
 
-        private static bool IsAllPunctuation(string filterText)
-        {
-            foreach (var ch in filterText)
-            {
-                if (!char.IsPunctuation(ch))
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
-
         /// <summary>
         /// A potential filter character is something that can filter a completion lists and is
         /// *guaranteed* to not be a commit character.

src/Features/Core/Portable/Completion/CompletionService.cs

@@ -363,6 +363,19 @@ internal static void FilterItems(
         }
     }
 
+    internal static bool IsAllPunctuation(string filterText)
+    {
+        foreach (var ch in filterText)
+        {
+            if (!char.IsPunctuation(ch))
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     /// <summary>
     /// Don't call. Used for pre-populating MEF providers only.
     /// </summary>

src/LanguageServer/Protocol/Handler/Completion/CompletionHandler.cs

@@ -3,8 +3,10 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Composition;
+using System.Diagnostics;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -99,16 +101,16 @@ public CompletionHandler(
             if (completionListResult == null)
                 return null;
 
-            var (list, isIncomplete, resultId) = completionListResult.Value;
+            var (list, isIncomplete, isHardSelection, resultId) = completionListResult.Value;
 
             var result = await CompletionResultFactory
-                .ConvertToLspCompletionListAsync(document, capabilityHelper, list, isIncomplete, resultId, cancellationToken)
+                .ConvertToLspCompletionListAsync(document, capabilityHelper, list, isIncomplete, isHardSelection, resultId, cancellationToken)
                 .ConfigureAwait(false);
 
             return result;
         }
 
-        private static async Task<(CompletionList CompletionList, bool IsIncomplete, long ResultId)?> GetFilteredCompletionListAsync(
+        private static async Task<(CompletionList CompletionList, bool IsIncomplete, bool isHardSelection, long ResultId)?> GetFilteredCompletionListAsync(
             LSP.CompletionContext? context,
             Document document,
             SourceText sourceText,
@@ -157,9 +159,9 @@ public CompletionHandler(
                 completionList = completionList.WithSpan(defaultSpan);
             }
 
-            var (filteredCompletionList, isIncomplete) = FilterCompletionList(completionList, completionListMaxSize, completionTrigger, sourceText);
+            var (filteredCompletionList, isIncomplete, isHardSelection) = FilterCompletionList(completionList, completionListMaxSize, completionTrigger, sourceText, capabilityHelper);
 
-            return (filteredCompletionList, isIncomplete, resultId);
+            return (filteredCompletionList, isIncomplete, isHardSelection, resultId);
         }
 
         private static async Task<(CompletionList CompletionList, long ResultId)?> CalculateListAsync(
@@ -184,18 +186,31 @@ public CompletionHandler(
             return (completionList, resultId);
         }
 
-        private static (CompletionList CompletionList, bool IsIncomplete) FilterCompletionList(
+        private static (CompletionList CompletionList, bool IsIncomplete, bool isHardSelection) FilterCompletionList(
             CompletionList completionList,
             int completionListMaxSize,
             CompletionTrigger completionTrigger,
-            SourceText sourceText)
+            SourceText sourceText,
+            CompletionCapabilityHelper completionCapabilityHelper)
         {
-            if (completionListMaxSize < 0 || completionListMaxSize >= completionList.ItemsList.Count)
-                return (completionList, false);
-
             var filterText = sourceText.GetSubText(completionList.Span).ToString();
             var filterReason = GetFilterReason(completionTrigger);
 
+            // Determine if the list should be hard selected or soft selected.
+            var isFilterTextAllPunctuation = CompletionService.IsAllPunctuation(filterText);
+
+            // If we only had punctuation - we set soft selection and the list to be incomplete so we get called back when the user continues typing.
+            // If they type something that is not punctuation, we may need to update the hard vs soft selection.
+            // For example, typing '_' should initially be soft selection, but if the user types 'o' we should hard select '_otherVar' (if it exists).
+            // This isn't perfect - ideally we would make this determination every time a filter character is typed, but we do not get called back
+            // for typing filter characters in LSP (unless we always set isIncomplete, which is expensive).
+            var isHardSelection = completionList.SuggestionModeItem is null && !isFilterTextAllPunctuation;
+            var isIncomplete = isFilterTextAllPunctuation;
+
+            // If our completion list hasn't hit the max size, we don't need to do anything filtering
+            if (completionListMaxSize < 0 || completionListMaxSize >= completionList.ItemsList.Count)
+                return (completionList, isIncomplete, isHardSelection);
+
             // Use pattern matching to determine which items are most relevant out of the calculated items.
             using var _ = ArrayBuilder<MatchResult>.GetInstance(out var matchResultsBuilder);
             var index = 0;
@@ -239,9 +254,11 @@ private static (CompletionList CompletionList, bool IsIncomplete) FilterCompleti
             // Currently the VS client does not remember to re-request, so the completion list only ever shows items from "Som"
             // so we always set the isIncomplete flag to true when the original list size (computed when no filter text was typed) is too large.
             // VS bug here - https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1335142
-            var isIncomplete = completionList.ItemsList.Count > newCompletionList.ItemsList.Count;
+            isIncomplete |= completionCapabilityHelper.SupportVSInternalClientCapabilities
+                ? completionList.ItemsList.Count > newCompletionList.ItemsList.Count
+                : matchResultsBuilder.Count > filteredList.Length;
 
-            return (newCompletionList, isIncomplete);
+            return (newCompletionList, isIncomplete, isHardSelection);
 
             static CompletionFilterReason GetFilterReason(CompletionTrigger trigger)
             {

src/LanguageServer/Protocol/Handler/Completion/CompletionResultFactory.cs

@@ -38,10 +38,11 @@ internal static class CompletionResultFactory
             CompletionCapabilityHelper capabilityHelper,
             CompletionList list,
             bool isIncomplete,
+            bool isHardSelection,
             long resultId,
             CancellationToken cancellationToken)
         {
-            var isSuggestionMode = list.SuggestionModeItem is not null;
+            var isSuggestionMode = !isHardSelection;
             if (list.ItemsList.Count == 0)
             {
                 return new LSP.VSInternalCompletionList
@@ -92,7 +93,7 @@ internal static class CompletionResultFactory
                 //
                 // If we have a suggestion mode item, we just need to keep the list in suggestion mode.
                 // We don't need to return the fake suggestion mode item.
-                SuggestionMode = list.SuggestionModeItem != null,
+                SuggestionMode = isSuggestionMode,
                 Data = capabilityHelper.SupportVSInternalCompletionListData ? resolveData : null,
             };
 

src/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs

@@ -809,6 +809,58 @@ public async Task TestSoftSelectionWhenFilterTextIsEmptyForPreselectItemAsync(bo
             Assert.Null(item.CommitCharacters);
     }
 
+    [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/vscode-csharp/issues/7623")]
+    public async Task TestSoftSelectionForDiscardAsync(bool mutatingLspWorkspace)
+    {
+        var markup =
+@"
+public class A
+{
+    public void M()
+    {
+        var _someDiscard = 1;
+        _{|caret:|}
+    }
+}";
+
+        await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, DefaultClientCapabilities);
+        var caretLocation = testLspServer.GetLocations("caret").Single();
+        await testLspServer.OpenDocumentAsync(caretLocation.Uri);
+
+        testLspServer.TestWorkspace.GlobalOptions.SetGlobalOption(CompletionOptionsStorage.TriggerInArgumentLists, LanguageNames.CSharp, true);
+
+        var completionParams = CreateCompletionParams(
+            caretLocation,
+            invokeKind: LSP.VSInternalCompletionInvokeKind.Typing,
+            triggerCharacter: "_",
+            triggerKind: LSP.CompletionTriggerKind.Invoked);
+
+        var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();
+        var results = await testLspServer.ExecuteRequestAsync<LSP.CompletionParams, LSP.CompletionList>(LSP.Methods.TextDocumentCompletionName, completionParams, CancellationToken.None).ConfigureAwait(false);
+        var actualItem = results.Items.First(i => i.Label == "_someDiscard");
+
+        Assert.True(results.IsIncomplete);
+        Assert.Empty(results.ItemDefaults.CommitCharacters);
+        Assert.Equal("_someDiscard", actualItem.Label);
+        Assert.Null(actualItem.CommitCharacters);
+
+        await testLspServer.InsertTextAsync(caretLocation.Uri, (caretLocation.Range.End.Line, caretLocation.Range.End.Character, "s"));
+
+        completionParams = CreateCompletionParams(
+            GetLocationPlusOne(caretLocation),
+            invokeKind: LSP.VSInternalCompletionInvokeKind.Typing,
+            triggerCharacter: "s",
+            triggerKind: LSP.CompletionTriggerKind.TriggerForIncompleteCompletions);
+
+        results = await testLspServer.ExecuteRequestAsync<LSP.CompletionParams, LSP.CompletionList>(LSP.Methods.TextDocumentCompletionName, completionParams, CancellationToken.None).ConfigureAwait(false);
+        actualItem = results.Items.First(i => i.Label == "_someDiscard");
+
+        Assert.False(results.IsIncomplete);
+        Assert.NotEmpty(results.ItemDefaults.CommitCharacters);
+        Assert.Equal("_someDiscard", actualItem.Label);
+        Assert.Null(actualItem.CommitCharacters);
+    }
+
     private sealed class CSharpLspThrowExceptionOnChangeCompletionService : CompletionService
     {
         private CSharpLspThrowExceptionOnChangeCompletionService(SolutionServices services, IAsynchronousOperationListenerProvider listenerProvider) : base(services, listenerProvider)