Partial fix for dotnet/vscode-csharp#7678 (#11681)

* Partial fix for dotnet/vscode-csharp#7678

* Fix C# commit characters

In VSCode case they are returned by Roslyn via CompletionList.ItemDefaults.CommitCharacters rather than VSInternalCompletionList.CommitCharacters, so make sure we don't lose ItemDefaults.CommitCharacters. Also make sure not to merge unnecessarily when razorCompletionList is non-null but empty.

* Remove unnecessary HashSet creation per CR

* Fixing tests

Author: alexgav

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/CompletionListMerger.cs

@@ -22,7 +22,9 @@ internal static class CompletionListMerger
     [return: NotNullIfNotNull(nameof(delegatedCompletionList))]
     public static RazorVSInternalCompletionList? Merge(RazorVSInternalCompletionList? razorCompletionList, RazorVSInternalCompletionList? delegatedCompletionList)
     {
-        if (razorCompletionList is null)
+        // In VSCode case we always think completion was invoked explicitly and create empty Razor completion list,
+        // so check for empty Items collection as well. 
+        if (razorCompletionList is null || razorCompletionList.Items.Length == 0)
         {
             return delegatedCompletionList;
         }
@@ -38,7 +40,8 @@ internal static class CompletionListMerger
         var mergedIsIncomplete = razorCompletionList.IsIncomplete || delegatedCompletionList.IsIncomplete;
         var mergedItems = razorCompletionList.Items.Concat(delegatedCompletionList.Items).ToArray();
         var mergedData = MergeData(razorCompletionList.Data, delegatedCompletionList.Data);
-        var mergedCommitCharacters = razorCompletionList.CommitCharacters ?? delegatedCompletionList.CommitCharacters;
+        var mergedCommitCharacters = razorCompletionList.CommitCharacters
+            ?? delegatedCompletionList.CommitCharacters;
         var mergedSuggestionMode = razorCompletionList.SuggestionMode || delegatedCompletionList.SuggestionMode;
 
         // We don't fully support merging continue characters currently. Razor doesn't currently use them so delegated completion lists always win.
@@ -60,6 +63,8 @@ internal static class CompletionListMerger
             {
                 Data = mergedItemDefaultsData,
                 EditRange = mergedItemDefaultsEditRange,
+                // VSCode won't use VSInternalCompletionList.CommitCharacters, make sure we don't lose item defaults
+                CommitCharacters = razorCompletionList.ItemDefaults?.CommitCharacters ?? delegatedCompletionList.ItemDefaults?.CommitCharacters
             }
         };
 

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/CompletionTriggerAndCommitCharacters.cs

@@ -18,7 +18,7 @@ internal class CompletionTriggerAndCommitCharacters
     private static readonly char[] s_vsCodeHtmlTriggerCharacters = ['#', '.', '!', ',', '-', '<'];
     private static readonly char[] s_razorTriggerCharacters = ['<', ':', ' '];
     private static readonly char[] s_csharpTriggerCharacters = [' ', '(', '=', '#', '.', '<', '[', '{', '"', '/', ':', '~'];
-    private static readonly char[] s_commitCharacters = [' ', '>', ';', '='];
+    private static readonly ImmutableArray<string> s_commitCharacters = [" ", ">", ";", "="];
 
     private readonly HashSet<char> _csharpTriggerCharacters;
     private readonly HashSet<char> _delegationTriggerCharacters;
@@ -67,15 +67,16 @@ public CompletionTriggerAndCommitCharacters(LanguageServerFeatureOptions languag
         allTriggerCharacters.UnionWith(razorTriggerCharacters);
         allTriggerCharacters.UnionWith(delegationTriggerCharacters);
 
-        var commitCharacters = new HashSet<char>();
-        commitCharacters.UnionWith(s_commitCharacters);
-
         _csharpTriggerCharacters = csharpTriggerCharacters;
         _htmlTriggerCharacters = htmlTriggerCharacters;
         _razorTriggerCharacters = razorTriggerCharacters;
         _delegationTriggerCharacters = delegationTriggerCharacters;
+
+        // We shouldn't specify commit characters for VSCode.
+        // It doesn't appear to need them and they interfere with normal item commit.
+        // E.g. see https://github.com/dotnet/vscode-csharp/issues/7678
+        AllCommitCharacters = languageServerFeatureOptions.UseVsCodeCompletionTriggerCharacters ? [] : s_commitCharacters;
         AllTriggerCharacters = allTriggerCharacters.SelectAsArray(static c => c.ToString());
-        AllCommitCharacters = commitCharacters.SelectAsArray(static c => c.ToString());
     }
 
     public bool IsValidCSharpTrigger(CompletionContext completionContext)

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Completion/CompletionListProviderTest.cs

@@ -21,8 +21,8 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Completion;
 
 public class CompletionListProviderTest : LanguageServerTestBase
 {
-    private readonly RazorVSInternalCompletionList _completionList1;
-    private readonly RazorVSInternalCompletionList _completionList2;
+    private readonly RazorVSInternalCompletionList _razorCompletionList;
+    private readonly RazorVSInternalCompletionList _delegatedCompletionList;
     private readonly RazorCompletionListProvider _razorCompletionProvider;
     private readonly DelegatedCompletionListProvider _delegatedCompletionProvider;
     private readonly VSInternalCompletionContext _completionContext;
@@ -34,10 +34,10 @@ public class CompletionListProviderTest : LanguageServerTestBase
     public CompletionListProviderTest(ITestOutputHelper testOutput)
         : base(testOutput)
     {
-        _completionList1 = new RazorVSInternalCompletionList() { Items = [] };
-        _completionList2 = new RazorVSInternalCompletionList() { Items = [] };
-        _razorCompletionProvider = new TestRazorCompletionListProvider(_completionList1, LoggerFactory);
-        _delegatedCompletionProvider = new TestDelegatedCompletionListProvider(_completionList2);
+        _razorCompletionList = new RazorVSInternalCompletionList() { Items = [] };
+        _delegatedCompletionList = new RazorVSInternalCompletionList() { Items = [] };
+        _razorCompletionProvider = new TestRazorCompletionListProvider(_razorCompletionList, LoggerFactory);
+        _delegatedCompletionProvider = new TestDelegatedCompletionListProvider(_delegatedCompletionList);
         _completionContext = new VSInternalCompletionContext();
         _documentContext = TestDocumentContext.Create("C:/path/to/file.cshtml");
         _clientCapabilities = new VSInternalClientCapabilities();
@@ -52,12 +52,13 @@ public async Task MultipleCompletionLists_Merges()
         var provider = new CompletionListProvider(_razorCompletionProvider, _delegatedCompletionProvider, _triggerAndCommitCharacters);
 
         // Act
-        var completionList = await provider.GetCompletionListAsync(
+        var mergedCompletionList = await provider.GetCompletionListAsync(
             absoluteIndex: 0, _completionContext, _documentContext, _clientCapabilities, _razorCompletionOptions, correlationId: Guid.Empty, cancellationToken: DisposalToken);
 
         // Assert
-        Assert.NotSame(_completionList1, completionList);
-        Assert.NotSame(_completionList2, completionList);
+        Assert.Empty(mergedCompletionList.Items);
+        Assert.NotSame(_razorCompletionList, mergedCompletionList);
+        Assert.Same(_delegatedCompletionList, mergedCompletionList);
     }
 
     [Fact]
@@ -75,7 +76,7 @@ public async Task MultipleCompletionLists_DifferentCommitCharacters_OnlyCallsApp
             absoluteIndex: 0, _completionContext, _documentContext, _clientCapabilities, _razorCompletionOptions, correlationId: Guid.Empty, cancellationToken: DisposalToken);
 
         // Assert
-        Assert.Same(_completionList2, completionList);
+        Assert.Same(_delegatedCompletionList, completionList);
     }
 
     private class TestDelegatedCompletionListProvider : DelegatedCompletionListProvider