Simplify determination of live vs. build diagnostics

src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs

@@ -21,6 +21,12 @@ public static class WellKnownDiagnosticTags
         /// <summary>
         /// Indicates that the diagnostic is related to build.
         /// </summary>
+        /// <remarks>
+        /// Build errors are recognized to potentially represent stale results from a point in the past when the computation occurred.
+        /// An example of when Roslyn produces non-live errors is with an explicit user gesture to "run code analysis".
+        /// Because these representerrors from the past, we do want them to be superseded by a more recent live run,
+        /// or a more recent build from another source.
+        /// </remarks>
         public const string Build = nameof(Build);
 
         /// <summary>

src/LanguageServer/ExternalAccess/VisualDiagnostics/Internal/HotReloadDiagnosticSource.cs

@@ -27,6 +27,5 @@ public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestCon
     public TextDocumentIdentifier? GetDocumentIdentifier() => new() { DocumentUri = textDocument.GetURI() };
     public ProjectOrDocumentId GetId() => new(textDocument.Id);
     public Project GetProject() => textDocument.Project;
-    public bool IsLiveSource() => true;
     public string ToDisplayString() => $"{this.GetType().Name}: {textDocument.FilePath ?? textDocument.Name} in {textDocument.Project.Name}";
 }

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

@@ -58,7 +58,8 @@ public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestCon
                     isEnabledByDefault: true,
                     // Warning level 0 is used as a placeholder when the diagnostic has error severity
                     warningLevel: 0,
-                    customTags: ImmutableArray<string>.Empty,
+                    // Mark these diagnostics as build errors so they can be overridden by diagnostics from an explicit build.
+                    customTags: [WellKnownDiagnosticTags.Build],
                     properties: ImmutableDictionary<string, string?>.Empty,
                     projectId: document.Project.Id,
                     location: new DiagnosticDataLocation(location, document.Id)
@@ -85,13 +86,6 @@ public Project GetProject()
             return document.Project;
         }
 
-        /// <summary>
-        /// These diagnostics are from the last time 'dotnet run-api' was invoked, which only occurs when a design time build is performed.
-        /// <seealso cref="IDiagnosticSource.IsLiveSource"/>.
-        /// </summary>
-        /// <returns></returns>
-        public bool IsLiveSource() => false;
-
         public string ToDisplayString() => nameof(VirtualProjectXmlDiagnosticSource);
     }
 }

src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs

@@ -17,23 +17,33 @@ namespace Microsoft.CodeAnalysis.LanguageServer;
 
 internal static partial class ProtocolConversions
 {
+    internal static ImmutableArray<DiagnosticData> AddBuildTagIfNotPresent(ImmutableArray<DiagnosticData> diagnostics)
+    {
+        return diagnostics.SelectAsArray(static d =>
+        {
+            if (d.CustomTags.Contains(WellKnownDiagnosticTags.Build))
+                return d;
+
+            return d.WithCustomTags(d.CustomTags.Add(WellKnownDiagnosticTags.Build));
+        });
+    }
+
     /// <summary>
     /// Converts from <see cref="DiagnosticData"/> to <see cref="LSP.Diagnostic"/>
     /// </summary>
     /// <param name="diagnosticData">The diagnostic to convert</param>
     /// <param name="supportsVisualStudioExtensions">Whether the client is Visual Studio</param>
     /// <param name="project">The project the diagnostic is relevant to</param>
-    /// <param name="isLiveSource">Whether the diagnostic is considered "live" and should supersede others</param>
     /// <param name="potentialDuplicate">Whether the diagnostic is potentially a duplicate to a build diagnostic</param>
     /// <param name="globalOptionService">The global options service</param>
-    public static ImmutableArray<LSP.Diagnostic> ConvertDiagnostic(DiagnosticData diagnosticData, bool supportsVisualStudioExtensions, Project project, bool isLiveSource, bool potentialDuplicate, IGlobalOptionService globalOptionService)
+    public static ImmutableArray<LSP.Diagnostic> ConvertDiagnostic(DiagnosticData diagnosticData, bool supportsVisualStudioExtensions, Project project, bool potentialDuplicate, IGlobalOptionService globalOptionService)
     {
         if (!ShouldIncludeHiddenDiagnostic(diagnosticData, supportsVisualStudioExtensions))
         {
             return [];
         }
 
-        var diagnostic = CreateLspDiagnostic(diagnosticData, project, isLiveSource, potentialDuplicate, supportsVisualStudioExtensions);
+        var diagnostic = CreateLspDiagnostic(diagnosticData, project, potentialDuplicate, supportsVisualStudioExtensions);
 
         // Check if we need to handle the unnecessary tag (fading).
         if (!diagnosticData.CustomTags.Contains(WellKnownDiagnosticTags.Unnecessary))
@@ -65,7 +75,7 @@ internal static partial class ProtocolConversions
             diagnosticsBuilder.Add(diagnostic);
             foreach (var location in unnecessaryLocations)
             {
-                var additionalDiagnostic = CreateLspDiagnostic(diagnosticData, project, isLiveSource, potentialDuplicate, supportsVisualStudioExtensions);
+                var additionalDiagnostic = CreateLspDiagnostic(diagnosticData, project, potentialDuplicate, supportsVisualStudioExtensions);
                 additionalDiagnostic.Severity = LSP.DiagnosticSeverity.Hint;
                 additionalDiagnostic.Range = GetRange(location);
                 additionalDiagnostic.Tags = [DiagnosticTag.Unnecessary, VSDiagnosticTags.HiddenInEditor, VSDiagnosticTags.HiddenInErrorList, VSDiagnosticTags.SuppressEditorToolTip];
@@ -94,7 +104,6 @@ internal static partial class ProtocolConversions
     private static LSP.VSDiagnostic CreateLspDiagnostic(
         DiagnosticData diagnosticData,
         Project project,
-        bool isLiveSource,
         bool potentialDuplicate,
         bool supportsVisualStudioExtensions)
     {
@@ -108,7 +117,7 @@ private static LSP.VSDiagnostic CreateLspDiagnostic(
             CodeDescription = ProtocolConversions.HelpLinkToCodeDescription(diagnosticData.GetValidHelpLinkUri()),
             Message = diagnosticData.Message,
             Severity = ConvertDiagnosticSeverity(diagnosticData.Severity),
-            Tags = ConvertTags(diagnosticData, isLiveSource, potentialDuplicate),
+            Tags = ConvertTags(diagnosticData, potentialDuplicate),
             DiagnosticRank = ConvertRank(diagnosticData),
             Range = GetRange(diagnosticData.DataLocation)
         };
@@ -223,7 +232,7 @@ private static LSP.DiagnosticSeverity ConvertDiagnosticSeverity(DiagnosticSeveri
     /// If you make change in this method, please also update the corresponding file in
     /// src\VisualStudio\Xaml\Impl\Implementation\LanguageServer\Handler\Diagnostics\AbstractPullDiagnosticHandler.cs
     /// </summary>
-    private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource, bool potentialDuplicate)
+    private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool potentialDuplicate)
     {
         using var _ = ArrayBuilder<DiagnosticTag>.GetInstance(out var result);
 
@@ -246,11 +255,8 @@ private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool i
         if (potentialDuplicate)
             result.Add(VSDiagnosticTags.PotentialDuplicate);
 
-        // Mark this also as a build error.  That way an explicitly kicked off build from a source like CPS can
+        // If tagged as build, mark this also as a build error.  That way an explicitly kicked off build from a source like CPS can
         // override it.
-        if (!isLiveSource)
-            result.Add(VSDiagnosticTags.BuildError);
-
         result.Add(diagnosticData.CustomTags.Contains(WellKnownDiagnosticTags.Build)
             ? VSDiagnosticTags.BuildError
             : VSDiagnosticTags.IntellisenseError);

src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_OpenDocument.cs

@@ -18,9 +18,6 @@ internal static partial class EditAndContinueDiagnosticSource
 {
     private sealed class OpenDocumentSource(Document document) : AbstractDocumentDiagnosticSource<Document>(document)
     {
-        public override bool IsLiveSource()
-            => true;
-
         public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
         {
             var designTimeDocument = Document;

src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_Workspace.cs

@@ -19,17 +19,13 @@ internal static partial class EditAndContinueDiagnosticSource
 {
     private sealed class ProjectSource(Project project, ImmutableArray<DiagnosticData> diagnostics) : AbstractProjectDiagnosticSource(project)
     {
-        public override bool IsLiveSource()
-            => true;
 
         public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
             => Task.FromResult(diagnostics);
     }
 
     private sealed class ClosedDocumentSource(TextDocument document, ImmutableArray<DiagnosticData> diagnostics) : AbstractWorkspaceDocumentDiagnosticSource(document)
     {
-        public override bool IsLiveSource()
-            => true;
 
         public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
             => Task.FromResult(diagnostics);

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

@@ -313,7 +313,6 @@ private void HandleRemovedDocuments(RequestContext context, HashSet<PreviousPull
             diagnosticData,
             capabilities.HasVisualStudioLspCapability(),
             diagnosticSource.GetProject(),
-            diagnosticSource.IsLiveSource(),
             PotentialDuplicate,
             GlobalOptions);
     }

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSourceProviders/DiagnosticSourceManager.cs

@@ -6,7 +6,6 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Composition;
-using System.Diagnostics;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -78,7 +77,7 @@ private static async ValueTask<ImmutableArray<IDiagnosticSource>> CreateDiagnost
         }
         else
         {
-            // VS Code (and legacy VS ?) pass null sourceName when requesting all sources.
+            // Some clients (legacy VS/VSCode, Razor) do not support multiple sources - a null source indicates that diagnostics from all sources should be returned.
             using var _ = ArrayBuilder<IDiagnosticSource>.GetInstance(out var sourcesBuilder);
             foreach (var (name, provider) in nameToProviderMap)
             {
@@ -106,7 +105,6 @@ public static ImmutableArray<IDiagnosticSource> AggregateSourcesIfNeeded(Immutab
         if (isDocument)
         {
             // Group all document sources into a single source.
-            Debug.Assert(sources.All(s => s.IsLiveSource()), "All document sources should be live");
             sources = [new AggregatedDocumentDiagnosticSource(sources)];
         }
         else
@@ -115,7 +113,7 @@ public static ImmutableArray<IDiagnosticSource> AggregateSourcesIfNeeded(Immutab
             // will have same value for GetDocumentIdentifier and GetProject(). Thus can be
             // aggregated in a single source which will return same values. See
             // AggregatedDocumentDiagnosticSource implementation for more details.
-            sources = [.. sources.GroupBy(s => (s.GetId(), s.IsLiveSource()), s => s).SelectMany(g => AggregatedDocumentDiagnosticSource.AggregateIfNeeded(g))];
+            sources = [.. sources.GroupBy(s => s.GetId(), s => s).SelectMany(g => AggregatedDocumentDiagnosticSource.AggregateIfNeeded(g))];
         }
 
         return sources;
@@ -141,7 +139,6 @@ public static ImmutableArray<IDiagnosticSource> AggregateIfNeeded(IEnumerable<ID
             return result;
         }
 
-        public bool IsLiveSource() => true;
         public Project GetProject() => sources[0].GetProject();
         public ProjectOrDocumentId GetId() => sources[0].GetId();
         public TextDocumentIdentifier? GetDocumentIdentifier() => sources[0].GetDocumentIdentifier();

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractDocumentDiagnosticSource.cs

@@ -16,8 +16,6 @@ internal abstract class AbstractDocumentDiagnosticSource<TDocument>(TDocument do
     public TDocument Document { get; } = document;
     public Solution Solution => this.Document.Project.Solution;
 
-    public abstract bool IsLiveSource();
-
     public abstract Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         RequestContext context, CancellationToken cancellationToken);
 

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs

@@ -7,6 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.LanguageServer;
 using Roslyn.LanguageServer.Protocol;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
@@ -23,7 +24,6 @@ public static AbstractProjectDiagnosticSource CreateForFullSolutionAnalysisDiagn
     public static AbstractProjectDiagnosticSource CreateForCodeAnalysisDiagnostics(Project project, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService)
         => new CodeAnalysisDiagnosticSource(project, codeAnalysisService);
 
-    public abstract bool IsLiveSource();
     public abstract Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken);
 
     public ProjectOrDocumentId GetId() => new(Project.Id);
@@ -37,12 +37,6 @@ public static AbstractProjectDiagnosticSource CreateForCodeAnalysisDiagnostics(P
     private sealed class FullSolutionAnalysisDiagnosticSource(Project project, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer)
         : AbstractProjectDiagnosticSource(project)
     {
-        /// <summary>
-        /// This is a normal project source that represents live/fresh diagnostics that should supersede everything else.
-        /// </summary>
-        public override bool IsLiveSource()
-            => true;
-
         public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
             RequestContext context,
             CancellationToken cancellationToken)
@@ -64,19 +58,17 @@ public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
     private sealed class CodeAnalysisDiagnosticSource(Project project, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService)
         : AbstractProjectDiagnosticSource(project)
     {
-        /// <summary>
-        /// This source provides the results of the *last* explicitly kicked off "run code analysis" command from the
-        /// user.  As such, it is definitely not "live" data, and it should be overridden by any subsequent fresh data
-        /// that has been produced.
-        /// </summary>
-        public override bool IsLiveSource()
-            => false;
-
         public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
             RequestContext context,
             CancellationToken cancellationToken)
         {
-            return Task.FromResult(codeAnalysisService.GetLastComputedProjectDiagnostics(Project.Id));
+            var diagnostics = codeAnalysisService.GetLastComputedProjectDiagnostics(Project.Id);
+
+            // This source provides the results of the *last* explicitly kicked off "run code analysis" command from the
+            // user.  As such, it is definitely not "live" data, and it should be overridden by any subsequent fresh data
+            // that has been produced.
+            diagnostics = ProtocolConversions.AddBuildTagIfNotPresent(diagnostics);
+            return Task.FromResult(diagnostics);
         }
     }
 }

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

@@ -32,12 +32,6 @@ private sealed class FullSolutionAnalysisDiagnosticSource(
         /// </summary>
         private static readonly ConditionalWeakTable<Project, AsyncLazy<ILookup<DocumentId, DiagnosticData>>> s_projectToDiagnostics = new();
 
-        /// <summary>
-        /// This is a normal document source that represents live/fresh diagnostics that should supersede everything else.
-        /// </summary>
-        public override bool IsLiveSource()
-            => true;
-
         public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
             RequestContext context,
             CancellationToken cancellationToken)
@@ -92,19 +86,17 @@ AsyncLazy<ILookup<DocumentId, DiagnosticData>> GetLazyDiagnostics()
     private sealed class CodeAnalysisDiagnosticSource(TextDocument document, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService)
         : AbstractWorkspaceDocumentDiagnosticSource(document)
     {
-        /// <summary>
-        /// This source provides the results of the *last* explicitly kicked off "run code analysis" command from the
-        /// user.  As such, it is definitely not "live" data, and it should be overridden by any subsequent fresh data
-        /// that has been produced.
-        /// </summary>
-        public override bool IsLiveSource()
-            => false;
-
         public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
             RequestContext context,
             CancellationToken cancellationToken)
         {
-            return Task.FromResult(codeAnalysisService.GetLastComputedDocumentDiagnostics(Document.Id));
+            var diagnostics = codeAnalysisService.GetLastComputedDocumentDiagnostics(Document.Id);
+
+            // This source provides the results of the *last* explicitly kicked off "run code analysis" command from the
+            // user.  As such, it is definitely not "live" data, and it should be overridden by any subsequent fresh data
+            // that has been produced.
+            diagnostics = ProtocolConversions.AddBuildTagIfNotPresent(diagnostics);
+            return Task.FromResult(diagnostics);
         }
     }
 }

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/DocumentDiagnosticSource.cs

@@ -15,12 +15,6 @@ internal sealed class DocumentDiagnosticSource(DiagnosticKind diagnosticKind, Te
 {
     public DiagnosticKind DiagnosticKind { get; } = diagnosticKind;
 
-    /// <summary>
-    /// This is a normal document source that represents live/fresh diagnostics that should supersede everything else.
-    /// </summary>
-    public override bool IsLiveSource()
-        => true;
-
     public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         RequestContext context, CancellationToken cancellationToken)
     {

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs

@@ -20,17 +20,6 @@ internal interface IDiagnosticSource
     ProjectOrDocumentId GetId();
     TextDocumentIdentifier? GetDocumentIdentifier();
     string ToDisplayString();
-
-    /// <summary>
-    /// True if this source produces diagnostics that are considered 'live' or not.  Live errors represent up to date
-    /// information that should supersede other sources.  Non 'live' errors (aka "build errors") are recognized to
-    /// potentially represent stale results from a point in the past when the computation occurred.  An example of when
-    /// Roslyn produces non-live errors is with an explicit user gesture to "run code analysis". Because these represent
-    /// errors from the past, we do want them to be superseded by a more recent live run, or a more recent build from
-    /// another source.
-    /// </summary>
-    bool IsLiveSource();
-
     Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         RequestContext context,
         CancellationToken cancellationToken);

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/NonLocalDocumentDiagnosticSource.cs

@@ -16,9 +16,6 @@ internal sealed class NonLocalDocumentDiagnosticSource(
 {
     private readonly Func<DiagnosticAnalyzer, bool>? _shouldIncludeAnalyzer = shouldIncludeAnalyzer;
 
-    public override bool IsLiveSource()
-        => true;
-
     public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         RequestContext context,
         CancellationToken cancellationToken)

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/TaskListDiagnosticSource.cs

@@ -29,9 +29,6 @@ internal sealed class TaskListDiagnosticSource(Document document, IGlobalOptionS
 
     private readonly IGlobalOptionService _globalOptions = globalOptions;
 
-    public override bool IsLiveSource()
-        => true;
-
     public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         RequestContext context, CancellationToken cancellationToken)
     {

src/LanguageServer/ProtocolUnitTests/Diagnostics/AdditionalFileDiagnosticsTests.cs

@@ -217,8 +217,6 @@ public Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext c
 
             public Project GetProject() => textDocument.Project;
 
-            public bool IsLiveSource() => true;
-
             public string ToDisplayString() => textDocument.ToString()!;
         }
     }