10BC0 Ensure RazorVSInternalCompletionParams is used for serialization of c… · dotnet/razor@cfbe7ee · GitHub
[go: up one dir, main page]
Skip to content

Commit cfbe7ee

Browse files
ToddGrunToddGrun
authored
Ensure RazorVSInternalCompletionParams is used for serialization of completion requests (#12271)
* Ensure RazorVSInternalCompletionParams is used for serialization of completion requests Without this type being used, serialization loses the VSInternalCompletionContext.InvokeKind, which is critical for webtools to determine the correct range to replace. Without this, webtools indicates a range to be replaced based on the assumption that this is a forced completion session, which in turns affects how vseditor determines filtering of items in the completion list. Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2581568 * 1) Fix ci build errors 2) Propagate RazorVSInternalCompletionParams usage a bit further out per David's suggestion
1 parent 06c0bfa commit cfbe7ee

File tree

6 files changed

+94
-21
lines changed

6 files changed

+94
-21
lines changed

‎src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Completion/CohostDocumentCompletionEndpoint.cs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ internal sealed class CohostDocumentCompletionEndpoint(
4646
CompletionListCache completionListCache,
4747
ITelemetryReporter telemetryReporter,
4848
ILoggerFactory loggerFactory)
49-
: AbstractCohostDocumentEndpoint<CompletionParams, RazorVSInternalCompletionList?>(incompatibleProjectService), IDynamicRegistrationProvider
49+
: AbstractCohostDocumentEndpoint<RazorVSInternalCompletionParams, RazorVSInternalCompletionList?>(incompatibleProjectService), IDynamicRegistrationProvider
5050
{
5151
private readonly IRemoteServiceInvoker _remoteServiceInvoker = remoteServiceInvoker;
5252
private readonly IClientSettingsManager _clientSettingsManager = clientSettingsManager;
@@ -82,13 +82,12 @@ public ImmutableArray<Registration> GetRegistrations(VSInternalClientCapabilitie
8282
return [];
8383
}
8484

85-
protected override RazorTextDocumentIdentifier? GetRazorTextDocumentIdentifier(CompletionParams request)
85+
protected override RazorTextDocumentIdentifier? GetRazorTextDocumentIdentifier(RazorVSInternalCompletionParams request)
8686
=> request.TextDocument?.ToRazorTextDocumentIdentifier();
8787

88-
protected override async Task<RazorVSInternalCompletionList?> HandleRequestAsync(CompletionParams request, TextDocument razorDocument, CancellationToken cancellationToken)
88+
protected override async Task<RazorVSInternalCompletionList?> HandleRequestAsync(RazorVSInternalCompletionParams request, TextDocument razorDocument, CancellationToken cancellationToken)
8989
{
90-
if (request.Context is null ||
91-
JsonHelpers.Convert<CompletionContext, VSInternalCompletionContext>(request.Context) is not { } completionContext)
90+
if (request.Context is not { } completionContext)
9291
{
9392
_logger.LogError("Completion request context is null");
9493
return null;
@@ -218,23 +217,23 @@ public ImmutableArray<Registration> GetRegistrations(VSInternalClientCapabilitie
218217
}
219218

220219
private async Task<RazorVSInternalCompletionList?> GetHtmlCompletionListAsync(
221-
CompletionParams request,
220+
RazorVSInternalCompletionParams completionParams,
222221
TextDocument razorDocument,
223222
RazorCompletionOptions razorCompletionOptions,
224223
Guid correlationId,
225224
CancellationToken cancellationToken)
226225
{
227-
var result = await _requestInvoker.MakeHtmlLspRequestAsync<CompletionParams, RazorVSInternalCompletionList>(
226+
var result = await _requestInvoker.MakeHtmlLspRequestAsync<RazorVSInternalCompletionParams, RazorVSInternalCompletionList>(
228227
razorDocument,
229228
Methods.TextDocumentCompletionName,
230-
request,
229+
completionParams,
231230
TelemetryThresholds.CompletionSubLSPTelemetryThreshold,
232231
correlationId,
233232
cancellationToken).ConfigureAwait(false);
234233

235234
var rewrittenResponse = DelegatedCompletionHelper.RewriteHtmlResponse(result, razorCompletionOptions);
236235

237-
var razorDocumentIdentifier = new TextDocumentIdentifierAndVersion(request.TextDocument, Version: 0);
236+
var razorDocumentIdentifier = new TextDocumentIdentifierAndVersion(completionParams.TextDocument, Version: 0);
238237
var resolutionContext = new DelegatedCompletionResolutionContext(razorDocumentIdentifier, RazorLanguageKind.Html, rewrittenResponse.Data ?? rewrittenResponse.ItemDefaults?.Data);
239238
var resultId = _completionListCache.Add(rewrittenResponse, resolutionContext);
240239
rewrittenResponse.SetResultId(resultId, _clientCapabilitiesService.ClientCapabilities);
@@ -285,7 +284,7 @@ ref builder.AsRef(),
285284
internal readonly struct TestAccessor(CohostDocumentCompletionEndpoint instance)
286285
{
287286
public Task<RazorVSInternalCompletionList?> HandleRequestAsync(
288-
CompletionParams request,
287+
RazorVSInternalCompletionParams request,
289288
TextDocument razorDocument,
290289
CancellationToken cancellationToken)
291290
=> instance.HandleRequestAsync(request, razorDocument, cancellationToken);

‎src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Cohost/HtmlRequestInvoker.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ internal sealed class HtmlRequestInvoker(
6868
_logger.LogDebug($"Making LSP request for {method} from {htmlDocument.Uri}{(request is ITextDocumentPositionParams positionParams ? $" at {positionParams.Position}" : "")}, checksum {syncResult.Checksum}.");
6969

7070
// Passing Guid.Empty to this method will mean no tracking
71-
using var _ = _telemetryReporter.TrackLspRequest(Methods.TextDocumentCodeActionName, RazorLSPConstants.HtmlLanguageServerName, threshold, correlationId);
71+
using var _ = _telemetryReporter.TrackLspRequest(method, RazorLSPConstants.HtmlLanguageServerName, threshold, correlationId);
7272

7373
var result = await _requestInvoker.ReinvokeRequestOnServerAsync<TRequest, TResponse?>(
7474
htmlDocument.Snapshot.TextBuffer,

‎src/Razor/test/Microsoft.AspNetCore.Razor.Test.Common.Cohosting/TestHtmlRequestInvoker.cs‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,29 @@ namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;
1313

1414
internal class TestHtmlRequestInvoker : IHtmlRequestInvoker
1515
{
16-
private readonly Dictionary<string, object?> _htmlResponses;
16+
private readonly Dictionary<string, Func<object, object?>> _getResponses;
17+
18+
public TestHtmlRequestInvoker()
19+
: this(Array.Empty<(string, Func<object, object?>)>())
20+
{
21+
}
1722

1823
public TestHtmlRequestInvoker(params (string method, object? response)[] htmlResponses)
24+
: this(htmlResponses.Select<(string method, object? response), (string, Func<object, object?>)>(kvp => (kvp.method, _ => kvp.response)).ToArray())
25+
{
26+
}
27+
28+
public TestHtmlRequestInvoker(params (string method, Func<object, object?> getResponse)[] htmlResponses)
1929
{
20-
_htmlResponses = htmlResponses.ToDictionary(kvp => kvp.method, kvp => kvp.response);
30+
_getResponses = htmlResponses.ToDictionary(kvp => kvp.method, kvp => kvp.getResponse);
2131
}
2232

2333
public Task<TResponse?> MakeHtmlLspRequestAsync<TRequest, TResponse>(TextDocument razorDocument, string method, TRequest request, TimeSpan threshold, Guid correlationId, CancellationToken cancellationToken) where TRequest : notnull
2434
{
25-
if (_htmlResponses is not null &&
26-
_htmlResponses.TryGetValue(method, out var response))
35+
if (_getResponses is not null &&
36+
_getResponses.TryGetValue(method, out var getResponse))
2737
{
28-
return Task.FromResult((TResponse?)response);
38+
return Task.FromResult((TResponse?)getResponse(request));
2939
}
3040

3141
return Task.FromResult<TResponse?>(default);

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/Shared/CohostDocumentCompletionEndpointTest.cs‎

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.Collections.Immutable;
67
using System.Linq;
@@ -12,6 +13,7 @@
1213
using Microsoft.AspNetCore.Razor.PooledObjects;
1314
using Microsoft.AspNetCore.Razor.Test.Common;
1415
using Microsoft.AspNetCore.Razor.Test.Common.Workspaces;
16+
using Microsoft.CodeAnalysis;
1517
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
1618
using Microsoft.CodeAnalysis.Razor.Completion;
1719
using Microsoft.CodeAnalysis.Razor.Protocol;
@@ -346,6 +348,23 @@ This is a Razor document.
346348
unexpectedItemLabels: ["snippet1", "snippet2"]);
347349
}
348350

351+
[Fact]
352+
public async Task HtmlElementNamesCompletion_UsesRazorVSInternalCompletionParams()
353+
{
354+
await VerifyCompletionListParamsTypeAsync(
355+
input: """
356+
This is a Razor document.
357+
358+
<$$1
359+
""",
360+
completionContext: new VSInternalCompletionContext()
361+
{
362+
InvokeKind = VSInternalCompletionInvokeKind.Typing,
363+
TriggerCharacter = "<",
364+
TriggerKind = CompletionTriggerKind.TriggerCharacter
365+
});
366+
}
367+
349368
[Fact]
350369
public async Task HtmlElementDoNotCommitWithSpace()
351370
{
@@ -792,7 +811,7 @@ private async Task<RazorVSInternalCompletionList> VerifyCompletionListAsync(
792811
NoOpTelemetryReporter.Instance,
793812
LoggerFactory);
794813

795-
var request = new CompletionParams()
814+
var request = new RazorVSInternalCompletionParams()
796815
{
797816
TextDocument = new TextDocumentIdentifier()
798817
{
@@ -826,7 +845,7 @@ private async Task<RazorVSInternalCompletionList> VerifyCompletionListAsync(
826845

827846
if (!commitElementsWithSpace)
828847
{
829-
Assert.False(result.Items.Any(item => item.CommitCharacters?.First().Contains(" ") ?? false));
848+
Assert.False(result.Items.Any(item => item.CommitCharacters?.First().Contains(' ') ?? false));
830849
}
831850

832851
if (!autoInsertAttributeQuotes)
@@ -855,6 +874,51 @@ private async Task<RazorVSInternalCompletionList> VerifyCompletionListAsync(
855874
return result;
856875
}
857876

877+
private async Task VerifyCompletionListParamsTypeAsync(
878+
TestCode input,
879+
VSInternalCompletionContext completionContext,
880+
RazorFileKind? fileKind = null)
881+
{
882+
var document = CreateProjectAndRazorDocument(input.Text, fileKind);
883+
var sourceText = await document.GetTextAsync(DisposalToken);
884+
885+
// Assert the request invoker is passed a RazorVSInternalCompletionParams
886+
var requestInvoker = new TestHtmlRequestInvoker((Methods.TextDocumentCompletionName, ValidateArgType));
887+
888+
var languageServerFeatureOptions = new TestLanguageServerFeatureOptions();
889+
890+
var completionListCache = new CompletionListCache();
891+
var endpoint = new CohostDocumentCompletionEndpoint(
892+
IncompatibleProjectService,
893+
RemoteServiceInvoker,
894+
ClientSettingsManager,
895+
ClientCapabilitiesService,
896+
snippetCompletionItemProvider: null,
897+
languageServerFeatureOptions,
898+
requestInvoker,
899+
completionListCache,
900+
NoOpTelemetryReporter.Instance,
901+
LoggerFactory);
902+
903+
var request = new RazorVSInternalCompletionParams()
904+
{
905+
TextDocument = new TextDocumentIdentifier()
906+
{
907+
DocumentUri = document.CreateDocumentUri()
908+
},
909+
Position = sourceText.GetPosition(input.Position),
910+
Context = completionContext
911+
};
912+
913+
await endpoint.GetTestAccessor().HandleRequestAsync(request, document, DisposalToken);
914+
915+
static RazorVSInternalCompletionList? ValidateArgType(object arg)
916+
{
917+
Assert.Equal(typeof(RazorVSInternalCompletionParams), arg.GetType());
918+
return default;
919+
}
920+
}
921+
858922
private async Task VerifyCompletionResolveAsync(CodeAnalysis.TextDocument document, CompletionListCache completionListCache, VSInternalCompletionItem item, string? expected, string expectedResolvedItemDescription)
859923
{
860924
// We expect data to be a JsonElement, so for tests we have to _not_ strongly type
@@ -917,7 +981,7 @@ private async Task VerifyCompletionResolveAsync(CodeAnalysis.TextDocument docume
917981
}
918982
}
919983

920-
private string? FlattenDescription(ClassifiedTextElement? description)
984+
private static string? FlattenDescription(ClassifiedTextElement? description)
921985
{
922986
if (description is null)
923987
{

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/Shared/CohostRenameEndpointTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ private async Task VerifyRenamesAsync(
178178
var inputText = await document.GetTextAsync(DisposalToken);
179179
var position = inputText.GetPosition(cursorPosition);
180180

181-
var requestInvoker = new TestHtmlRequestInvoker([(Methods.TextDocumentRenameName, null)]);
181+
var requestInvoker = new TestHtmlRequestInvoker([(Methods.TextDocumentRenameName, (object?)null)]);
182182

183183
var endpoint = new CohostRenameEndpoint(IncompatibleProjectService, RemoteServiceInvoker, requestInvoker);
184184

‎src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/Cohost/Shared/CohostSignatureHelpEndpointTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ private async Task VerifySignatureHelpAsync(string input, string expected, bool
9494

9595
ClientSettingsManager.Update(ClientCompletionSettings.Default with { AutoListParams = autoListParams });
9696

97-
var requestInvoker = new TestHtmlRequestInvoker([(Methods.TextDocumentSignatureHelpName, null)]);
97+
var requestInvoker = new TestHtmlRequestInvoker([(Methods.TextDocumentSignatureHelpName, (object?)null)]);
9898

9999
var endpoint = new CohostSignatureHelpEndpoint(IncompatibleProjectService, RemoteServiceInvoker, ClientSettingsManager, requestInvoker);
100100

0 commit comments

Comments
 (0)
0