Skip to content

Commit 7a165d5

Browse files
authored
Use work done progress to handle restore server side (#81233)
Followup to the canonical misc changes. This PR has two parts ## Move server initiated restore entirely to server Previously restores initiated by the server had to call out to the client to setup the correct UI, which then called back to the server to run the restore, all using custom LSP requests. This wasn't ideal as it meant extra custom client side code and an extra server -> client -> server loop. Instead, we can utilize the standard LSP [server initiated work done progress](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#serverInitiatedProgress) API to display progress from the server on the client side. This means server initiated restores no longer need any custom client code, and the server can choose when to display a UI without a client code change. This did require a bit of complex code to handle either the server or client cancelling the request. Additionally, we still have custom requests for manually initiated restores from the client. ## Hide restore progress for canonical files Utilizing the above change, I modified the server behavior to not show any progress when restoring the canonical file. This is an internal server operation and does not need to be displayed client side (based on feedback it only causes confusion). CLient side change - dotnet/vscode-csharp#8780
1 parent 935ec83 commit 7a165d5

File tree

14 files changed

+386
-56
lines changed

14 files changed

+386
-56
lines changed

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerClientTests.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Roslyn.LanguageServer.Protocol;
1515
using Roslyn.Test.Utilities;
1616
using Roslyn.Utilities;
17+
using StreamJsonRpc;
1718
using Xunit.Abstractions;
1819
using LSP = Roslyn.LanguageServer.Protocol;
1920

@@ -74,9 +75,10 @@ await File.WriteAllTextAsync(projectPath, $"""
7475
documents: files,
7576
locations: annotatedLocations);
7677

77-
// Perform restore and mock up project restore client handler
78+
// Perform restore
7879
ProcessUtilities.Run("dotnet", $"restore --project {projectPath}");
79-
lspClient.AddClientLocalRpcTarget(ProjectDependencyHelper.ProjectNeedsRestoreName, (string[] projectFilePaths) => { });
80+
81+
lspClient.AddClientLocalRpcTarget(new WorkDoneProgressTarget());
8082

8183
// Listen for project initialization
8284
var projectInitialized = new TaskCompletionSource();
@@ -92,6 +94,15 @@ await File.WriteAllTextAsync(projectPath, $"""
9294
return lspClient;
9395
}
9496

97+
private class WorkDoneProgressTarget
98+
{
99+
[JsonRpcMethod(Methods.WindowWorkDoneProgressCreateName, UseSingleObjectParameterDeserialization = true)]
100+
public Task HandleCreateWorkDoneProgress(WorkDoneProgressCreateParams _1, CancellationToken _2) => Task.CompletedTask;
101+
102+
[JsonRpcMethod(Methods.ProgressNotificationName, UseSingleObjectParameterDeserialization = true)]
103+
public Task HandleProgress((string token, object value) _1, CancellationToken _2) => Task.CompletedTask;
104+
}
105+
95106
private protected static Dictionary<string, IList<LSP.Location>> GetAnnotatedLocations(DocumentUri codeUri, SourceText text, ImmutableDictionary<string, ImmutableArray<TextSpan>> spanMap)
96107
{
97108
var locations = new Dictionary<string, IList<LSP.Location>>();

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/CanonicalMiscFilesProjectLoader.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ internal sealed class CanonicalMiscFilesProjectLoader : LanguageServerProjectLoa
2929
{
3030
private readonly Lazy<string> _canonicalDocumentPath;
3131

32+
/// <summary>
33+
/// Avoid showing restore notifications for misc files - it ends up being noisy and confusing
34+
/// as every file is a misc file on first open until we detect a project for it.
35+
/// </summary>
36+
protected override bool EnableProgressReporting => false;
37+
3238
public CanonicalMiscFilesProjectLoader(
3339
LanguageServerWorkspaceFactory workspaceFactory,
3440
IFileChangeWatcher fileChangeWatcher,
@@ -37,7 +43,8 @@ public CanonicalMiscFilesProjectLoader(
3743
IAsynchronousOperationListenerProvider listenerProvider,
3844
ProjectLoadTelemetryReporter projectLoadTelemetry,
3945
ServerConfigurationFactory serverConfigurationFactory,
40-
IBinLogPathProvider binLogPathProvider)
46+
IBinLogPathProvider binLogPathProvider,
47+
DotnetCliHelper dotnetCliHelper)
4148
: base(
4249
workspaceFactory,
4350
fileChangeWatcher,
@@ -46,7 +53,8 @@ public CanonicalMiscFilesProjectLoader(
4653
listenerProvider,
4754
projectLoadTelemetry,
4855
serverConfigurationFactory,
49-
binLogPathProvider)
56+
binLogPathProvider,
57+
dotnetCliHelper)
5058
{
5159
_canonicalDocumentPath = new Lazy<string>(() =>
5260
{

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/FileBasedProgramsProjectSystem.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ public FileBasedProgramsProjectSystem(
4444
IAsynchronousOperationListenerProvider listenerProvider,
4545
ProjectLoadTelemetryReporter projectLoadTelemetry,
4646
ServerConfigurationFactory serverConfigurationFactory,
47-
IBinLogPathProvider binLogPathProvider)
47+
IBinLogPathProvider binLogPathProvider,
48+
DotnetCliHelper dotnetCliHelper)
4849
: base(
4950
workspaceFactory,
5051
fileChangeWatcher,
@@ -53,7 +54,8 @@ public FileBasedProgramsProjectSystem(
5354
listenerProvider,
5455
projectLoadTelemetry,
5556
serverConfigurationFactory,
56-
binLogPathProvider)
57+
binLogPathProvider,
58+
dotnetCliHelper)
5759
{
5860
_lspServices = lspServices;
5961
_logger = loggerFactory.CreateLogger<FileBasedProgramsProjectSystem>();
@@ -66,7 +68,8 @@ public FileBasedProgramsProjectSystem(
6668
listenerProvider,
6769
projectLoadTelemetry,
6870
serverConfigurationFactory,
69-
binLogPathProvider);
71+
binLogPathProvider,
72+
dotnetCliHelper);
7073
}
7174

7275
private string GetDocumentFilePath(DocumentUri uri) => uri.ParsedUri is { } parsedUri ? ProtocolConversions.GetDocumentFilePathFromUri(parsedUri) : uri.UriString;

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/FileBasedProgramsWorkspaceProviderFactory.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ internal sealed class FileBasedProgramsWorkspaceProviderFactory(
3434
IAsynchronousOperationListenerProvider listenerProvider,
3535
ProjectLoadTelemetryReporter projectLoadTelemetry,
3636
ServerConfigurationFactory serverConfigurationFactory,
37-
IBinLogPathProvider binLogPathProvider) : ILspMiscellaneousFilesWorkspaceProviderFactory
37+
IBinLogPathProvider binLogPathProvider,
38+
DotnetCliHelper dotnetCliHelper) : ILspMiscellaneousFilesWorkspaceProviderFactory
3839
{
3940
public ILspMiscellaneousFilesWorkspaceProvider CreateLspMiscellaneousFilesWorkspaceProvider(ILspServices lspServices, HostServices hostServices)
4041
{
@@ -48,6 +49,7 @@ public ILspMiscellaneousFilesWorkspaceProvider CreateLspMiscellaneousFilesWorksp
4849
listenerProvider,
4950
projectLoadTelemetry,
5051
serverConfigurationFactory,
51-
binLogPathProvider);
52+
binLogPathProvider,
53+
dotnetCliHelper);
5254
}
5355
}

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ internal abstract class LanguageServerProjectLoader
3636
private readonly ILogger _logger;
3737
private readonly ProjectLoadTelemetryReporter _projectLoadTelemetryReporter;
3838
private readonly IBinLogPathProvider _binLogPathProvider;
39+
private readonly DotnetCliHelper _dotnetCliHelper;
3940
protected readonly ImmutableDictionary<string, string> AdditionalProperties;
4041

4142
/// <summary>
@@ -84,6 +85,11 @@ public sealed record Primordial(ProjectSystemProjectFactory PrimordialProjectFac
8485
public sealed record LoadedTargets(ImmutableArray<LoadedProject> LoadedProjectTargets) : ProjectLoadState;
8586
}
8687

88+
/// <summary>
89+
/// Indicates whether loads should report UI progress to the client for this loader.
90+
/// </summary>
91+
protected virtual bool EnableProgressReporting => true;
92+
8793
protected LanguageServerProjectLoader(
8894
LanguageServerWorkspaceFactory workspaceFactory,
8995
IFileChangeWatcher fileChangeWatcher,
@@ -92,7 +98,8 @@ protected LanguageServerProjectLoader(
9298
IAsynchronousOperationListenerProvider listenerProvider,
9399
ProjectLoadTelemetryReporter projectLoadTelemetry,
94100
ServerConfigurationFactory serverConfigurationFactory,
95-
IBinLogPathProvider binLogPathProvider)
101+
IBinLogPathProvider binLogPathProvider,
102+
DotnetCliHelper dotnetCliHelper)
96103
{
97104
_workspaceFactory = workspaceFactory;
98105
_fileChangeWatcher = fileChangeWatcher;
@@ -101,6 +108,7 @@ protected LanguageServerProjectLoader(
101108
_logger = loggerFactory.CreateLogger(nameof(LanguageServerProjectLoader));
102109
_projectLoadTelemetryReporter = projectLoadTelemetry;
103110
_binLogPathProvider = binLogPathProvider;
111+
_dotnetCliHelper = dotnetCliHelper;
104112

105113
AdditionalProperties = BuildAdditionalProperties(serverConfigurationFactory.ServerConfiguration);
106114

@@ -176,12 +184,8 @@ private async ValueTask ReloadProjectsAsync(ImmutableSegmentedList<ProjectToLoad
176184

177185
if (GlobalOptionService.GetOption(LanguageServerProjectSystemOptionsStorage.EnableAutomaticRestore) && projectsThatNeedRestore.Any())
178186
{
179-
// Tell the client to restore any projects with unresolved dependencies.
180-
// This should eventually move entirely server side once we have a mechanism for reporting generic project load progress.
181-
// Tracking: https://github.com/dotnet/vscode-csharp/issues/6675
182-
//
183-
// The request blocks to ensure we aren't trying to run a design time build at the same time as a restore.
184-
await ProjectDependencyHelper.SendProjectNeedsRestoreRequestAsync(projectsThatNeedRestore, cancellationToken);
187+
// This request blocks to ensure we aren't trying to run a design time build at the same time as a restore.
188+
await ProjectDependencyHelper.RestoreProjectsAsync(projectsThatNeedRestore, EnableProgressReporting, _dotnetCliHelper, _logger, cancellationToken);
185189
}
186190
}
187191
finally

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectSystem.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ public LanguageServerProjectSystem(
3535
IAsynchronousOperationListenerProvider listenerProvider,
3636
ProjectLoadTelemetryReporter projectLoadTelemetry,
3737
ServerConfigurationFactory serverConfigurationFactory,
38-
IBinLogPathProvider binLogPathProvider)
38+
IBinLogPathProvider binLogPathProvider,
39+
DotnetCliHelper dotnetCliHelper)
3940
: base(
4041
workspaceFactory,
4142
fileChangeWatcher,
@@ -44,7 +45,8 @@ public LanguageServerProjectSystem(
4445
listenerProvider,
4546
projectLoadTelemetry,
4647
serverConfigurationFactory,
47-
binLogPathProvider)
48+
binLogPathProvider,
49+
dotnetCliHelper)
4850
{
4951
_logger = loggerFactory.CreateLogger(nameof(LanguageServerProjectSystem));
5052
_hostProjectFactory = workspaceFactory.HostProjectFactory;

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/ProjectDependencyHelper.cs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Collections.Immutable;
6-
using System.Text.Json.Serialization;
6+
using Microsoft.CodeAnalysis.LanguageServer.Handler;
77
using Microsoft.CodeAnalysis.LanguageServer.LanguageServer;
88
using Microsoft.CodeAnalysis.MSBuild;
99
using Microsoft.CodeAnalysis.PooledObjects;
@@ -16,8 +16,6 @@ namespace Microsoft.CodeAnalysis.LanguageServer.HostWorkspace;
1616

1717
internal static class ProjectDependencyHelper
1818
{
19-
internal const string ProjectNeedsRestoreName = "workspace/_roslyn_projectNeedsRestore";
20-
2119
internal static bool NeedsRestore(ProjectFileInfo newProjectFileInfo, ProjectFileInfo? previousProjectFileInfo, ILogger logger)
2220
{
2321
if (previousProjectFileInfo is null)
@@ -121,20 +119,24 @@ static bool SatisfiesVersion(VersionRange requestedVersionRange, NuGetVersion pr
121119
}
122120
}
123121

124-
internal static async Task SendProjectNeedsRestoreRequestAsync(ImmutableArray<string> projectPaths, CancellationToken cancellationToken)
122+
internal static async Task RestoreProjectsAsync(ImmutableArray<string> projectPaths, bool enableProgressReporting, DotnetCliHelper dotnetCliHelper, ILogger logger, CancellationToken cancellationToken)
125123
{
126124
if (projectPaths.IsEmpty)
127125
return;
128126

129127
Contract.ThrowIfNull(LanguageServerHost.Instance, "We don't have an LSP channel yet to send this request through.");
130128

131-
var languageServerManager = LanguageServerHost.Instance.GetRequiredLspService<IClientLanguageServerManager>();
129+
var workDoneProgressManager = LanguageServerHost.Instance.GetRequiredLspService<WorkDoneProgressManager>();
132130

133-
// Ensure we only pass unique paths back to be restored.
134-
var unresolvedParams = new UnresolvedDependenciesParams([.. projectPaths.Distinct()]);
135-
await languageServerManager.SendRequestAsync(ProjectNeedsRestoreName, unresolvedParams, cancellationToken);
131+
try
132+
{
133+
await RestoreHandler.RestoreAsync(projectPaths, workDoneProgressManager, dotnetCliHelper, logger, enableProgressReporting, cancellationToken);
134+
}
135+
catch (OperationCanceledException)
136+
{
137+
// Restore was cancelled. This is not a failure, it just leaves the project unrestored or partially restored (same as if the user cancelled a CLI restore).
138+
// We don't want this exception to bubble up to the project load queue however as it may need to additional work after this call.
139+
logger.LogWarning("Project restore was canceled.");
140+
}
136141
}
137-
138-
private sealed record UnresolvedDependenciesParams(
139-
[property: JsonPropertyName("projectFilePaths")] string[] ProjectFilePaths);
140142
}

0 commit comments

Comments
 (0)