Simplify determination of live vs. build diagnostics #79691

dibarbet · 2025-07-30T22:58:40Z

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

dibarbet · 2025-07-30T23:11:16Z

...t.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlDiagnosticSourceProvider.cs

                    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],


We are hitting a debug assertion in DiagnosticSourceManager (see below) when attempting to aggregate document diagnostics from multiple sources when a client calls into us that does not support multiple sources (these are Razor, super old versions of VS/VSCode, or third party LSPs). We could not aggregate live and non-live diagnostic sources into a single source as the diagnostics returned are modified based on the source.

We recently introduced a new non-live document diagnostic source, which triggered the assert (see below) when Razor calls into us.

After looking at this, it appeared to me as though a diagnostic source being 'live' or 'not live' was not the correct abstraction. At the end of the day, IsLiveSource was only ever read to set a tag on the diagnostic indicating if it was a build diagnostic. Instead of having IsLiveSource be on the IDiagnosticSource, we can just create diagnostics with the correct build tag to begin with!

dibarbet · 2025-07-30T23:13:22Z

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

+        // 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);


as above, this was the only place IDiagnosticSource.isLiveSource eventually got read. And we already set the build tag based on our build tag below. So we can just set the build tag on the diagnostics directly instead of this whole method on the source.

dibarbet · 2025-07-30T23:14:00Z

...uageServer/Protocol/Handler/Diagnostics/DiagnosticSourceProviders/DiagnosticSourceManager.cs

        if (isDocument)
        {
            // Group all document sources into a single source.
-            Debug.Assert(sources.All(s => s.IsLiveSource()), "All document sources should be live");


this was the assert that was firing. Now there is no concept of 'live' vs. non-live sources, just live vs. non-live diagnostics

dibarbet · 2025-07-30T23:14:46Z

...uageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs

+            //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 = [.. diagnostics.Select(d => d.WithCustomTags(d.CustomTags.Add(WellKnownDiagnosticTags.Build)))];


explicitly tag the diagnostics as build diagnostics in the impl, instead of using IsLiveSource

Is there any possibility of the diagnostics already having Build tags?

yeah. might want a helper that only adds if not alreadythere. and call that single helper from all these places.

jasonmalinowski

Seems reasonable to me, I'm not the expert here though.

...uageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs

jasonmalinowski · 2025-07-31T01:00:11Z

...uageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs

+            //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 = [.. diagnostics.Select(d => d.WithCustomTags(d.CustomTags.Add(WellKnownDiagnosticTags.Build)))];


Is there any possibility of the diagnostics already having Build tags?

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

...uageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs

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

CyrusNajmabadi

Overall seems good. tiny cleanup maybe.

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

Co-authored-by: Rikki Gibson <[email protected]>

Simplify determination of live vs. build diagnostics

c5150d5

dotnet-policy-service bot added the VSCode label Jul 30, 2025

dibarbet commented Jul 30, 2025

View reviewed changes

dibarbet marked this pull request as ready for review July 30, 2025 23:31

dibarbet requested a review from a team as a code owner July 30, 2025 23:31

jasonmalinowski approved these changes Jul 31, 2025

View reviewed changes

RikkiGibson reviewed Jul 31, 2025

View reviewed changes

src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs Show resolved Hide resolved

CyrusNajmabadi approved these changes Jul 31, 2025

View reviewed changes

Update based on review feedback

15f46ce

dibarbet requested a review from a team as a code owner July 31, 2025 18:15

RikkiGibson reviewed Jul 31, 2025

View reviewed changes

src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs Outdated Show resolved Hide resolved

RikkiGibson approved these changes Jul 31, 2025

View reviewed changes

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

c640e34

Co-authored-by: Rikki Gibson <[email protected]>

dibarbet merged commit 9a48772 into dotnet:main Jul 31, 2025
28 checks passed

dibarbet deleted the simplify_live_diagnostics branch July 31, 2025 20:34

dotnet-policy-service bot added this to the Next milestone Jul 31, 2025

This was referenced Aug 5, 2025

[Automated] PRs inserted in VS build main-10904.79 #79768

Closed

[Automated] PRs inserted in VS build feature.debugger.main-10907.206 #79846

Closed

RikkiGibson modified the milestones: Next, 18.0 P1 Aug 19, 2025

dotnet-bot mentioned this pull request Aug 22, 2025

[Automated] PRs inserted in VS build feature.Wix5-10921.95 #80011

Closed

Simplify determination of live vs. build diagnostics #79691

Simplify determination of live vs. build diagnostics #79691

Uh oh!

Conversation

dibarbet commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dibarbet Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dibarbet Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

dibarbet Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

dibarbet Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

jasonmalinowski Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

jasonmalinowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jasonmalinowski Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dibarbet commented Jul 30, 2025 •

edited

Loading

dibarbet Jul 30, 2025 •

edited

Loading