Skip to content

Commit 09542f8

Browse files
Audit GetCompletionListAsync methods
This change audits and refactors to the GetCompletionListAsync methods on CompletionListProvider, RazorCompletionListProvider and DelegatedCompletionListProvider in favor of removing async state machines where possible.
1 parent 4a33dc1 commit 09542f8

File tree

13 files changed

+262
-165
lines changed

13 files changed

+262
-165
lines changed

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

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77
using System.Threading;
88
using System.Threading.Tasks;
99
using BenchmarkDotNet.Attributes;
10+
using Microsoft.AspNetCore.Razor.Language;
1011
using Microsoft.AspNetCore.Razor.LanguageServer;
1112
using Microsoft.AspNetCore.Razor.LanguageServer.Completion;
1213
using Microsoft.AspNetCore.Razor.LanguageServer.Completion.Delegation;
1314
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
1415
using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
1516
using Microsoft.AspNetCore.Razor.ProjectSystem;
17+
using Microsoft.AspNetCore.Razor.Telemetry;
1618
using Microsoft.CodeAnalysis.Razor.Completion;
1719
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
1820
using Microsoft.CodeAnalysis.Razor.Logging;
@@ -48,7 +50,7 @@ public async Task SetupAsync()
4850
var completionListProvider = new CompletionListProvider(razorCompletionListProvider, delegatedCompletionListProvider, triggerAndCommitCharacters);
4951
var configurationService = new DefaultRazorConfigurationService(clientConnection, loggerFactory);
5052
var optionsMonitor = new RazorLSPOptionsMonitor(configurationService, RazorLSPOptions.Default);
51-
CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, triggerAndCommitCharacters, telemetryReporter: null, optionsMonitor);
53+
CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, triggerAndCommitCharacters, NoOpTelemetryReporter.Instance, optionsMonitor);
5254

5355
var clientCapabilities = new VSInternalClientCapabilities
5456
{
@@ -145,24 +147,20 @@ public TestDelegatedCompletionListProvider(
145147
IDocumentMappingService documentMappingService,
146148
IClientConnection clientConnection,
147149
CompletionListCache completionListCache,
148-
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters)
149-
: base(documentMappingService, clientConnection, completionListCache, completionTriggerAndCommitCharacters)
150+
CompletionTriggerAndCommitCharacters triggerAndCommitCharacters)
151+
: base(documentMappingService, clientConnection, completionListCache, triggerAndCommitCharacters)
150152
{
151153
}
152154

153-
public override Task<VSInternalCompletionList?> GetCompletionListAsync(
155+
public override ValueTask<VSInternalCompletionList?> GetCompletionListAsync(
156+
RazorCodeDocument codeDocument,
154157
int absoluteIndex,
155158
VSInternalCompletionContext completionContext,
156159
DocumentContext documentContext,
157160
VSInternalClientCapabilities clientCapabilities,
158161
RazorCompletionOptions completionOptions,
159162
Guid correlationId,
160163
CancellationToken cancellationToken)
161-
{
162-
return Task.FromResult<VSInternalCompletionList?>(
163-
new VSInternalCompletionList
164-
{
165-
});
166-
}
164+
=> new(new VSInternalCompletionList());
167165
}
168166
}

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

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.Linq;
78
using System.Threading;
89
using System.Threading.Tasks;
@@ -22,7 +23,7 @@ internal class CompletionListProvider(
2223
private readonly DelegatedCompletionListProvider _delegatedCompletionListProvider = delegatedCompletionListProvider;
2324
private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = triggerAndCommitCharacters;
2425

25-
public async Task<VSInternalCompletionList?> GetCompletionListAsync(
26+
public ValueTask<VSInternalCompletionList?> GetCompletionListAsync(
2627
int absoluteIndex,
2728
VSInternalCompletionContext completionContext,
2829
DocumentContext documentContext,
@@ -31,37 +32,78 @@ internal class CompletionListProvider(
3132
Guid correlationId,
3233
CancellationToken cancellationToken)
3334
{
34-
// First we delegate to get completion items from the individual language server
35-
var delegatedCompletionList = _triggerAndCommitCharacters.IsValidDelegationTrigger(completionContext)
36-
? await _delegatedCompletionListProvider.GetCompletionListAsync(
35+
var isDelegationTrigger = _triggerAndCommitCharacters.IsValidDelegationTrigger(completionContext);
36+
var isRazorTrigger = _triggerAndCommitCharacters.IsValidRazorTrigger(completionContext);
37+
38+
// We don't have a valid trigger, so we can't provide completions
39+
return isDelegationTrigger || isRazorTrigger
40+
? new(GetCompletionListCoreAsync(
3741
absoluteIndex,
3842
completionContext,
3943
documentContext,
4044
clientCapabilities,
4145
razorCompletionOptions,
4246
correlationId,
43-
cancellationToken).ConfigureAwait(false)
44-
: null;
47+
isDelegationTrigger,
48+
isRazorTrigger,
49+
cancellationToken))
50+
: default;
51+
}
52+
53+
private async Task<VSInternalCompletionList?> GetCompletionListCoreAsync(
54+
int absoluteIndex,
55+
VSInternalCompletionContext completionContext,
56+
DocumentContext documentContext,
57+
VSInternalClientCapabilities clientCapabilities,
58+
RazorCompletionOptions razorCompletionOptions,
59+
Guid correlationId,
60+
bool isDelegationTrigger,
61+
bool isRazorTrigger,
62+
CancellationToken cancellationToken)
63+
{
64+
Debug.Assert(isDelegationTrigger || isRazorTrigger);
65+
66+
var codeDocument = await documentContext.GetCodeDocumentAsync(cancellationToken).ConfigureAwait(false);
67+
68+
// First we delegate to get completion items from the individual language server
69+
VSInternalCompletionList? delegatedCompletionList = null;
70+
HashSet<string>? existingItems = null;
4571

46-
// Extract the items we got back from the delegated server, to inform tag helper completion
47-
var existingItems = delegatedCompletionList?.Items != null
48-
? new HashSet<string>(delegatedCompletionList.Items.Select(i => i.Label))
49-
: null;
72+
if (isDelegationTrigger)
73+
{
74+
delegatedCompletionList = await _delegatedCompletionListProvider
75+
.GetCompletionListAsync(
76+
codeDocument,
77+
absoluteIndex,
78+
completionContext,
79+
documentContext,
80+
clientCapabilities,
81+
razorCompletionOptions,
82+
correlationId,
83+
cancellationToken)
84+
.ConfigureAwait(false);
85+
86+
// Extract the items we got back from the delegated server, to inform tag helper completion
87+
if (delegatedCompletionList?.Items is { } delegatedItems)
88+
{
89+
existingItems = [.. delegatedItems.Select(static i => i.Label)];
90+
}
91+
}
5092

5193
// Now we get the Razor completion list, using information from the actual language server if necessary
52-
var razorCompletionList = _triggerAndCommitCharacters.IsValidRazorTrigger(completionContext)
53-
? await _razorCompletionListProvider.GetCompletionListAsync(
94+
VSInternalCompletionList? razorCompletionList = null;
95+
96+
if (isRazorTrigger)
97+
{
98+
razorCompletionList = _razorCompletionListProvider.GetCompletionList(
99+
codeDocument,
54100
absoluteIndex,
55101
completionContext,
56-
documentContext,
57102
clientCapabilities,
58103
existingItems,
59-
razorCompletionOptions,
60-
cancellationToken).ConfigureAwait(false)
61-
: null;
62-
63-
var finalCompletionList = CompletionListMerger.Merge(razorCompletionList, delegatedCompletionList);
104+
razorCompletionOptions);
105+
}
64106

65-
return finalCompletionList;
107+
return CompletionListMerger.Merge(razorCompletionList, delegatedCompletionList);
66108
}
67109
}

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

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ public DelegatedCompletionListProvider(
4242
}
4343

4444
// virtual for tests
45-
public virtual async Task<VSInternalCompletionList?> GetCompletionListAsync(
45+
public virtual ValueTask<VSInternalCompletionList?> GetCompletionListAsync(
46+
RazorCodeDocument codeDocument,
4647
int absoluteIndex,
4748
VSInternalCompletionContext completionContext,
4849
DocumentContext documentContext,
@@ -51,13 +52,11 @@ public DelegatedCompletionListProvider(
5152
Guid correlationId,
5253
CancellationToken cancellationToken)
5354
{
54-
var codeDocument = await documentContext.GetCodeDocumentAsync(cancellationToken).ConfigureAwait(false);
55-
5655
var positionInfo = _documentMappingService.GetPositionInfo(codeDocument, absoluteIndex);
5756
if (positionInfo.LanguageKind == RazorLanguageKind.Razor)
5857
{
5958
// Nothing to delegate to.
60-
return null;
59+
return default;
6160
}
6261

6362
TextEdit? provisionalTextEdit = null;
@@ -69,7 +68,7 @@ public DelegatedCompletionListProvider(
6968

7069
if (DelegatedCompletionHelper.RewriteContext(completionContext, positionInfo.LanguageKind, _triggerAndCommitCharacters) is not { } rewrittenContext)
7170
{
72-
return null;
71+
return default;
7372
}
7473

7574
completionContext = rewrittenContext;
@@ -80,22 +79,51 @@ public DelegatedCompletionListProvider(
8079
// Their values are not related in any way.
8180
var shouldIncludeDelegationSnippets = DelegatedCompletionHelper.ShouldIncludeSnippets(codeDocument, absoluteIndex);
8281

83-
var delegatedParams = new DelegatedCompletionParams(
82+
return new(GetDelegatedCompletionListAsync(
83+
codeDocument,
84+
absoluteIndex,
85+
completionContext,
8486
documentContext.GetTextDocumentIdentifierAndVersion(),
87+
clientCapabilities,
88+
razorCompletionOptions,
89+
correlationId,
90+
positionInfo,
91+
provisionalTextEdit,
92+
shouldIncludeDelegationSnippets,
93+
cancellationToken));
94+
}
95+
96+
private async Task<VSInternalCompletionList?> GetDelegatedCompletionListAsync(
97+
RazorCodeDocument codeDocument,
98+
int absoluteIndex,
99+
VSInternalCompletionContext completionContext,
100+
TextDocumentIdentifierAndVersion identifier,
101+
VSInternalClientCapabilities clientCapabilities,
102+
RazorCompletionOptions razorCompletionOptions,
103+
Guid correlationId,
104+
DocumentPositionInfo positionInfo,
105+
TextEdit? provisionalTextEdit,
106+
bool shouldIncludeDelegationSnippets,
107+
CancellationToken cancellationToken)
108+
{
109+
var delegatedParams = new DelegatedCompletionParams(
110+
identifier,
85111
positionInfo.Position,
86112
positionInfo.LanguageKind,
87113
completionContext,
88114
provisionalTextEdit,
89115
shouldIncludeDelegationSnippets,
90116
correlationId);
91117

92-
var delegatedResponse = await _clientConnection.SendRequestAsync<DelegatedCompletionParams, VSInternalCompletionList?>(
93-
LanguageServerConstants.RazorCompletionEndpointName,
94-
delegatedParams,
95-
cancellationToken).ConfigureAwait(false);
118+
var delegatedResponse = await _clientConnection
119+
.SendRequestAsync<DelegatedCompletionParams, VSInternalCompletionList?>(
120+
LanguageServerConstants.RazorCompletionEndpointName,
121+
delegatedParams,
122+
cancellationToken)
123+
.ConfigureAwait(false);
96124

97-
var rewrittenResponse = delegatedParams.ProjectedKind == RazorLanguageKind.CSharp
98-
? DelegatedCompletionHelper.RewriteCSharpResponse(delegatedResponse, absoluteIndex, codeDocument, delegatedParams.ProjectedPosition, razorCompletionOptions)
125+
var rewrittenResponse = positionInfo.LanguageKind == RazorLanguageKind.CSharp
126+
? DelegatedCompletionHelper.RewriteCSharpResponse(delegatedResponse, absoluteIndex, codeDocument, positionInfo.Position, razorCompletionOptions)
99127
: DelegatedCompletionHelper.RewriteHtmlResponse(delegatedResponse, razorCompletionOptions);
100128

101129
var completionCapability = clientCapabilities?.TextDocument?.Completion as VSInternalCompletionSetting;

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

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Completion;
1919
[RazorLanguageServerEndpoint(Methods.TextDocumentCompletionName)]
2020
internal class RazorCompletionEndpoint(
2121
CompletionListProvider completionListProvider,
22-
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters,
23-
ITelemetryReporter? telemetryReporter,
22+
CompletionTriggerAndCommitCharacters triggerAndCommitCharacters,
23+
ITelemetryReporter telemetryReporter,
2424
RazorLSPOptionsMonitor optionsMonitor)
2525
: IRazorRequestHandler<CompletionParams, VSInternalCompletionList?>, ICapabilitiesProvider
2626
{
2727
private readonly CompletionListProvider _completionListProvider = completionListProvider;
28-
private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = completionTriggerAndCommitCharacters;
29-
private readonly ITelemetryReporter? _telemetryReporter = telemetryReporter;
28+
private readonly CompletionTriggerAndCommitCharacters _triggerAndCommitCharacters = triggerAndCommitCharacters;
29+
private readonly ITelemetryReporter _telemetryReporter = telemetryReporter;
3030
private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor;
3131

3232
private VSInternalClientCapabilities? _clientCapabilities;
@@ -52,46 +52,43 @@ public TextDocumentIdentifier GetTextDocumentIdentifier(CompletionParams request
5252

5353
public async Task<VSInternalCompletionList?> HandleRequestAsync(CompletionParams request, RazorRequestContext requestContext, CancellationToken cancellationToken)
5454
{
55-
var documentContext = requestContext.DocumentContext;
56-
57-
if (request.Context is null || documentContext is null)
58-
{
59-
return null;
60-
}
61-
62-
var sourceText = await documentContext.GetSourceTextAsync(cancellationToken).ConfigureAwait(false);
63-
if (!sourceText.TryGetAbsoluteIndex(request.Position, out var hostDocumentIndex))
55+
if (request.Context is not VSInternalCompletionContext completionContext ||
56+
requestContext.DocumentContext is not { } documentContext)
6457
{
6558
return null;
6659
}
6760

68-
if (request.Context is not VSInternalCompletionContext completionContext)
61+
var autoShownCompletion = completionContext.InvokeKind != VSInternalCompletionInvokeKind.Explicit;
62+
var options = _optionsMonitor.CurrentValue;
63+
if (autoShownCompletion && !options.AutoShowCompletion)
6964
{
70-
Debug.Fail("Completion context should never be null in practice");
7165
return null;
7266
}
7367

74-
var autoShownCompletion = completionContext.InvokeKind != VSInternalCompletionInvokeKind.Explicit;
75-
if (autoShownCompletion && !_optionsMonitor.CurrentValue.AutoShowCompletion)
68+
var sourceText = await documentContext.GetSourceTextAsync(cancellationToken).ConfigureAwait(false);
69+
if (!sourceText.TryGetAbsoluteIndex(request.Position, out var hostDocumentIndex))
7670
{
7771
return null;
7872
}
7973

8074
var correlationId = Guid.NewGuid();
81-
using var _ = _telemetryReporter?.TrackLspRequest(Methods.TextDocumentCompletionName, LanguageServerConstants.RazorLanguageServerName, TelemetryThresholds.CompletionRazorTelemetryThreshold, correlationId);
75+
using (_telemetryReporter.TrackLspRequest(Methods.TextDocumentCompletionName, LanguageServerConstants.RazorLanguageServerName, TelemetryThresholds.CompletionRazorTelemetryThreshold, correlationId))
76+
{
77+
var razorCompletionOptions = new RazorCompletionOptions(
78+
SnippetsSupported: true,
79+
AutoInsertAttributeQuotes: options.AutoInsertAttributeQuotes,
80+
CommitElementsWithSpace: options.CommitElementsWithSpace);
8281

83-
var razorCompletionOptions = new RazorCompletionOptions(
84-
SnippetsSupported: true,
85-
AutoInsertAttributeQuotes: _optionsMonitor.CurrentValue.AutoInsertAttributeQuotes,
86-
CommitElementsWithSpace: _optionsMonitor.CurrentValue.CommitElementsWithSpace);
87-
var completionList = await _completionListProvider.GetCompletionListAsync(
88-
hostDocumentIndex,
89-
completionContext,
90-
documentContext,
91-
_clientCapabilities!,
92-
razorCompletionOptions,
93-
correlationId,
94-
cancellationToken).ConfigureAwait(false);
95-
return completionList;
82+
return await _completionListProvider
83+
.GetCompletionListAsync(
84+
hostDocumentIndex,
85+
completionContext,
86+
documentContext,
87+
_clientCapabilities.AssumeNotNull(),
88+
razorCompletionOptions,
89+
correlationId,
90+
cancellationToken)
91+
.ConfigureAwait(false);
92+
}
9693
}
9794
}

0 commit comments

Comments
 (0)