From c5150d54b5def12189dd80f22d7f26d6844d6c2d Mon Sep 17 00:00:00 2001 From: David Barbet Date: Wed, 30 Jul 2025 15:57:19 -0700 Subject: [PATCH 1/3] Simplify determination of live vs. build diagnostics --- .../Internal/HotReloadDiagnosticSource.cs | 1 - ...rtualProjectXmlDiagnosticSourceProvider.cs | 10 ++------ .../ProtocolConversions.Diagnostics.cs | 17 +++++-------- ...ndContinueDiagnosticSource_OpenDocument.cs | 3 --- ...itAndContinueDiagnosticSource_Workspace.cs | 4 ---- .../AbstractPullDiagnosticHandler.cs | 1 - .../DiagnosticSourceManager.cs | 7 ++---- .../AbstractDocumentDiagnosticSource.cs | 2 -- .../AbstractProjectDiagnosticSource.cs | 24 +++++++------------ ...stractWorkspaceDocumentDiagnosticSource.cs | 22 ++++++----------- .../DocumentDiagnosticSource.cs | 6 ----- .../DiagnosticSources/IDiagnosticSource.cs | 1 - .../NonLocalDocumentDiagnosticSource.cs | 3 --- .../TaskListDiagnosticSource.cs | 3 --- .../AdditionalFileDiagnosticsTests.cs | 2 -- .../Diagnostics/DiagnosticsPullCacheTests.cs | 5 ---- .../Features/Cohost/Handlers/Diagnostics.cs | 4 +--- .../Xaml/Internal/XamlDiagnosticSource.cs | 1 - .../Portable/Diagnostics/DiagnosticData.cs | 5 ++++ 19 files changed, 31 insertions(+), 90 deletions(-) diff --git a/src/LanguageServer/ExternalAccess/VisualDiagnostics/Internal/HotReloadDiagnosticSource.cs b/src/LanguageServer/ExternalAccess/VisualDiagnostics/Internal/HotReloadDiagnosticSource.cs index 137716f79981a..75ac3d15a7820 100644 --- a/src/LanguageServer/ExternalAccess/VisualDiagnostics/Internal/HotReloadDiagnosticSource.cs +++ b/src/LanguageServer/ExternalAccess/VisualDiagnostics/Internal/HotReloadDiagnosticSource.cs @@ -27,6 +27,5 @@ public async Task> 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}"; } diff --git a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlDiagnosticSourceProvider.cs b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlDiagnosticSourceProvider.cs index 6ede8079c41de..fad1ea38ae007 100644 --- a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlDiagnosticSourceProvider.cs +++ b/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlDiagnosticSourceProvider.cs @@ -58,7 +58,8 @@ public async Task> GetDiagnosticsAsync(RequestCon isEnabledByDefault: true, // Warning level 0 is used as a placeholder when the diagnostic has error severity warningLevel: 0, - customTags: ImmutableArray.Empty, + // Mark these diagnostics as build errors so they can be overridden by diagnostics from an explicit build. + customTags: [WellKnownDiagnosticTags.Build], properties: ImmutableDictionary.Empty, projectId: document.Project.Id, location: new DiagnosticDataLocation(location, document.Id) @@ -85,13 +86,6 @@ public Project GetProject() return document.Project; } - /// - /// These diagnostics are from the last time 'dotnet run-api' was invoked, which only occurs when a design time build is performed. - /// . - /// - /// - public bool IsLiveSource() => false; - public string ToDisplayString() => nameof(VirtualProjectXmlDiagnosticSource); } } diff --git a/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs b/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs index 0bf58091d3323..bd3947c5d4962 100644 --- a/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs +++ b/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs @@ -23,17 +23,16 @@ internal static partial class ProtocolConversions /// The diagnostic to convert /// Whether the client is Visual Studio /// The project the diagnostic is relevant to - /// Whether the diagnostic is considered "live" and should supersede others /// Whether the diagnostic is potentially a duplicate to a build diagnostic /// The global options service - public static ImmutableArray ConvertDiagnostic(DiagnosticData diagnosticData, bool supportsVisualStudioExtensions, Project project, bool isLiveSource, bool potentialDuplicate, IGlobalOptionService globalOptionService) + public static ImmutableArray 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 +64,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 +93,6 @@ internal static partial class ProtocolConversions private static LSP.VSDiagnostic CreateLspDiagnostic( DiagnosticData diagnosticData, Project project, - bool isLiveSource, bool potentialDuplicate, bool supportsVisualStudioExtensions) { @@ -108,7 +106,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 +221,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 /// - private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool isLiveSource, bool potentialDuplicate) + private static DiagnosticTag[] ConvertTags(DiagnosticData diagnosticData, bool potentialDuplicate) { using var _ = ArrayBuilder.GetInstance(out var result); @@ -246,11 +244,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); diff --git a/src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_OpenDocument.cs b/src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_OpenDocument.cs index c4b2b704f4e96..db886e9f5f633 100644 --- a/src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_OpenDocument.cs +++ b/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) { - public override bool IsLiveSource() - => true; - public override async Task> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken) { var designTimeDocument = Document; diff --git a/src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_Workspace.cs b/src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_Workspace.cs index 4188a319bc83e..9069b39d0e3fc 100644 --- a/src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_Workspace.cs +++ b/src/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource_Workspace.cs @@ -19,8 +19,6 @@ internal static partial class EditAndContinueDiagnosticSource { private sealed class ProjectSource(Project project, ImmutableArray diagnostics) : AbstractProjectDiagnosticSource(project) { - public override bool IsLiveSource() - => true; public override Task> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken) => Task.FromResult(diagnostics); @@ -28,8 +26,6 @@ public override Task> GetDiagnosticsAsync(Request private sealed class ClosedDocumentSource(TextDocument document, ImmutableArray diagnostics) : AbstractWorkspaceDocumentDiagnosticSource(document) { - public override bool IsLiveSource() - => true; public override Task> GetDiagnosticsAsync(RequestContext context, CancellationToken cancellationToken) => Task.FromResult(diagnostics); diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs index 33ce7c29a33fc..b1490635f95ef 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/AbstractPullDiagnosticHandler.cs @@ -313,7 +313,6 @@ private void HandleRemovedDocuments(RequestContext context, HashSet> 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.GetInstance(out var sourcesBuilder); foreach (var (name, provider) in nameToProviderMap) { @@ -106,7 +105,6 @@ public static ImmutableArray 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 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 AggregateIfNeeded(IEnumerable true; public Project GetProject() => sources[0].GetProject(); public ProjectOrDocumentId GetId() => sources[0].GetId(); public TextDocumentIdentifier? GetDocumentIdentifier() => sources[0].GetDocumentIdentifier(); diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractDocumentDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractDocumentDiagnosticSource.cs index c92822a6b9cd6..c77cfb567f96e 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractDocumentDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractDocumentDiagnosticSource.cs @@ -16,8 +16,6 @@ internal abstract class AbstractDocumentDiagnosticSource(TDocument do public TDocument Document { get; } = document; public Solution Solution => this.Document.Project.Solution; - public abstract bool IsLiveSource(); - public abstract Task> GetDiagnosticsAsync( RequestContext context, CancellationToken cancellationToken); diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs index a2e58d7ac8a3d..3aef13fe0147a 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Immutable; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.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> 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? shouldIncludeAnalyzer) : AbstractProjectDiagnosticSource(project) { - /// - /// This is a normal project source that represents live/fresh diagnostics that should supersede everything else. - /// - public override bool IsLiveSource() - => true; - public override async Task> GetDiagnosticsAsync( RequestContext context, CancellationToken cancellationToken) @@ -64,19 +58,17 @@ public override async Task> GetDiagnosticsAsync( private sealed class CodeAnalysisDiagnosticSource(Project project, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService) : AbstractProjectDiagnosticSource(project) { - /// - /// 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. - /// - public override bool IsLiveSource() - => false; - public override Task> 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 = [.. diagnostics.Select(d => d.WithCustomTags(d.CustomTags.Add(WellKnownDiagnosticTags.Build)))]; + return Task.FromResult(diagnostics); } } } diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs index 9e0a50253403e..e1c7faac79bb9 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs @@ -32,12 +32,6 @@ private sealed class FullSolutionAnalysisDiagnosticSource( /// private static readonly ConditionalWeakTable>> s_projectToDiagnostics = new(); - /// - /// This is a normal document source that represents live/fresh diagnostics that should supersede everything else. - /// - public override bool IsLiveSource() - => true; - public override async Task> GetDiagnosticsAsync( RequestContext context, CancellationToken cancellationToken) @@ -92,19 +86,17 @@ AsyncLazy> GetLazyDiagnostics() private sealed class CodeAnalysisDiagnosticSource(TextDocument document, ICodeAnalysisDiagnosticAnalyzerService codeAnalysisService) : AbstractWorkspaceDocumentDiagnosticSource(document) { - /// - /// 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. - /// - public override bool IsLiveSource() - => false; - public override Task> 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 = [.. diagnostics.Select(d => d.WithCustomTags(d.CustomTags.Add(WellKnownDiagnosticTags.Build)))]; + return Task.FromResult(diagnostics); } } } diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/DocumentDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/DocumentDiagnosticSource.cs index 9960d4dc5a7e5..448a9e5b9497e 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/DocumentDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/DocumentDiagnosticSource.cs @@ -15,12 +15,6 @@ internal sealed class DocumentDiagnosticSource(DiagnosticKind diagnosticKind, Te { public DiagnosticKind DiagnosticKind { get; } = diagnosticKind; - /// - /// This is a normal document source that represents live/fresh diagnostics that should supersede everything else. - /// - public override bool IsLiveSource() - => true; - public override async Task> GetDiagnosticsAsync( RequestContext context, CancellationToken cancellationToken) { diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs index 1fb04c9c18ed5..698c96a4670c0 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs @@ -29,7 +29,6 @@ internal interface IDiagnosticSource /// 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. /// - bool IsLiveSource(); Task> GetDiagnosticsAsync( RequestContext context, diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/NonLocalDocumentDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/NonLocalDocumentDiagnosticSource.cs index 2551c6592a82b..4b6df55b2bf57 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/NonLocalDocumentDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/NonLocalDocumentDiagnosticSource.cs @@ -16,9 +16,6 @@ internal sealed class NonLocalDocumentDiagnosticSource( { private readonly Func? _shouldIncludeAnalyzer = shouldIncludeAnalyzer; - public override bool IsLiveSource() - => true; - public override async Task> GetDiagnosticsAsync( RequestContext context, CancellationToken cancellationToken) diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/TaskListDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/TaskListDiagnosticSource.cs index fe6ebc182c6f9..f3c0adfe559fc 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/TaskListDiagnosticSource.cs +++ b/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> GetDiagnosticsAsync( RequestContext context, CancellationToken cancellationToken) { diff --git a/src/LanguageServer/ProtocolUnitTests/Diagnostics/AdditionalFileDiagnosticsTests.cs b/src/LanguageServer/ProtocolUnitTests/Diagnostics/AdditionalFileDiagnosticsTests.cs index a40f429582e35..037f2a333e3ca 100644 --- a/src/LanguageServer/ProtocolUnitTests/Diagnostics/AdditionalFileDiagnosticsTests.cs +++ b/src/LanguageServer/ProtocolUnitTests/Diagnostics/AdditionalFileDiagnosticsTests.cs @@ -217,8 +217,6 @@ public Task> GetDiagnosticsAsync(RequestContext c public Project GetProject() => textDocument.Project; - public bool IsLiveSource() => true; - public string ToDisplayString() => textDocument.ToString()!; } } diff --git a/src/LanguageServer/ProtocolUnitTests/Diagnostics/DiagnosticsPullCacheTests.cs b/src/LanguageServer/ProtocolUnitTests/Diagnostics/DiagnosticsPullCacheTests.cs index 1017a75137049..ab67bd1e23670 100644 --- a/src/LanguageServer/ProtocolUnitTests/Diagnostics/DiagnosticsPullCacheTests.cs +++ b/src/LanguageServer/ProtocolUnitTests/Diagnostics/DiagnosticsPullCacheTests.cs @@ -125,11 +125,6 @@ public override Task> GetDiagnosticsAsync(Request isEnabledByDefault: true, warningLevel: 0, [], ImmutableDictionary.Empty,context.Document!.Project.Id, new DiagnosticDataLocation(new FileLinePositionSpan(context.Document!.FilePath!, new Text.LinePosition(0, 0), new Text.LinePosition(0, 0))))]); } - - public override bool IsLiveSource() - { - return true; - } } [Export(typeof(IDiagnosticSourceProvider)), Shared, PartNotDiscoverable] diff --git a/src/Tools/ExternalAccess/Razor/Features/Cohost/Handlers/Diagnostics.cs b/src/Tools/ExternalAccess/Razor/Features/Cohost/Handlers/Diagnostics.cs index 6e39c496e756b..ddb3c019a66bd 100644 --- a/src/Tools/ExternalAccess/Razor/Features/Cohost/Handlers/Diagnostics.cs +++ b/src/Tools/ExternalAccess/Razor/Features/Cohost/Handlers/Diagnostics.cs @@ -25,8 +25,6 @@ internal static class Diagnostics document, range: null, DiagnosticKind.All, cancellationToken).ConfigureAwait(false); var project = document.Project; - // isLiveSource means build might override a diagnostics, but this method is only used by tooling, so builds aren't relevant - const bool IsLiveSource = false; // Potential duplicate is only set for workspace diagnostics const bool PotentialDuplicate = false; @@ -34,7 +32,7 @@ internal static class Diagnostics foreach (var diagnostic in diagnostics) { if (!diagnostic.IsSuppressed) - result.AddRange(ProtocolConversions.ConvertDiagnostic(diagnostic, supportsVisualStudioExtensions, project, IsLiveSource, PotentialDuplicate, globalOptionsService)); + result.AddRange(ProtocolConversions.ConvertDiagnostic(diagnostic, supportsVisualStudioExtensions, project, PotentialDuplicate, globalOptionsService)); } return result.ToImmutableAndFree(); diff --git a/src/Tools/ExternalAccess/Xaml/Internal/XamlDiagnosticSource.cs b/src/Tools/ExternalAccess/Xaml/Internal/XamlDiagnosticSource.cs index 2da252a3ab76a..6c94990e317f3 100644 --- a/src/Tools/ExternalAccess/Xaml/Internal/XamlDiagnosticSource.cs +++ b/src/Tools/ExternalAccess/Xaml/Internal/XamlDiagnosticSource.cs @@ -16,7 +16,6 @@ namespace Microsoft.CodeAnalysis.ExternalAccess.Xaml; internal sealed class XamlDiagnosticSource(IXamlDiagnosticSource xamlDiagnosticSource, TextDocument document) : IDiagnosticSource { - bool IDiagnosticSource.IsLiveSource() => true; Project IDiagnosticSource.GetProject() => document.Project; ProjectOrDocumentId IDiagnosticSource.GetId() => new(document.Id); TextDocumentIdentifier? IDiagnosticSource.GetDocumentIdentifier() => new() { DocumentUri = document.GetURI() }; diff --git a/src/Workspaces/Core/Portable/Diagnostics/DiagnosticData.cs b/src/Workspaces/Core/Portable/Diagnostics/DiagnosticData.cs index a18295c96e5fd..00233f4dadee1 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/DiagnosticData.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/DiagnosticData.cs @@ -102,6 +102,11 @@ public DiagnosticData WithLocations(DiagnosticDataLocation location, ImmutableAr WarningLevel, CustomTags, Properties, ProjectId, location, additionalLocations, Language, Title, Description, HelpLink, IsSuppressed); + public DiagnosticData WithCustomTags(ImmutableArray customTags) + => new(Id, Category, Message, Severity, DefaultSeverity, IsEnabledByDefault, + WarningLevel, customTags, Properties, ProjectId, DataLocation, AdditionalLocations, + Language, Title, Description, HelpLink, IsSuppressed); + public DocumentId? DocumentId => DataLocation.DocumentId; public override bool Equals(object? obj) From 15f46cecd102846bfa81da0ea05b5f02b39d6bb5 Mon Sep 17 00:00:00 2001 From: David Barbet Date: Thu, 31 Jul 2025 11:15:05 -0700 Subject: [PATCH 2/3] Update based on review feedback --- .../Portable/Diagnostic/WellKnownDiagnosticTags.cs | 6 ++++++ .../Extensions/ProtocolConversions.Diagnostics.cs | 11 +++++++++++ .../AbstractProjectDiagnosticSource.cs | 6 +++--- .../AbstractWorkspaceDocumentDiagnosticSource.cs | 4 ++-- .../DiagnosticSources/IDiagnosticSource.cs | 10 ---------- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs index 0d5bd50ce6504..40d0c45354b42 100644 --- a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs +++ b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs @@ -21,6 +21,12 @@ public static class WellKnownDiagnosticTags /// /// Indicates that the diagnostic is related to build. /// + /// + /// 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. + /// public const string Build = nameof(Build); /// diff --git a/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs b/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs index bd3947c5d4962..4d7216e45b5b0 100644 --- a/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs +++ b/src/LanguageServer/Protocol/Extensions/ProtocolConversions.Diagnostics.cs @@ -17,6 +17,17 @@ namespace Microsoft.CodeAnalysis.LanguageServer; internal static partial class ProtocolConversions { + internal static ImmutableArray AddBuildTagIfNotPresent(ImmutableArray diagnostics) + { + return diagnostics.SelectAsArray(static d => + { + if (d.CustomTags.Contains(WellKnownDiagnosticTags.Build)) + return d; + + return d.WithCustomTags(d.CustomTags.Add(WellKnownDiagnosticTags.Build)); + }); + } + /// /// Converts from to /// diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs index 3aef13fe0147a..04e70a6e08763 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractProjectDiagnosticSource.cs @@ -4,10 +4,10 @@ using System; using System.Collections.Immutable; -using System.Linq; 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; @@ -64,10 +64,10 @@ public override Task> GetDiagnosticsAsync( { var diagnostics = codeAnalysisService.GetLastComputedProjectDiagnostics(Project.Id); - //This source provides the results of the *last* explicitly kicked off "run code analysis" command from the + // 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)))]; + diagnostics = ProtocolConversions.AddBuildTagIfNotPresent(diagnostics); return Task.FromResult(diagnostics); } } diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs index e1c7faac79bb9..e243c04ef9404 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/AbstractWorkspaceDocumentDiagnosticSource.cs @@ -92,10 +92,10 @@ public override Task> GetDiagnosticsAsync( { var diagnostics = codeAnalysisService.GetLastComputedDocumentDiagnostics(Document.Id); - //This source provides the results of the *last* explicitly kicked off "run code analysis" command from the + // 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)))]; + diagnostics = ProtocolConversions.AddBuildTagIfNotPresent(diagnostics); return Task.FromResult(diagnostics); } } diff --git a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs index 698c96a4670c0..030cdd7d924b0 100644 --- a/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs +++ b/src/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/IDiagnosticSource.cs @@ -20,16 +20,6 @@ internal interface IDiagnosticSource ProjectOrDocumentId GetId(); TextDocumentIdentifier? GetDocumentIdentifier(); string ToDisplayString(); - - /// - /// 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. - /// - Task> GetDiagnosticsAsync( RequestContext context, CancellationToken cancellationToken); From c640e3408944b501c4da4f61464bae8a0df35d82 Mon Sep 17 00:00:00 2001 From: David Barbet Date: Thu, 31 Jul 2025 11:26:37 -0700 Subject: [PATCH 3/3] Update src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs Co-authored-by: Rikki Gibson --- .../Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs index 40d0c45354b42..96a543c1857aa 100644 --- a/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs +++ b/src/Compilers/Core/Portable/Diagnostic/WellKnownDiagnosticTags.cs @@ -24,7 +24,7 @@ public static class WellKnownDiagnosticTags /// /// 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, + /// 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. /// public const string Build = nameof(Build);