Cleanup CompletionTriggerAndCommitCharacters (#11600)

Recently, I was taking a look at completion performance and noticed that
`CompletionTriggerAndCommitCharacters` has gotten into a halfway state
where it's sometimes accessed statically, and sometimes as an instance.
This change refactors `CompletionTriggerAndCommitCharacters` to be fully
instance-based and cleans it up significantly.

Author: DustinCampbell

src/Razor/benchmarks/Microsoft.AspNetCore.Razor.Microbenchmarks/LanguageServer/RazorCompletionBenchmark.cs

@@ -41,14 +41,14 @@ public async Task SetupAsync()
         var documentMappingService = lspServices.GetRequiredService<IDocumentMappingService>();
         var clientConnection = lspServices.GetRequiredService<IClientConnection>();
         var completionListCache = lspServices.GetRequiredService<CompletionListCache>();
-        var completionTriggerAndCommitCharacters = lspServices.GetRequiredService<CompletionTriggerAndCommitCharacters>();
+        var triggerAndCommitCharacters = lspServices.GetRequiredService<CompletionTriggerAndCommitCharacters>();
         var loggerFactory = lspServices.GetRequiredService<ILoggerFactory>();
 
-        var delegatedCompletionListProvider = new TestDelegatedCompletionListProvider(documentMappingService, clientConnection, completionListCache, completionTriggerAndCommitCharacters);
-        var completionListProvider = new CompletionListProvider(razorCompletionListProvider, delegatedCompletionListProvider);
+        var delegatedCompletionListProvider = new TestDelegatedCompletionListProvider(documentMappingService, clientConnection, completionListCache, triggerAndCommitCharacters);
+        var completionListProvider = new CompletionListProvider(razorCompletionListProvider, delegatedCompletionListProvider, triggerAndCommitCharacters);
         var configurationService = new DefaultRazorConfigurationService(clientConnection, loggerFactory);
         var optionsMonitor = new RazorLSPOptionsMonitor(configurationService, RazorLSPOptions.Default);
-        CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, completionTriggerAndCommitCharacters, telemetryReporter: null, optionsMonitor);
+        CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, triggerAndCommitCharacters, telemetryReporter: null, optionsMonitor);
 
         var clientCapabilities = new VSInternalClientCapabilities
         {

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Completion/CompletionListProvider.cs

@@ -13,16 +13,14 @@
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer.Completion;
 
-internal class CompletionListProvider
+internal class CompletionListProvider(
+    RazorCompletionListProvider razorCompletionListProvider,
+    DelegatedCompletionListProvider delegatedCompletionListProvider,
+    CompletionTriggerAndCommitCharacters triggerAndCommitCharacters)
 {
-    private readonly RazorCompletionListProvider _razorCompletionListProvider;
-    private readonly DelegatedCompletionListProvider _delegatedCompletionListProvider;
-
-    public CompletionListProvider(RazorCompletionListProvider razorCompletionListProvider, DelegatedCompletionListProvider delegatedCompletionListProvider)
-    {
-        _razorCompletionListProvider = razorCompletionListProvider;
-        _delegatedCompletionListProvider = delegatedCompletionListProvider;
-    }
+    private readonly RazorCompletionListProvider _razorCompletionListProvider = razorCompletionListProvider;
+    private readonly DelegatedCompletionListProvider _delegatedCompletionListProvider = delegatedCompletionListProvider;
+    private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = triggerAndCommitCharacters;
 
     public async Task<VSInternalCompletionList?> GetCompletionListAsync(
         int absoluteIndex,
@@ -34,7 +32,7 @@ public CompletionListProvider(RazorCompletionListProvider razorCompletionListPro
         CancellationToken cancellationToken)
     {
         // First we delegate to get completion items from the individual language server
-        var delegatedCompletionList = CompletionTriggerAndCommitCharacters.IsValidTrigger(_delegatedCompletionListProvider.TriggerCharacters, completionContext)
+        var delegatedCompletionList = _triggerAndCommitCharacters.IsValidDelegationTrigger(completionContext)
             ? await _delegatedCompletionListProvider.GetCompletionListAsync(
                 absoluteIndex,
                 completionContext,
@@ -51,7 +49,7 @@ public CompletionListProvider(RazorCompletionListProvider razorCompletionListPro
             : null;
 
         // Now we get the Razor completion list, using information from the actual language server if necessary
-        var razorCompletionList = CompletionTriggerAndCommitCharacters.IsValidTrigger(_razorCompletionListProvider.TriggerCharacters, completionContext)
+        var razorCompletionList = _triggerAndCommitCharacters.IsValidRazorTrigger(completionContext)
             ? await _razorCompletionListProvider.GetCompletionListAsync(
                 absoluteIndex,
                 completionContext,

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Completion/Delegation/DelegatedCompletionListProvider.cs

@@ -27,7 +27,7 @@ internal class DelegatedCompletionListProvider
     private readonly IDocumentMappingService _documentMappingService;
     private readonly IClientConnection _clientConnection;
     private readonly CompletionListCache _completionListCache;
-    private readonly CompletionTriggerAndCommitCharacters _completionTriggerAndCommitCharacters;
+    private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters;
 
     public DelegatedCompletionListProvider(
         IDocumentMappingService documentMappingService,
@@ -38,12 +38,9 @@ public DelegatedCompletionListProvider(
         _documentMappingService = documentMappingService;
         _clientConnection = clientConnection;
         _completionListCache = completionListCache;
-        _completionTriggerAndCommitCharacters = completionTriggerAndCommitCharacters;
+        _triggerAndCommitCharacters = completionTriggerAndCommitCharacters;
     }
 
-    // virtual for tests
-    public virtual FrozenSet<string> TriggerCharacters => _completionTriggerAndCommitCharacters.AllDelegationTriggerCharacters;
-
     // virtual for tests
     public virtual async Task<VSInternalCompletionList?> GetCompletionListAsync(
         int absoluteIndex,
@@ -77,7 +74,7 @@ public DelegatedCompletionListProvider(
             positionInfo = provisionalCompletionValue.DocumentPositionInfo;
         }
 
-        if (DelegatedCompletionHelper.RewriteContext(completionContext, positionInfo.LanguageKind, _completionTriggerAndCommitCharacters) is not { } rewrittenContext)
+        if (DelegatedCompletionHelper.RewriteContext(completionContext, positionInfo.LanguageKind, _triggerAndCommitCharacters) is not { } rewrittenContext)
         {
             return null;
         }

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Completion/RazorCompletionEndpoint.cs

@@ -25,7 +25,7 @@ internal class RazorCompletionEndpoint(
     : IRazorRequestHandler<CompletionParams, VSInternalCompletionList?>, ICapabilitiesProvider
 {
     private readonly CompletionListProvider _completionListProvider = completionListProvider;
-    private readonly CompletionTriggerAndCommitCharacters _completionTriggerAndCommitCharacters = completionTriggerAndCommitCharacters;
+    private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = completionTriggerAndCommitCharacters;
     private readonly ITelemetryReporter? _telemetryReporter = telemetryReporter;
     private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor;
 
@@ -40,8 +40,8 @@ public void ApplyCapabilities(VSInternalServerCapabilities serverCapabilities, V
         serverCapabilities.CompletionProvider = new CompletionOptions()
         {
             ResolveProvider = true,
-            TriggerCharacters = _completionTriggerAndCommitCharacters.AllTriggerCharacters,
-            AllCommitCharacters = CompletionTriggerAndCommitCharacters.AllCommitCharacters
+            TriggerCharacters = [.. _triggerAndCommitCharacters.AllTriggerCharacters],
+            AllCommitCharacters = [.. _triggerAndCommitCharacters.AllCommitCharacters]
         };
     }
 

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

@@ -0,0 +1,113 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT license. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis.Razor.Workspaces;
+using Microsoft.VisualStudio.LanguageServer.Protocol;
+
+namespace Microsoft.CodeAnalysis.Razor.Completion;
+
+internal class CompletionTriggerAndCommitCharacters
+{
+    /// <summary>
+    ///  Trigger character that can trigger both Razor and Delegation completion
+    /// </summary>
+    private const char TransitionCharacter = '@';
+
+    private static readonly char[] s_vsHtmlTriggerCharacters = [':', '#', '.', '!', '*', ',', '(', '[', '-', '<', '&', '\\', '/', '\'', '"', '=', ':', ' ', '`'];
+    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 readonly HashSet<char> _csharpTriggerCharacters;
+    private readonly HashSet<char> _delegationTriggerCharacters;
+    private readonly HashSet<char> _htmlTriggerCharacters;
+    private readonly HashSet<char> _razorTriggerCharacters;
+
+    public ImmutableArray<string> AllTriggerCharacters { get; }
+
+    /// <summary>
+    /// This is the intersection of C# and HTML commit characters.
+    /// </summary>
+    // We need to specify it so that platform can correctly calculate ApplicableToSpan in
+    // https://devdiv.visualstudio.com/DevDiv/_git/VSLanguageServerClient?path=/src/product/RemoteLanguage/Impl/Features/Completion/AsyncCompletionSource.cs&version=GBdevelop&line=855&lineEnd=855&lineStartColumn=9&lineEndColumn=49&lineStyle=plain&_a=contents
+    // This is needed to fix https://github.com/dotnet/razor/issues/10787 in particular
+    public ImmutableArray<string> AllCommitCharacters { get; }
+
+    public CompletionTriggerAndCommitCharacters(LanguageServerFeatureOptions languageServerFeatureOptions)
+    {
+        // C# trigger characters (do NOT include '@')
+        var csharpTriggerCharacters = new HashSet<char>();
+        csharpTriggerCharacters.UnionWith(s_csharpTriggerCharacters);
+
+        // HTML trigger characters (include '@' + HTML trigger characters)
+        var htmlTriggerCharacters = new HashSet<char>() { TransitionCharacter };
+
+        if (languageServerFeatureOptions.UseVsCodeCompletionTriggerCharacters)
+        {
+            htmlTriggerCharacters.UnionWith(s_vsCodeHtmlTriggerCharacters);
+        }
+        else
+        {
+            htmlTriggerCharacters.UnionWith(s_vsHtmlTriggerCharacters);
+        }
+
+        // Delegation trigger characters (include '@' + C# and HTML trigger characters)
+        var delegationTriggerCharacters = new HashSet<char> { TransitionCharacter };
+        delegationTriggerCharacters.UnionWith(csharpTriggerCharacters);
+        delegationTriggerCharacters.UnionWith(htmlTriggerCharacters);
+
+        // Razor trigger characters (include '@' + Razor trigger characters)
+        var razorTriggerCharacters = new HashSet<char>() { TransitionCharacter };
+        razorTriggerCharacters.UnionWith(s_razorTriggerCharacters);
+
+        // All trigger characters (include Razor + Delegation trigger characters)
+        var allTriggerCharacters = new HashSet<char>();
+        allTriggerCharacters.UnionWith(razorTriggerCharacters);
+        allTriggerCharacters.UnionWith(delegationTriggerCharacters);
+
+        var commitCharacters = new HashSet<char>();
+        commitCharacters.UnionWith(s_commitCharacters);
+
+        _csharpTriggerCharacters = csharpTriggerCharacters;
+        _htmlTriggerCharacters = htmlTriggerCharacters;
+        _razorTriggerCharacters = razorTriggerCharacters;
+        _delegationTriggerCharacters = delegationTriggerCharacters;
+        AllTriggerCharacters = allTriggerCharacters.SelectAsArray(static c => c.ToString());
+        AllCommitCharacters = commitCharacters.SelectAsArray(static c => c.ToString());
+    }
+
+    public bool IsValidCSharpTrigger(CompletionContext completionContext)
+        => IsValidTrigger(completionContext, _csharpTriggerCharacters);
+
+    public bool IsValidDelegationTrigger(CompletionContext completionContext)
+        => IsValidTrigger(completionContext, _delegationTriggerCharacters);
+
+    public bool IsValidHtmlTrigger(CompletionContext completionContext)
+        => IsValidTrigger(completionContext, _htmlTriggerCharacters);
+
+    public bool IsValidRazorTrigger(CompletionContext completionContext)
+        => IsValidTrigger(completionContext, _razorTriggerCharacters);
+
+    private static bool IsValidTrigger(CompletionContext completionContext, HashSet<char> triggerCharacters)
+        => completionContext.TriggerKind != CompletionTriggerKind.TriggerCharacter ||
+           completionContext.TriggerCharacter is not [var c] ||
+           triggerCharacters.Contains(c);
+
+    public bool IsCSharpTriggerCharacter(string ch)
+        => ch is [var c] && _csharpTriggerCharacters.Contains(c);
+
+    public bool IsDelegationTriggerCharacter(string ch)
+        => ch is [var c] && _delegationTriggerCharacters.Contains(c);
+
+    public bool IsHtmlTriggerCharacter(string ch)
+        => ch is [var c] && _htmlTriggerCharacters.Contains(c);
+
+    public bool IsRazorTriggerCharacter(string ch)
+        => ch is [var c] && _razorTriggerCharacters.Contains(c);
+
+    public bool IsTransitionCharacter(string ch)
+        => ch is [TransitionCharacter];
+}

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

@@ -37,15 +37,15 @@ internal static class DelegatedCompletionHelper
     /// </summary>
     /// <param name="context">Original completion context passed to the completion handler</param>
     /// <param name="languageKind">Language of the completion position</param>
-    /// <param name="completionTriggerAndCommitCharacters">Per-client set of trigger and commit characters</param>
+    /// <param name="triggerAndCommitCharacters">Per-client set of trigger and commit characters</param>
     /// <returns>Possibly modified completion context</returns>
     /// <remarks>For example, if we invoke C# completion in Razor via @ character, we will not
     /// want C# to see @ as the trigger character and instead will transform completion context
     /// into "invoked" and "explicit" rather than "typing", without a trigger character</remarks>
     public static VSInternalCompletionContext? RewriteContext(
         VSInternalCompletionContext context,
         RazorLanguageKind languageKind,
-        CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters)
+        CompletionTriggerAndCommitCharacters triggerAndCommitCharacters)
     {
         Debug.Assert(languageKind != RazorLanguageKind.Razor,
             $"{nameof(RewriteContext)} should be called for delegated completion only");
@@ -58,7 +58,7 @@ internal static class DelegatedCompletionHelper
         }
 
         if (languageKind == RazorLanguageKind.CSharp
-            && CompletionTriggerAndCommitCharacters.CSharpTriggerCharacters.Contains(triggerCharacter))
+            && triggerAndCommitCharacters.IsCSharpTriggerCharacter(triggerCharacter))
         {
             // C# trigger character for C# content
             return context;
@@ -69,7 +69,7 @@ internal static class DelegatedCompletionHelper
             // For HTML we don't want to delegate to HTML language server is completion is due to a trigger characters that is not
             // HTML trigger character. Doing so causes bad side effects in VSCode HTML client as we will end up with non-matching
             // completion entries
-            return completionTriggerAndCommitCharacters.HtmlTriggerCharacters.Contains(triggerCharacter) ? context : null;
+            return triggerAndCommitCharacters.IsHtmlTriggerCharacter(triggerCharacter) ? context : null;
         }
 
         // Trigger character not associated with the current language. Transform the context into an invoked context.
@@ -80,7 +80,7 @@ internal static class DelegatedCompletionHelper
         };
 
         if (languageKind == RazorLanguageKind.CSharp
-            && CompletionTriggerAndCommitCharacters.RazorDelegationTriggerCharacters.Contains(triggerCharacter))
+            && triggerAndCommitCharacters.IsTransitionCharacter(triggerCharacter))
         {
             // The C# language server will not return any completions for the '@' character unless we
             // send the completion request explicitly.

src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/Delegation/DelegatedCompletionHelper.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Frozen;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
@@ -32,9 +31,6 @@ internal class RazorCompletionListProvider(
         Title = SR.ReTrigger_Completions_Title,
     };
 
-    // virtual for tests
-    public virtual FrozenSet<string> TriggerCharacters => CompletionTriggerAndCommitCharacters.RazorTriggerCharacters;
-
     // virtual for tests
     public virtual async Task<VSInternalCompletionList?> GetCompletionListAsync(
         int absoluteIndex,