Skip to content

Use work done progress to handle restore server side #81233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 19, 2025
Merged

Use work done progress to handle restore server side #81233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7759992
Use work done progress to handle restore server side
dibarbet Nov 12, 2025
faaf857
Updated based on review feedback
dibarbet Nov 18, 2025
9841b74
remove unused using
dibarbet Nov 18, 2025
58e2ed3
feedback
dibarbet Nov 18, 2025
f8c7e1e
fix test crash
dibarbet Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions ...soft.CodeAnalysis.LanguageServer.UnitTests/Utilities/AbstractLanguageServerClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Roslyn.LanguageServer.Protocol;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
using StreamJsonRpc;
using Xunit.Abstractions;
using LSP = Roslyn.LanguageServer.Protocol;

Expand Down Expand Up @@ -74,9 +75,10 @@ await File.WriteAllTextAsync(projectPath, $"""
documents: files,
locations: annotatedLocations);

// Perform restore and mock up project restore client handler
// Perform restore
ProcessUtilities.Run("dotnet", $"restore --project {projectPath}");
Copy link
Member

@RikkiGibson RikkiGibson Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you feel the new WorkDoneProgress code is exercised well by the existing tests?

Copy link
Member Author

@dibarbet dibarbet Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restore wasn't really exercised by the tests at all really. We likely need integration tests for this that actually do the full load / restore loop.

lspClient.AddClientLocalRpcTarget(ProjectDependencyHelper.ProjectNeedsRestoreName, (string[] projectFilePaths) => { });

lspClient.AddClientLocalRpcTarget(new WorkDoneProgressTarget());

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

private class WorkDoneProgressTarget
{
[JsonRpcMethod(Methods.WindowWorkDoneProgressCreateName, UseSingleObjectParameterDeserialization = true)]
public Task HandleCreateWorkDoneProgress(WorkDoneProgressCreateParams _1, CancellationToken _2) => Task.CompletedTask;

[JsonRpcMethod(Methods.ProgressNotificationName, UseSingleObjectParameterDeserialization = true)]
public Task HandleProgress((string token, object value) _1, CancellationToken _2) => Task.CompletedTask;
}

private protected static Dictionary<string, IList<LSP.Location>> GetAnnotatedLocations(DocumentUri codeUri, SourceText text, ImmutableDictionary<string, ImmutableArray<TextSpan>> spanMap)
{
var locations = new Dictionary<string, IList<LSP.Location>>();
Expand Down
12 changes: 10 additions & 2 deletions ...icrosoft.CodeAnalysis.LanguageServer/FileBasedPrograms/CanonicalMiscFilesProjectLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ internal sealed class CanonicalMiscFilesProjectLoader : LanguageServerProjectLoa
{
private readonly Lazy<string> _canonicalDocumentPath;

/// <summary>
/// Avoid showing restore notifications for misc files - it ends up being noisy and confusing
/// as every file is a misc file on first open until we detect a project for it.
/// </summary>
protected override bool EnableProgressReporting => false;

public CanonicalMiscFilesProjectLoader(
LanguageServerWorkspaceFactory workspaceFactory,
IFileChangeWatcher fileChangeWatcher,
Expand All @@ -37,7 +43,8 @@ public CanonicalMiscFilesProjectLoader(
IAsynchronousOperationListenerProvider listenerProvider,
ProjectLoadTelemetryReporter projectLoadTelemetry,
ServerConfigurationFactory serverConfigurationFactory,
IBinLogPathProvider binLogPathProvider)
IBinLogPathProvider binLogPathProvider,
DotnetCliHelper dotnetCliHelper)
: base(
workspaceFactory,
fileChangeWatcher,
Expand All @@ -46,7 +53,8 @@ public CanonicalMiscFilesProjectLoader(
listenerProvider,
projectLoadTelemetry,
serverConfigurationFactory,
binLogPathProvider)
binLogPathProvider,
dotnetCliHelper)
{
_canonicalDocumentPath = new Lazy<string>(() =>
{
Expand Down
9 changes: 6 additions & 3 deletions ...Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/FileBasedProgramsProjectSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public FileBasedProgramsProjectSystem(
IAsynchronousOperationListenerProvider listenerProvider,
ProjectLoadTelemetryReporter projectLoadTelemetry,
ServerConfigurationFactory serverConfigurationFactory,
IBinLogPathProvider binLogPathProvider)
IBinLogPathProvider binLogPathProvider,
DotnetCliHelper dotnetCliHelper)
: base(
workspaceFactory,
fileChangeWatcher,
Expand All @@ -53,7 +54,8 @@ public FileBasedProgramsProjectSystem(
listenerProvider,
projectLoadTelemetry,
serverConfigurationFactory,
binLogPathProvider)
binLogPathProvider,
dotnetCliHelper)
{
_lspServices = lspServices;
_logger = loggerFactory.CreateLogger<FileBasedProgramsProjectSystem>();
Expand All @@ -66,7 +68,8 @@ public FileBasedProgramsProjectSystem(
listenerProvider,
projectLoadTelemetry,
serverConfigurationFactory,
binLogPathProvider);
binLogPathProvider,
dotnetCliHelper);
}

private string GetDocumentFilePath(DocumentUri uri) => uri.ParsedUri is { } parsedUri ? ProtocolConversions.GetDocumentFilePathFromUri(parsedUri) : uri.UriString;
Expand Down
6 changes: 4 additions & 2 deletions ...odeAnalysis.LanguageServer/FileBasedPrograms/FileBasedProgramsWorkspaceProviderFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ internal sealed class FileBasedProgramsWorkspaceProviderFactory(
IAsynchronousOperationListenerProvider listenerProvider,
ProjectLoadTelemetryReporter projectLoadTelemetry,
ServerConfigurationFactory serverConfigurationFactory,
IBinLogPathProvider binLogPathProvider) : ILspMiscellaneousFilesWorkspaceProviderFactory
IBinLogPathProvider binLogPathProvider,
DotnetCliHelper dotnetCliHelper) : ILspMiscellaneousFilesWorkspaceProviderFactory
{
public ILspMiscellaneousFilesWorkspaceProvider CreateLspMiscellaneousFilesWorkspaceProvider(ILspServices lspServices, HostServices hostServices)
{
Expand All @@ -48,6 +49,7 @@ public ILspMiscellaneousFilesWorkspaceProvider CreateLspMiscellaneousFilesWorksp
listenerProvider,
projectLoadTelemetry,
serverConfigurationFactory,
binLogPathProvider);
binLogPathProvider,
dotnetCliHelper);
}
}
18 changes: 11 additions & 7 deletions ...Server/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ internal abstract class LanguageServerProjectLoader
private readonly ILogger _logger;
private readonly ProjectLoadTelemetryReporter _projectLoadTelemetryReporter;
private readonly IBinLogPathProvider _binLogPathProvider;
private readonly DotnetCliHelper _dotnetCliHelper;
protected readonly ImmutableDictionary<string, string> AdditionalProperties;

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

/// <summary>
/// Indicates whether loads should report UI progress to the client for this loader.
/// </summary>
protected virtual bool EnableProgressReporting => true;

protected LanguageServerProjectLoader(
LanguageServerWorkspaceFactory workspaceFactory,
IFileChangeWatcher fileChangeWatcher,
Expand All @@ -92,7 +98,8 @@ protected LanguageServerProjectLoader(
IAsynchronousOperationListenerProvider listenerProvider,
ProjectLoadTelemetryReporter projectLoadTelemetry,
ServerConfigurationFactory serverConfigurationFactory,
IBinLogPathProvider binLogPathProvider)
IBinLogPathProvider binLogPathProvider,
DotnetCliHelper dotnetCliHelper)
{
_workspaceFactory = workspaceFactory;
_fileChangeWatcher = fileChangeWatcher;
Expand All @@ -101,6 +108,7 @@ protected LanguageServerProjectLoader(
_logger = loggerFactory.CreateLogger(nameof(LanguageServerProjectLoader));
_projectLoadTelemetryReporter = projectLoadTelemetry;
_binLogPathProvider = binLogPathProvider;
_dotnetCliHelper = dotnetCliHelper;

AdditionalProperties = BuildAdditionalProperties(serverConfigurationFactory.ServerConfiguration);

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

if (GlobalOptionService.GetOption(LanguageServerProjectSystemOptionsStorage.EnableAutomaticRestore) && projectsThatNeedRestore.Any())
{
// Tell the client to restore any projects with unresolved dependencies.
// This should eventually move entirely server side once we have a mechanism for reporting generic project load progress.
// Tracking: https://github.com/dotnet/vscode-csharp/issues/6675
//
// The request blocks to ensure we aren't trying to run a design time build at the same time as a restore.
await ProjectDependencyHelper.SendProjectNeedsRestoreRequestAsync(projectsThatNeedRestore, cancellationToken);
// This request blocks to ensure we aren't trying to run a design time build at the same time as a restore.
await ProjectDependencyHelper.RestoreProjectsAsync(projectsThatNeedRestore, EnableProgressReporting, _dotnetCliHelper, _logger, cancellationToken);
}
}
finally
Expand Down
6 changes: 4 additions & 2 deletions ...Server/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public LanguageServerProjectSystem(
IAsynchronousOperationListenerProvider listenerProvider,
ProjectLoadTelemetryReporter projectLoadTelemetry,
ServerConfigurationFactory serverConfigurationFactory,
IBinLogPathProvider binLogPathProvider)
IBinLogPathProvider binLogPathProvider,
DotnetCliHelper dotnetCliHelper)
: base(
workspaceFactory,
fileChangeWatcher,
Expand All @@ -44,7 +45,8 @@ public LanguageServerProjectSystem(
listenerProvider,
projectLoadTelemetry,
serverConfigurationFactory,
binLogPathProvider)
binLogPathProvider,
dotnetCliHelper)
{
_logger = loggerFactory.CreateLogger(nameof(LanguageServerProjectSystem));
_hostProjectFactory = workspaceFactory.HostProjectFactory;
Expand Down
24 changes: 13 additions & 11 deletions ...uageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/ProjectDependencyHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Text.Json.Serialization;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.LanguageServer.LanguageServer;
using Microsoft.CodeAnalysis.MSBuild;
using Microsoft.CodeAnalysis.PooledObjects;
Expand All @@ -16,8 +16,6 @@ namespace Microsoft.CodeAnalysis.LanguageServer.HostWorkspace;

internal static class ProjectDependencyHelper
{
internal const string ProjectNeedsRestoreName = "workspace/_roslyn_projectNeedsRestore";

internal static bool NeedsRestore(ProjectFileInfo newProjectFileInfo, ProjectFileInfo? previousProjectFileInfo, ILogger logger)
{
if (previousProjectFileInfo is null)
Expand Down Expand Up @@ -121,20 +119,24 @@ static bool SatisfiesVersion(VersionRange requestedVersionRange, NuGetVersion pr
}
}

internal static async Task SendProjectNeedsRestoreRequestAsync(ImmutableArray<string> projectPaths, CancellationToken cancellationToken)
internal static async Task RestoreProjectsAsync(ImmutableArray<string> projectPaths, bool enableProgressReporting, DotnetCliHelper dotnetCliHelper, ILogger logger, CancellationToken cancellationToken)
{
if (projectPaths.IsEmpty)
return;

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

var languageServerManager = LanguageServerHost.Instance.GetRequiredLspService<IClientLanguageServerManager>();
var workDoneProgressManager = LanguageServerHost.Instance.GetRequiredLspService<WorkDoneProgressManager>();

// Ensure we only pass unique paths back to be restored.
var unresolvedParams = new UnresolvedDependenciesParams([.. projectPaths.Distinct()]);
await languageServerManager.SendRequestAsync(ProjectNeedsRestoreName, unresolvedParams, cancellationToken);
try
{
await RestoreHandler.RestoreAsync(projectPaths, workDoneProgressManager, dotnetCliHelper, logger, enableProgressReporting, cancellationToken);
}
catch (OperationCanceledException)
{
// 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).
// 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.
logger.LogWarning("Project restore was canceled.");
}
}

private sealed record UnresolvedDependenciesParams(
[property: JsonPropertyName("projectFilePaths")] string[] ProjectFilePaths);
}
Loading
Loading