review feedback

src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratedDocumentCache.cs

@@ -14,7 +14,7 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler.SourceGenerators;
 
 internal record struct SourceGeneratedDocumentGetTextState(Document Document);
 
-internal class SourceGeneratedDocumentCache(string uniqueKey) : VersionedPullCache<(SourceGeneratorExecutionVersion, VersionStamp), object?, SourceGeneratedDocumentGetTextState, SourceText?>(uniqueKey), ILspService
+internal sealed class SourceGeneratedDocumentCache(string uniqueKey) : VersionedPullCache<(SourceGeneratorExecutionVersion, VersionStamp), object?, SourceGeneratedDocumentGetTextState, SourceText?>(uniqueKey), ILspService
 {
     public override async Task<(SourceGeneratorExecutionVersion, VersionStamp)> ComputeCheapVersionAsync(SourceGeneratedDocumentGetTextState state, CancellationToken cancellationToken)
     {
@@ -41,6 +41,7 @@ public override Checksum ComputeChecksum(SourceText? data)
         // get through didOpen/didChanges, like any other file. That way operations in LSP file are in sync with the
         // contents the user has. However in this case, we don't want to look at that frozen text, but look at what the
         // generator would generate if we ran it again. Otherwise, we'll get "stuck" and never update the file with something new.
+        // This can return null when the source generated file has been removed (but the queue itself is using the frozen non-null document).
         var unfrozenDocument = await state.Document.Project.Solution.WithoutFrozenSourceGeneratedDocuments().GetDocumentAsync(state.Document.Id, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
         return unfrozenDocument == null
             ? null

src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratedDocumentGetTextHandler.cs

@@ -9,6 +9,7 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
 using LSP = Roslyn.LanguageServer.Protocol;
 
@@ -52,7 +53,7 @@ public async Task<SourceGeneratedDocumentText> HandleRequestAsync(SourceGenerato
         var cache = context.GetRequiredLspService<SourceGeneratedDocumentCache>();
         var projectOrDocument = new ProjectOrDocumentId(document.Id);
 
-        var previousPullResults = new Dictionary<ProjectOrDocumentId, PreviousPullResult>();
+        using var _ = PooledDictionary<ProjectOrDocumentId, PreviousPullResult>.GetInstance(out var previousPullResults);
         if (request.ResultId is not null)
         {
             previousPullResults.Add(projectOrDocument, new PreviousPullResult(request.ResultId, request.TextDocument));
@@ -64,10 +65,11 @@ public async Task<SourceGeneratedDocumentText> HandleRequestAsync(SourceGenerato
         {
             Contract.ThrowIfNull(request.ResultId, "Attempted to reuse cache entry but given no resultId");
             // The generated document is the same, we can return the same resultId.
-            return new SourceGeneratedDocumentText(request.ResultId, null);
+            return new SourceGeneratedDocumentText(request.ResultId, Text: null);
         }
         else
         {
+            // We may get no text back if the unfrozen source generated file no longer exists.
             var data = newResult.Value.Data?.ToString();
             return new SourceGeneratedDocumentText(newResult.Value.ResultId, data);
         }

src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratedDocumentText.cs

@@ -6,6 +6,16 @@
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler;
 
+/// <summary>
+/// Source generated file text result.  The client uses the resultId to inform what the text value is.
+/// 
+/// An unchanged result has a non-null resultId (same as client request resultId) + null text.
+/// 
+/// A changed result has a new non-null resultId + possibly null text (if the sg document no longer exists).
+/// 
+/// In rare circumstances it is possible to get a null resultId + null text - this happens when
+/// the source generated document is not open AND the source generated document no longer exists
+/// </summary>
 internal sealed record SourceGeneratedDocumentText(
     [property: JsonPropertyName("resultId")] string? ResultId,
     [property: JsonPropertyName("text")] string? Text);

src/LanguageServer/Protocol/Handler/SourceGenerators/SourceGeneratorRefreshQueue.cs

@@ -14,10 +14,10 @@
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.SourceGenerators;
 
-internal class SourceGeneratorRefreshQueue :
-        IOnInitialized,
-        ILspService,
-        IDisposable
+internal sealed class SourceGeneratorRefreshQueue :
+    IOnInitialized,
+    ILspService,
+    IDisposable
 {
     private const string RefreshSourceGeneratedDocumentName = "workspace/refreshSourceGeneratedDocument";
 
@@ -29,10 +29,10 @@ internal class SourceGeneratorRefreshQueue :
     private readonly AsyncBatchingWorkQueue _refreshQueue;
 
     public SourceGeneratorRefreshQueue(
-                IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider,
-                LspWorkspaceRegistrationService lspWorkspaceRegistrationService,
-                LspWorkspaceManager lspWorkspaceManager,
-                IClientLanguageServerManager notificationManager)
+        IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider,
+        LspWorkspaceRegistrationService lspWorkspaceRegistrationService,
+        LspWorkspaceManager lspWorkspaceManager,
+        IClientLanguageServerManager notificationManager)
     {
         _lspWorkspaceRegistrationService = lspWorkspaceRegistrationService;
         _lspWorkspaceManager = lspWorkspaceManager;
@@ -43,7 +43,7 @@ public SourceGeneratorRefreshQueue(
         // every 2 seconds - long enough to avoid spamming the client with notifications, but short enough to refresh
         // the source generated files relatively frequently.
         _refreshQueue = _refreshQueue = new AsyncBatchingWorkQueue(
-            delay: TimeSpan.FromMilliseconds(2000),
+            delay: DelayTimeSpan.Idle,
             processBatchAsync: RefreshSourceGeneratedDocumentsAsync,
             asyncListener: _asyncListener,
             _disposalTokenSource.Token);
@@ -63,6 +63,14 @@ public Task OnInitializedAsync(ClientCapabilities clientCapabilities, RequestCon
     }
 
     private void OnLspSolutionChanged(object? sender, WorkspaceChangeEventArgs e)
+    {
+        var asyncToken = _asyncListener.BeginAsyncOperation($"{nameof(SourceGeneratorRefreshQueue)}.{nameof(OnLspSolutionChanged)}");
+        _ = OnLspSolutionChangedAsync(e)
+            .CompletesAsyncOperation(asyncToken)
+            .ReportNonFatalErrorUnlessCancelledAsync(_disposalTokenSource.Token);
+    }
+
+    private async Task OnLspSolutionChangedAsync(WorkspaceChangeEventArgs e)
     {
         var projectId = e.ProjectId ?? e.DocumentId?.ProjectId;
         if (projectId is not null)
@@ -85,8 +93,7 @@ private void OnLspSolutionChanged(object? sender, WorkspaceChangeEventArgs e)
 
             if (oldProject != null && newProject != null)
             {
-                var asyncToken = _asyncListener.BeginAsyncOperation($"{nameof(SourceGeneratorRefreshQueue)}.{nameof(OnLspSolutionChanged)}");
-                CheckDependentVersionsAsync(oldProject, newProject, _disposalTokenSource.Token).CompletesAsyncOperation(asyncToken);
+                await CheckDependentVersionsAsync(oldProject, newProject).ConfigureAwait(false);
             }
         }
         else
@@ -98,10 +105,10 @@ private void OnLspSolutionChanged(object? sender, WorkspaceChangeEventArgs e)
             }
         }
 
-        async Task CheckDependentVersionsAsync(Project oldProject, Project newProject, CancellationToken cancellationToken)
+        async Task CheckDependentVersionsAsync(Project oldProject, Project newProject)
         {
-            if (await oldProject.GetDependentVersionAsync(cancellationToken).ConfigureAwait(false) !=
-                await newProject.GetDependentVersionAsync(cancellationToken).ConfigureAwait(false))
+            if (await oldProject.GetDependentVersionAsync(_disposalTokenSource.Token).ConfigureAwait(false) !=
+                await newProject.GetDependentVersionAsync(_disposalTokenSource.Token).ConfigureAwait(false))
             {
                 _refreshQueue.AddWork();
             }