Do not report unchanged files in workspace-pull diagnostics

src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs

@@ -942,81 +942,6 @@ void M()
             }
         }
 
-        [Theory, CombinatorialData]
-        internal async Task TestCancellationDuringDiagnosticComputation_InProc(AnalyzerRegisterActionKind actionKind)
-        {
-            // This test verifies that we do no attempt to re-use CompilationWithAnalyzers instance in IDE in-proc diagnostic computation in presence of an OperationCanceledException during analysis.
-            // Attempting to do so has led to large number of reliability issues and flakiness in diagnostic computation, which we want to avoid.
-
-            var source = @"
-class A
-{
-    void M()
-    {
-        int x = 0;
-    }
-}";
-
-            using var workspace = EditorTestWorkspace.CreateCSharp(source,
-                composition: s_editorFeaturesCompositionWithMockDiagnosticUpdateSourceRegistrationService.AddParts(typeof(TestDocumentTrackingService)));
-            Assert.True(workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(
-                workspace.CurrentSolution.Options.WithChangedOption(new OptionKey(DiagnosticOptionsStorage.PullDiagnosticsFeatureFlag), false))));
-
-            var analyzer = new CancellationTestAnalyzer(actionKind);
-            var analyzerReference = new AnalyzerImageReference(ImmutableArray.Create<DiagnosticAnalyzer>(analyzer));
-            workspace.TryApplyChanges(workspace.CurrentSolution.WithAnalyzerReferences(new[] { analyzerReference }));
-
-            var project = workspace.CurrentSolution.Projects.Single();
-            var document = project.Documents.Single();
-
-            Assert.IsType<MockDiagnosticUpdateSourceRegistrationService>(workspace.GetService<IDiagnosticUpdateSourceRegistrationService>());
-            var service = Assert.IsType<DiagnosticAnalyzerService>(workspace.GetService<IDiagnosticAnalyzerService>());
-            var globalOptions = workspace.GetService<IGlobalOptionService>();
-
-            DiagnosticData diagnostic = null;
-            service.DiagnosticsUpdated += (s, eCollection) =>
-            {
-                foreach (var e in eCollection)
-                {
-                    var diagnostics = e.Diagnostics;
-                    if (diagnostics.IsEmpty)
-                    {
-                        continue;
-                    }
-
-                    Assert.Null(diagnostic);
-                    diagnostic = Assert.Single(diagnostics);
-                }
-            };
-
-            var incrementalAnalyzer = service.CreateIncrementalAnalyzer(workspace);
-
-            OpenDocumentAndMakeActive(document, workspace);
-
-            // First invoke analysis with cancellation token, and verify canceled compilation and no reported diagnostics.
-            Assert.Empty(analyzer.CanceledCompilations);
-            try
-            {
-                await incrementalAnalyzer.ForceAnalyzeProjectAsync(project, analyzer.CancellationToken);
-
-                throw ExceptionUtilities.Unreachable();
-            }
-            catch (OperationCanceledException ex) when (ex.CancellationToken == analyzer.CancellationToken)
-            {
-            }
-
-            Assert.Single(analyzer.CanceledCompilations);
-            Assert.Null(diagnostic);
-
-            // Then invoke analysis without cancellation token, and verify non-cancelled diagnostic.
-            await incrementalAnalyzer.ForceAnalyzeProjectAsync(project, CancellationToken.None);
-
-            await ((AsynchronousOperationListener)service.Listener).ExpeditedWaitAsync();
-
-            Assert.True(diagnostic != null);
-            Assert.Equal(CancellationTestAnalyzer.DiagnosticId, diagnostic.Id);
-        }
-
         [Theory, CombinatorialData]
         [WorkItem("https://github.com/dotnet/roslyn/issues/49698")]
         internal async Task TestOnlyRequiredAnalyzerExecutedDuringDiagnosticComputation(bool documentAnalysis)

src/Features/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs

@@ -6,6 +6,7 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -20,17 +21,16 @@
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics
 {
-    internal abstract class AbstractDocumentPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>
-        : AbstractPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>, ITextDocumentIdentifierHandler<TDiagnosticsParams, TextDocumentIdentifier?>
+    internal abstract class AbstractDocumentPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>(
+        IDiagnosticAnalyzerService diagnosticAnalyzerService,
+        IDiagnosticsRefresher diagnosticRefresher,
+        IGlobalOptionService globalOptions)
+        : AbstractPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>(
+            diagnosticAnalyzerService,
+            diagnosticRefresher,
+            globalOptions), ITextDocumentIdentifierHandler<TDiagnosticsParams, TextDocumentIdentifier?>
         where TDiagnosticsParams : IPartialResultParams<TReport>
     {
-        public AbstractDocumentPullDiagnosticHandler(
-            IDiagnosticAnalyzerService diagnosticAnalyzerService,
-            IDiagnosticsRefresher diagnosticRefresher,
-            IGlobalOptionService globalOptions) : base(diagnosticAnalyzerService, diagnosticRefresher, globalOptions)
-        {
-        }
-
         public abstract LSP.TextDocumentIdentifier? GetTextDocumentIdentifier(TDiagnosticsParams diagnosticsParams);
     }
 
@@ -100,9 +100,11 @@ protected abstract ValueTask<ImmutableArray<IDiagnosticSource>> GetOrderedDiagno
         protected abstract TReport CreateReport(TextDocumentIdentifier identifier, LSP.Diagnostic[] diagnostics, string resultId);
 
         /// <summary>
-        /// Creates the appropriate LSP type to report unchanged diagnostics.
+        /// Creates the appropriate LSP type to report unchanged diagnostics. Can return <see langword="false"/> to
+        /// indicate nothing should be reported.  This should be done for workspace requests to avoiding sending a huge
+        /// amount of "nothing changed" responses for most files.
         /// </summary>
-        protected abstract TReport CreateUnchangedReport(TextDocumentIdentifier identifier, string resultId);
+        protected abstract bool TryCreateUnchangedReport(TextDocumentIdentifier identifier, string resultId, [NotNullWhen(true)] out TReport? report);
 
         /// <summary>
         /// Creates the appropriate LSP type to report a removed file.
@@ -187,8 +189,12 @@ await ComputeAndReportCurrentDiagnosticsAsync(
                     // Nothing changed between the last request and this one.  Report a (null-diagnostics,
                     // same-result-id) response to the client as that means they should just preserve the current
                     // diagnostics they have for this file.
+                    //
+                    // Note: if this is a workspace request, we can do nothing, as that will be interpreted by the
+                    // client as nothing having been changed for that document.
                     var previousParams = documentToPreviousDiagnosticParams[diagnosticSource.GetId()];
-                    progress.Report(CreateUnchangedReport(previousParams.TextDocument, previousParams.PreviousResultId));
+                    if (TryCreateUnchangedReport(previousParams.TextDocument, previousParams.PreviousResultId, out var report))
+                        progress.Report(report);
                 }
             }
 

src/Features/LanguageServer/Protocol/Handler/Diagnostics/AbstractWorkspacePullDiagnosticsHandler.cs

@@ -18,6 +18,7 @@
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
+
 internal abstract class AbstractWorkspacePullDiagnosticsHandler<TDiagnosticsParams, TReport, TReturn>
     : AbstractPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>, IDisposable
     where TDiagnosticsParams : IPartialResultParams<TReport>

src/Features/LanguageServer/Protocol/Handler/Diagnostics/DocumentPullDiagnosticHandler.cs

@@ -49,8 +49,11 @@ protected override VSInternalDiagnosticReport[] CreateReport(TextDocumentIdentif
         protected override VSInternalDiagnosticReport[] CreateRemovedReport(TextDocumentIdentifier identifier)
             => CreateReport(identifier, diagnostics: null, resultId: null);
 
-        protected override VSInternalDiagnosticReport[] CreateUnchangedReport(TextDocumentIdentifier identifier, string resultId)
-            => CreateReport(identifier, diagnostics: null, resultId);
+        protected override bool TryCreateUnchangedReport(TextDocumentIdentifier identifier, string resultId, out VSInternalDiagnosticReport[] report)
+        {
+            report = CreateReport(identifier, diagnostics: null, resultId);
+            return true;
+        }
 
         protected override ImmutableArray<PreviousPullResult>? GetPreviousResults(VSInternalDocumentDiagnosticsParams diagnosticsParams)
         {

src/Features/LanguageServer/Protocol/Handler/Diagnostics/Public/PublicDocumentPullDiagnosticsHandler.cs

@@ -66,11 +66,14 @@ protected override DocumentDiagnosticPartialReport CreateRemovedReport(TextDocum
             Items = [],
         });
 
-    protected override DocumentDiagnosticPartialReport CreateUnchangedReport(TextDocumentIdentifier identifier, string resultId)
-        => new(new RelatedUnchangedDocumentDiagnosticReport
+    protected override bool TryCreateUnchangedReport(TextDocumentIdentifier identifier, string resultId, out DocumentDiagnosticPartialReport report)
+    {
+        report = new RelatedUnchangedDocumentDiagnosticReport
         {
             ResultId = resultId
-        });
+        };
+        return true;
+    }
 
     protected override DocumentDiagnosticReport? CreateReturn(BufferedProgress<DocumentDiagnosticPartialReport> progress)
     {

src/Features/LanguageServer/Protocol/Handler/Diagnostics/Public/PublicWorkspacePullDiagnosticsHandler.cs

@@ -70,20 +70,13 @@ protected override WorkspaceDiagnosticPartialReport CreateRemovedReport(TextDocu
             ]
         });
 
-    protected override WorkspaceDiagnosticPartialReport CreateUnchangedReport(TextDocumentIdentifier identifier, string resultId)
-        => new(new WorkspaceDiagnosticReport
-        {
-            Items =
-            [
-                new WorkspaceUnchangedDocumentDiagnosticReport
-                {
-                    Uri = identifier.Uri,
-                    // The documents provided by workspace reports are never open, so we return null.
-                    Version = null,
-                    ResultId = resultId,
-                }
-            ]
-        });
+    protected override bool TryCreateUnchangedReport(TextDocumentIdentifier identifier, string resultId, out WorkspaceDiagnosticPartialReport report)
+    {
+        // Skip reporting 'unchanged' document reports for workspace pull diagnostics.  There are often a ton of
+        // these and we can save a lot of memory not serializing/deserializing all of this.
+        report = default;
+        return false;
+    }
 
     protected override WorkspaceDiagnosticReport? CreateReturn(BufferedProgress<WorkspaceDiagnosticPartialReport> progress)
     {

src/Features/LanguageServer/Protocol/Handler/Diagnostics/WorkspacePullDiagnosticHandler.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Options;
@@ -40,8 +41,13 @@ protected override VSInternalWorkspaceDiagnosticReport[] CreateReport(TextDocume
         protected override VSInternalWorkspaceDiagnosticReport[] CreateRemovedReport(TextDocumentIdentifier identifier)
             => CreateReport(identifier, diagnostics: null, resultId: null);
 
-        protected override VSInternalWorkspaceDiagnosticReport[] CreateUnchangedReport(TextDocumentIdentifier identifier, string resultId)
-            => CreateReport(identifier, diagnostics: null, resultId);
+        protected override bool TryCreateUnchangedReport(TextDocumentIdentifier identifier, string resultId, [NotNullWhen(true)] out VSInternalWorkspaceDiagnosticReport[]? report)
+        {
+            // Skip reporting 'unchanged' document reports for workspace pull diagnostics.  There are often a ton of
+            // these and we can save a lot of memory not serializing/deserializing all of this.
+            report = null;
+            return false;
+        }
 
         protected override ImmutableArray<PreviousPullResult>? GetPreviousResults(VSInternalWorkspaceDiagnosticsParams diagnosticsParams)
             => diagnosticsParams.PreviousResults?.Where(d => d.PreviousResultId != null).Select(d => new PreviousPullResult(d.PreviousResultId!, d.TextDocument!)).ToImmutableArray();

src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/AdditionalFileDiagnosticsTests.cs

@@ -47,10 +47,7 @@ public async Task TestWorkspaceDiagnosticsReportsAdditionalFileDiagnostic(bool u
 
         // Asking again should give us back an unchanged diagnostic.
         var results2 = await RunGetWorkspacePullDiagnosticsAsync(testLspServer, useVSDiagnostics, previousResults: CreateDiagnosticParamsFromPreviousReports(results));
-        Assert.Null(results2[0].Diagnostics);
-        Assert.Null(results2[1].Diagnostics);
-        Assert.Equal(results[1].ResultId, results2[1].ResultId);
-        Assert.Null(results2[2].Diagnostics);
+        Assert.Empty(results2);
     }
 
     [Theory, CombinatorialData]
@@ -121,8 +118,7 @@ public async Task TestWorkspaceDiagnosticsWithAdditionalFileInMultipleProjects(b
 
         // Asking again should give us back an unchanged diagnostic.
         var results2 = await RunGetWorkspacePullDiagnosticsAsync(testLspServer, useVSDiagnostics: true, previousResults: CreateDiagnosticParamsFromPreviousReports(results));
-        Assert.Equal(results[1].ResultId, results2[1].ResultId);
-        Assert.Equal(results[4].ResultId, results2[4].ResultId);
+        Assert.Empty(results2);
     }
 
     protected override TestComposition Composition => base.Composition.AddParts(typeof(MockAdditionalFileDiagnosticAnalyzer));

src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/PullDiagnosticTests.cs

@@ -1347,14 +1347,8 @@ public async Task TestNoChangeIfWorkspaceDiagnosticsCalledTwice(bool useVSDiagno
 
             var results2 = await RunGetWorkspacePullDiagnosticsAsync(testLspServer, useVSDiagnostics, previousResults: CreateDiagnosticParamsFromPreviousReports(results));
 
-            Assert.Equal(3, results2.Length);
-            Assert.Null(results2[0].Diagnostics);
-            Assert.Null(results2[1].Diagnostics);
-            Assert.Null(results2[2].Diagnostics);
-
-            Assert.Equal(results[0].ResultId, results2[0].ResultId);
-            Assert.Equal(results[1].ResultId, results2[1].ResultId);
-            Assert.Equal(results[2].ResultId, results2[2].ResultId);
+            // 'no changes' will be reported as an empty array.
+            Assert.Empty(results2);
         }
 
         [Theory, CombinatorialData]
@@ -1657,19 +1651,14 @@ public class {|caret:|} { }
             var previousResultIds = CreateDiagnosticParamsFromPreviousReports(results);
             results = await RunGetWorkspacePullDiagnosticsAsync(testLspServer, useVSDiagnostics, previousResultIds);
             AssertEx.NotNull(results);
-            Assert.Equal(4, results.Length);
-
-            // Verify the diagnostic result for A.cs is unchanged as A.cs does not reference CSProj2.
-            Assert.Null(results[0].Diagnostics);
-            Assert.Equal(previousResultIds[0].resultId, results[0].ResultId);
-            Assert.Null(results[1].Diagnostics);
-            Assert.Equal(previousResultIds[1].resultId, results[1].ResultId);
+            Assert.Equal(2, results.Length);
 
+            // Note: tehre will be no results for A.cs as it is unchanged and does not reference CSProj2.
             // Verify that the diagnostics result for B.cs reflects the change we made to it.
-            Assert.Empty(results[2].Diagnostics);
-            Assert.NotEqual(previousResultIds[2].resultId, results[2].ResultId);
-            Assert.Empty(results[3].Diagnostics);
-            Assert.NotEqual(previousResultIds[3].resultId, results[3].ResultId);
+            Assert.Empty(results[0].Diagnostics);
+            Assert.NotEqual(previousResultIds[2].resultId, results[0].ResultId);
+            Assert.Empty(results[1].Diagnostics);
+            Assert.NotEqual(previousResultIds[3].resultId, results[1].ResultId);
         }
 
         [Theory, CombinatorialData]
@@ -1728,7 +1717,7 @@ public class {|caret:|} { }
         }
 
         [Theory, CombinatorialData]
-        public async Task TestWorkspaceDiagnosticsWithDependentProjectReloadedUnChanged(bool useVSDiagnostics, bool mutatingLspWorkspace)
+        public async Task TestWorkspaceDiagnosticsWithDependentProjectReloadedUnchanged(bool useVSDiagnostics, bool mutatingLspWorkspace)
         {
             var markup1 =
 @"namespace M
@@ -1774,14 +1763,10 @@ public class {|caret:|} { }
             results = await RunGetWorkspacePullDiagnosticsAsync(testLspServer, useVSDiagnostics, previousResults: previousResultIds);
 
             // Verify that since no actual changes have been made we report unchanged diagnostics.
+            // We get an empty array here as this is workspace diagnostics, and we do not report unchanged
+            // docs there for efficiency.
             AssertEx.NotNull(results);
-            Assert.Equal(4, results.Length);
-
-            // Diagnostics should be unchanged as the referenced project was only unloaded / reloaded, but did not actually change.
-            Assert.Null(results[0].Diagnostics);
-            Assert.Equal(previousResultIds[0].resultId, results[0].ResultId);
-            Assert.Null(results[2].Diagnostics);
-            Assert.Equal(previousResultIds[2].resultId, results[2].ResultId);
+            Assert.Empty(results);
         }
 
         [Theory, CombinatorialData]
@@ -1830,13 +1815,10 @@ class A : B { }
             results = await RunGetWorkspacePullDiagnosticsAsync(testLspServer, useVSDiagnostics, previousResults: previousResults);
 
             // Verify that since no actual changes have been made we report unchanged diagnostics.
+            // We get an empty array here as this is workspace diagnostics, and we do not report unchanged
+            // docs there for efficiency.
             AssertEx.NotNull(results);
-            Assert.Equal(6, results.Length);
-
-            // Diagnostics should be unchanged as a referenced project was unloaded and reloaded.  Order should not matter.
-            Assert.Null(results[0].Diagnostics);
-            Assert.All(results, result => Assert.Null(result.Diagnostics));
-            Assert.All(results, result => Assert.True(previousResultIds.Contains(result.ResultId)));
+            Assert.Empty(results);
         }
 
         [Theory, CombinatorialData]

src/Features/LanguageServer/ProtocolUnitTests/Diagnostics/WorkspaceProjectDiagnosticsTests.cs

@@ -37,9 +37,7 @@ public async Task TestWorkspaceDiagnosticsReportsProjectDiagnostic(bool useVSDia
 
         // Asking again should give us back an unchanged diagnostic.
         var results2 = await RunGetWorkspacePullDiagnosticsAsync(testLspServer, useVSDiagnostics, previousResults: CreateDiagnosticParamsFromPreviousReports(results));
-        Assert.Null(results2[0].Diagnostics);
-        Assert.Null(results2[1].Diagnostics);
-        Assert.Equal(results[1].ResultId, results2[1].ResultId);
+        Assert.Empty(results2);
     }
 
     [Theory, CombinatorialData]