Use work done progress to handle restore server side

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

@@ -74,9 +74,8 @@ 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}");
-        lspClient.AddClientLocalRpcTarget(ProjectDependencyHelper.ProjectNeedsRestoreName, (string[] projectFilePaths) => { });
 
         // Listen for project initialization
         var projectInitialized = new TaskCompletionSource();

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

@@ -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,
@@ -37,7 +43,8 @@ public CanonicalMiscFilesProjectLoader(
         IAsynchronousOperationListenerProvider listenerProvider,
         ProjectLoadTelemetryReporter projectLoadTelemetry,
         ServerConfigurationFactory serverConfigurationFactory,
-        IBinLogPathProvider binLogPathProvider)
+        IBinLogPathProvider binLogPathProvider,
+        DotnetCliHelper dotnetCliHelper)
             : base(
                 workspaceFactory,
                 fileChangeWatcher,
@@ -46,7 +53,8 @@ public CanonicalMiscFilesProjectLoader(
                 listenerProvider,
                 projectLoadTelemetry,
                 serverConfigurationFactory,
-                binLogPathProvider)
+                binLogPathProvider,
+                dotnetCliHelper)
     {
         _canonicalDocumentPath = new Lazy<string>(() =>
         {

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

@@ -44,7 +44,8 @@ public FileBasedProgramsProjectSystem(
         IAsynchronousOperationListenerProvider listenerProvider,
         ProjectLoadTelemetryReporter projectLoadTelemetry,
         ServerConfigurationFactory serverConfigurationFactory,
-        IBinLogPathProvider binLogPathProvider)
+        IBinLogPathProvider binLogPathProvider,
+        DotnetCliHelper dotnetCliHelper)
             : base(
                 workspaceFactory,
                 fileChangeWatcher,
@@ -53,7 +54,8 @@ public FileBasedProgramsProjectSystem(
                 listenerProvider,
                 projectLoadTelemetry,
                 serverConfigurationFactory,
-                binLogPathProvider)
+                binLogPathProvider,
+                dotnetCliHelper)
     {
         _lspServices = lspServices;
         _logger = loggerFactory.CreateLogger<FileBasedProgramsProjectSystem>();
@@ -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;

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

@@ -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)
     {
@@ -48,6 +49,7 @@ public ILspMiscellaneousFilesWorkspaceProvider CreateLspMiscellaneousFilesWorksp
             listenerProvider,
             projectLoadTelemetry,
             serverConfigurationFactory,
-            binLogPathProvider);
+            binLogPathProvider,
+            dotnetCliHelper);
     }
 }

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

@@ -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>
@@ -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,
@@ -92,7 +98,8 @@ protected LanguageServerProjectLoader(
         IAsynchronousOperationListenerProvider listenerProvider,
         ProjectLoadTelemetryReporter projectLoadTelemetry,
         ServerConfigurationFactory serverConfigurationFactory,
-        IBinLogPathProvider binLogPathProvider)
+        IBinLogPathProvider binLogPathProvider,
+        DotnetCliHelper dotnetCliHelper)
     {
         _workspaceFactory = workspaceFactory;
         _fileChangeWatcher = fileChangeWatcher;
@@ -101,6 +108,7 @@ protected LanguageServerProjectLoader(
         _logger = loggerFactory.CreateLogger(nameof(LanguageServerProjectLoader));
         _projectLoadTelemetryReporter = projectLoadTelemetry;
         _binLogPathProvider = binLogPathProvider;
+        _dotnetCliHelper = dotnetCliHelper;
 
         AdditionalProperties = BuildAdditionalProperties(serverConfigurationFactory.ServerConfiguration);
 
@@ -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

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

@@ -35,7 +35,8 @@ public LanguageServerProjectSystem(
         IAsynchronousOperationListenerProvider listenerProvider,
         ProjectLoadTelemetryReporter projectLoadTelemetry,
         ServerConfigurationFactory serverConfigurationFactory,
-        IBinLogPathProvider binLogPathProvider)
+        IBinLogPathProvider binLogPathProvider,
+        DotnetCliHelper dotnetCliHelper)
             : base(
                 workspaceFactory,
                 fileChangeWatcher,
@@ -44,7 +45,8 @@ public LanguageServerProjectSystem(
                 listenerProvider,
                 projectLoadTelemetry,
                 serverConfigurationFactory,
-                binLogPathProvider)
+                binLogPathProvider,
+                dotnetCliHelper)
     {
         _logger = loggerFactory.CreateLogger(nameof(LanguageServerProjectSystem));
         _hostProjectFactory = workspaceFactory.HostProjectFactory;

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

@@ -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;
@@ -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)
@@ -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);
 }

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/LanguageServer/Handler/Restore/RestoreHandler.cs

@@ -4,8 +4,11 @@
 
 using System.Collections.Immutable;
 using System.Composition;
+using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.Extensions.Logging;
+using Microsoft.VisualStudio.Threading;
+using Roslyn.LanguageServer.Protocol;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler;
@@ -18,7 +21,7 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler;
 [Method(MethodName)]
 [method: ImportingConstructor]
 [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-internal sealed class RestoreHandler(DotnetCliHelper dotnetCliHelper, ILoggerFactory loggerFactory) : ILspServiceRequestHandler<RestoreParams, RestorePartialResult[]>
+internal sealed class RestoreHandler(DotnetCliHelper dotnetCliHelper, ILoggerFactory loggerFactory) : ILspServiceRequestHandler<RestoreParams, RestoreResult>
 {
     internal const string MethodName = "workspace/_roslyn_restore";
 
@@ -28,26 +31,24 @@ internal sealed class RestoreHandler(DotnetCliHelper dotnetCliHelper, ILoggerFac
 
     private readonly ILogger<RestoreHandler> _logger = loggerFactory.CreateLogger<RestoreHandler>();
 
-    public async Task<RestorePartialResult[]> HandleRequestAsync(RestoreParams request, RequestContext context, CancellationToken cancellationToken)
+    public async Task<RestoreResult> HandleRequestAsync(RestoreParams request, RequestContext context, CancellationToken cancellationToken)
     {
         Contract.ThrowIfNull(context.Solution);
-        using var progress = BufferedProgress.Create(request.PartialResultToken);
-
-        progress.Report(new RestorePartialResult(LanguageServerResources.Restore, LanguageServerResources.Restore_started));
 
         var restorePaths = GetRestorePaths(request, context.Solution, context);
         if (restorePaths.IsEmpty)
         {
             _logger.LogDebug($"Restore was requested but no paths were provided.");
-            progress.Report(new RestorePartialResult(LanguageServerResources.Restore, LanguageServerResources.Nothing_found_to_restore));
-            return progress.GetValues() ?? [];
+            return new RestoreResult(true);
         }
 
+        var workDoneProgressManager = context.GetRequiredService<WorkDoneProgressManager>();
         _logger.LogDebug($"Running restore on {restorePaths.Length} paths, starting with '{restorePaths.First()}'.");
-        bool success = await RestoreAsync(restorePaths, progress, cancellationToken);
 
-        progress.Report(new RestorePartialResult(LanguageServerResources.Restore, $"{LanguageServerResources.Restore_complete}{Environment.NewLine}"));
-        if (success)
+        // We let cancellation here bubble up to the client as this is a client initiated operation.
+        var didSucceed = await RestoreAsync(restorePaths, workDoneProgressManager, dotnetCliHelper, _logger, enableProgressReporting: true, cancellationToken);
+
+        if (didSucceed)
         {
             _logger.LogDebug($"Restore completed successfully.");
         }
@@ -56,13 +57,44 @@ public async Task<RestorePartialResult[]> HandleRequestAsync(RestoreParams reque
             _logger.LogError($"Restore completed with errors.");
         }
 
-        return progress.GetValues() ?? [];
+        return new RestoreResult(didSucceed);
     }
 
     /// <returns>True if all restore invocations exited with code 0. Otherwise, false.</returns>
-    private async Task<bool> RestoreAsync(ImmutableArray<string> pathsToRestore, BufferedProgress<RestorePartialResult> progress, CancellationToken cancellationToken)
+    public static async Task<bool> RestoreAsync(
+        ImmutableArray<string> pathsToRestore,
+        WorkDoneProgressManager workDoneProgressManager,
+        DotnetCliHelper dotnetCliHelper,
+        ILogger logger,
+        bool enableProgressReporting,
+        CancellationToken cancellationToken)
     {
-        bool success = true;
+        using var progress = await workDoneProgressManager.CreateWorkDoneProgressAsync(reportProgressToClient: enableProgressReporting, cancellationToken);
+        // Ensure we're observing cancellation token from the work done progress (to allow client cancellation).
+        cancellationToken = progress.CancellationToken;
+        return await RestoreCoreAsync(pathsToRestore, progress, dotnetCliHelper, logger, cancellationToken);
+
+    }
+
+    private static async Task<bool> RestoreCoreAsync(
+        ImmutableArray<string> pathsToRestore,
+        IWorkDoneProgressReporter progress,
+        DotnetCliHelper dotnetCliHelper,
+        ILogger logger,
+        CancellationToken cancellationToken)
+    {
+        // Report the start of the work done progress to the client.
+        progress.Report(new WorkDoneProgressBegin()
+        {
+            Title = LanguageServerResources.Restore,
+            // Adds a cancel button to the client side progress UI.
+            // Cancellation here is fine, it just means the restore will be incomplete (same as a cntrl+C for a CLI restore).
+            Cancellable = true,
+            Message = LanguageServerResources.Restore_started,
+            Percentage = 0,
+        });
+
+        var success = true;
         foreach (var path in pathsToRestore)
         {
             var arguments = new string[] { "restore", path };
@@ -77,8 +109,8 @@ private async Task<bool> RestoreAsync(ImmutableArray<string> pathsToRestore, Buf
                 process?.Kill();
             });
 
-            process.OutputDataReceived += (sender, args) => ReportProgress(progress, stageName, args.Data);
-            process.ErrorDataReceived += (sender, args) => ReportProgress(progress, stageName, args.Data);
+            process.OutputDataReceived += (sender, args) => ReportProgressInEvent(progress, stageName, args.Data);
+            process.ErrorDataReceived += (sender, args) => ReportProgressInEvent(progress, stageName, args.Data);
             process.BeginOutputReadLine();
             process.BeginErrorReadLine();
 
@@ -91,14 +123,43 @@ private async Task<bool> RestoreAsync(ImmutableArray<string> pathsToRestore, Buf
             }
         }
 
+        // Report work done progress completion
+        progress.Report(
+            new WorkDoneProgressEnd()
+            {
+                Message = LanguageServerResources.Restore_complete
+            });
+
+        logger.LogInformation(LanguageServerResources.Restore_complete);
         return success;
 
-        static void ReportProgress(BufferedProgress<RestorePartialResult> progress, string stage, string? restoreOutput)
+        void ReportProgressInEvent(IWorkDoneProgressReporter progress, string stage, string? restoreOutput)
         {
-            if (restoreOutput != null)
+            if (restoreOutput == null)
+                return;
+
+            try
             {
-                progress.Report(new RestorePartialResult(stage, restoreOutput));
+                ReportProgress(progress, stage, restoreOutput);
             }
+            catch (Exception)
+            {
+                // Catch everything to ensure the exception doesn't escape the event handler.
+                // Errors already reported via ReportNonFatalErrorUnlessCancelledAsync.
+            }
+        }
+
+        void ReportProgress(IWorkDoneProgressReporter progress, string stage, string message)
+        {
+            logger.LogInformation("{stage}: {Output}", stage, message);
+            var report = new WorkDoneProgressReport()
+            {
+                Message = stage,
+                Percentage = null,
+                Cancellable = true,
+            };
+
+            progress.Report(report);
         }
     }
 

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/LanguageServer/Handler/Restore/RestoreParams.cs

@@ -10,9 +10,4 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler;
 internal sealed record RestoreParams(
     // An empty set of project file paths means restore all projects in the workspace.
     [property: JsonPropertyName("projectFilePaths")] string[] ProjectFilePaths
-) : IPartialResultParams<RestorePartialResult>
-{
-    [JsonPropertyName(Methods.PartialResultTokenName)]
-    [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
-    public IProgress<RestorePartialResult>? PartialResultToken { get; set; }
-}
+);

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/LanguageServer/Handler/Restore/RestorePartialResult.cs → src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/LanguageServer/Handler/Restore/RestoreResult.cs

@@ -6,7 +6,6 @@
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler;
 
-internal sealed record RestorePartialResult(
-    [property: JsonPropertyName("stage")] string Stage,
-    [property: JsonPropertyName("message")] string Message
+internal sealed record RestoreResult(
+    [property: JsonPropertyName("success")] bool Success
 );