Skip to content

Commit aa1041b

Browse files
Fix overly aggressive delegated completion (#11965)
* Fix overly aggressive delegated completion We were losing VSInternalCompletionContext.InvokeKind value when re-invoking completion request on delegated servers (C#, HTML). That caused default value of "Invoked" being used in delegated server, which basically means Ctrl+J/Show Completion command. That caused overly aggressive behavior as if completion was forced rather than auto-shown by typing. This change properly types Context of CompletionParams so that VSInternalCompletionContext is properly serialized/deserialized and InvokeKind value doesn't get lost. * Formatting suggestion Co-authored-by: Dustin Campbell <[email protected]> * Formatting suggestion Co-authored-by: Dustin Campbell <[email protected]> * Adding link to the CompletionParams source per CR suggestion --------- Co-authored-by: Dustin Campbell <[email protected]>
1 parent 0d68401 commit aa1041b

File tree

3 files changed

+89
-2
lines changed

3 files changed

+89
-2
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT license. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Text.Json.Serialization;
6+
7+
namespace Roslyn.LanguageServer.Protocol;
8+
9+
/// <summary>
10+
/// A replacement of the LSP protocol <see cref="CompletionParams"/> that ensures correct serialization between LSP servers.
11+
/// </summary>
12+
/// <remarks>
13+
/// This is the same as the LSP protocol <see cref="CompletionParams"/> except that it strongly types the <see cref="Context"/> property as VSInternalCompletionContext,
14+
/// because our custom message target gets handled by a JsonRpc connection set up by the editor, that has no Roslyn converters.
15+
/// Without using this class, we lose VSInternalCompletionContext.InvokeKind property when calling Roslyn or HTML servers from Razor
16+
/// which results in default value of "Invoked" to be used and can cause overly aggressive completion list.
17+
///
18+
/// See original CompletionParams here https://github.com/dotnet/roslyn/blob/98d41b80f6a192230c045a6576e2a283a407980b/src/LanguageServer/Protocol/Protocol/CompletionParams.cs
19+
/// </remarks>
20+
internal sealed class RazorVSInternalCompletionParams : TextDocumentPositionParams, IPartialResultParams<SumType<CompletionItem[], CompletionList>?>, IWorkDoneProgressOptions
21+
{
22+
public RazorVSInternalCompletionParams()
23+
{
24+
}
25+
26+
/// <summary>
27+
/// The completion context. This is only available if the client specifies the
28+
/// client capability <see cref="CompletionSetting.ContextSupport"/>.
29+
/// </summary>
30+
[JsonPropertyName("context")]
31+
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
32+
public VSInternalCompletionContext? Context { get; set; }
33+
34+
/// <inheritdoc/>
35+
[JsonPropertyName(Methods.PartialResultTokenName)]
36+
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
37+
public IProgress<SumType<CompletionItem[], CompletionList>?>? PartialResultToken { get; set; }
38+
39+
/// <inheritdoc/>
40+
[JsonPropertyName("workDoneProgress")]
41+
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
42+
public bool WorkDoneProgress { get; init; }
43+
}

src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/LanguageClient/Endpoints/RazorCustomMessageTarget_Completion.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ internal partial class RazorCustomMessageTarget
9797
return null;
9898
}
9999

100-
var completionParams = new CompletionParams()
100+
var completionParams = new RazorVSInternalCompletionParams()
101101
{
102102
Context = request.Context,
103103
Position = request.ProjectedPosition,
@@ -135,7 +135,7 @@ internal partial class RazorCustomMessageTarget
135135
ReinvocationResponse<RazorVSInternalCompletionList?>? response;
136136
using (_telemetryReporter.TrackLspRequest(lspMethodName, languageServerName, TelemetryThresholds.CompletionSubLSPTelemetryThreshold, request.CorrelationId))
137137
{
138-
response = await _requestInvoker.ReinvokeRequestOnServerAsync<CompletionParams, RazorVSInternalCompletionList?>(
138+
response = await _requestInvoker.ReinvokeRequestOnServerAsync<RazorVSInternalCompletionParams, RazorVSInternalCompletionList?>(
139139
textBuffer,
140140
lspMethodName,
141141
languageServerName,

src/Razor/test/Microsoft.VisualStudio.Razor.IntegrationTests/CompletionIntegrationTests.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,50 @@ await VerifyTypeAndCommitCompletionAsync(
165165
expectedSelectedItemLabel: "@onactivate");
166166
}
167167

168+
// Regression test for https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2505611
169+
[IdeFact]
170+
public async Task CompletionCommit_NoCommitOnTypingInDocComments()
171+
{
172+
await TestServices.SolutionExplorer.AddFileAsync(
173+
RazorProjectConstants.BlazorProjectName,
174+
"Test.razor",
175+
"""
176+
@page "/test"
177+
178+
<PageTitle>Test</PageTitle>
179+
180+
@{
181+
///
182+
}
183+
""",
184+
open: true,
185+
ControlledHangMitigatingCancellationToken);
186+
187+
var textView = await TestServices.Editor.GetActiveTextViewAsync(HangMitigatingCancellationToken);
188+
await TestServices.Editor.WaitForComponentClassificationAsync(ControlledHangMitigatingCancellationToken);
189+
190+
await TestServices.Editor.PlaceCaretAsync("/// ", charsOffset: 1, ControlledHangMitigatingCancellationToken);
191+
TestServices.Input.Send("add a function");
192+
193+
// Make sure extra text didn't get commited from an unexpected completion list
194+
var currentLineText = await TestServices.Editor.GetCurrentLineTextAsync(HangMitigatingCancellationToken);
195+
Assert.Contains("/// add a function", currentLineText);
196+
197+
// Make sure completion doesn't come up for 15 seconds
198+
var completionSession = await TestServices.Editor.WaitForCompletionSessionAsync(s_snippetTimeout, HangMitigatingCancellationToken);
199+
var items = completionSession?.GetComputedItems(HangMitigatingCancellationToken);
200+
201+
if (items is null)
202+
{
203+
// No items to check, we're good
204+
return;
205+
}
206+
207+
// If completion did pop up with something like "Processing", make sure no doccomment items are present
208+
Assert.DoesNotContain("summary", items.Items.Select(i => i.DisplayText));
209+
210+
}
211+
168212
[IdeFact]
169213
public async Task CompletionCommit_HtmlTag()
170214
{

0 commit comments

Comments
 (0)