Skip to content

Commit 9a48772

Browse files
authored
Simplify determination of live vs. build diagnostics (#79691)
Removes IsLive from the diagnostic source, and relies on diagnostic tags to correctly set the `Build` diagnostic tag. This fixes a debug assert crash when using Razor with the latest main changes ``` 2025-07-30 14:46:02.141 [error] [stderr] Process terminated. Assertion failed. All document sources should be live at Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.DiagnosticSourceManager.AggregateSourcesIfNeeded(ImmutableArray`1 sources, Boolean isDocument) in C:\Users\xiaoyuz\source\repo\roslyn\src\LanguageServer\Protocol\Handler\Diagnostics\DiagnosticSourceProviders\DiagnosticSourceManager.cs:line 109 at Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.DiagnosticSourceManager.CreateDiagnosticSourcesAsync(RequestContext context, String providerName, ImmutableDictionary`2 nameToProviderMap, Boolean isDocument, CancellationToken cancellationToken) in C:\Users\xiaoyuz\source\repo\roslyn\src\LanguageServer\Protocol\Handler\Diagnostics\DiagnosticSourceProviders\DiagnosticSourceManager.cs:line 95 at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) at Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.DiagnosticSourceManager.CreateDiagnosticSourcesAsync(RequestContext context, String providerName, ImmutableDictionary`2 nameToProviderMap, Boolean isDocument, CancellationToken cancellationToken) at Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.DiagnosticSourceManager.CreateDocumentDiagnosticSourcesAsync(RequestContext context, String providerName, CancellationToken cancellationToken) in C:\Users\xiaoyuz\source\repo\roslyn\src\LanguageServer\Protocol\Handler\Diagnostics\DiagnosticSourceProviders\DiagnosticSourceManager.cs:line 55 at Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.AbstractDocumentPullDiagnosticHandler`3.GetOrderedDiagnosticSourcesAsync(TDiagnosticsParams diagnosticsParams, String requestDiagnosticCategory, RequestContext context, CancellationToken cancellationToken) in C:\Users\xiaoyuz\source\repo\roslyn\src\LanguageServer\Protocol\Handler\Diagnostics\AbstractDocumentPullDiagnosticHandler.cs:line 50 at Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.AbstractPullDiagnosticHandler`3.HandleRequestAsync(TDiagnosticsParams diagnosticsParams, RequestContext context, CancellationToken cancellationToken) in C:\Users\xiaoyuz\source\repo\roslyn\src\LanguageServer\Protocol\Handler\Diagnostics\AbstractPullDiagnosticHandler.cs:line 142 at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) at Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics.AbstractPullDiagnosticHandler`3.HandleRequestAsync(TDiagnosticsParams diagnosticsParams, RequestContext context, CancellationToken cancellationToken) at Microsoft.CommonLanguageServerProtocol.Framework.QueueItem`1.StartRequestAsync[TRequest,TResponse](TRequest request, TRequestContext context, IMethodHandler handler, String language, CancellationToken cancellationToken) in C:\Users\xiaoyuz\source\repo\roslyn\src\LanguageServer\Microsoft.CommonLanguageServerProtocol.Framework\QueueItem.cs:line 191 at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) at Microsoft.CommonLanguageServerProtocol.Framework.QueueItem`1.StartRequestAsync[TRequest,TResponse](TRequest request, TRequestContext context, IMethodHandler handler, String language, CancellationToken cancellationToken) at Microsoft.CommonLanguageServerProtocol.Framework.RequestExecutionQueue`1.<>c__DisplayClass18_0`2.<ProcessQueueCoreAsync>b__2() in C:\Users\xiaoyuz\source\repo\roslyn\src\LanguageServer\Microsoft.CommonLanguageServerProtocol.Framework\RequestExecutionQueue.cs:line 378 at System.Threading.Tasks.Task`1.InnerInvoke() at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state) at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) at System.Threading.ThreadPoolWorkQueue.Dispatch() at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart() ``` this bug was revealed in #79421 when a non-live doc diagnostic source was added
2 parents 0be066d + c640e34 commit 9a48772

File tree

20 files changed

+48
-100
lines changed

20 files changed

+48
-100
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ public static class WellKnownDiagnosticTags
2121
/// <summary>
2222
/// Indicates that the diagnostic is related to build.
2323
/// </summary>
24+
/// <remarks>
25+
/// Build errors are recognized to potentially represent stale results from a point in the past when the computation occurred.
26+
/// An example of when Roslyn produces non-live errors is with an explicit user gesture to "run code analysis".
27+
/// Because these represent errors from the past, we do want them to be superseded by a more recent live run,
28+
/// or a more recent build from another source.
29+
/// </remarks>
2430
public const string Build = nameof(Build);
2531

2632
/// <summary>

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,5 @@ public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestCon
2727
public TextDocumentIdentifier? GetDocumentIdentifier() => new() { DocumentUri = textDocument.GetURI() };
2828
public ProjectOrDocumentId GetId() => new(textDocument.Id);
2929
public Project GetProject() => textDocument.Project;
30-
public bool IsLiveSource() => true;
3130
public string ToDisplayString() => $"{this.GetType().Name}: {textDocument.FilePath ?? textDocument.Name} in {textDocument.Project.Name}";
3231
}

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ public async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestCon
5858
isEnabledByDefault: true,
5959
// Warning level 0 is used as a placeholder when the diagnostic has error severity
6060
warningLevel: 0,
61-
customTags: ImmutableArray<string>.Empty,
61+
// Mark these diagnostics as build errors so they can be overridden by diagnostics from an explicit build.
62+
customTags: [WellKnownDiagnosticTags.Build],
6263
properties: ImmutableDictionary<string, string?>.Empty,
6364
projectId: document.Project.Id,
6465
location: new DiagnosticDataLocation(location, document.Id)
@@ -85,13 +86,6 @@ public Project GetProject()
8586
return document.Project;
8687
}
8788

88-
/// <summary>
89-
/// These diagnostics are from the last time 'dotnet run-api' was invoked, which only occurs when a design time build is performed.
90-
/// <seealso cref="IDiagnosticSource.IsLiveSource"/>.
91-
/// </summary>
92-
/// <returns></returns>
93-
public bool IsLiveSource() => false;
94-
9589
public string ToDisplayString() => nameof(VirtualProjectXmlDiagnosticSource);
9690
}
9791
}

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,33 @@ namespace Microsoft.CodeAnalysis.LanguageServer;
1717

1818
internal static partial class ProtocolConversions
1919
{
20+
internal static ImmutableArray<DiagnosticData> AddBuildTagIfNotPresent(ImmutableArray<DiagnosticData> diagnostics)
21+
{
22+
return diagnostics.SelectAsArray(static d =>
23+
{
24+
if (d.CustomTags.Contains(WellKnownDiagnosticTags.Build))
25+
return d;
26+
27+
return d.WithCustomTags(d.CustomTags.Add(WellKnownDiagnosticTags.Build));
28+
});
29+
}
30+
2031
/// <summary>
2132
/// Converts from <see cref="DiagnosticData"/> to <see cref="LSP.Diagnostic"/>
2233
/// </summary>
2334
/// <param name="diagnosticData">The diagnostic to convert</param>
2435
/// <param name="supportsVisualStudioExtensions">Whether the client is Visual Studio</param>
2536
/// <param name="project">The project the diagnostic is relevant to</param>
26-
/// <param name="isLiveSource">Whether the diagnostic is considered "live" and should supersede others</param>
2737
/// <param name="potentialDuplicate">Whether the diagnostic is potentially a duplicate to a build diagnostic</param>
2838
/// <param name="globalOptionService">The global options service</param>
29-
public static ImmutableArray<LSP.Diagnostic> ConvertDiagnostic(DiagnosticData diagnosticData, bool supportsVisualStudioExtensions, Project project, bool isLiveSource, bool potentialDuplicate, IGlobalOptionService globalOptionService)
39+
public static ImmutableArray<LSP.Diagnostic> ConvertDiagnostic(DiagnosticData diagnosticData, bool supportsVisualStudioExtensions, Project project, bool potentialDuplicate, IGlobalOptionService globalOptionService)
3040
{
3141
if (!ShouldIncludeHiddenDiagnostic(diagnosticData, supportsVisualStudioExtensions))
3242
{
3343
return [];
3444
}
3545

36-
var diagnostic = CreateLspDiagnostic(diagnosticData, project, isLiveSource, potentialDuplicate, supportsVisualStudioExtensions);
46+
var diagnostic = CreateLspDiagnostic(diagnosticData, project, potentialDuplicate, supportsVisualStudioExtensions);
3747

3848
// Check if we need to handle the unnecessary tag (fading).
3949
if (!diagnosticData.CustomTags.Contains(WellKnownDiagnosticTags.Unnecessary))
@@ -65,7 +75,7 @@ internal static partial class ProtocolConversions
6575
diagnosticsBuilder.Add(diagnostic);
6676
foreach (var location in unnecessaryLocations)
6777
{
68-
var additionalDiagnostic = CreateLspDiagnostic(diagnosticData, project, isLiveSource, potentialDuplicate, supportsVisualStudioExtensions);
78+
var additionalDiagnostic = CreateLspDiagnostic(diagnosticData, project, potentialDuplicate, supportsVisualStudioExtensions);
6979
additionalDiagnostic.Severity = LSP.DiagnosticSeverity.Hint;
7080
additionalDiagnostic.Range = GetRange(location);
7181
additionalDiagnostic.Tags = [DiagnosticTag.Unnecessary, VSDiagnosticTags.HiddenInEditor, VSDiagnosticTags.HiddenInErrorList, VSDiagnosticTags.SuppressEditorToolTip];
@@ -94,7 +104,6 @@ internal static partial class ProtocolConversions
94104
private static LSP.VSDiagnostic CreateLspDiagnostic(
95105
DiagnosticData diagnosticData,
96106
Project project,
97-
bool isLiveSource,
98107
bool potentialDuplicate,
99108
bool supportsVisualStudioExtensions)
100109
{
@@ -108,7 +117,7 @@ private static LSP.VSDiagnostic CreateLspDiagnostic(
108117
CodeDescription = ProtocolConversions.HelpLinkToCodeDescription(diagnosticData.GetValidHelpLinkUri()),
109118
Message = diagnosticData.Message,
110119
Severity = ConvertDiagnosticSeverity(diagnosticData.Severity),
111-
Tags = ConvertTags(diagnosticData, isLiveSource, potentialDuplicate),
120+
Tags = ConvertTags(diagnosticData, potentialDuplicate),
112121
DiagnosticRank = ConvertRank(diagnosticData),
113122
Range = GetRange(diagnosticData.DataLocation)
114123
};
@@ -223,7 +232,7 @@ private static LSP.DiagnosticSeverity ConvertDiagnosticSeverity(DiagnosticSeveri
223232
/// If you make change in this method, please also update the corresponding file in
224233
/// src\VisualStudio\Xaml\Impl\Implementation\LanguageServer\Handler\Diagnostics\AbstractPullDiagnosticHandler.cs
225234
/// </summary>
226-
private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource, bool potentialDuplicate)
235+
private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool potentialDuplicate)
227236
{
228237
using var _ = ArrayBuilder<DiagnosticTag>.GetInstance(out var result);
229238

@@ -246,11 +255,8 @@ private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool i
246255
if (potentialDuplicate)
247256
result.Add(VSDiagnosticTags.PotentialDuplicate);
248257

249-
// Mark this also as a build error. That way an explicitly kicked off build from a source like CPS can
258+
// If tagged as build, mark this also as a build error. That way an explicitly kicked off build from a source like CPS can
250259
// override it.
251-
if (!isLiveSource)
252-
result.Add(VSDiagnosticTags.BuildError);
253-
254260
result.Add(diagnosticData.CustomTags.Contains(WellKnownDiagnosticTags.Build)
255261
? VSDiagnosticTags.BuildError
256262
: VSDiagnosticTags.IntellisenseError);

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ internal static partial class EditAndContinueDiagnosticSource
1818
{
1919
private sealed class OpenDocumentSource(Document document) : AbstractDocumentDiagnosticSource<Document>(document)
2020
{
21-
public override bool IsLiveSource()
22-
=> true;
23-
2421
public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
2522
{
2623
var designTimeDocument = Document;

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,13 @@ internal static partial class EditAndContinueDiagnosticSource
1919
{
2020
private sealed class ProjectSource(Project project, ImmutableArray<DiagnosticData> diagnostics) : AbstractProjectDiagnosticSource(project)
2121
{
22-
public override bool IsLiveSource()
23-
=> true;
2422

2523
public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
2624
=> Task.FromResult(diagnostics);
2725
}
2826

2927
private sealed class ClosedDocumentSource(TextDocument document, ImmutableArray<DiagnosticData> diagnostics) : AbstractWorkspaceDocumentDiagnosticSource(document)
3028
{
31-
public override bool IsLiveSource()
32-
=> true;
3329

3430
public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken)
3531
=> Task.FromResult(diagnostics);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ private void HandleRemovedDocuments(RequestContext context, HashSet<PreviousPull
313313
diagnosticData,
314314
capabilities.HasVisualStudioLspCapability(),
315315
diagnosticSource.GetProject(),
316-
diagnosticSource.IsLiveSource(),
317316
PotentialDuplicate,
318317
GlobalOptions);
319318
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using System.Collections.Generic;
77
using System.Collections.Immutable;
88
using System.Composition;
9-
using System.Diagnostics;
109
using System.Linq;
1110
using System.Threading;
1211
using System.Threading.Tasks;
@@ -78,7 +77,7 @@ private static async ValueTask<ImmutableArray<IDiagnosticSource>> CreateDiagnost
7877
}
7978
else
8079
{
81-
// VS Code (and legacy VS ?) pass null sourceName when requesting all sources.
80+
// Some clients (legacy VS/VSCode, Razor) do not support multiple sources - a null source indicates that diagnostics from all sources should be returned.
8281
using var _ = ArrayBuilder<IDiagnosticSource>.GetInstance(out var sourcesBuilder);
8382
foreach (var (name, provider) in nameToProviderMap)
8483
{
@@ -106,7 +105,6 @@ public static ImmutableArray<IDiagnosticSource> AggregateSourcesIfNeeded(Immutab
106105
if (isDocument)
107106
{
108107
// Group all document sources into a single source.
109-
Debug.Assert(sources.All(s => s.IsLiveSource()), "All document sources should be live");
110108
sources = [new AggregatedDocumentDiagnosticSource(sources)];
111109
}
112110
else
@@ -115,7 +113,7 @@ public static ImmutableArray<IDiagnosticSource> AggregateSourcesIfNeeded(Immutab
115113
// will have same value for GetDocumentIdentifier and GetProject(). Thus can be
116114
// aggregated in a single source which will return same values. See
117115
// AggregatedDocumentDiagnosticSource implementation for more details.
118-
sources = [.. sources.GroupBy(s => (s.GetId(), s.IsLiveSource()), s => s).SelectMany(g => AggregatedDocumentDiagnosticSource.AggregateIfNeeded(g))];
116+
sources = [.. sources.GroupBy(s => s.GetId(), s => s).SelectMany(g => AggregatedDocumentDiagnosticSource.AggregateIfNeeded(g))];
119117
}
120118

121119
return sources;
@@ -141,7 +139,6 @@ public static ImmutableArray<IDiagnosticSource> AggregateIfNeeded(IEnumerable<ID
141139
return result;
142140
}
143141

144-
public bool IsLiveSource() => true;
145142
public Project GetProject() => sources[0].GetProject();
146143
public ProjectOrDocumentId GetId() => sources[0].GetId();
147144
public TextDocumentIdentifier? GetDocumentIdentifier() => sources[0].GetDocumentIdentifier();

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ internal abstract class AbstractDocumentDiagnosticSource<TDocument>(TDocument do
1616
public TDocument Document { get; } = document;
1717
public Solution Solution => this.Document.Project.Solution;
1818

19-
public abstract bool IsLiveSource();
20-
2119
public abstract Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
2220
RequestContext context, CancellationToken cancellationToken);
2321

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

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Threading;
88
using System.Threading.Tasks;
99
using Microsoft.CodeAnalysis.Diagnostics;
10+
using Microsoft.CodeAnalysis.LanguageServer;
1011
using Roslyn.LanguageServer.Protocol;
1112

1213
namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
@@ -23,7 +24,6 @@ public static AbstractProjectDiagnosticSource CreateForFullSolutionAnalysisDiagn
2324
public static AbstractProjectDiagnosticSource CreateForCodeAnalysisDiagnostics(Project project, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService)
2425
=> new CodeAnalysisDiagnosticSource(project, codeAnalysisService);
2526

26-
public abstract bool IsLiveSource();
2727
public abstract Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken);
2828

2929
public ProjectOrDocumentId GetId() => new(Project.Id);
@@ -37,12 +37,6 @@ public static AbstractProjectDiagnosticSource CreateForCodeAnalysisDiagnostics(P
3737
private sealed class FullSolutionAnalysisDiagnosticSource(Project project, Func<DiagnosticAnalyzer, bool>? shouldIncludeAnalyzer)
3838
: AbstractProjectDiagnosticSource(project)
3939
{
40-
/// <summary>
41-
/// This is a normal project source that represents live/fresh diagnostics that should supersede everything else.
42-
/// </summary>
43-
public override bool IsLiveSource()
44-
=> true;
45-
4640
public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
4741
RequestContext context,
4842
CancellationToken cancellationToken)
@@ -64,19 +58,17 @@ public override async Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
6458
private sealed class CodeAnalysisDiagnosticSource(Project project, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService)
6559
: AbstractProjectDiagnosticSource(project)
6660
{
67-
/// <summary>
68-
/// This source provides the results of the *last* explicitly kicked off "run code analysis" command from the
69-
/// user. As such, it is definitely not "live" data, and it should be overridden by any subsequent fresh data
70-
/// that has been produced.
71-
/// </summary>
72-
public override bool IsLiveSource()
73-
=> false;
74-
7561
public override Task<ImmutableArray<DiagnosticData>> GetDiagnosticsAsync(
7662
RequestContext context,
7763
CancellationToken cancellationToken)
7864
{
79-
return Task.FromResult(codeAnalysisService.GetLastComputedProjectDiagnostics(Project.Id));
65+
var diagnostics = codeAnalysisService.GetLastComputedProjectDiagnostics(Project.Id);
66+
67+
// This source provides the results of the *last* explicitly kicked off "run code analysis" command from the
68+
// user. As such, it is definitely not "live" data, and it should be overridden by any subsequent fresh data
69+
// that has been produced.
70+
diagnostics = ProtocolConversions.AddBuildTagIfNotPresent(diagnostics);
71+
return Task.FromResult(diagnostics);
8072
}
8173
}
8274
}

0 commit comments

Comments
 (0)