Merge pull request #70402 from CyrusNajmabadi/liveSource

Mark diagnostics as being 'live' or not in LSP

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

@@ -20,7 +20,8 @@
 
 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>
+        : AbstractPullDiagnosticHandler<TDiagnosticsParams, TReport, TReturn>, ITextDocumentIdentifierHandler<TDiagnosticsParams, TextDocumentIdentifier?>
         where TDiagnosticsParams : IPartialResultParams<TReport>
     {
         public AbstractDocumentPullDiagnosticHandler(
@@ -113,7 +114,7 @@ protected abstract ValueTask<ImmutableArray<IDiagnosticSource>> GetOrderedDiagno
         /// <summary>
         /// Generate the right diagnostic tags for a particular diagnostic.
         /// </summary>
-        protected abstract DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData);
+        protected abstract DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource);
 
         protected abstract string? GetDiagnosticCategory(TDiagnosticsParams diagnosticsParams);
 
@@ -400,12 +401,11 @@ LSP.VSDiagnostic CreateLspDiagnostic(
                     CodeDescription = ProtocolConversions.HelpLinkToCodeDescription(diagnosticData.GetValidHelpLinkUri()),
                     Message = diagnosticData.Message,
                     Severity = ConvertDiagnosticSeverity(diagnosticData.Severity, capabilities),
-                    Tags = ConvertTags(diagnosticData),
+                    Tags = ConvertTags(diagnosticData, diagnosticSource.IsLiveSource()),
                     DiagnosticRank = ConvertRank(diagnosticData),
+                    Range = GetRange(diagnosticData.DataLocation)
                 };
 
-                diagnostic.Range = GetRange(diagnosticData.DataLocation);
-
                 if (capabilities.HasVisualStudioLspCapability())
                 {
                     diagnostic.DiagnosticType = diagnosticData.Category;
@@ -522,7 +522,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>
-        protected static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool potentialDuplicate)
+        protected static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource, bool potentialDuplicate)
         {
             using var _ = ArrayBuilder<DiagnosticTag>.GetInstance(out var result);
 
@@ -540,9 +540,16 @@ protected static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool
             if (diagnosticData.CustomTags.Contains(PullDiagnosticConstants.TaskItemCustomTag))
                 result.Add(VSDiagnosticTags.TaskItem);
 
+            // Let the host know that these errors represent potentially stale information from the past that should
+            // be superseded by fresher info.
             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
+            // override it.
+            if (!isLiveSource)
+                result.Add(VSDiagnosticTags.BuildError);
+
             result.Add(diagnosticData.CustomTags.Contains(WellKnownDiagnosticTags.Build)
                 ? VSDiagnosticTags.BuildError
                 : VSDiagnosticTags.IntellisenseError);

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

@@ -10,13 +10,15 @@
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
 
-internal abstract class AbstractDocumentDiagnosticSource<TDocument> : IDiagnosticSource
+internal abstract class AbstractDocumentDiagnosticSource<TDocument>(TDocument document) : IDiagnosticSource
     where TDocument : TextDocument
 {
-    public TDocument Document { get; }
+    public TDocument Document { get; } = document;
 
-    public AbstractDocumentDiagnosticSource(TDocument document)
-        => Document = document;
+    public abstract bool IsLiveSource();
+
+    public abstract Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
+        IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken);
 
     public ProjectOrDocumentId GetId() => new(Document.Id);
     public Project GetProject() => Document.Project;
@@ -27,7 +29,4 @@ public AbstractDocumentDiagnosticSource(TDocument document)
             : null;
 
     public string ToDisplayString() => $"{this.GetType().Name}: {Document.FilePath ?? Document.Name} in {Document.Project.Name}";
-
-    public abstract Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
-        IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken);
 }

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

@@ -22,6 +22,7 @@ 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(IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken);
 
     public ProjectOrDocumentId GetId() => new(Project.Id);
@@ -34,6 +35,12 @@ 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(
             IDiagnosticAnalyzerService diagnosticAnalyzerService,
             RequestContext context,
@@ -50,6 +57,14 @@ 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(
             IDiagnosticAnalyzerService diagnosticAnalyzerService,
             RequestContext context,

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

@@ -18,8 +18,15 @@ public static AbstractWorkspaceDocumentDiagnosticSource CreateForFullSolutionAna
     public static AbstractWorkspaceDocumentDiagnosticSource CreateForCodeAnalysisDiagnostics(TextDocument document, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService)
         => new CodeAnalysisDiagnosticSource(document, codeAnalysisService);
 
-    private sealed class FullSolutionAnalysisDiagnosticSource(TextDocument document, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer) : AbstractWorkspaceDocumentDiagnosticSource(document)
+    private sealed class FullSolutionAnalysisDiagnosticSource(TextDocument document, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer)
+        : AbstractWorkspaceDocumentDiagnosticSource(document)
     {
+        /// <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(
             IDiagnosticAnalyzerService diagnosticAnalyzerService,
             RequestContext context,
@@ -45,8 +52,17 @@ public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         }
     }
 
-    private sealed class CodeAnalysisDiagnosticSource(TextDocument document, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService) : AbstractWorkspaceDocumentDiagnosticSource(document)
+    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(
             IDiagnosticAnalyzerService diagnosticAnalyzerService,
             RequestContext context,

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

@@ -9,16 +9,16 @@
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
 
-internal sealed class DocumentDiagnosticSource
-    : AbstractDocumentDiagnosticSource<Document>
+internal sealed class DocumentDiagnosticSource(DiagnosticKind diagnosticKind, Document document)
+    : AbstractDocumentDiagnosticSource<Document>(document)
 {
-    public DiagnosticKind DiagnosticKind { get; }
+    public DiagnosticKind DiagnosticKind { get; } = diagnosticKind;
 
-    public DocumentDiagnosticSource(DiagnosticKind diagnosticKind, Document document)
-        : base(document)
-    {
-        DiagnosticKind = 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(
         IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken)

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

@@ -22,6 +22,16 @@ internal interface IDiagnosticSource
     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.  The only time
+    /// Roslyn produces non-live errors through 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(
         IDiagnosticAnalyzerService diagnosticAnalyzerService,
         RequestContext context,

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

@@ -10,10 +10,14 @@
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
 
-internal sealed class NonLocalDocumentDiagnosticSource(TextDocument document, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer) : AbstractDocumentDiagnosticSource<TextDocument>(document)
+internal sealed class NonLocalDocumentDiagnosticSource(TextDocument document, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer)
+    : AbstractDocumentDiagnosticSource<TextDocument>(document)
 {
     private readonly Func<DiagnosticAnalyzer, bool>? _shouldIncludeAnalyzer = shouldIncludeAnalyzer;
 
+    public override bool IsLiveSource()
+        => true;
+
     public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         IDiagnosticAnalyzerService diagnosticAnalyzerService,
         RequestContext context,

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

@@ -16,7 +16,7 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
 
 using static PullDiagnosticConstants;
 
-internal sealed class TaskListDiagnosticSource : AbstractDocumentDiagnosticSource<Document>
+internal sealed class TaskListDiagnosticSource(Document document, IGlobalOptionService globalOptions) : AbstractDocumentDiagnosticSource<Document>(document)
 {
     private static readonly ImmutableArray<string> s_todoCommentCustomTags = ImmutableArray.Create(TaskItemCustomTag);
 
@@ -27,13 +27,10 @@ internal sealed class TaskListDiagnosticSource : AbstractDocumentDiagnosticSourc
     private static Tuple<ImmutableArray<string>, ImmutableArray<TaskListItemDescriptor>> s_lastRequestedTokens =
         Tuple.Create(ImmutableArray<string>.Empty, ImmutableArray<TaskListItemDescriptor>.Empty);
 
-    private readonly IGlobalOptionService _globalOptions;
+    private readonly IGlobalOptionService _globalOptions = globalOptions;
 
-    public TaskListDiagnosticSource(Document document, IGlobalOptionService globalOptions)
-        : base(document)
-    {
-        _globalOptions = globalOptions;
-    }
+    public override bool IsLiveSource()
+        => true;
 
     public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
         IDiagnosticAnalyzerService diagnosticAnalyzerService, RequestContext context, CancellationToken cancellationToken)

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

@@ -2,23 +2,20 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
 using System.Collections.Immutable;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Diagnostics;
-using Microsoft.CodeAnalysis.EditAndContinue;
 using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.SolutionCrawler;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
-using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics
 {
     [Method(VSInternalMethods.DocumentPullDiagnosticName)]
-    internal partial class DocumentPullDiagnosticHandler : AbstractDocumentPullDiagnosticHandler<VSInternalDocumentDiagnosticsParams, VSInternalDiagnosticReport[], VSInternalDiagnosticReport[]>
+    internal partial class DocumentPullDiagnosticHandler
+        : AbstractDocumentPullDiagnosticHandler<VSInternalDocumentDiagnosticsParams, VSInternalDiagnosticReport[], VSInternalDiagnosticReport[]>
     {
         public DocumentPullDiagnosticHandler(
             IDiagnosticAnalyzerService analyzerService,
@@ -67,8 +64,8 @@ protected override VSInternalDiagnosticReport[] CreateUnchangedReport(TextDocume
             return null;
         }
 
-        protected override DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData)
-            => ConvertTags(diagnosticData, potentialDuplicate: false);
+        protected override DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource)
+            => ConvertTags(diagnosticData, isLiveSource, potentialDuplicate: false);
 
         protected override ValueTask<ImmutableArray<IDiagnosticSource>> GetOrderedDiagnosticSourcesAsync(
             VSInternalDocumentDiagnosticsParams diagnosticsParams, RequestContext context, CancellationToken cancellationToken)

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

@@ -49,27 +49,25 @@ public PublicDocumentPullDiagnosticsHandler(
 
     protected override string? GetDiagnosticSourceIdentifier(DocumentDiagnosticParams diagnosticsParams) => diagnosticsParams.Identifier;
 
-    protected override DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData)
-    {
-        return ConvertTags(diagnosticData, potentialDuplicate: false);
-    }
+    protected override DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource)
+        => ConvertTags(diagnosticData, isLiveSource, potentialDuplicate: false);
 
     protected override DocumentDiagnosticPartialReport CreateReport(TextDocumentIdentifier identifier, VisualStudio.LanguageServer.Protocol.Diagnostic[] diagnostics, string resultId)
-        => new DocumentDiagnosticPartialReport(new RelatedFullDocumentDiagnosticReport
+        => new(new RelatedFullDocumentDiagnosticReport
         {
             ResultId = resultId,
             Items = diagnostics,
         });
 
     protected override DocumentDiagnosticPartialReport CreateRemovedReport(TextDocumentIdentifier identifier)
-        => new DocumentDiagnosticPartialReport(new RelatedFullDocumentDiagnosticReport
+        => new(new RelatedFullDocumentDiagnosticReport
         {
             ResultId = null,
             Items = Array.Empty<VisualStudio.LanguageServer.Protocol.Diagnostic>(),
         });
 
     protected override DocumentDiagnosticPartialReport CreateUnchangedReport(TextDocumentIdentifier identifier, string resultId)
-        => new DocumentDiagnosticPartialReport(new RelatedUnchangedDocumentDiagnosticReport
+        => new(new RelatedUnchangedDocumentDiagnosticReport
         {
             ResultId = resultId
         });

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

@@ -62,13 +62,11 @@ public void Dispose()
     protected override string? GetDiagnosticCategory(WorkspaceDiagnosticParams diagnosticsParams)
         => null;
 
-    protected override DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData)
-    {
-        return ConvertTags(diagnosticData, potentialDuplicate: false);
-    }
+    protected override DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource)
+        => ConvertTags(diagnosticData, isLiveSource, potentialDuplicate: false);
 
     protected override WorkspaceDiagnosticPartialReport CreateReport(TextDocumentIdentifier identifier, VisualStudio.LanguageServer.Protocol.Diagnostic[] diagnostics, string resultId)
-        => new WorkspaceDiagnosticPartialReport(new WorkspaceDiagnosticReport
+        => new(new WorkspaceDiagnosticReport
         {
             Items = new SumType<WorkspaceFullDocumentDiagnosticReport, WorkspaceUnchangedDocumentDiagnosticReport>[]
             {
@@ -84,7 +82,7 @@ protected override WorkspaceDiagnosticPartialReport CreateReport(TextDocumentIde
         });
 
     protected override WorkspaceDiagnosticPartialReport CreateRemovedReport(TextDocumentIdentifier identifier)
-        => new WorkspaceDiagnosticPartialReport(new WorkspaceDiagnosticReport
+        => new(new WorkspaceDiagnosticReport
         {
             Items = new SumType<WorkspaceFullDocumentDiagnosticReport, WorkspaceUnchangedDocumentDiagnosticReport>[]
             {
@@ -100,7 +98,7 @@ protected override WorkspaceDiagnosticPartialReport CreateRemovedReport(TextDocu
         });
 
     protected override WorkspaceDiagnosticPartialReport CreateUnchangedReport(TextDocumentIdentifier identifier, string resultId)
-        => new WorkspaceDiagnosticPartialReport(new WorkspaceDiagnosticReport
+        => new(new WorkspaceDiagnosticReport
         {
             Items = new SumType<WorkspaceFullDocumentDiagnosticReport, WorkspaceUnchangedDocumentDiagnosticReport>[]
             {