WIP: Ensure we always cache diag results.

src/Features/Core/Portable/Diagnostics/DiagnosticAnalyzerExtensions.cs

@@ -7,7 +7,6 @@
 using System.Linq;
 using System.Reflection;
 using Microsoft.CodeAnalysis.Diagnostics.Telemetry;
-using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.Simplification;
 using Roslyn.Utilities;
 

src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.Executor.cs

@@ -4,15 +4,13 @@
 
 using System;
 using System.Collections.Immutable;
-using System.Diagnostics;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Diagnostics.Telemetry;
 using Microsoft.CodeAnalysis.ErrorReporting;
 using Microsoft.CodeAnalysis.Internal.Log;
 using Microsoft.CodeAnalysis.Simplification;
-using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.CodeAnalysis.Workspaces.Diagnostics;
 using Roslyn.Utilities;
 
@@ -26,34 +24,55 @@ internal partial class DiagnosticIncrementalAnalyzer
         private async Task<ProjectAnalysisData> GetProjectAnalysisDataAsync(
             CompilationWithAnalyzers? compilationWithAnalyzers, Project project, IdeAnalyzerOptions ideOptions, ImmutableArray<StateSet> stateSets, CancellationToken cancellationToken)
         {
-            using (Logger.LogBlock(FunctionId.Diagnostics_ProjectDiagnostic, GetProjectLogMessage, project, stateSets, cancellationToken))
+            // Compute the diagnostic data for all the requested state sets for this particular project.
+            var projectAnalysisData = await GetProjectAnalysisDataWorkerAsync().ConfigureAwait(false);
+
+            // Now, save all that data to the in-memory storage, so it can be retrieved later when making requests for
+            // other documents within the same project.
+            await SaveAllStatesToInMemoryStorageAsync().ConfigureAwait(false);
+
+            return projectAnalysisData;
+
+            async Task SaveAllStatesToInMemoryStorageAsync()
+            {
+                foreach (var stateSet in stateSets)
+                {
+                    var state = stateSet.GetOrCreateProjectState(project.Id);
+
+                    if (projectAnalysisData.TryGetResult(stateSet.Analyzer, out var analyzerResult))
+                        await state.SaveToInMemoryStorageAsync(project, analyzerResult).ConfigureAwait(false);
+                }
+            }
+
+            async Task<ProjectAnalysisData> GetProjectAnalysisDataWorkerAsync()
             {
-                try
+                using (Logger.LogBlock(FunctionId.Diagnostics_ProjectDiagnostic, GetProjectLogMessage, project, stateSets, cancellationToken))
                 {
-                    // PERF: We need to flip this to false when we do actual diffing.
-                    var avoidLoadingData = true;
-                    var version = await GetDiagnosticVersionAsync(project, cancellationToken).ConfigureAwait(false);
-                    var existingData = await ProjectAnalysisData.CreateAsync(project, stateSets, avoidLoadingData, cancellationToken).ConfigureAwait(false);
-
-                    // We can't return here if we have open file only analyzers since saved data for open file only analyzer
-                    // is incomplete -- it only contains info on open files rather than whole project.
-                    if (existingData.Version == version && !CompilationHasOpenFileOnlyAnalyzers(compilationWithAnalyzers, ideOptions.CleanupOptions?.SimplifierOptions))
+                    try
                     {
-                        return existingData;
-                    }
+                        // PERF: We need to flip this to false when we do actual diffing.
+                        var avoidLoadingData = true;
+                        var version = await GetDiagnosticVersionAsync(project, cancellationToken).ConfigureAwait(false);
+                        var existingData = await ProjectAnalysisData.CreateAsync(project, stateSets, avoidLoadingData, cancellationToken).ConfigureAwait(false);
 
-                    var result = await ComputeDiagnosticsAsync(compilationWithAnalyzers, project, ideOptions, stateSets, existingData.Result, cancellationToken).ConfigureAwait(false);
+                        // We can't return here if we have open file only analyzers since saved data for open file only analyzer
+                        // is incomplete -- it only contains info on open files rather than whole project.
+                        if (existingData.Version == version && !CompilationHasOpenFileOnlyAnalyzers(compilationWithAnalyzers, ideOptions.CleanupOptions?.SimplifierOptions))
+                            return existingData;
 
-                    // If project is not loaded successfully, get rid of any semantic errors from compiler analyzer.
-                    // Note: In the past when project was not loaded successfully we did not run any analyzers on the project.
-                    // Now we run analyzers but filter out some information. So on such projects, there will be some perf degradation.
-                    result = await RemoveCompilerSemanticErrorsIfProjectNotLoadedAsync(result, project, cancellationToken).ConfigureAwait(false);
+                        var result = await ComputeDiagnosticsAsync(compilationWithAnalyzers, project, ideOptions, stateSets, existingData.Result, cancellationToken).ConfigureAwait(false);
 
-                    return new ProjectAnalysisData(project.Id, version, existingData.Result, result);
-                }
-                catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken))
-                {
-                    throw ExceptionUtilities.Unreachable();
+                        // If project is not loaded successfully, get rid of any semantic errors from compiler analyzer.
+                        // Note: In the past when project was not loaded successfully we did not run any analyzers on the project.
+                        // Now we run analyzers but filter out some information. So on such projects, there will be some perf degradation.
+                        result = await RemoveCompilerSemanticErrorsIfProjectNotLoadedAsync(result, project, cancellationToken).ConfigureAwait(false);
+
+                        return new ProjectAnalysisData(project.Id, version, existingData.Result, result);
+                    }
+                    catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken))
+                    {
+                        throw ExceptionUtilities.Unreachable();
+                    }
                 }
             }
         }

src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs

@@ -263,10 +263,10 @@ protected override async Task AppendDiagnosticsAsync(Project project, IEnumerabl
 
                 var ideOptions = Owner.AnalyzerService.GlobalOptions.GetIdeAnalyzerOptions(project);
 
-                // unlike the suppressed (disabled) analyzer, we will include hidden diagnostic only analyzers here.
-                var compilation = await CreateCompilationWithAnalyzersAsync(project, ideOptions, stateSets, IncludeSuppressedDiagnostics, cancellationToken).ConfigureAwait(false);
+                var compilationWithAnalyzers = await CreateCompilationWithAnalyzersAsync(
+                    project, ideOptions, stateSets, IncludeSuppressedDiagnostics, cancellationToken).ConfigureAwait(false);
 
-                var result = await Owner.GetProjectAnalysisDataAsync(compilation, project, ideOptions, stateSets, cancellationToken).ConfigureAwait(false);
+                var result = await Owner.GetProjectAnalysisDataAsync(compilationWithAnalyzers, project, ideOptions, stateSets, cancellationToken).ConfigureAwait(false);
 
                 foreach (var stateSet in stateSets)
                 {

src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs

@@ -22,35 +22,27 @@ public async Task<ImmutableArray<DiagnosticData>> ForceAnalyzeProjectAsync(Proje
         {
             try
             {
-                var stateSets = GetStateSetsForFullSolutionAnalysis(_stateManager.GetOrUpdateStateSets(project), project);
-
                 // get driver only with active analyzers.
                 var ideOptions = AnalyzerService.GlobalOptions.GetIdeAnalyzerOptions(project);
 
-                // PERF: get analyzers that are not suppressed and marked as open file only
-                // this is perf optimization. we cache these result since we know the result. (no diagnostics)
-                var activeAnalyzers = stateSets
-                                        .Select(s => s.Analyzer)
-                                        .Where(a => !a.IsOpenFileOnly(ideOptions.CleanupOptions?.SimplifierOptions));
-
-                CompilationWithAnalyzers? compilationWithAnalyzers = null;
+                var stateSets = GetStateSetsForFullSolutionAnalysis(_stateManager.GetOrUpdateStateSets(project), project, ideOptions);
 
-                compilationWithAnalyzers = await DocumentAnalysisExecutor.CreateCompilationWithAnalyzersAsync(project, ideOptions, activeAnalyzers, includeSuppressedDiagnostics: true, cancellationToken).ConfigureAwait(false);
+                var compilationWithAnalyzers = await CreateCompilationWithAnalyzersAsync(
+                    project, ideOptions, stateSets, includeSuppressedDiagnostics: true, cancellationToken).ConfigureAwait(false);
 
                 var result = await GetProjectAnalysisDataAsync(compilationWithAnalyzers, project, ideOptions, stateSets, cancellationToken).ConfigureAwait(false);
 
                 using var _ = ArrayBuilder<DiagnosticData>.GetInstance(out var diagnostics);
 
                 // no cancellation after this point.
+                cancellationToken = CancellationToken.None;
+
                 foreach (var stateSet in stateSets)
                 {
                     var state = stateSet.GetOrCreateProjectState(project.Id);
 
                     if (result.TryGetResult(stateSet.Analyzer, out var analyzerResult))
-                    {
                         diagnostics.AddRange(analyzerResult.GetAllDiagnostics());
-                        await state.SaveToInMemoryStorageAsync(project, analyzerResult).ConfigureAwait(false);
-                    }
                 }
 
                 return diagnostics.ToImmutableAndClear();
@@ -76,7 +68,8 @@ private async Task TextDocumentOpenAsync(TextDocument document, CancellationToke
         /// <summary>
         /// Return list of <see cref="StateSet"/> to be used for full solution analysis.
         /// </summary>
-        private ImmutableArray<StateSet> GetStateSetsForFullSolutionAnalysis(ImmutableArray<StateSet> stateSets, Project project)
+        private ImmutableArray<StateSet> GetStateSetsForFullSolutionAnalysis(
+            ImmutableArray<StateSet> stateSets, Project project, IdeAnalyzerOptions ideOptions)
         {
             // If full analysis is off, remove state that is created from build.
             // this will make sure diagnostics from build (converted from build to live) will never be cleared
@@ -104,10 +97,11 @@ private ImmutableArray<StateSet> GetStateSetsForFullSolutionAnalysis(ImmutableAr
 
             // Include only analyzers we want to run for full solution analysis.
             // Analyzers not included here will never be saved because result is unknown.
-            return stateSets.WhereAsArray(s => IsCandidateForFullSolutionAnalysis(s.Analyzer, project));
+            return stateSets.WhereAsArray(s => IsCandidateForFullSolutionAnalysis(s.Analyzer, project, ideOptions));
         }
 
-        private bool IsCandidateForFullSolutionAnalysis(DiagnosticAnalyzer analyzer, Project project)
+        private bool IsCandidateForFullSolutionAnalysis(
+            DiagnosticAnalyzer analyzer, Project project, IdeAnalyzerOptions ideOptions)
         {
             // PERF: Don't query descriptors for compiler analyzer or workspace load analyzer, always execute them.
             if (analyzer == FileContentLoadAnalyzer.Instance ||
@@ -117,17 +111,18 @@ private bool IsCandidateForFullSolutionAnalysis(DiagnosticAnalyzer analyzer, Pro
                 return true;
             }
 
+            // Open-file-only analyzers are never candidates for full solution analysis, as they have explicitly marked
+            // themselves as only being reportable for open files.
+            if (analyzer.IsOpenFileOnly(ideOptions.CleanupOptions?.SimplifierOptions))
+                return false;
+
             if (analyzer.IsBuiltInAnalyzer())
             {
-                // always return true for builtin analyzer. we can't use
-                // descriptor check since many builtin analyzer always return 
-                // hidden descriptor regardless what descriptor it actually
-                // return on runtime. they do this so that they can control
-                // severity through option page rather than rule set editor.
-                // this is special behavior only ide analyzer can do. we hope
-                // once we support editorconfig fully, third party can use this
-                // ability as well and we can remove this kind special treatment on builtin
-                // analyzer.
+                // always return true for builtin analyzer. we can't use descriptor check since many builtin analyzer
+                // always return hidden descriptor regardless what descriptor it actually return on runtime. they do
+                // this so that they can control severity through option page rather than rule set editor. this is
+                // special behavior only ide analyzer can do. we hope once we support editorconfig fully, third party
+                // can use this ability as well and we can remove this kind special treatment on builtin analyzer.
                 return true;
             }
 

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

@@ -13,7 +13,6 @@
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.PooledObjects;
-using Microsoft.CodeAnalysis.Shared.Collections;
 using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.CodeAnalysis.TaskList;
 using Roslyn.LanguageServer.Protocol;
@@ -141,21 +140,20 @@ protected override async Task WaitForChangesAsync(RequestContext context, Cancel
 
     /// <summary>
     /// There are three potential sources for reporting workspace diagnostics:
-    ///
-    ///  1. Full solution analysis: If the user has enabled Full solution analysis, we always run analysis on the latest
-    ///                             project snapshot and return up-to-date diagnostics computed from this analysis.
-    ///
-    ///  2. Code analysis service: Otherwise, if full solution analysis is disabled, and if we have diagnostics from an explicitly
-    ///                            triggered code analysis execution on either the current or a prior project snapshot, we return
-    ///                            diagnostics from this execution. These diagnostics may be stale with respect to the current
-    ///                            project snapshot, but they match user's intent of not enabling continuous background analysis
-    ///                            for always having up-to-date workspace diagnostics, but instead computing them explicitly on
-    ///                            specific project snapshots by manually running the "Run Code Analysis" command on a project or solution.
-    ///
-    ///  3. EnC analysis: Emit and debugger diagnostics associated with a closed document or not associated with any document.
-    ///
-    /// If full solution analysis is disabled AND code analysis was never executed for the given project,
-    /// we have no workspace diagnostics to report and bail out.
+    /// <list type="number">
+    /// <item>Full solution analysis: If the user has enabled Full solution analysis, we always run analysis on the
+    /// latest project snapshot and return up-to-date diagnostics computed from this analysis.</item>
+    /// <item>Code analysis service: Otherwise, if full solution analysis is disabled, and if we have diagnostics from
+    /// an explicitly triggered code analysis execution on either the current or a prior project snapshot, we return
+    /// diagnostics from this execution. These diagnostics may be stale with respect to the current project snapshot,
+    /// but they match user's intent of not enabling continuous background analysis for always having up-to-date
+    /// workspace diagnostics, but instead computing them explicitly on specific project snapshots by manually running
+    /// the "Run Code Analysis" command on a project or solution.</item>
+    /// <item>EnC analysis: Emit and debugger diagnostics associated with a closed document or not associated with any
+    /// document.</item>
+    /// </list>
+    /// If full solution analysis is disabled AND code analysis was never executed for the given project, we have no
+    /// workspace diagnostics to report and bail out.
     /// </summary>
     public static async ValueTask<ImmutableArray<IDiagnosticSource>> GetDiagnosticSourcesAsync(
         RequestContext context, IGlobalOptionService globalOptions, CancellationToken cancellationToken)
@@ -179,8 +177,9 @@ async Task AddDocumentsAndProjectAsync(Project project, CancellationToken cancel
             if (!fullSolutionAnalysisEnabled && !codeAnalysisService.HasProjectBeenAnalyzed(project.Id))
                 return;
 
-            Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer = !compilerFullSolutionAnalysisEnabled || !analyzersFullSolutionAnalysisEnabled
-                ? ShouldIncludeAnalyzer : null;
+            var ideOptions = globalOptions.GetIdeAnalyzerOptions(project);
+
+            Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer = ShouldIncludeAnalyzer;
 
             AddDocumentSources(project.Documents);
             AddDocumentSources(project.AdditionalDocuments);
@@ -221,7 +220,15 @@ void AddProjectSource()
             }
 
             bool ShouldIncludeAnalyzer(DiagnosticAnalyzer analyzer)
-                => analyzer.IsCompilerAnalyzer() ? compilerFullSolutionAnalysisEnabled : analyzersFullSolutionAnalysisEnabled;
+            {
+                // We never want to return open-file-only analyzers for workspace diagnostics.  Workspace diags are
+                // always for the closed documents.  Not doing this also breaks caching in the diagnostic subsystem
+                // layer.
+                if (analyzer.IsOpenFileOnly(ideOptions.CleanupOptions?.SimplifierOptions))
+                    return false;
+
+                return analyzer.IsCompilerAnalyzer() ? compilerFullSolutionAnalysisEnabled : analyzersFullSolutionAnalysisEnabled;
+            }
         }
     }
 

src/Features/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs

@@ -40,9 +40,10 @@ public override bool IsLiveSource()
             }
             else
             {
-                // We call GetDiagnosticsForIdsAsync as we want to ensure we get the full set of diagnostics for this document
-                // including those reported as a compilation end diagnostic.  These are not included in document pull (uses GetDiagnosticsForSpan) due to cost.
-                // However we can include them as a part of workspace pull when FSA is on.
+                // We call GetDiagnosticsForIdsAsync as we want to ensure we get the full set of diagnostics for this
+                // document including those reported as a compilation end diagnostic.  These are not included in
+                // document pull (uses GetDiagnosticsForSpan) due to cost. However we can include them as a part of
+                // workspace pull when FSA is on.
                 var documentDiagnostics = await diagnosticAnalyzerService.GetDiagnosticsForIdsAsync(
                     Document.Project.Solution, Document.Project.Id, Document.Id,
                     diagnosticIds: null, shouldIncludeAnalyzer, includeSuppressedDiagnostics: false,