From da79db058266e0945ea1a4490cb5eb0626316d74 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 20 Sep 2023 09:48:39 -0500 Subject: [PATCH 1/5] Enable nullable reference types for portions of diagnostic service --- .../Diagnostics/DiagnosticServiceTests.cs | 16 ++++----- .../Squiggles/TestDiagnosticTagProducer.cs | 12 +++---- .../AbstractHostDiagnosticUpdateSource.cs | 2 +- .../Diagnostics/IDiagnosticUpdateSource.cs | 2 +- .../EditAndContinueDiagnosticUpdateSource.cs | 2 +- .../Features/Diagnostics/DiagnosticService.cs | 35 +++++++++---------- ...Service_UpdateSourceRegistrationService.cs | 2 -- .../ExternalErrorDiagnosticUpdateSource.cs | 2 +- 8 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs index 273174f124b9a..9702c11e8cd4c 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Immutable; using System.Linq; @@ -163,7 +161,7 @@ void MarkSet(object sender, DiagnosticsUpdatedArgs args) } } - private static DiagnosticData RaiseDiagnosticEvent(ManualResetEvent set, TestDiagnosticUpdateSource source, TestWorkspace workspace, ProjectId projectId, DocumentId documentId, object id) + private static DiagnosticData RaiseDiagnosticEvent(ManualResetEvent set, TestDiagnosticUpdateSource source, TestWorkspace workspace, ProjectId? projectId, DocumentId? documentId, object id) { set.Reset(); @@ -177,7 +175,7 @@ private static DiagnosticData RaiseDiagnosticEvent(ManualResetEvent set, TestDia return diagnostic; } - private static DiagnosticData CreateDiagnosticData(ProjectId projectId, DocumentId documentId) + private static DiagnosticData CreateDiagnosticData(ProjectId? projectId, DocumentId? documentId) { return new DiagnosticData( id: "test1", @@ -188,7 +186,7 @@ private static DiagnosticData CreateDiagnosticData(ProjectId projectId, Document isEnabledByDefault: false, warningLevel: 1, customTags: ImmutableArray.Empty, - properties: ImmutableDictionary.Empty, + properties: ImmutableDictionary.Empty, projectId, location: new DiagnosticDataLocation(new("originalFile1", new(10, 10), new(20, 20)), documentId)); } @@ -198,17 +196,17 @@ private class TestDiagnosticUpdateSource : IDiagnosticUpdateSource private readonly bool _support; private readonly ImmutableArray _diagnosticData; - public TestDiagnosticUpdateSource(bool support, DiagnosticData[] diagnosticData) + public TestDiagnosticUpdateSource(bool support, DiagnosticData[]? diagnosticData) { _support = support; _diagnosticData = (diagnosticData ?? Array.Empty()).ToImmutableArray(); } public bool SupportGetDiagnostics { get { return _support; } } - public event EventHandler DiagnosticsUpdated; - public event EventHandler DiagnosticsCleared; + public event EventHandler? DiagnosticsUpdated; + public event EventHandler? DiagnosticsCleared; - public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) + public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) => new(_support ? _diagnosticData : ImmutableArray.Empty); public void RaiseDiagnosticsUpdatedEvent(DiagnosticsUpdatedArgs args) diff --git a/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs b/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs index 2714a528640ab..77578e85d7658 100644 --- a/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs +++ b/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Generic; using System.Collections.Immutable; @@ -27,7 +25,7 @@ internal sealed class TestDiagnosticTagProducer { internal static Task<(ImmutableArray, ImmutableArray>)> GetDiagnosticsAndErrorSpans( TestWorkspace workspace, - IReadOnlyDictionary> analyzerMap = null) + IReadOnlyDictionary>? analyzerMap = null) { return SquiggleUtilities.GetDiagnosticsAndErrorSpansAsync(workspace, analyzerMap); } @@ -57,6 +55,8 @@ internal static async Task>> GetErrorsFromUpdateSource(Test internal static DiagnosticData CreateDiagnosticData(TestHostDocument document, TextSpan span) { + Contract.ThrowIfNull(document.FilePath); + var sourceText = document.GetTextBuffer().CurrentSnapshot.AsText(); var linePosSpan = sourceText.Lines.GetLinePositionSpan(span); return new DiagnosticData( @@ -69,7 +69,7 @@ internal static DiagnosticData CreateDiagnosticData(TestHostDocument document, T warningLevel: 0, projectId: document.Project.Id, customTags: ImmutableArray.Empty, - properties: ImmutableDictionary.Empty, + properties: ImmutableDictionary.Empty, location: new DiagnosticDataLocation(new FileLinePositionSpan(document.FilePath, linePosSpan), document.Id), language: document.Project.Language); } @@ -84,12 +84,12 @@ public void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs args) DiagnosticsUpdated?.Invoke(this, args); } - public event EventHandler DiagnosticsUpdated; + public event EventHandler? DiagnosticsUpdated; public event EventHandler DiagnosticsCleared { add { } remove { } } public bool SupportGetDiagnostics => false; - public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) + public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) => new(includeSuppressedDiagnostics ? _diagnostics : _diagnostics.WhereAsArray(d => !d.IsSuppressed)); } } diff --git a/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs b/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs index 5353d2f2fd97f..889e2287ea64d 100644 --- a/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs +++ b/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs @@ -25,7 +25,7 @@ internal abstract class AbstractHostDiagnosticUpdateSource : IDiagnosticUpdateSo public bool SupportGetDiagnostics => false; - public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) + public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) => new(ImmutableArray.Empty); public event EventHandler? DiagnosticsUpdated; diff --git a/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs b/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs index 9644b6d7913a8..b9a97b4703b1d 100644 --- a/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs +++ b/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs @@ -33,6 +33,6 @@ internal interface IDiagnosticUpdateSource /// /// Get diagnostics stored in the source. /// - ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken); + ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken); } } diff --git a/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs b/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs index 171565b249eb2..079b2198a64cb 100644 --- a/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs +++ b/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs @@ -50,7 +50,7 @@ internal EditAndContinueDiagnosticUpdateSource() /// public bool SupportGetDiagnostics => false; - public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) + public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) => new(ImmutableArray.Empty); /// diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs index ddefb20a9cf22..82aa095792291 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs @@ -2,19 +2,17 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Common; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.TestHooks; using Roslyn.Utilities; @@ -188,25 +186,25 @@ private bool ClearDiagnosticsReportedBySource(IDiagnosticUpdateSource source, Li } } - private void OnDiagnosticsUpdated(object sender, DiagnosticsUpdatedArgs e) + private void OnDiagnosticsUpdated(object? sender, DiagnosticsUpdatedArgs e) { AssertIfNull(e.Diagnostics); // all events are serialized by async event handler - RaiseDiagnosticsUpdated((IDiagnosticUpdateSource)sender, e); + RaiseDiagnosticsUpdated((IDiagnosticUpdateSource)sender!, e); } - private void OnCleared(object sender, EventArgs e) + private void OnCleared(object? sender, EventArgs e) { // all events are serialized by async event handler - RaiseDiagnosticsCleared((IDiagnosticUpdateSource)sender); + RaiseDiagnosticsCleared((IDiagnosticUpdateSource)sender!); } public ValueTask> GetDiagnosticsAsync( Workspace workspace, - ProjectId projectId, - DocumentId documentId, - object id, + ProjectId? projectId, + DocumentId? documentId, + object? id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) { @@ -220,7 +218,7 @@ public ValueTask> GetDiagnosticsAsync( return GetDiagnosticsAsync(workspace, projectId, documentId, includeSuppressedDiagnostics, cancellationToken); } - private async ValueTask> GetSpecificDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) + private async ValueTask> GetSpecificDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) { using var _ = ArrayBuilder.GetInstance(out var buffer); @@ -254,7 +252,7 @@ private async ValueTask> GetSpecificDiagnosticsAs } private async ValueTask> GetDiagnosticsAsync( - Workspace workspace, ProjectId projectId, DocumentId documentId, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) + Workspace workspace, ProjectId? projectId, DocumentId? documentId, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) { using var _1 = ArrayBuilder.GetInstance(out var result); using var _2 = ArrayBuilder.GetInstance(out var buffer); @@ -288,8 +286,8 @@ private async ValueTask> GetDiagnosticsAsync( public ImmutableArray GetDiagnosticBuckets( Workspace workspace, - ProjectId projectId, - DocumentId documentId, + ProjectId? projectId, + DocumentId? documentId, CancellationToken cancellationToken) { using var _1 = ArrayBuilder.GetInstance(out var result); @@ -309,7 +307,7 @@ public ImmutableArray GetDiagnosticBuckets( } private void AppendMatchingData( - IDiagnosticUpdateSource source, Workspace workspace, ProjectId projectId, DocumentId documentId, object id, ArrayBuilder list) + IDiagnosticUpdateSource source, Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, ArrayBuilder list) { Contract.ThrowIfNull(workspace); @@ -343,7 +341,8 @@ private void AppendMatchingData( } } - private static bool TryAddData(Workspace workspace, T key, Data data, Func keyGetter, ArrayBuilder result) where T : class + private static bool TryAddData(Workspace workspace, [NotNullWhen(true)] T? key, Data data, Func keyGetter, ArrayBuilder result) + where T : class { if (key == null) { @@ -385,8 +384,8 @@ private static void AssertIfNull(T obj) where T : class private readonly struct Data { public readonly Workspace Workspace; - public readonly ProjectId ProjectId; - public readonly DocumentId DocumentId; + public readonly ProjectId? ProjectId; + public readonly DocumentId? DocumentId; public readonly object Id; public readonly ImmutableArray Diagnostics; diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService_UpdateSourceRegistrationService.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService_UpdateSourceRegistrationService.cs index 4bc8e510627a5..f586248984e64 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService_UpdateSourceRegistrationService.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService_UpdateSourceRegistrationService.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Composition; namespace Microsoft.CodeAnalysis.Diagnostics diff --git a/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs b/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs index 4db89cca6dcf5..c0ffec18f9033 100644 --- a/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs +++ b/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs @@ -559,7 +559,7 @@ private void RaiseBuildProgressChanged(BuildProgress progress) public bool SupportGetDiagnostics { get { return false; } } public ValueTask> GetDiagnosticsAsync( - Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) + Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) { return new ValueTask>(ImmutableArray.Empty); } From 4ac7c3da75b074f0e431e894aa5848a5d2877c83 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 20 Sep 2023 13:25:19 -0500 Subject: [PATCH 2/5] Send batch events for DiagnosticsUpdatedArgs --- .../VSTypeScriptDiagnosticService.cs | 23 +- ...ntSources.DiagnosticsChangedEventSource.cs | 10 +- .../DiagnosticAnalyzerServiceTests.cs | 205 ++++++++++-------- .../Diagnostics/DiagnosticServiceTests.cs | 10 +- .../Test/Diagnostics/MockDiagnosticService.cs | 6 +- .../RemoteEditAndContinueServiceTests.cs | 2 +- .../Squiggles/TestDiagnosticTagProducer.cs | 8 +- .../AbstractHostDiagnosticUpdateSource.cs | 32 +-- .../Diagnostics/IDiagnosticUpdateSource.cs | 2 +- .../EditAndContinueDiagnosticUpdateSource.cs | 18 +- .../Api/IVSTypeScriptDiagnosticService.cs | 4 +- .../DefaultDiagnosticAnalyzerService.cs | 16 +- .../DiagnosticAnalyzerService_UpdateSource.cs | 36 ++- .../Features/Diagnostics/DiagnosticService.cs | 114 +++++----- .../EngineV2/DiagnosticIncrementalAnalyzer.cs | 39 ++-- ...IncrementalAnalyzer_IncrementalAnalyzer.cs | 98 ++++++--- .../Diagnostics/IDiagnosticService.cs | 2 +- .../AnalyzerDependencyCheckingService.cs | 8 +- .../AnalyzerFileWatcherService.cs | 16 +- .../VisualStudioDiagnosticAnalyzerService.cs | 10 +- ...DiagnosticListTable.LiveTableDataSource.cs | 53 ++--- .../ExternalErrorDiagnosticUpdateSource.cs | 139 +++++++++--- .../TaskList/HostDiagnosticUpdateSource.cs | 39 +++- .../DefaultDiagnosticUpdateSourceTests.vb | 4 +- .../DiagnosticTableDataSourceTests.vb | 18 +- .../ExternalDiagnosticUpdateSourceTests.vb | 35 +-- .../VisualStudioAnalyzerTests.vb | 11 +- 27 files changed, 613 insertions(+), 345 deletions(-) diff --git a/src/EditorFeatures/Core/ExternalAccess/VSTypeScript/VSTypeScriptDiagnosticService.cs b/src/EditorFeatures/Core/ExternalAccess/VSTypeScript/VSTypeScriptDiagnosticService.cs index 8336036122405..64387ca43879d 100644 --- a/src/EditorFeatures/Core/ExternalAccess/VSTypeScript/VSTypeScriptDiagnosticService.cs +++ b/src/EditorFeatures/Core/ExternalAccess/VSTypeScript/VSTypeScriptDiagnosticService.cs @@ -33,18 +33,37 @@ public async Task> GetPushDiagnostics return result.SelectAsArray(data => new VSTypeScriptDiagnosticData(data)); } + [Obsolete] public IDisposable RegisterDiagnosticsUpdatedEventHandler(Action action) => new EventHandlerWrapper(_service, action); + public IDisposable RegisterDiagnosticsUpdatedEventHandler(Action> action) + => new EventHandlerWrapper(_service, action); + private sealed class EventHandlerWrapper : IDisposable { private readonly IDiagnosticService _service; - private readonly EventHandler _handler; + private readonly EventHandler> _handler; + [Obsolete] internal EventHandlerWrapper(IDiagnosticService service, Action action) { _service = service; - _handler = (sender, args) => action(new VSTypeScriptDiagnosticsUpdatedArgsWrapper(args)); + _handler = (sender, argsCollection) => + { + foreach (var args in argsCollection) + action(new VSTypeScriptDiagnosticsUpdatedArgsWrapper(args)); + }; + _service.DiagnosticsUpdated += _handler; + } + + internal EventHandlerWrapper(IDiagnosticService service, Action> action) + { + _service = service; + _handler = (sender, argsCollection) => + { + action(ImmutableArray.CreateRange(argsCollection, static args => new VSTypeScriptDiagnosticsUpdatedArgsWrapper(args))); + }; _service.DiagnosticsUpdated += _handler; } diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs index 253976fdfd608..794b38e6c849a 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs @@ -2,6 +2,8 @@ // 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.Collections.Immutable; +using System.Linq; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; @@ -15,11 +17,13 @@ private class DiagnosticsChangedEventSource(ITextBuffer subjectBuffer, IDiagnost private readonly ITextBuffer _subjectBuffer = subjectBuffer; private readonly IDiagnosticService _service = service; - private void OnDiagnosticsUpdated(object? sender, DiagnosticsUpdatedArgs e) + private void OnDiagnosticsUpdated(object? sender, ImmutableArray e) { - var documentId = e.Workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); + if (e.FirstOrDefault() is not { } first) + return; - if (documentId == e.DocumentId) + var documentId = first.Workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); + if (e.Any(static (args, expectedDocumentId) => args.DocumentId == expectedDocumentId, documentId)) { this.RaiseChanged(); } diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs index 5410f2f48b305..143af7d011dbd 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs @@ -7,6 +7,7 @@ using System; using System.Collections.Immutable; using System.Linq; +using System.Security.Cryptography; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeActions; @@ -83,8 +84,7 @@ public async Task TestHasSuccessfullyLoadedBeingFalse() // check empty since this could be called to clear up existing diagnostics service.DiagnosticsUpdated += (s, a) => { - var diagnostics = a.Diagnostics; - Assert.Empty(diagnostics); + Assert.All(a, e => Assert.Empty(e.Diagnostics)); }; // now call each analyze method. none of them should run. @@ -207,23 +207,26 @@ public async Task TestDisabledByDefaultAnalyzerEnabledWithEditorConfig(bool enab var syntaxDiagnostic = false; var semanticDiagnostic = false; var compilationDiagnostic = false; - service.DiagnosticsUpdated += (s, a) => + service.DiagnosticsUpdated += (s, aCollection) => { - var diagnostics = a.Diagnostics; - var diagnostic = Assert.Single(diagnostics); - Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); - - if (diagnostic.Id == DisabledByDefaultAnalyzer.s_syntaxRule.Id) - { - syntaxDiagnostic = true; - } - else if (diagnostic.Id == DisabledByDefaultAnalyzer.s_semanticRule.Id) - { - semanticDiagnostic = true; - } - else if (diagnostic.Id == DisabledByDefaultAnalyzer.s_compilationRule.Id) + foreach (var a in aCollection) { - compilationDiagnostic = true; + var diagnostics = a.Diagnostics; + var diagnostic = Assert.Single(diagnostics); + Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); + + if (diagnostic.Id == DisabledByDefaultAnalyzer.s_syntaxRule.Id) + { + syntaxDiagnostic = true; + } + else if (diagnostic.Id == DisabledByDefaultAnalyzer.s_semanticRule.Id) + { + semanticDiagnostic = true; + } + else if (diagnostic.Id == DisabledByDefaultAnalyzer.s_compilationRule.Id) + { + compilationDiagnostic = true; + } } }; @@ -260,10 +263,13 @@ private static async Task TestAnalyzerAsync( var semantic = false; // listen to events - service.DiagnosticsUpdated += (s, a) => + service.DiagnosticsUpdated += (s, aCollection) => { - var diagnostics = a.Diagnostics; - (syntax, semantic) = resultSetter(syntax, semantic, diagnostics); + foreach (var a in aCollection) + { + var diagnostics = a.Diagnostics; + (syntax, semantic) = resultSetter(syntax, semantic, diagnostics); + } }; // now call each analyze method. none of them should run. @@ -303,22 +309,25 @@ public async Task TestOpenFileOnlyAnalyzerDiagnostics() var analyzer = service.CreateIncrementalAnalyzer(workspace); // listen to events - service.DiagnosticsUpdated += (s, a) => + service.DiagnosticsUpdated += (s, aCollection) => { - if (workspace.IsDocumentOpen(a.DocumentId)) + foreach (var a in aCollection) { - var diagnostics = a.Diagnostics; - // check the diagnostics are reported - Assert.Equal(document.Id, a.DocumentId); - Assert.Equal(1, diagnostics.Length); - Assert.Equal(OpenFileOnlyAnalyzer.s_syntaxRule.Id, diagnostics[0].Id); - } + if (workspace.IsDocumentOpen(a.DocumentId)) + { + var diagnostics = a.Diagnostics; + // check the diagnostics are reported + Assert.Equal(document.Id, a.DocumentId); + Assert.Equal(1, diagnostics.Length); + Assert.Equal(OpenFileOnlyAnalyzer.s_syntaxRule.Id, diagnostics[0].Id); + } - if (a.DocumentId == document.Id && !workspace.IsDocumentOpen(a.DocumentId)) - { - // check the diagnostics reported are cleared - var diagnostics = a.Diagnostics; - Assert.Equal(0, diagnostics.Length); + if (a.DocumentId == document.Id && !workspace.IsDocumentOpen(a.DocumentId)) + { + // check the diagnostics reported are cleared + var diagnostics = a.Diagnostics; + Assert.Equal(0, diagnostics.Length); + } } }; @@ -374,19 +383,22 @@ public async Task TestSynchronizeWithBuild() var syntax = false; // listen to events - service.DiagnosticsUpdated += (s, a) => + service.DiagnosticsUpdated += (s, aCollection) => { - var diagnostics = a.Diagnostics; - switch (diagnostics.Length) + foreach (var a in aCollection) { - case 0: - return; - case 1: - syntax |= diagnostics[0].Id == NoNameAnalyzer.s_syntaxRule.Id; - return; - default: - AssertEx.Fail("shouldn't reach here"); - return; + var diagnostics = a.Diagnostics; + switch (diagnostics.Length) + { + case 0: + continue; + case 1: + syntax |= diagnostics[0].Id == NoNameAnalyzer.s_syntaxRule.Id; + continue; + default: + AssertEx.Fail("shouldn't reach here"); + continue; + } } }; @@ -493,18 +505,21 @@ public async Task TestHostAnalyzerErrorNotLeaking() var service = Assert.IsType(exportProvider.GetExportedValue()); var called = false; - service.DiagnosticsUpdated += (s, e) => + service.DiagnosticsUpdated += (s, eCollection) => { - var diagnostics = e.Diagnostics; - if (diagnostics.Length == 0) + foreach (var e in eCollection) { - return; - } + var diagnostics = e.Diagnostics; + if (diagnostics.Length == 0) + { + continue; + } - var liveId = (LiveDiagnosticUpdateArgsId)e.Id; - Assert.False(liveId.Analyzer is ProjectDiagnosticAnalyzer); + var liveId = (LiveDiagnosticUpdateArgsId)e.Id; + Assert.False(liveId.Analyzer is ProjectDiagnosticAnalyzer); - called = true; + called = true; + } }; var incrementalAnalyzer = (DiagnosticIncrementalAnalyzer)service.CreateIncrementalAnalyzer(workspace); @@ -599,18 +614,21 @@ private static async Task TestFullSolutionAnalysisForProjectAsync(AdhocWorkspace var globalOptions = exportProvider.GetExportedValue(); var called = false; - service.DiagnosticsUpdated += (s, e) => + service.DiagnosticsUpdated += (s, eCollection) => { - var diagnostics = e.Diagnostics; - if (diagnostics.Length == 0) + foreach (var e in eCollection) { - return; - } + var diagnostics = e.Diagnostics; + if (diagnostics.Length == 0) + { + return; + } - var liveId = (LiveDiagnosticUpdateArgsId)e.Id; - Assert.True(liveId.Analyzer is NamedTypeAnalyzer); + var liveId = (LiveDiagnosticUpdateArgsId)e.Id; + Assert.True(liveId.Analyzer is NamedTypeAnalyzer); - called = true; + called = true; + } }; var incrementalAnalyzer = (DiagnosticIncrementalAnalyzer)service.CreateIncrementalAnalyzer(project.Solution.Workspace); @@ -657,9 +675,10 @@ internal async Task TestAdditionalFileAnalyzer(bool registerFromInitialize, bool var service = Assert.IsType(exportProvider.GetExportedValue()); var diagnostics = new ConcurrentSet(); - service.DiagnosticsUpdated += (s, e) => + service.DiagnosticsUpdated += (s, eCollection) => { - diagnostics.AddRange(e.Diagnostics); + foreach (var e in eCollection) + diagnostics.AddRange(e.Diagnostics); }; var incrementalAnalyzer = (DiagnosticIncrementalAnalyzer)service.CreateIncrementalAnalyzer(workspace); @@ -763,15 +782,18 @@ internal async Task TestDiagnosticSuppressor(bool includeAnalyzer, bool includeS var globalOptions = workspace.GetService(); DiagnosticData diagnostic = null; - service.DiagnosticsUpdated += (s, e) => + service.DiagnosticsUpdated += (s, eCollection) => { - var diagnostics = e.Diagnostics; - if (diagnostics.Length == 0) + foreach (var e in eCollection) { - return; - } + var diagnostics = e.Diagnostics; + if (diagnostics.Length == 0) + { + return; + } - diagnostic = Assert.Single(diagnostics); + diagnostic = Assert.Single(diagnostics); + } }; var incrementalAnalyzer = (DiagnosticIncrementalAnalyzer)service.CreateIncrementalAnalyzer(workspace); @@ -897,12 +919,15 @@ void M() var diagnostics = ArrayBuilder.GetInstance(); var text = await document.GetTextAsync(); - service.DiagnosticsUpdated += (s, e) => + service.DiagnosticsUpdated += (s, eCollection) => { - diagnostics.AddRange( - e.Diagnostics - .Where(d => d.Id == IDEDiagnosticIds.RemoveUnnecessarySuppressionDiagnosticId) - .OrderBy(d => d.DataLocation.UnmappedFileSpan.GetClampedTextSpan(text))); + foreach (var e in eCollection) + { + diagnostics.AddRange( + e.Diagnostics + .Where(d => d.Id == IDEDiagnosticIds.RemoveUnnecessarySuppressionDiagnosticId) + .OrderBy(d => d.DataLocation.UnmappedFileSpan.GetClampedTextSpan(text))); + } }; var incrementalAnalyzer = (DiagnosticIncrementalAnalyzer)service.CreateIncrementalAnalyzer(workspace); @@ -994,16 +1019,19 @@ void M() var globalOptions = workspace.GetService(); DiagnosticData diagnostic = null; - service.DiagnosticsUpdated += (s, e) => + service.DiagnosticsUpdated += (s, eCollection) => { - var diagnostics = e.Diagnostics; - if (diagnostics.IsEmpty) + foreach (var e in eCollection) { - return; - } + var diagnostics = e.Diagnostics; + if (diagnostics.IsEmpty) + { + return; + } - Assert.Null(diagnostic); - diagnostic = Assert.Single(diagnostics); + Assert.Null(diagnostic); + diagnostic = Assert.Single(diagnostics); + } }; var incrementalAnalyzer = (DiagnosticIncrementalAnalyzer)service.CreateIncrementalAnalyzer(workspace); @@ -1237,15 +1265,18 @@ internal async Task TestGeneratorProducedDiagnostics(bool fullSolutionAnalysis) var service = Assert.IsType(workspace.GetService()); var gotDiagnostics = false; - service.DiagnosticsUpdated += (s, e) => + service.DiagnosticsUpdated += (s, eCollection) => { - var diagnostics = e.Diagnostics; - if (diagnostics.Length == 0) - return; + foreach (var e in eCollection) + { + var diagnostics = e.Diagnostics; + if (diagnostics.Length == 0) + return; - var liveId = (LiveDiagnosticUpdateArgsId)e.Id; - if (liveId.Analyzer is GeneratorDiagnosticsPlaceholderAnalyzer) - gotDiagnostics = true; + var liveId = (LiveDiagnosticUpdateArgsId)e.Id; + if (liveId.Analyzer is GeneratorDiagnosticsPlaceholderAnalyzer) + gotDiagnostics = true; + } }; var incrementalAnalyzer = (DiagnosticIncrementalAnalyzer)service.CreateIncrementalAnalyzer(workspace); diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs index 9702c11e8cd4c..6c40aa56f0003 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs @@ -146,7 +146,7 @@ public async Task TestCleared() var data2 = await diagnosticService.GetDiagnosticsAsync(workspace, null, null, null, includeSuppressedDiagnostics: false, CancellationToken.None); Assert.Equal(2, data2.Count()); - void MarkCalled(object sender, DiagnosticsUpdatedArgs args) + void MarkCalled(object sender, ImmutableArray args) { // event is serialized. no concurrent call if (++count == 3) @@ -155,7 +155,7 @@ void MarkCalled(object sender, DiagnosticsUpdatedArgs args) } } - void MarkSet(object sender, DiagnosticsUpdatedArgs args) + void MarkSet(object sender, ImmutableArray args) { mutex.Set(); } @@ -168,7 +168,7 @@ private static DiagnosticData RaiseDiagnosticEvent(ManualResetEvent set, TestDia var diagnostic = CreateDiagnosticData(projectId, documentId); source.RaiseDiagnosticsUpdatedEvent( - DiagnosticsUpdatedArgs.DiagnosticsCreated(id, workspace, workspace.CurrentSolution, projectId, documentId, ImmutableArray.Create(diagnostic))); + ImmutableArray.Create(DiagnosticsUpdatedArgs.DiagnosticsCreated(id, workspace, workspace.CurrentSolution, projectId, documentId, ImmutableArray.Create(diagnostic)))); set.WaitOne(); @@ -203,13 +203,13 @@ public TestDiagnosticUpdateSource(bool support, DiagnosticData[]? diagnosticData } public bool SupportGetDiagnostics { get { return _support; } } - public event EventHandler? DiagnosticsUpdated; + public event EventHandler>? DiagnosticsUpdated; public event EventHandler? DiagnosticsCleared; public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default) => new(_support ? _diagnosticData : ImmutableArray.Empty); - public void RaiseDiagnosticsUpdatedEvent(DiagnosticsUpdatedArgs args) + public void RaiseDiagnosticsUpdatedEvent(ImmutableArray args) => DiagnosticsUpdated?.Invoke(this, args); public void RaiseDiagnosticsClearedEvent() diff --git a/src/EditorFeatures/Test/Diagnostics/MockDiagnosticService.cs b/src/EditorFeatures/Test/Diagnostics/MockDiagnosticService.cs index 1725bafe79833..bfc3ce9fac104 100644 --- a/src/EditorFeatures/Test/Diagnostics/MockDiagnosticService.cs +++ b/src/EditorFeatures/Test/Diagnostics/MockDiagnosticService.cs @@ -24,7 +24,7 @@ internal class MockDiagnosticService : IDiagnosticService private DiagnosticData? _diagnosticData; - public event EventHandler? DiagnosticsUpdated; + public event EventHandler>? DiagnosticsUpdated; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] @@ -65,10 +65,10 @@ internal void CreateDiagnosticAndFireEvents(Workspace workspace, MockDiagnosticA document); analyzerService.AddDiagnostic(_diagnosticData, diagnosticKind); - DiagnosticsUpdated?.Invoke(this, DiagnosticsUpdatedArgs.DiagnosticsCreated( + DiagnosticsUpdated?.Invoke(this, ImmutableArray.Create(DiagnosticsUpdatedArgs.DiagnosticsCreated( this, workspace, workspace.CurrentSolution, GetProjectId(workspace), GetDocumentId(workspace), - ImmutableArray.Create(_diagnosticData))); + ImmutableArray.Create(_diagnosticData)))); } private static DocumentId GetDocumentId(Workspace workspace) diff --git a/src/EditorFeatures/Test/EditAndContinue/RemoteEditAndContinueServiceTests.cs b/src/EditorFeatures/Test/EditAndContinue/RemoteEditAndContinueServiceTests.cs index 91d9aca97048e..7fafa8ccaa269 100644 --- a/src/EditorFeatures/Test/EditAndContinue/RemoteEditAndContinueServiceTests.cs +++ b/src/EditorFeatures/Test/EditAndContinue/RemoteEditAndContinueServiceTests.cs @@ -109,7 +109,7 @@ void VerifyReanalyzeInvocation(ImmutableArray documentIds) var diagnosticUpdateSource = new EditAndContinueDiagnosticUpdateSource(); var emitDiagnosticsUpdated = new List(); var emitDiagnosticsClearedCount = 0; - diagnosticUpdateSource.DiagnosticsUpdated += (object sender, DiagnosticsUpdatedArgs args) => emitDiagnosticsUpdated.Add(args); + diagnosticUpdateSource.DiagnosticsUpdated += (object sender, ImmutableArray args) => emitDiagnosticsUpdated.AddRange(args); diagnosticUpdateSource.DiagnosticsCleared += (object sender, EventArgs args) => emitDiagnosticsClearedCount++; var span1 = new LinePositionSpan(new LinePosition(1, 2), new LinePosition(1, 5)); diff --git a/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs b/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs index 77578e85d7658..6d6ed7acd7e4d 100644 --- a/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs +++ b/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs @@ -43,7 +43,7 @@ internal static async Task>> GetErrorsFromUpdateSource(Test var analyzerServer = (MockDiagnosticAnalyzerService)workspace.GetService(); analyzerServer.AddDiagnostics(updateArgs.Diagnostics, diagnosticKind); - source.RaiseDiagnosticsUpdated(updateArgs); + source.RaiseDiagnosticsUpdated(ImmutableArray.Create(updateArgs)); await wrapper.WaitForTags(); @@ -78,13 +78,13 @@ private class TestDiagnosticUpdateSource : IDiagnosticUpdateSource { private ImmutableArray _diagnostics = ImmutableArray.Empty; - public void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs args) + public void RaiseDiagnosticsUpdated(ImmutableArray args) { - _diagnostics = args.Diagnostics; + _diagnostics = args.SelectManyAsArray(e => e.Diagnostics); DiagnosticsUpdated?.Invoke(this, args); } - public event EventHandler? DiagnosticsUpdated; + public event EventHandler>? DiagnosticsUpdated; public event EventHandler DiagnosticsCleared { add { } remove { } } public bool SupportGetDiagnostics => false; diff --git a/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs b/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs index 889e2287ea64d..d28e73296f5a0 100644 --- a/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs +++ b/src/Features/Core/Portable/Diagnostics/AbstractHostDiagnosticUpdateSource.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Shared.Collections; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Diagnostics @@ -28,11 +29,14 @@ internal abstract class AbstractHostDiagnosticUpdateSource : IDiagnosticUpdateSo public ValueTask> GetDiagnosticsAsync(Workspace workspace, ProjectId? projectId, DocumentId? documentId, object? id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken) => new(ImmutableArray.Empty); - public event EventHandler? DiagnosticsUpdated; + public event EventHandler>? DiagnosticsUpdated; public event EventHandler DiagnosticsCleared { add { } remove { } } - public void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs args) - => DiagnosticsUpdated?.Invoke(this, args); + public void RaiseDiagnosticsUpdated(ImmutableArray args) + { + if (!args.IsEmpty) + DiagnosticsUpdated?.Invoke(this, args); + } public void ReportAnalyzerDiagnostic(DiagnosticAnalyzer analyzer, Diagnostic diagnostic, ProjectId? projectId) { @@ -67,7 +71,7 @@ public void ReportAnalyzerDiagnostic(DiagnosticAnalyzer analyzer, DiagnosticData if (raiseDiagnosticsUpdated) { - RaiseDiagnosticsUpdated(MakeCreatedArgs(analyzer, dxs, project)); + RaiseDiagnosticsUpdated(ImmutableArray.Create(MakeCreatedArgs(analyzer, dxs, project))); } } @@ -78,27 +82,29 @@ public void ClearAnalyzerReferenceDiagnostics(AnalyzerFileReference analyzerRefe if (_analyzerHostDiagnosticsMap.Count == 0) return; + using var argsBuilder = TemporaryArray.Empty; var analyzers = analyzerReference.GetAnalyzers(language); - ClearAnalyzerDiagnostics(analyzers, projectId); + AddArgsToClearAnalyzerDiagnostics(ref argsBuilder.AsRef(), analyzers, projectId); + RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); } - public void ClearAnalyzerDiagnostics(ImmutableArray analyzers, ProjectId projectId) + public void AddArgsToClearAnalyzerDiagnostics(ref TemporaryArray builder, ImmutableArray analyzers, ProjectId projectId) { foreach (var analyzer in analyzers) { - ClearAnalyzerDiagnostics(analyzer, projectId); + AddArgsToClearAnalyzerDiagnostics(ref builder, analyzer, projectId); } } - public void ClearAnalyzerDiagnostics(ProjectId projectId) + public void AddArgsToClearAnalyzerDiagnostics(ref TemporaryArray builder, ProjectId projectId) { foreach (var (analyzer, _) in _analyzerHostDiagnosticsMap) { - ClearAnalyzerDiagnostics(analyzer, projectId); + AddArgsToClearAnalyzerDiagnostics(ref builder, analyzer, projectId); } } - private void ClearAnalyzerDiagnostics(DiagnosticAnalyzer analyzer, ProjectId projectId) + private void AddArgsToClearAnalyzerDiagnostics(ref TemporaryArray builder, DiagnosticAnalyzer analyzer, ProjectId projectId) { if (!_analyzerHostDiagnosticsMap.TryGetValue(analyzer, out var existing)) { @@ -114,17 +120,17 @@ private void ClearAnalyzerDiagnostics(DiagnosticAnalyzer analyzer, ProjectId pro ImmutableInterlocked.TryUpdate(ref _analyzerHostDiagnosticsMap, analyzer, newDiags, existing)) { var project = Workspace.CurrentSolution.GetProject(projectId); - RaiseDiagnosticsUpdated(MakeRemovedArgs(analyzer, project)); + builder.Add(MakeRemovedArgs(analyzer, project)); } } else if (ImmutableInterlocked.TryRemove(ref _analyzerHostDiagnosticsMap, analyzer, out existing)) { var project = Workspace.CurrentSolution.GetProject(projectId); - RaiseDiagnosticsUpdated(MakeRemovedArgs(analyzer, project)); + builder.Add(MakeRemovedArgs(analyzer, project)); if (existing.Any(d => d.ProjectId == null)) { - RaiseDiagnosticsUpdated(MakeRemovedArgs(analyzer, project: null)); + builder.Add(MakeRemovedArgs(analyzer, project: null)); } } } diff --git a/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs b/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs index b9a97b4703b1d..f2478f52dad85 100644 --- a/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs +++ b/src/Features/Core/Portable/Diagnostics/IDiagnosticUpdateSource.cs @@ -17,7 +17,7 @@ internal interface IDiagnosticUpdateSource /// /// Raise this when new diagnostics are found /// - event EventHandler DiagnosticsUpdated; + event EventHandler> DiagnosticsUpdated; /// /// Raise this when all diagnostics reported from this update source has cleared diff --git a/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs b/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs index 079b2198a64cb..3c063603e07a9 100644 --- a/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs +++ b/src/Features/Core/Portable/EditAndContinue/EditAndContinueDiagnosticUpdateSource.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; using System.Diagnostics.CodeAnalysis; @@ -12,7 +11,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Host.Mef; -using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Collections; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.EditAndContinue @@ -42,7 +41,7 @@ internal EditAndContinueDiagnosticUpdateSource() { } - public event EventHandler? DiagnosticsUpdated; + public event EventHandler>? DiagnosticsUpdated; public event EventHandler? DiagnosticsCleared; /// @@ -102,13 +101,15 @@ public void ReportDiagnostics(Workspace workspace, Solution solution, ImmutableA var projectDiagnostics = diagnostics.WhereAsArray(d => d.DocumentId == null && d.ProjectId != null); var solutionDiagnostics = diagnostics.WhereAsArray(d => d.DocumentId == null && d.ProjectId == null); + using var argsBuilder = TemporaryArray.Empty; + if (documentDiagnostics.Length > 0) { foreach (var (documentId, diagnosticData) in documentDiagnostics.GroupBy(static data => data.DocumentId!)) { var diagnosticGroupId = (this, documentId); - updateEvent(this, DiagnosticsUpdatedArgs.DiagnosticsCreated( + argsBuilder.Add(DiagnosticsUpdatedArgs.DiagnosticsCreated( diagnosticGroupId, workspace, solution, @@ -124,7 +125,7 @@ public void ReportDiagnostics(Workspace workspace, Solution solution, ImmutableA { var diagnosticGroupId = (this, projectId); - updateEvent(this, DiagnosticsUpdatedArgs.DiagnosticsCreated( + argsBuilder.Add(DiagnosticsUpdatedArgs.DiagnosticsCreated( diagnosticGroupId, workspace, solution, @@ -138,7 +139,7 @@ public void ReportDiagnostics(Workspace workspace, Solution solution, ImmutableA { var diagnosticGroupId = this; - updateEvent(this, DiagnosticsUpdatedArgs.DiagnosticsCreated( + argsBuilder.Add(DiagnosticsUpdatedArgs.DiagnosticsCreated( diagnosticGroupId, workspace, solution, @@ -146,6 +147,11 @@ public void ReportDiagnostics(Workspace workspace, Solution solution, ImmutableA documentId: null, diagnostics: solutionDiagnostics)); } + + if (argsBuilder.Count > 0) + { + updateEvent(this, argsBuilder.ToImmutableAndClear()); + } } } } diff --git a/src/Features/Core/Portable/ExternalAccess/VSTypeScript/Api/IVSTypeScriptDiagnosticService.cs b/src/Features/Core/Portable/ExternalAccess/VSTypeScript/Api/IVSTypeScriptDiagnosticService.cs index c3393483bdba4..56e5998b63016 100644 --- a/src/Features/Core/Portable/ExternalAccess/VSTypeScript/Api/IVSTypeScriptDiagnosticService.cs +++ b/src/Features/Core/Portable/ExternalAccess/VSTypeScript/Api/IVSTypeScriptDiagnosticService.cs @@ -6,7 +6,6 @@ using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Diagnostics; namespace Microsoft.CodeAnalysis.ExternalAccess.VSTypeScript.Api { @@ -14,6 +13,9 @@ internal interface IVSTypeScriptDiagnosticService { Task> GetPushDiagnosticsAsync(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics, CancellationToken cancellationToken); + [Obsolete] IDisposable RegisterDiagnosticsUpdatedEventHandler(Action action); + + IDisposable RegisterDiagnosticsUpdatedEventHandler(Action> action); } } diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DefaultDiagnosticAnalyzerService.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DefaultDiagnosticAnalyzerService.cs index 69c1578335dca..bdee3c762fd1c 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DefaultDiagnosticAnalyzerService.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DefaultDiagnosticAnalyzerService.cs @@ -49,7 +49,7 @@ public IIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace workspace) return new DefaultDiagnosticIncrementalAnalyzer(this, workspace); } - public event EventHandler DiagnosticsUpdated; + public event EventHandler> DiagnosticsUpdated; public event EventHandler DiagnosticsCleared { add { } remove { } } // this only support push model, pull model will be provided by DiagnosticService by caching everything this one pushed @@ -61,7 +61,7 @@ public ValueTask> GetDiagnosticsAsync(Workspace w return new ValueTask>(ImmutableArray.Empty); } - internal void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs state) + internal void RaiseDiagnosticsUpdated(ImmutableArray state) => DiagnosticsUpdated?.Invoke(this, state); private sealed class DefaultDiagnosticIncrementalAnalyzer : IIncrementalAnalyzer @@ -129,9 +129,11 @@ private async Task AnalyzeForKindAsync(TextDocument document, AnalysisKind kind, { var diagnosticData = await GetDiagnosticsAsync(document, kind, cancellationToken).ConfigureAwait(false); - _service.RaiseDiagnosticsUpdated( + // TODO: Consider raising these with a batching work queue to aggregate results from analyzers that + // complete quickly. + _service.RaiseDiagnosticsUpdated(ImmutableArray.Create( DiagnosticsUpdatedArgs.DiagnosticsCreated(new DefaultUpdateArgsId(_workspace.Kind, kind, document.Id), - _workspace, document.Project.Solution, document.Project.Id, document.Id, diagnosticData)); + _workspace, document.Project.Solution, document.Project.Id, document.Id, diagnosticData))); } /// @@ -220,8 +222,10 @@ public Task NonSourceDocumentCloseAsync(TextDocument textDocument, CancellationT private void RaiseEmptyDiagnosticUpdated(AnalysisKind kind, DocumentId documentId) { - _service.RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs.DiagnosticsRemoved( - new DefaultUpdateArgsId(_workspace.Kind, kind, documentId), _workspace, null, documentId.ProjectId, documentId)); + // TODO: Consider raising these with a batching work queue to aggregate results from analyzers that + // complete quickly. + _service.RaiseDiagnosticsUpdated(ImmutableArray.Create(DiagnosticsUpdatedArgs.DiagnosticsRemoved( + new DefaultUpdateArgsId(_workspace.Kind, kind, documentId), _workspace, null, documentId.ProjectId, documentId))); } public Task AnalyzeProjectAsync(Project project, bool semanticsChanged, InvocationReasons reasons, CancellationToken cancellationToken) diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs index c6e7ea8071815..98868ee19d1af 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs @@ -14,7 +14,7 @@ namespace Microsoft.CodeAnalysis.Diagnostics { internal partial class DiagnosticAnalyzerService : IDiagnosticUpdateSource { - public event EventHandler DiagnosticsUpdated + public event EventHandler> DiagnosticsUpdated { add { @@ -40,41 +40,59 @@ public event EventHandler DiagnosticsCleared } } - internal void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs args) + internal void RaiseDiagnosticsUpdated(ImmutableArray args) { // all diagnostics events are serialized. - var ev = _eventMap.GetEventHandlers>(DiagnosticsUpdatedEventName); + var ev = _eventMap.GetEventHandlers>>(DiagnosticsUpdatedEventName); if (ev.HasHandlers) { _eventQueue.ScheduleTask(nameof(RaiseDiagnosticsUpdated), () => ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)), CancellationToken.None); } } - internal void RaiseBulkDiagnosticsUpdated(Action> eventAction) + internal void RaiseBulkDiagnosticsUpdated(Action>> eventAction) { // all diagnostics events are serialized. - var ev = _eventMap.GetEventHandlers>(DiagnosticsUpdatedEventName); + var ev = _eventMap.GetEventHandlers>>(DiagnosticsUpdatedEventName); if (ev.HasHandlers) { // we do this bulk update to reduce number of tasks (with captured data) enqueued. // we saw some "out of memory" due to us having long list of pending tasks in memory. // this is to reduce for such case to happen. - void raiseEvents(DiagnosticsUpdatedArgs args) => ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); + void raiseEvents(ImmutableArray args) + { + ev.RaiseEvent( + static (handler, arg) => + { + if (!arg.args.IsEmpty) + handler(arg.self, arg.args); + }, + (self: this, args)); + } _eventQueue.ScheduleTask(nameof(RaiseDiagnosticsUpdated), () => eventAction(raiseEvents), CancellationToken.None); } } - internal void RaiseBulkDiagnosticsUpdated(Func, Task> eventActionAsync) + internal void RaiseBulkDiagnosticsUpdated(Func>, Task> eventActionAsync) { // all diagnostics events are serialized. - var ev = _eventMap.GetEventHandlers>(DiagnosticsUpdatedEventName); + var ev = _eventMap.GetEventHandlers>>(DiagnosticsUpdatedEventName); if (ev.HasHandlers) { // we do this bulk update to reduce number of tasks (with captured data) enqueued. // we saw some "out of memory" due to us having long list of pending tasks in memory. // this is to reduce for such case to happen. - void raiseEvents(DiagnosticsUpdatedArgs args) => ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); + void raiseEvents(ImmutableArray args) + { + ev.RaiseEvent( + static (handler, arg) => + { + if (!arg.args.IsEmpty) + handler(arg.self, arg.args); + }, + (self: this, args)); + } _eventQueue.ScheduleTask(nameof(RaiseDiagnosticsUpdated), () => eventActionAsync(raiseEvents), CancellationToken.None); } diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs index 82aa095792291..20526cd08c802 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticService.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.TestHooks; using Roslyn.Utilities; @@ -51,7 +52,7 @@ public DiagnosticService( _eventListenerTracker = new EventListenerTracker(eventListeners, WellKnownEventListeners.DiagnosticService); } - public event EventHandler DiagnosticsUpdated + public event EventHandler> DiagnosticsUpdated { add { @@ -64,34 +65,42 @@ public event EventHandler DiagnosticsUpdated } } - private void RaiseDiagnosticsUpdated(IDiagnosticUpdateSource source, DiagnosticsUpdatedArgs args) + private void RaiseDiagnosticsUpdated(IDiagnosticUpdateSource source, ImmutableArray argsCollection) { - _eventListenerTracker.EnsureEventListener(args.Workspace, this); + Workspace? previousWorkspace = null; + foreach (var args in argsCollection) + { + if (args.Workspace != previousWorkspace) + { + _eventListenerTracker.EnsureEventListener(args.Workspace, this); + previousWorkspace = args.Workspace; + } + } - var ev = _eventMap.GetEventHandlers>(DiagnosticsUpdatedEventName); + var ev = _eventMap.GetEventHandlers>>(DiagnosticsUpdatedEventName); _eventQueue.ScheduleTask(DiagnosticsUpdatedEventName, () => { - if (!UpdateDataMap(source, args)) + var updatedArgsCollection = UpdateDataMap(source, argsCollection); + if (updatedArgsCollection.IsEmpty) { // there is no change, nothing to raise events for. return; } - ev.RaiseEvent(static (handler, arg) => handler(arg.source, arg.args), (source, args)); + ev.RaiseEvent(static (handler, arg) => handler(arg.source, arg.updatedArgsCollection), (source, updatedArgsCollection)); }, CancellationToken.None); } private void RaiseDiagnosticsCleared(IDiagnosticUpdateSource source) { - var ev = _eventMap.GetEventHandlers>(DiagnosticsUpdatedEventName); + var ev = _eventMap.GetEventHandlers>>(DiagnosticsUpdatedEventName); _eventQueue.ScheduleTask(DiagnosticsUpdatedEventName, () => { - using var pooledObject = SharedPools.Default>().GetPooledObject(); + using var argsBuilder = TemporaryArray.Empty; - var removed = pooledObject.Object; - if (!ClearDiagnosticsReportedBySource(source, removed)) + if (!ClearDiagnosticsReportedBySource(source, ref argsBuilder.AsRef())) { // there is no change, nothing to raise events for. return; @@ -99,66 +108,68 @@ private void RaiseDiagnosticsCleared(IDiagnosticUpdateSource source) // don't create event listener if it haven't created yet. if there is a diagnostic to remove // listener should have already created since all events are done in the serialized queue - foreach (var args in removed) - { - ev.RaiseEvent(static (handler, arg) => handler(arg.source, arg.args), (source, args)); - } + ev.RaiseEvent(static (handler, arg) => handler(arg.source, arg.args), (source, args: argsBuilder.ToImmutableAndClear())); }, CancellationToken.None); } - private bool UpdateDataMap(IDiagnosticUpdateSource source, DiagnosticsUpdatedArgs args) + private ImmutableArray UpdateDataMap(IDiagnosticUpdateSource source, ImmutableArray argsCollection) { // we expect source who uses this ability to have small number of diagnostics. lock (_gate) { - Debug.Assert(_updateSources.Contains(source)); + var result = argsCollection.WhereAsArray(args => + { + Debug.Assert(_updateSources.Contains(source)); - var diagnostics = args.Diagnostics; + var diagnostics = args.Diagnostics; - // check cheap early bail out - if (diagnostics.Length == 0 && !_map.ContainsKey(source)) - { - // no new diagnostic, and we don't have update source for it. - return false; - } + // check cheap early bail out + if (diagnostics.Length == 0 && !_map.ContainsKey(source)) + { + // no new diagnostic, and we don't have update source for it. + return false; + } - // 2 different workspaces (ex, PreviewWorkspaces) can return same Args.Id, we need to - // distinguish them. so we separate diagnostics per workspace map. - var workspaceMap = _map.GetOrAdd(source, _ => new Dictionary>()); + // 2 different workspaces (ex, PreviewWorkspaces) can return same Args.Id, we need to + // distinguish them. so we separate diagnostics per workspace map. + var workspaceMap = _map.GetOrAdd(source, _ => new Dictionary>()); - if (diagnostics.Length == 0 && !workspaceMap.ContainsKey(args.Workspace)) - { - // no new diagnostic, and we don't have workspace for it. - return false; - } + if (diagnostics.Length == 0 && !workspaceMap.ContainsKey(args.Workspace)) + { + // no new diagnostic, and we don't have workspace for it. + return false; + } - var diagnosticDataMap = workspaceMap.GetOrAdd(args.Workspace, _ => new Dictionary()); + var diagnosticDataMap = workspaceMap.GetOrAdd(args.Workspace, _ => new Dictionary()); - diagnosticDataMap.Remove(args.Id); - if (diagnosticDataMap.Count == 0 && diagnostics.Length == 0) - { - workspaceMap.Remove(args.Workspace); + diagnosticDataMap.Remove(args.Id); + if (diagnosticDataMap.Count == 0 && diagnostics.Length == 0) + { + workspaceMap.Remove(args.Workspace); - if (workspaceMap.Count == 0) + if (workspaceMap.Count == 0) + { + _map.Remove(source); + } + + return true; + } + + if (diagnostics.Length > 0) { - _map.Remove(source); + // save data only if there is a diagnostic + var data = source.SupportGetDiagnostics ? new Data(args) : new Data(args, diagnostics); + diagnosticDataMap.Add(args.Id, data); } return true; - } - - if (diagnostics.Length > 0) - { - // save data only if there is a diagnostic - var data = source.SupportGetDiagnostics ? new Data(args) : new Data(args, diagnostics); - diagnosticDataMap.Add(args.Id, data); - } + }); - return true; + return result; } } - private bool ClearDiagnosticsReportedBySource(IDiagnosticUpdateSource source, List removed) + private bool ClearDiagnosticsReportedBySource(IDiagnosticUpdateSource source, ref TemporaryArray removed) { // we expect source who uses this ability to have small number of diagnostics. lock (_gate) @@ -186,9 +197,9 @@ private bool ClearDiagnosticsReportedBySource(IDiagnosticUpdateSource source, Li } } - private void OnDiagnosticsUpdated(object? sender, DiagnosticsUpdatedArgs e) + private void OnDiagnosticsUpdated(object? sender, ImmutableArray e) { - AssertIfNull(e.Diagnostics); + AssertIfNull(e.SelectManyAsArray(e => e.Diagnostics)); // all events are serialized by async event handler RaiseDiagnosticsUpdated((IDiagnosticUpdateSource)sender!, e); @@ -373,7 +384,8 @@ private static void AssertIfNull(ImmutableArray diagnostics) } [Conditional("DEBUG")] - private static void AssertIfNull(T obj) where T : class + private static void AssertIfNull(T obj) + where T : class { if (obj == null) { diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs index 218e1e8ebbe3a..09dbae84f46f7 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.cs @@ -14,6 +14,7 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Simplification; using Microsoft.CodeAnalysis.SolutionCrawler; using Microsoft.CodeAnalysis.Workspaces.Diagnostics; @@ -112,6 +113,7 @@ public void Shutdown() { var handleActiveFile = true; using var _ = PooledHashSet.GetInstance(out var documentSet); + using var argsBuilder = TemporaryArray.Empty; foreach (var stateSet in stateSets) { @@ -119,10 +121,12 @@ public void Shutdown() foreach (var projectId in projectIds) { stateSet.CollectDocumentsWithDiagnostics(projectId, documentSet); - RaiseProjectDiagnosticsRemoved(stateSet, projectId, documentSet, handleActiveFile, raiseEvents); + AddProjectDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), stateSet, projectId, documentSet, handleActiveFile); documentSet.Clear(); } } + + raiseEvents(argsBuilder.ToImmutableAndClear()); }); } @@ -131,6 +135,7 @@ private void ClearAllDiagnostics(ImmutableArray stateSets, ProjectId p AnalyzerService.RaiseBulkDiagnosticsUpdated(raiseEvents => { using var _ = PooledHashSet.GetInstance(out var documentSet); + using var argsBuilder = TemporaryArray.Empty; foreach (var stateSet in stateSets) { @@ -141,19 +146,22 @@ private void ClearAllDiagnostics(ImmutableArray stateSets, ProjectId p // PERF: don't fire events for ones that we dont have any diagnostics on if (documentSet.Count > 0) { - RaiseProjectDiagnosticsRemoved(stateSet, projectId, documentSet, handleActiveFile: true, raiseEvents); + AddProjectDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), stateSet, projectId, documentSet, handleActiveFile: true); documentSet.Clear(); } } + + raiseEvents(argsBuilder.ToImmutableAndClear()); }); } - private void RaiseDiagnosticsCreated( - Project project, DiagnosticAnalyzer analyzer, ImmutableArray items, Action raiseEvents) + private void AddDiagnosticsCreatedArgs( + ref TemporaryArray builder, + Project project, DiagnosticAnalyzer analyzer, ImmutableArray items) { Contract.ThrowIfFalse(project.Solution.Workspace == Workspace); - raiseEvents(DiagnosticsUpdatedArgs.DiagnosticsCreated( + builder.Add(DiagnosticsUpdatedArgs.DiagnosticsCreated( CreateId(analyzer, project.Id, AnalysisKind.NonLocal), project.Solution.Workspace, project.Solution, @@ -162,12 +170,13 @@ private void RaiseDiagnosticsCreated( diagnostics: items)); } - private void RaiseDiagnosticsRemoved( - ProjectId projectId, Solution? solution, DiagnosticAnalyzer analyzer, Action raiseEvents) + private void AddDiagnosticsRemovedArgs( + ref TemporaryArray builder, + ProjectId projectId, Solution? solution, DiagnosticAnalyzer analyzer) { Contract.ThrowIfFalse(solution == null || solution.Workspace == Workspace); - raiseEvents(DiagnosticsUpdatedArgs.DiagnosticsRemoved( + builder.Add(DiagnosticsUpdatedArgs.DiagnosticsRemoved( CreateId(analyzer, projectId, AnalysisKind.NonLocal), Workspace, solution, @@ -175,12 +184,13 @@ private void RaiseDiagnosticsRemoved( documentId: null)); } - private void RaiseDiagnosticsCreated( - TextDocument document, DiagnosticAnalyzer analyzer, AnalysisKind kind, ImmutableArray items, Action raiseEvents) + private void AddDiagnosticsCreatedArgs( + ref TemporaryArray builder, + TextDocument document, DiagnosticAnalyzer analyzer, AnalysisKind kind, ImmutableArray items) { Contract.ThrowIfFalse(document.Project.Solution.Workspace == Workspace); - raiseEvents(DiagnosticsUpdatedArgs.DiagnosticsCreated( + builder.Add(DiagnosticsUpdatedArgs.DiagnosticsCreated( CreateId(analyzer, document.Id, kind), document.Project.Solution.Workspace, document.Project.Solution, @@ -189,12 +199,13 @@ private void RaiseDiagnosticsCreated( items)); } - private void RaiseDiagnosticsRemoved( - DocumentId documentId, Solution? solution, DiagnosticAnalyzer analyzer, AnalysisKind kind, Action raiseEvents) + private void AddDiagnosticsRemovedArgs( + ref TemporaryArray builder, + DocumentId documentId, Solution? solution, DiagnosticAnalyzer analyzer, AnalysisKind kind) { Contract.ThrowIfFalse(solution == null || solution.Workspace == Workspace); - raiseEvents(DiagnosticsUpdatedArgs.DiagnosticsRemoved( + builder.Add(DiagnosticsUpdatedArgs.DiagnosticsRemoved( CreateId(analyzer, documentId, kind), Workspace, solution, diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs index bd7eb16046721..dcbff64d7d562 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.SolutionCrawler; using Microsoft.CodeAnalysis.Workspaces.Diagnostics; @@ -68,6 +69,7 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi // First attempt to fetch diagnostics from the cache, while computing the analyzers that are not cached. using var _ = ArrayBuilder<(DiagnosticAnalyzer analyzer, ActiveFileState state)>.GetInstance(out var nonCachedAnalyzersAndStates); + using var argsBuilder = TemporaryArray.Empty; foreach (var stateSet in stateSets) { var (activeFileState, existingData) = TryGetCachedDocumentAnalysisData(document, stateSet, kind, version, @@ -75,7 +77,7 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi isOpenDocument, isGeneratedRazorDocument, cancellationToken, out var isAnalyzerSuppressed); if (existingData.HasValue) { - PersistAndRaiseDiagnosticsIfNeeded(existingData.Value, stateSet.Analyzer, activeFileState); + PersistAndAddDiagnosticsArgsIfNeeded(ref argsBuilder.AsRef(), existingData.Value, stateSet.Analyzer, activeFileState); } else if (!isAnalyzerSuppressed) { @@ -83,6 +85,11 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi } } + if (argsBuilder.Count > 0) + { + AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + } + // Then, compute the diagnostics for non-cached state sets, and cache and raise diagnostic reported events for these diagnostics. if (nonCachedAnalyzersAndStates.Count > 0) { @@ -92,7 +99,13 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi foreach (var (analyzer, state) in nonCachedAnalyzersAndStates) { var computedData = await ComputeDocumentAnalysisDataAsync(executor, analyzer, state, logTelemetry, cancellationToken).ConfigureAwait(false); - PersistAndRaiseDiagnosticsIfNeeded(computedData, analyzer, state); + + PersistAndAddDiagnosticsArgsIfNeeded(ref argsBuilder.AsRef(), computedData, analyzer, state); + + if (argsBuilder.Count > 0) + { + AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + } } } } @@ -101,18 +114,20 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi throw ExceptionUtilities.Unreachable(); } - void PersistAndRaiseDiagnosticsIfNeeded(DocumentAnalysisData result, DiagnosticAnalyzer analyzer, ActiveFileState state) + void PersistAndAddDiagnosticsArgsIfNeeded( + ref TemporaryArray builder, + DocumentAnalysisData result, DiagnosticAnalyzer analyzer, ActiveFileState state) { if (result.FromCache == true) { - RaiseDocumentDiagnosticsIfNeeded(document, analyzer, kind, result.Items); + AddDocumentDiagnosticsArgsIfNeeded(ref builder, document, analyzer, kind, result.Items); return; } // no cancellation after this point. state.Save(kind, result.ToPersistData()); - RaiseDocumentDiagnosticsIfNeeded(document, analyzer, kind, result.OldItems, result.Items); + AddDocumentDiagnosticsArgsIfNeeded(ref builder, document, analyzer, kind, result.OldItems, result.Items); } void OnAnalysisException() @@ -321,13 +336,16 @@ private void RaiseDiagnosticsRemovedForDocument(DocumentId documentId, IEnumerab // remove all diagnostics for the document AnalyzerService.RaiseBulkDiagnosticsUpdated(raiseEvents => { + using var argsBuilder = TemporaryArray.Empty; foreach (var stateSet in stateSets) { // clear all doucment diagnostics - RaiseDiagnosticsRemoved(documentId, solution: null, stateSet.Analyzer, AnalysisKind.Syntax, raiseEvents); - RaiseDiagnosticsRemoved(documentId, solution: null, stateSet.Analyzer, AnalysisKind.Semantic, raiseEvents); - RaiseDiagnosticsRemoved(documentId, solution: null, stateSet.Analyzer, AnalysisKind.NonLocal, raiseEvents); + AddDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), documentId, solution: null, stateSet.Analyzer, AnalysisKind.Syntax); + AddDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), documentId, solution: null, stateSet.Analyzer, AnalysisKind.Semantic); + AddDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), documentId, solution: null, stateSet.Analyzer, AnalysisKind.NonLocal); } + + raiseEvents(argsBuilder.ToImmutableAndClear()); }); } @@ -348,11 +366,14 @@ public Task RemoveProjectAsync(ProjectId projectId, CancellationToken cancellati // remove all diagnostics for the project AnalyzerService.RaiseBulkDiagnosticsUpdated(raiseEvents => { + using var argsBuilder = TemporaryArray.Empty; foreach (var stateSet in stateSets) { // clear all project diagnostics - RaiseDiagnosticsRemoved(projectId, solution: null, stateSet.Analyzer, raiseEvents); + AddDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), projectId, solution: null, stateSet.Analyzer); } + + raiseEvents(argsBuilder.ToImmutableAndClear()); }); } } @@ -467,6 +488,7 @@ private void RaiseProjectDiagnosticsIfNeeded( AnalyzerService.RaiseBulkDiagnosticsUpdated(async raiseEvents => { + using var argsBuilder = TemporaryArray.Empty; foreach (var stateSet in stateSets) { var analyzer = stateSet.Analyzer; @@ -488,14 +510,14 @@ private void RaiseProjectDiagnosticsIfNeeded( RoslynDebug.Assert(oldAnalysisResult.DocumentIds != null); // remove old diagnostics - RaiseProjectDiagnosticsRemoved(stateSet, oldAnalysisResult.ProjectId, oldAnalysisResult.DocumentIds, handleActiveFile: false, raiseEvents); + AddProjectDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), stateSet, oldAnalysisResult.ProjectId, oldAnalysisResult.DocumentIds, handleActiveFile: false); continue; } if (oldAnalysisResult.IsEmpty && !newAnalysisResult.IsEmpty) { // add new diagnostics - await RaiseProjectDiagnosticsCreatedAsync(project, stateSet, oldAnalysisResult, newAnalysisResult, raiseEvents, CancellationToken.None).ConfigureAwait(false); + argsBuilder.AddRange(await CreateProjectDiagnosticsCreatedArgsAsync(project, stateSet, oldAnalysisResult, newAnalysisResult, CancellationToken.None).ConfigureAwait(false)); continue; } @@ -505,27 +527,32 @@ private void RaiseProjectDiagnosticsIfNeeded( // first remove ones no longer needed. var documentsToRemove = oldAnalysisResult.DocumentIds.Except(newAnalysisResult.DocumentIds); - RaiseProjectDiagnosticsRemoved(stateSet, oldAnalysisResult.ProjectId, documentsToRemove, handleActiveFile: false, raiseEvents); + AddProjectDiagnosticsRemovedArgs(ref argsBuilder.AsRef(), stateSet, oldAnalysisResult.ProjectId, documentsToRemove, handleActiveFile: false); // next update or create new ones - await RaiseProjectDiagnosticsCreatedAsync(project, stateSet, oldAnalysisResult, newAnalysisResult, raiseEvents, CancellationToken.None).ConfigureAwait(false); + argsBuilder.AddRange(await CreateProjectDiagnosticsCreatedArgsAsync(project, stateSet, oldAnalysisResult, newAnalysisResult, CancellationToken.None).ConfigureAwait(false)); } + + raiseEvents(argsBuilder.ToImmutableAndClear()); }); } - private void RaiseDocumentDiagnosticsIfNeeded(TextDocument document, DiagnosticAnalyzer analyzer, AnalysisKind kind, ImmutableArray items) - => RaiseDocumentDiagnosticsIfNeeded(document, analyzer, kind, ImmutableArray.Empty, items); + private void AddDocumentDiagnosticsArgsIfNeeded( + ref TemporaryArray builder, + TextDocument document, DiagnosticAnalyzer analyzer, AnalysisKind kind, ImmutableArray items) + => AddDocumentDiagnosticsArgsIfNeeded(ref builder, document, analyzer, kind, ImmutableArray.Empty, items); - private void RaiseDocumentDiagnosticsIfNeeded( + private void AddDocumentDiagnosticsArgsIfNeeded( + ref TemporaryArray builder, TextDocument document, DiagnosticAnalyzer analyzer, AnalysisKind kind, ImmutableArray oldItems, ImmutableArray newItems) { - RaiseDocumentDiagnosticsIfNeeded(document, analyzer, kind, oldItems, newItems, AnalyzerService.RaiseDiagnosticsUpdated, forceUpdate: false); + AddDocumentDiagnosticsArgsIfNeeded(ref builder, document, analyzer, kind, oldItems, newItems, forceUpdate: false); } - private void RaiseDocumentDiagnosticsIfNeeded( + private void AddDocumentDiagnosticsArgsIfNeeded( + ref TemporaryArray builder, TextDocument document, DiagnosticAnalyzer analyzer, AnalysisKind kind, - DiagnosticAnalysisResult oldResult, DiagnosticAnalysisResult newResult, - Action raiseEvents) + DiagnosticAnalysisResult oldResult, DiagnosticAnalysisResult newResult) { // if our old result is from build and we don't have actual data, don't try micro-optimize and always refresh diagnostics. // most of time, we don't actually load or hold the old data in memory from persistent storage due to perf reasons. @@ -540,13 +567,13 @@ private void RaiseDocumentDiagnosticsIfNeeded( var oldItems = oldResult.GetDocumentDiagnostics(document.Id, kind); var newItems = newResult.GetDocumentDiagnostics(document.Id, kind); - RaiseDocumentDiagnosticsIfNeeded(document, analyzer, kind, oldItems, newItems, raiseEvents, forceUpdate); + AddDocumentDiagnosticsArgsIfNeeded(ref builder, document, analyzer, kind, oldItems, newItems, forceUpdate); } - private void RaiseDocumentDiagnosticsIfNeeded( + private void AddDocumentDiagnosticsArgsIfNeeded( + ref TemporaryArray builder, TextDocument document, DiagnosticAnalyzer analyzer, AnalysisKind kind, ImmutableArray oldItems, ImmutableArray newItems, - Action raiseEvents, bool forceUpdate) { if (!forceUpdate && oldItems.IsEmpty && newItems.IsEmpty) @@ -555,13 +582,14 @@ private void RaiseDocumentDiagnosticsIfNeeded( return; } - RaiseDiagnosticsCreated(document, analyzer, kind, newItems, raiseEvents); + AddDiagnosticsCreatedArgs(ref builder, document, analyzer, kind, newItems); } - private async Task RaiseProjectDiagnosticsCreatedAsync(Project project, StateSet stateSet, DiagnosticAnalysisResult oldAnalysisResult, DiagnosticAnalysisResult newAnalysisResult, Action raiseEvents, CancellationToken cancellationToken) + private async Task> CreateProjectDiagnosticsCreatedArgsAsync(Project project, StateSet stateSet, DiagnosticAnalysisResult oldAnalysisResult, DiagnosticAnalysisResult newAnalysisResult, CancellationToken cancellationToken) { RoslynDebug.Assert(newAnalysisResult.DocumentIds != null); + using var argsBuilder = TemporaryArray.Empty; foreach (var documentId in newAnalysisResult.DocumentIds) { var document = project.GetTextDocument(documentId); @@ -586,7 +614,7 @@ private async Task RaiseProjectDiagnosticsCreatedAsync(Project project, StateSet continue; } - RaiseDocumentDiagnosticsIfNeeded(document, stateSet.Analyzer, AnalysisKind.NonLocal, oldAnalysisResult, newAnalysisResult, raiseEvents); + AddDocumentDiagnosticsArgsIfNeeded(ref argsBuilder.AsRef(), document, stateSet.Analyzer, AnalysisKind.NonLocal, oldAnalysisResult, newAnalysisResult); // we don't raise events for active file. it will be taken cared by active file analysis if (stateSet.IsActiveFile(documentId)) @@ -594,18 +622,20 @@ private async Task RaiseProjectDiagnosticsCreatedAsync(Project project, StateSet continue; } - RaiseDocumentDiagnosticsIfNeeded(document, stateSet.Analyzer, AnalysisKind.Syntax, oldAnalysisResult, newAnalysisResult, raiseEvents); - RaiseDocumentDiagnosticsIfNeeded(document, stateSet.Analyzer, AnalysisKind.Semantic, oldAnalysisResult, newAnalysisResult, raiseEvents); + AddDocumentDiagnosticsArgsIfNeeded(ref argsBuilder.AsRef(), document, stateSet.Analyzer, AnalysisKind.Syntax, oldAnalysisResult, newAnalysisResult); + AddDocumentDiagnosticsArgsIfNeeded(ref argsBuilder.AsRef(), document, stateSet.Analyzer, AnalysisKind.Semantic, oldAnalysisResult, newAnalysisResult); } - RaiseDiagnosticsCreated(project, stateSet.Analyzer, newAnalysisResult.GetOtherDiagnostics(), raiseEvents); + AddDiagnosticsCreatedArgs(ref argsBuilder.AsRef(), project, stateSet.Analyzer, newAnalysisResult.GetOtherDiagnostics()); + + return argsBuilder.ToImmutableAndClear(); } - private void RaiseProjectDiagnosticsRemoved(StateSet stateSet, ProjectId projectId, IEnumerable documentIds, bool handleActiveFile, Action raiseEvents) + private void AddProjectDiagnosticsRemovedArgs(ref TemporaryArray builder, StateSet stateSet, ProjectId projectId, IEnumerable documentIds, bool handleActiveFile) { foreach (var documentId in documentIds) { - RaiseDiagnosticsRemoved(documentId, solution: null, stateSet.Analyzer, AnalysisKind.NonLocal, raiseEvents); + AddDiagnosticsRemovedArgs(ref builder, documentId, solution: null, stateSet.Analyzer, AnalysisKind.NonLocal); // we don't raise events for active file. it will be taken care of by active file analysis if (!handleActiveFile && stateSet.IsActiveFile(documentId)) @@ -613,11 +643,11 @@ private void RaiseProjectDiagnosticsRemoved(StateSet stateSet, ProjectId project continue; } - RaiseDiagnosticsRemoved(documentId, solution: null, stateSet.Analyzer, AnalysisKind.Syntax, raiseEvents); - RaiseDiagnosticsRemoved(documentId, solution: null, stateSet.Analyzer, AnalysisKind.Semantic, raiseEvents); + AddDiagnosticsRemovedArgs(ref builder, documentId, solution: null, stateSet.Analyzer, AnalysisKind.Syntax); + AddDiagnosticsRemovedArgs(ref builder, documentId, solution: null, stateSet.Analyzer, AnalysisKind.Semantic); } - RaiseDiagnosticsRemoved(projectId, solution: null, stateSet.Analyzer, raiseEvents); + AddDiagnosticsRemovedArgs(ref builder, projectId, solution: null, stateSet.Analyzer); } } } diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/IDiagnosticService.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/IDiagnosticService.cs index 8c384d6e89838..5dc18b498f1c0 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/IDiagnosticService.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/IDiagnosticService.cs @@ -20,7 +20,7 @@ internal interface IDiagnosticService /// Notifications for this event are serialized to preserve order. /// However, individual event notifications may occur on any thread. /// - event EventHandler DiagnosticsUpdated; + event EventHandler> DiagnosticsUpdated; /// /// Get current diagnostics stored in IDiagnosticUpdateSource. diff --git a/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerDependencyCheckingService.cs b/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerDependencyCheckingService.cs index 5fc6cc62986a2..d078533c2fee5 100644 --- a/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerDependencyCheckingService.cs +++ b/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerDependencyCheckingService.cs @@ -15,6 +15,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList; using Roslyn.Utilities; @@ -24,7 +25,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Implementation internal sealed class AnalyzerDependencyCheckingService { /// - /// Object given as key for . + /// Object given as key for . /// private static readonly object s_dependencyConflictErrorId = new(); private static readonly IIgnorableAssemblyList s_systemPrefixList = new IgnorableAssemblyNamePrefixList("System"); @@ -130,6 +131,7 @@ private static void AnalyzeAndReportConflictsInSolution( var conflicts = results.Conflicts; var missingDependencies = results.MissingDependencies; + using var argsBuilder = TemporaryArray.Empty; foreach (var project in solution.Projects) { builder.Clear(); @@ -169,9 +171,11 @@ private static void AnalyzeAndReportConflictsInSolution( } } - hostDiagnosticUpdateSource.UpdateDiagnosticsForProject(project.Id, s_dependencyConflictErrorId, builder.ToImmutable()); + hostDiagnosticUpdateSource.UpdateAndAddDiagnosticsArgsForProject(ref argsBuilder.AsRef(), project.Id, s_dependencyConflictErrorId, builder.ToImmutable()); } + hostDiagnosticUpdateSource.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + foreach (var conflict in conflicts) { LogConflict(conflict); diff --git a/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerFileWatcherService.cs b/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerFileWatcherService.cs index 9a23321c50303..4811660423374 100644 --- a/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerFileWatcherService.cs +++ b/src/VisualStudio/Core/Def/AnalyzerDependency/AnalyzerFileWatcherService.cs @@ -10,6 +10,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem; using Microsoft.VisualStudio.LanguageServices.Implementation.TaskList; using Microsoft.VisualStudio.Shell; @@ -56,17 +57,15 @@ public AnalyzerFileWatcherService( _updateSource = hostDiagnosticUpdateSource; _fileChangeService = (IVsFileChangeEx)serviceProvider.GetService(typeof(SVsFileChangeEx)); } - internal void RemoveAnalyzerAlreadyLoadedDiagnostics(ProjectId projectId, string analyzerPath) - => _updateSource.ClearDiagnosticsForProject(projectId, Tuple.Create(s_analyzerChangedErrorId, analyzerPath)); - private void RaiseAnalyzerChangedWarning(ProjectId projectId, string analyzerPath) + private void AddAnalyzerChangedWarningArgs(ref TemporaryArray builder, ProjectId projectId, string analyzerPath) { var messageArguments = new string[] { analyzerPath }; var project = _workspace.CurrentSolution.GetProject(projectId); if (project != null && DiagnosticData.TryCreate(_analyzerChangedRule, messageArguments, project, out var diagnostic)) { - _updateSource.UpdateDiagnosticsForProject(projectId, Tuple.Create(s_analyzerChangedErrorId, analyzerPath), SpecializedCollections.SingletonEnumerable(diagnostic)); + _updateSource.UpdateAndAddDiagnosticsArgsForProject(ref builder, projectId, Tuple.Create(s_analyzerChangedErrorId, analyzerPath), SpecializedCollections.SingletonEnumerable(diagnostic)); } } @@ -110,7 +109,9 @@ internal void TrackFilePathAndReportErrorIfChanged(string filePath, ProjectId pr { if (currentFileUpdateTime != assemblyUpdatedTime) { - RaiseAnalyzerChangedWarning(projectId, filePath); + using var argsBuilder = TemporaryArray.Empty; + AddAnalyzerChangedWarningArgs(ref argsBuilder.AsRef(), projectId, filePath); + _updateSource.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); } // If the the tracker is in place, at this point we can stop checking any further for this assembly @@ -154,15 +155,18 @@ private void Tracker_UpdatedOnDisk(object sender, EventArgs e) // Traverse the chain of requesting assemblies to get back to the original analyzer // assembly. + using var argsBuilder = TemporaryArray.Empty; foreach (var project in _workspace.CurrentSolution.Projects) { var analyzerFileReferences = project.AnalyzerReferences.OfType(); if (analyzerFileReferences.Any(a => a.FullPath.Equals(filePath, StringComparison.OrdinalIgnoreCase))) { - RaiseAnalyzerChangedWarning(project.Id, filePath); + AddAnalyzerChangedWarningArgs(ref argsBuilder.AsRef(), project.Id, filePath); } } + + _updateSource.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); } } } diff --git a/src/VisualStudio/Core/Def/Diagnostics/VisualStudioDiagnosticAnalyzerService.cs b/src/VisualStudio/Core/Def/Diagnostics/VisualStudioDiagnosticAnalyzerService.cs index 6b93736f2052a..71343341b61e6 100644 --- a/src/VisualStudio/Core/Def/Diagnostics/VisualStudioDiagnosticAnalyzerService.cs +++ b/src/VisualStudio/Core/Def/Diagnostics/VisualStudioDiagnosticAnalyzerService.cs @@ -16,6 +16,7 @@ using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SolutionCrawler; using Microsoft.VisualStudio.LanguageServices.EditorConfigSettings; @@ -374,11 +375,14 @@ void HandleProjectsWithDisabledAnalysis() // First clear all special host diagostics for all involved projects. var projects = project != null ? otherProjectsForMultiTfmProject.Add(project) : solution.Projects; + using var argsBuilder = TemporaryArray.Empty; foreach (var project in projects) { - _hostDiagnosticUpdateSource.ClearDiagnosticsForProject(project.Id, key: this); + _hostDiagnosticUpdateSource.ClearAndAddDiagnosticsArgsForProject(ref argsBuilder.AsRef(), project.Id, key: this); } + _hostDiagnosticUpdateSource.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + // Now compute the new host diagostics for all projects with disabled analysis. var projectsWithDisabledAnalysis = isAnalysisDisabled ? projects.ToImmutableArray() @@ -403,8 +407,10 @@ void HandleProjectsWithDisabledAnalysis() { var project = projectsWithDisabledAnalysis[index]; var diagnostics = tasks[index].Result; - _hostDiagnosticUpdateSource.UpdateDiagnosticsForProject(project.Id, key: this, diagnostics); + _hostDiagnosticUpdateSource.UpdateAndAddDiagnosticsArgsForProject(ref argsBuilder.AsRef(), project.Id, key: this, diagnostics); } + + _hostDiagnosticUpdateSource.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); } } } diff --git a/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs b/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs index 5f6bbdd9811ad..254c97d7a245c 100644 --- a/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs +++ b/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs @@ -197,35 +197,38 @@ private void PopulateInitialData(Workspace workspace, IDiagnosticService diagnos } } - private void OnDiagnosticsUpdated(object sender, DiagnosticsUpdatedArgs e) + private void OnDiagnosticsUpdated(object sender, ImmutableArray argsCollection) { - using (Logger.LogBlock(FunctionId.LiveTableDataSource_OnDiagnosticsUpdated, static e => GetDiagnosticUpdatedMessage(e), e, CancellationToken.None)) + foreach (var e in argsCollection) { - if (_workspace != e.Workspace) + using (Logger.LogBlock(FunctionId.LiveTableDataSource_OnDiagnosticsUpdated, static e => GetDiagnosticUpdatedMessage(e), e, CancellationToken.None)) { - return; + if (_workspace != e.Workspace) + { + return; + } + + // if we're in lsp mode we never respond to any diagnostics we hear about, lsp client is fully + // responsible for populating the error list. + var diagnostics = GlobalOptions.IsLspPullDiagnostics() + ? ImmutableArray.Empty + : e.Diagnostics; + + if (diagnostics.Length == 0) + { + OnDataRemoved(e); + return; + } + + var count = diagnostics.Where(ShouldInclude).Count(); + if (count <= 0) + { + OnDataRemoved(e); + return; + } + + OnDataAddedOrChanged(e); } - - // if we're in lsp mode we never respond to any diagnostics we hear about, lsp client is fully - // responsible for populating the error list. - var diagnostics = GlobalOptions.IsLspPullDiagnostics() - ? ImmutableArray.Empty - : e.Diagnostics; - - if (diagnostics.Length == 0) - { - OnDataRemoved(e); - return; - } - - var count = diagnostics.Where(ShouldInclude).Count(); - if (count <= 0) - { - OnDataRemoved(e); - return; - } - - OnDataAddedOrChanged(e); } } diff --git a/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs b/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs index c0ffec18f9033..44d4cb3b16a31 100644 --- a/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs +++ b/src/VisualStudio/Core/Def/TaskList/ExternalErrorDiagnosticUpdateSource.cs @@ -15,6 +15,7 @@ using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Notification; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.SolutionCrawler; using Roslyn.Utilities; @@ -120,7 +121,7 @@ internal ExternalErrorDiagnosticUpdateSource( /// Event generated from the serialized whenever build-only diagnostics are reported during a build in Visual Studio. /// These diagnostics are not supported from intellisense and only get refreshed during actual build. /// - public event EventHandler? DiagnosticsUpdated; + public event EventHandler>? DiagnosticsUpdated; /// /// Event generated from the serialized whenever build-only diagnostics are cleared during a build in Visual Studio. @@ -224,7 +225,11 @@ async ValueTask ClearErrorsCoreAsync(ProjectId projectId, Solution solution, InP // when 'ClearErrors' is invoked for multiple dependent projects. // Finally, we update build progress state so error list gets refreshed. - ClearBuildOnlyProjectErrors(solution, projectId); + using (var argsBuilder = TemporaryArray.Empty) + { + AddArgsToClearBuildOnlyProjectErrors(ref argsBuilder.AsRef(), solution, projectId); + ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + } await SetLiveErrorsForProjectAsync(projectId, ImmutableArray.Empty, GetApplicableCancellationToken(state)).ConfigureAwait(false); @@ -234,26 +239,57 @@ async ValueTask ClearErrorsCoreAsync(ProjectId projectId, Solution solution, InP } } - // internal for testing purposes only. - internal void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + private void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) { // Clear relevant build-only errors on workspace events such as solution added/removed/reloaded, // project added/removed/reloaded, etc. switch (e.Kind) { case WorkspaceChangeKind.SolutionAdded: - _taskQueue.ScheduleTask("OnSolutionAdded", () => e.OldSolution.ProjectIds.Do(p => ClearBuildOnlyProjectErrors(e.OldSolution, p)), _disposalToken); + _taskQueue.ScheduleTask( + "OnSolutionAdded", + () => + { + using var argsBuilder = TemporaryArray.Empty; + foreach (var projectId in e.OldSolution.ProjectIds) + { + AddArgsToClearBuildOnlyProjectErrors(ref argsBuilder.AsRef(), e.OldSolution, projectId); + } + + ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + }, + _disposalToken); break; case WorkspaceChangeKind.SolutionRemoved: case WorkspaceChangeKind.SolutionCleared: case WorkspaceChangeKind.SolutionReloaded: - _taskQueue.ScheduleTask("OnSolutionChanged", () => e.OldSolution.ProjectIds.Do(p => ClearBuildOnlyProjectErrors(e.OldSolution, p)), _disposalToken); + _taskQueue.ScheduleTask( + "OnSolutionChanged", + () => + { + using var argsBuilder = TemporaryArray.Empty; + foreach (var projectId in e.OldSolution.ProjectIds) + { + AddArgsToClearBuildOnlyProjectErrors(ref argsBuilder.AsRef(), e.OldSolution, projectId); + } + + ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + }, + _disposalToken); break; case WorkspaceChangeKind.ProjectRemoved: case WorkspaceChangeKind.ProjectReloaded: - _taskQueue.ScheduleTask("OnProjectChanged", () => ClearBuildOnlyProjectErrors(e.OldSolution, e.ProjectId), _disposalToken); + _taskQueue.ScheduleTask( + "OnProjectChanged", + () => + { + using var argsBuilder = TemporaryArray.Empty; + AddArgsToClearBuildOnlyProjectErrors(ref argsBuilder.AsRef(), e.OldSolution, e.ProjectId); + ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + }, + _disposalToken); break; case WorkspaceChangeKind.DocumentRemoved: @@ -262,7 +298,15 @@ internal void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) case WorkspaceChangeKind.AdditionalDocumentReloaded: case WorkspaceChangeKind.AnalyzerConfigDocumentRemoved: case WorkspaceChangeKind.AnalyzerConfigDocumentReloaded: - _taskQueue.ScheduleTask("OnDocumentRemoved", () => ClearBuildOnlyDocumentErrors(e.OldSolution, e.ProjectId, e.DocumentId), _disposalToken); + _taskQueue.ScheduleTask( + "OnDocumentRemoved", + () => + { + using var argsBuilder = TemporaryArray.Empty; + AddArgsToClearBuildOnlyDocumentErrors(ref argsBuilder.AsRef(), e.OldSolution, e.ProjectId, e.DocumentId); + ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + }, + _disposalToken); break; case WorkspaceChangeKind.DocumentChanged: @@ -274,7 +318,15 @@ internal void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) // do not get automatically removed/refreshed while typing. // See https://github.com/dotnet/docs/issues/26708 and https://github.com/dotnet/roslyn/issues/64659 // for additional details. - _taskQueue.ScheduleTask("OnDocumentChanged", () => ClearBuildOnlyDocumentErrors(e.OldSolution, e.ProjectId, e.DocumentId), _disposalToken); + _taskQueue.ScheduleTask( + "OnDocumentChanged", + () => + { + using var argsBuilder = TemporaryArray.Empty; + AddArgsToClearBuildOnlyDocumentErrors(ref argsBuilder.AsRef(), e.OldSolution, e.ProjectId, e.DocumentId); + ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + }, + _disposalToken); break; case WorkspaceChangeKind.ProjectAdded: @@ -354,6 +406,7 @@ private ValueTask SyncBuildErrorsAndReportOnBuildCompletedAsync(DiagnosticAnalyz var (allLiveErrors, pendingLiveErrorsToSync) = inProgressState.GetLiveErrors(); // Raise events for build only errors + using var argsBuilder = TemporaryArray.Empty; var buildErrors = GetBuildErrors().Except(allLiveErrors).GroupBy(k => k.DocumentId); foreach (var group in buildErrors) { @@ -364,36 +417,37 @@ private ValueTask SyncBuildErrorsAndReportOnBuildCompletedAsync(DiagnosticAnalyz foreach (var projectGroup in group.GroupBy(g => g.ProjectId)) { Contract.ThrowIfNull(projectGroup.Key); - ReportBuildErrors(projectGroup.Key, solution, projectGroup.ToImmutableArray()); + argsBuilder.Add(CreateArgsToReportBuildErrors(projectGroup.Key, solution, projectGroup.ToImmutableArray())); } continue; } - ReportBuildErrors(group.Key, solution, group.ToImmutableArray()); + argsBuilder.Add(CreateArgsToReportBuildErrors(group.Key, solution, group.ToImmutableArray())); } + ProcessAndRaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + // Report pending live errors return diagnosticService.SynchronizeWithBuildAsync(_workspace, pendingLiveErrorsToSync, _postBuildAndErrorListRefreshTaskQueue, onBuildCompleted: true, cancellationToken); } - private void ReportBuildErrors(T item, Solution solution, ImmutableArray buildErrors) + private DiagnosticsUpdatedArgs CreateArgsToReportBuildErrors(T item, Solution solution, ImmutableArray buildErrors) { if (item is ProjectId projectId) { - RaiseDiagnosticsCreated(projectId, solution, projectId, null, buildErrors); - return; + return CreateDiagnosticsCreatedArgs(projectId, solution, projectId, documentId: null, buildErrors); } RoslynDebug.Assert(item is DocumentId); var documentId = (DocumentId)(object)item; - RaiseDiagnosticsCreated(documentId, solution, documentId.ProjectId, documentId, buildErrors); + return CreateDiagnosticsCreatedArgs(documentId, solution, documentId.ProjectId, documentId, buildErrors); } - private void ClearBuildOnlyProjectErrors(Solution solution, ProjectId? projectId) + private void AddArgsToClearBuildOnlyProjectErrors(ref TemporaryArray builder, Solution solution, ProjectId? projectId) { // Remove all project errors - RaiseDiagnosticsRemoved(projectId, solution, projectId, documentId: null); + builder.Add(CreateDiagnosticsRemovedArgs(projectId, solution, projectId, documentId: null)); var project = solution.GetProject(projectId); if (project == null) @@ -404,12 +458,12 @@ private void ClearBuildOnlyProjectErrors(Solution solution, ProjectId? projectId // Remove all document errors foreach (var documentId in project.DocumentIds.Concat(project.AdditionalDocumentIds).Concat(project.AnalyzerConfigDocumentIds)) { - ClearBuildOnlyDocumentErrors(solution, projectId, documentId); + AddArgsToClearBuildOnlyDocumentErrors(ref builder, solution, projectId, documentId); } } - private void ClearBuildOnlyDocumentErrors(Solution solution, ProjectId? projectId, DocumentId? documentId) - => RaiseDiagnosticsRemoved(documentId, solution, projectId, documentId); + private void AddArgsToClearBuildOnlyDocumentErrors(ref TemporaryArray builder, Solution solution, ProjectId? projectId, DocumentId? documentId) + => builder.Add(CreateDiagnosticsRemovedArgs(documentId, solution, projectId, documentId)); public void AddNewErrors(ProjectId projectId, DiagnosticData diagnostic) { @@ -536,18 +590,38 @@ private InProgressState GetOrCreateInProgressState() } } - private void RaiseDiagnosticsCreated(object? id, Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableArray items) + private DiagnosticsUpdatedArgs CreateDiagnosticsCreatedArgs(object? id, Solution solution, ProjectId? projectId, DocumentId? documentId, ImmutableArray items) { - _buildOnlyDiagnosticsService.AddBuildOnlyDiagnostics(solution, projectId, documentId, items); - DiagnosticsUpdated?.Invoke(this, DiagnosticsUpdatedArgs.DiagnosticsCreated( - CreateArgumentKey(id), _workspace, solution, projectId, documentId, items)); + return DiagnosticsUpdatedArgs.DiagnosticsCreated(CreateArgumentKey(id), _workspace, solution, projectId, documentId, items); } - private void RaiseDiagnosticsRemoved(object? id, Solution solution, ProjectId? projectId, DocumentId? documentId) + private DiagnosticsUpdatedArgs CreateDiagnosticsRemovedArgs(object? id, Solution solution, ProjectId? projectId, DocumentId? documentId) { - _buildOnlyDiagnosticsService.ClearBuildOnlyDiagnostics(solution, projectId, documentId); - DiagnosticsUpdated?.Invoke(this, DiagnosticsUpdatedArgs.DiagnosticsRemoved( - CreateArgumentKey(id), _workspace, solution, projectId, documentId)); + return DiagnosticsUpdatedArgs.DiagnosticsRemoved(CreateArgumentKey(id), _workspace, solution, projectId, documentId); + } + + private void ProcessAndRaiseDiagnosticsUpdated(ImmutableArray argsCollection) + { + if (argsCollection.IsEmpty) + { + return; + } + + foreach (var args in argsCollection) + { + if (args.Kind == DiagnosticsUpdatedKind.DiagnosticsCreated) + { + RoslynDebug.AssertNotNull(args.Solution); + _buildOnlyDiagnosticsService.AddBuildOnlyDiagnostics(args.Solution, args.ProjectId, args.DocumentId, args.Diagnostics); + } + else if (args.Kind == DiagnosticsUpdatedKind.DiagnosticsRemoved) + { + RoslynDebug.AssertNotNull(args.Solution); + _buildOnlyDiagnosticsService.ClearBuildOnlyDiagnostics(args.Solution, args.ProjectId, args.DocumentId); + } + } + + DiagnosticsUpdated?.Invoke(this, argsCollection); } private static ArgumentKey CreateArgumentKey(object? id) => new(id); @@ -565,6 +639,9 @@ public ValueTask> GetDiagnosticsAsync( } #endregion + internal TestAccessor GetTestAccessor() + => new(this); + internal enum BuildProgress { Started, @@ -572,6 +649,12 @@ internal enum BuildProgress Done } + internal readonly struct TestAccessor(ExternalErrorDiagnosticUpdateSource instance) + { + internal void OnWorkspaceChanged(object sender, WorkspaceChangeEventArgs e) + => instance.OnWorkspaceChanged(sender, e); + } + private sealed class InProgressState { private readonly ExternalErrorDiagnosticUpdateSource _owner; diff --git a/src/VisualStudio/Core/Def/TaskList/HostDiagnosticUpdateSource.cs b/src/VisualStudio/Core/Def/TaskList/HostDiagnosticUpdateSource.cs index fbc57d001a5ab..6c3cf0996c0db 100644 --- a/src/VisualStudio/Core/Def/TaskList/HostDiagnosticUpdateSource.cs +++ b/src/VisualStudio/Core/Def/TaskList/HostDiagnosticUpdateSource.cs @@ -10,6 +10,7 @@ using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Workspaces.ProjectSystem; using Roslyn.Utilities; @@ -43,7 +44,7 @@ public override Workspace Workspace } } - private void RaiseDiagnosticsCreatedForProject(ProjectId projectId, object key, IEnumerable items) + private void AddDiagnosticsCreatedArgsForProject(ref TemporaryArray builder, ProjectId projectId, object key, IEnumerable items) { var args = DiagnosticsUpdatedArgs.DiagnosticsCreated( CreateId(projectId, key), @@ -53,10 +54,10 @@ private void RaiseDiagnosticsCreatedForProject(ProjectId projectId, object key, documentId: null, diagnostics: items.AsImmutableOrEmpty()); - RaiseDiagnosticsUpdated(args); + builder.Add(args); } - private void RaiseDiagnosticsRemovedForProject(ProjectId projectId, object key) + private void AddDiagnosticsRemovedArgsForProject(ref TemporaryArray builder, ProjectId projectId, object key) { var args = DiagnosticsUpdatedArgs.DiagnosticsRemoved( CreateId(projectId, key), @@ -65,12 +66,12 @@ private void RaiseDiagnosticsRemovedForProject(ProjectId projectId, object key) projectId: projectId, documentId: null); - RaiseDiagnosticsUpdated(args); + builder.Add(args); } private object CreateId(ProjectId projectId, object key) => Tuple.Create(this, projectId, key); - public void UpdateDiagnosticsForProject(ProjectId projectId, object key, IEnumerable items) + public void UpdateAndAddDiagnosticsArgsForProject(ref TemporaryArray builder, ProjectId projectId, object key, IEnumerable items) { Contract.ThrowIfNull(projectId); Contract.ThrowIfNull(key); @@ -81,10 +82,17 @@ public void UpdateDiagnosticsForProject(ProjectId projectId, object key, IEnumer _diagnosticMap.GetOrAdd(projectId, id => new HashSet()).Add(key); } - RaiseDiagnosticsCreatedForProject(projectId, key, items); + AddDiagnosticsCreatedArgsForProject(ref builder, projectId, key, items); } - public void ClearAllDiagnosticsForProject(ProjectId projectId) + void IProjectSystemDiagnosticSource.UpdateDiagnosticsForProject(ProjectId projectId, object key, IEnumerable items) + { + using var argsBuilder = TemporaryArray.Empty; + UpdateAndAddDiagnosticsArgsForProject(ref argsBuilder.AsRef(), projectId, key, items); + RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + } + + void IProjectSystemDiagnosticSource.ClearAllDiagnosticsForProject(ProjectId projectId) { Contract.ThrowIfNull(projectId); @@ -97,18 +105,20 @@ public void ClearAllDiagnosticsForProject(ProjectId projectId) } } + using var argsBuilder = TemporaryArray.Empty; if (projectDiagnosticKeys != null) { foreach (var key in projectDiagnosticKeys) { - RaiseDiagnosticsRemovedForProject(projectId, key); + AddDiagnosticsRemovedArgsForProject(ref argsBuilder.AsRef(), projectId, key); } } - ClearAnalyzerDiagnostics(projectId); + AddArgsToClearAnalyzerDiagnostics(ref argsBuilder.AsRef(), projectId); + RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); } - public void ClearDiagnosticsForProject(ProjectId projectId, object key) + internal void ClearAndAddDiagnosticsArgsForProject(ref TemporaryArray builder, ProjectId projectId, object key) { Contract.ThrowIfNull(projectId); Contract.ThrowIfNull(key); @@ -124,10 +134,17 @@ public void ClearDiagnosticsForProject(ProjectId projectId, object key) if (raiseEvent) { - RaiseDiagnosticsRemovedForProject(projectId, key); + AddDiagnosticsRemovedArgsForProject(ref builder, projectId, key); } } + void IProjectSystemDiagnosticSource.ClearDiagnosticsForProject(ProjectId projectId, object key) + { + using var argsBuilder = TemporaryArray.Empty; + ClearAndAddDiagnosticsArgsForProject(ref argsBuilder.AsRef(), projectId, key); + RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); + } + public DiagnosticData CreateAnalyzerLoadFailureDiagnostic(AnalyzerLoadFailureEventArgs e, string fullPath, ProjectId projectId, string language) { return DocumentAnalysisExecutor.CreateAnalyzerLoadFailureDiagnostic(e, fullPath, projectId, language); diff --git a/src/VisualStudio/Core/Test/Diagnostics/DefaultDiagnosticUpdateSourceTests.vb b/src/VisualStudio/Core/Test/Diagnostics/DefaultDiagnosticUpdateSourceTests.vb index b540b86b249f7..b842a19845c7d 100644 --- a/src/VisualStudio/Core/Test/Diagnostics/DefaultDiagnosticUpdateSourceTests.vb +++ b/src/VisualStudio/Core/Test/Diagnostics/DefaultDiagnosticUpdateSourceTests.vb @@ -246,7 +246,7 @@ class 123 { } Dim buildTool = String.Empty AddHandler miscService.DiagnosticsUpdated, Sub(e, a) - Dim id = DirectCast(a.Id, BuildToolId) + Dim id = DirectCast(a.Single().Id, BuildToolId) buildTool = id.BuildTool End Sub @@ -275,7 +275,7 @@ End Class Dim buildTool = String.Empty AddHandler miscService.DiagnosticsUpdated, Sub(e, a) - Dim id = DirectCast(a.Id, BuildToolId) + Dim id = DirectCast(a.Single().Id, BuildToolId) buildTool = id.BuildTool End Sub diff --git a/src/VisualStudio/Core/Test/Diagnostics/DiagnosticTableDataSourceTests.vb b/src/VisualStudio/Core/Test/Diagnostics/DiagnosticTableDataSourceTests.vb index efdd394f61d1f..e0e2718c03ffa 100644 --- a/src/VisualStudio/Core/Test/Diagnostics/DiagnosticTableDataSourceTests.vb +++ b/src/VisualStudio/Core/Test/Diagnostics/DiagnosticTableDataSourceTests.vb @@ -799,7 +799,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics Me.Items = items End Sub - Public Event DiagnosticsUpdated As EventHandler(Of DiagnosticsUpdatedArgs) Implements IDiagnosticService.DiagnosticsUpdated + Public Event DiagnosticsUpdated As EventHandler(Of ImmutableArray(Of DiagnosticsUpdatedArgs)) Implements IDiagnosticService.DiagnosticsUpdated Public Function GetDiagnosticsAsync(workspace As Workspace, projectId As ProjectId, documentId As DocumentId, id As Object, includeSuppressedDiagnostics As Boolean, cancellationToken As CancellationToken) As ValueTask(Of ImmutableArray(Of DiagnosticData)) Implements IDiagnosticService.GetDiagnosticsAsync Return New ValueTask(Of ImmutableArray(Of DiagnosticData))(GetDiagnostics(workspace, projectId, documentId, includeSuppressedDiagnostics)) @@ -873,29 +873,29 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics Dim item = items(0) Dim id = If(CObj(item.DocumentId), item.ProjectId) - RaiseEvent DiagnosticsUpdated(Me, DiagnosticsUpdatedArgs.DiagnosticsCreated( - New ErrorId(Me, id), workspace, workspace.CurrentSolution, item.ProjectId, item.DocumentId, items.ToImmutableArray())) + RaiseEvent DiagnosticsUpdated(Me, ImmutableArray.Create(DiagnosticsUpdatedArgs.DiagnosticsCreated( + New ErrorId(Me, id), workspace, workspace.CurrentSolution, item.ProjectId, item.DocumentId, items.ToImmutableArray()))) End Sub Public Sub RaiseDiagnosticsUpdated(workspace As Workspace) Dim documentMap = Items.Where(Function(t) t.DocumentId IsNot Nothing).ToLookup(Function(t) t.DocumentId) For Each group In documentMap - RaiseEvent DiagnosticsUpdated(Me, DiagnosticsUpdatedArgs.DiagnosticsCreated( - New ErrorId(Me, group.Key), workspace, workspace.CurrentSolution, group.Key.ProjectId, group.Key, group.ToImmutableArrayOrEmpty())) + RaiseEvent DiagnosticsUpdated(Me, ImmutableArray.Create(DiagnosticsUpdatedArgs.DiagnosticsCreated( + New ErrorId(Me, group.Key), workspace, workspace.CurrentSolution, group.Key.ProjectId, group.Key, group.ToImmutableArrayOrEmpty()))) Next Dim projectMap = Items.Where(Function(t) t.DocumentId Is Nothing).ToLookup(Function(t) t.ProjectId) For Each group In projectMap - RaiseEvent DiagnosticsUpdated(Me, DiagnosticsUpdatedArgs.DiagnosticsCreated( - New ErrorId(Me, group.Key), workspace, workspace.CurrentSolution, group.Key, Nothing, group.ToImmutableArrayOrEmpty())) + RaiseEvent DiagnosticsUpdated(Me, ImmutableArray.Create(DiagnosticsUpdatedArgs.DiagnosticsCreated( + New ErrorId(Me, group.Key), workspace, workspace.CurrentSolution, group.Key, Nothing, group.ToImmutableArrayOrEmpty()))) Next End Sub Public Sub RaiseClearDiagnosticsUpdated(workspace As Microsoft.CodeAnalysis.Workspace, projectId As ProjectId, documentId As DocumentId) - RaiseEvent DiagnosticsUpdated(Me, DiagnosticsUpdatedArgs.DiagnosticsRemoved( - New ErrorId(Me, documentId), workspace, workspace.CurrentSolution, projectId, documentId)) + RaiseEvent DiagnosticsUpdated(Me, ImmutableArray.Create(DiagnosticsUpdatedArgs.DiagnosticsRemoved( + New ErrorId(Me, documentId), workspace, workspace.CurrentSolution, projectId, documentId))) End Sub Private Class ErrorId diff --git a/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb b/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb index 507de5e426315..e2c133ef1acd1 100644 --- a/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb +++ b/src/VisualStudio/Core/Test/Diagnostics/ExternalDiagnosticUpdateSourceTests.vb @@ -55,8 +55,9 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics Dim diagnostic = GetDiagnosticData(project.Id) Dim expected = 1 - AddHandler source.DiagnosticsUpdated, Sub(o, a) - Dim diagnostics = a.Diagnostics + AddHandler source.DiagnosticsUpdated, Sub(o, argsCollection) + Dim args = argsCollection.Single() + Dim diagnostics = args.Diagnostics Assert.Equal(expected, diagnostics.Length) If expected = 1 Then Assert.Equal(diagnostics(0), diagnostic) @@ -133,8 +134,9 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics source.AddNewErrors(project.Id, New HashSet(Of DiagnosticData)(SpecializedCollections.SingletonEnumerable(diagnostic)), map) - AddHandler source.DiagnosticsUpdated, Sub(o, a) - Dim diagnostics = a.Diagnostics + AddHandler source.DiagnosticsUpdated, Sub(o, argsCollection) + Dim args = argsCollection.Single() + Dim diagnostics = args.Diagnostics Assert.Equal(1, diagnostics.Length) End Sub @@ -255,8 +257,9 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics source.AddNewErrors(project.Id, diagnostic) source.AddNewErrors(project.Id, diagnostic) - AddHandler source.DiagnosticsUpdated, Sub(o, a) - Dim diagnostics = a.Diagnostics + AddHandler source.DiagnosticsUpdated, Sub(o, argsCollection) + Dim args = argsCollection.Single() + Dim diagnostics = args.Diagnostics Assert.Equal(1, diagnostics.Length) End Sub @@ -293,10 +296,11 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics source.AddNewErrors(project.Id, diagnostic) Dim buildDiagnosticCallbackSeen = False - AddHandler source.DiagnosticsUpdated, Sub(o, a) + AddHandler source.DiagnosticsUpdated, Sub(o, argsCollection) + Dim args = argsCollection.Single() buildDiagnosticCallbackSeen = True - Dim diagnostics = a.Diagnostics + Dim diagnostics = args.Diagnostics Assert.Equal(1, diagnostics.Length) Assert.Equal(diagnostics(0).Properties(WellKnownDiagnosticPropertyNames.Origin), WellKnownDiagnosticTags.Build) End Sub @@ -344,8 +348,9 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics Dim diagnostic = GetDiagnosticData(project.Id, id:=analyzer.SupportedDiagnostics(0).Id) source.AddNewErrors(project.Id, diagnostic) - AddHandler source.DiagnosticsUpdated, Sub(o, a) - Dim diagnostics = a.Diagnostics + AddHandler source.DiagnosticsUpdated, Sub(o, argsCollection) + Dim args = argsCollection.Single() + Dim diagnostics = args.Diagnostics Assert.Equal(1, diagnostics.Length) Assert.Equal(diagnostics(0).Properties(WellKnownDiagnosticPropertyNames.Origin), WellKnownDiagnosticTags.Build) @@ -469,7 +474,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics location:=New DiagnosticDataLocation(New FileLinePositionSpan("Test.txt", New LinePosition(4, 4), New LinePosition(4, 4)), documentId:=Nothing), language:=project.Language) - AddHandler service.DiagnosticsUpdated, Sub(o, args) + AddHandler service.DiagnosticsUpdated, Sub(o, argsCollection) + Dim args = argsCollection.Single() Dim diagnostics = args.Diagnostics Assert.Single(diagnostics) @@ -528,7 +534,8 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics Dim actualDiagnostic As DiagnosticData = Nothing Dim diagnosticAdded = False Dim diagnosticRemoved = False - AddHandler source.DiagnosticsUpdated, Sub(o, args) + AddHandler source.DiagnosticsUpdated, Sub(o, argsCollection) + Dim args = argsCollection.Single() Assert.Equal(document.Id, args.DocumentId) Dim diagnostics = args.Diagnostics If args.Kind = DiagnosticsUpdatedKind.DiagnosticsCreated Then @@ -561,7 +568,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics ' Verify build-only diagnostics cleared after document changed event document = document.WithText(SourceText.From("class C2 { }")) - source.OnWorkspaceChanged(workspace, New WorkspaceChangeEventArgs(WorkspaceChangeKind.DocumentChanged, + source.GetTestAccessor().OnWorkspaceChanged(workspace, New WorkspaceChangeEventArgs(WorkspaceChangeKind.DocumentChanged, oldSolution:=workspace.CurrentSolution, newSolution:=document.Project.Solution, project.Id, document.Id)) Await waiter.ExpeditedWaitAsync() @@ -657,7 +664,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.Diagnostics End Get End Property - Public Event DiagnosticsUpdated As EventHandler(Of DiagnosticsUpdatedArgs) Implements IDiagnosticUpdateSource.DiagnosticsUpdated + Public Event DiagnosticsUpdated As EventHandler(Of ImmutableArray(Of DiagnosticsUpdatedArgs)) Implements IDiagnosticUpdateSource.DiagnosticsUpdated Public Event DiagnosticsCleared As EventHandler Implements IDiagnosticUpdateSource.DiagnosticsCleared Public Function GetDiagnosticsAsync(workspace As Workspace, projectId As ProjectId, documentId As DocumentId, id As Object, includeSuppressedDiagnostics As Boolean, cancellationToken As CancellationToken) As ValueTask(Of ImmutableArray(Of DiagnosticData)) Implements IDiagnosticUpdateSource.GetDiagnosticsAsync diff --git a/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioAnalyzerTests.vb b/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioAnalyzerTests.vb index 5a705c1da70df..d22fe45fbdd0d 100644 --- a/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioAnalyzerTests.vb +++ b/src/VisualStudio/Core/Test/ProjectSystemShim/VisualStudioAnalyzerTests.vb @@ -2,6 +2,7 @@ ' The .NET Foundation licenses this file to you under the MIT license. ' See the LICENSE file in the project root for more information. +Imports System.Collections.Immutable Imports System.IO Imports System.Reflection Imports Microsoft.CodeAnalysis @@ -65,7 +66,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim reference.GetAnalyzers(LanguageNames.VisualBasic) RemoveHandler hostDiagnosticUpdateSource.DiagnosticsUpdated, AddressOf eventHandler.DiagnosticAddedTest - AddHandler hostDiagnosticUpdateSource.DiagnosticsUpdated, AddressOf eventHandler.DiagnosticRemovedTest + AddHandler hostDiagnosticUpdateSource.DiagnosticsUpdated, AddressOf EventHandlers.DiagnosticRemovedTest End Using IO.File.Delete(file) @@ -79,16 +80,16 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim Me.File = file End Sub - Public Sub DiagnosticAddedTest(o As Object, e As DiagnosticsUpdatedArgs) - Dim diagnostics = e.Diagnostics + Public Sub DiagnosticAddedTest(o As Object, e As ImmutableArray(Of DiagnosticsUpdatedArgs)) + Dim diagnostics = e.Single().Diagnostics Assert.Equal(1, diagnostics.Length) Dim diagnostic As DiagnosticData = diagnostics.First() Assert.Equal("BC42378", diagnostic.Id) Assert.Contains(File, diagnostic.Message, StringComparison.Ordinal) End Sub - Public Sub DiagnosticRemovedTest(o As Object, e As DiagnosticsUpdatedArgs) - Dim diagnostics = e.Diagnostics + Public Shared Sub DiagnosticRemovedTest(o As Object, e As ImmutableArray(Of DiagnosticsUpdatedArgs)) + Dim diagnostics = e.Single().Diagnostics Assert.Equal(0, diagnostics.Length) End Sub End Class From a918e4bc72bccfa1ea7ff3e57f4c66305f28b8f6 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 4 Oct 2023 13:02:06 -0500 Subject: [PATCH 3/5] Address feedback --- ...ntSources.DiagnosticsChangedEventSource.cs | 24 +++++++++++++++---- .../DiagnosticAnalyzerServiceTests.cs | 8 +++---- .../DiagnosticAnalyzerService_UpdateSource.cs | 12 ++++++---- ...IncrementalAnalyzer_IncrementalAnalyzer.cs | 10 ++------ ...DiagnosticListTable.LiveTableDataSource.cs | 6 ++--- 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs index 794b38e6c849a..41e1d525af722 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.DiagnosticsChangedEventSource.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; -using System.Linq; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; @@ -19,11 +18,26 @@ private class DiagnosticsChangedEventSource(ITextBuffer subjectBuffer, IDiagnost private void OnDiagnosticsUpdated(object? sender, ImmutableArray e) { - if (e.FirstOrDefault() is not { } first) - return; + var textContainer = _subjectBuffer.AsTextContainer(); + var anyChanged = false; + Workspace? lastWorkspace = null; + DocumentId? documentId = null; + foreach (var args in e) + { + if (args.Workspace != lastWorkspace) + { + lastWorkspace = args.Workspace; + documentId = args.Workspace.GetDocumentIdInCurrentContext(textContainer); + } + + if (args.DocumentId == documentId) + { + anyChanged = true; + break; + } + } - var documentId = first.Workspace.GetDocumentIdInCurrentContext(_subjectBuffer.AsTextContainer()); - if (e.Any(static (args, expectedDocumentId) => args.DocumentId == expectedDocumentId, documentId)) + if (anyChanged) { this.RaiseChanged(); } diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs index 143af7d011dbd..60bd529b90c14 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs @@ -621,7 +621,7 @@ private static async Task TestFullSolutionAnalysisForProjectAsync(AdhocWorkspace var diagnostics = e.Diagnostics; if (diagnostics.Length == 0) { - return; + continue; } var liveId = (LiveDiagnosticUpdateArgsId)e.Id; @@ -789,7 +789,7 @@ internal async Task TestDiagnosticSuppressor(bool includeAnalyzer, bool includeS var diagnostics = e.Diagnostics; if (diagnostics.Length == 0) { - return; + continue; } diagnostic = Assert.Single(diagnostics); @@ -1026,7 +1026,7 @@ void M() var diagnostics = e.Diagnostics; if (diagnostics.IsEmpty) { - return; + continue; } Assert.Null(diagnostic); @@ -1271,7 +1271,7 @@ internal async Task TestGeneratorProducedDiagnostics(bool fullSolutionAnalysis) { var diagnostics = e.Diagnostics; if (diagnostics.Length == 0) - return; + continue; var liveId = (LiveDiagnosticUpdateArgsId)e.Id; if (liveId.Analyzer is GeneratorDiagnosticsPlaceholderAnalyzer) diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs index 98868ee19d1af..d1f8618ebeb69 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService_UpdateSource.cs @@ -42,6 +42,9 @@ public event EventHandler DiagnosticsCleared internal void RaiseDiagnosticsUpdated(ImmutableArray args) { + if (args.IsEmpty) + return; + // all diagnostics events are serialized. var ev = _eventMap.GetEventHandlers>>(DiagnosticsUpdatedEventName); if (ev.HasHandlers) @@ -61,12 +64,11 @@ internal void RaiseBulkDiagnosticsUpdated(Action args) { + if (args.IsEmpty) + return; + ev.RaiseEvent( - static (handler, arg) => - { - if (!arg.args.IsEmpty) - handler(arg.self, arg.args); - }, + static (handler, arg) => handler(arg.self, arg.args), (self: this, args)); } diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs index dcbff64d7d562..50d33629a416f 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs @@ -85,10 +85,7 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi } } - if (argsBuilder.Count > 0) - { - AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); - } + AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); // Then, compute the diagnostics for non-cached state sets, and cache and raise diagnostic reported events for these diagnostics. if (nonCachedAnalyzersAndStates.Count > 0) @@ -102,10 +99,7 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi PersistAndAddDiagnosticsArgsIfNeeded(ref argsBuilder.AsRef(), computedData, analyzer, state); - if (argsBuilder.Count > 0) - { - AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); - } + AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); } } } diff --git a/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs b/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs index 254c97d7a245c..058a698e43cb1 100644 --- a/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs +++ b/src/VisualStudio/Core/Def/TableDataSource/VisualStudioBaseDiagnosticListTable.LiveTableDataSource.cs @@ -205,7 +205,7 @@ private void OnDiagnosticsUpdated(object sender, ImmutableArray Date: Wed, 4 Oct 2023 16:46:41 -0500 Subject: [PATCH 4/5] Fix test deadlocks --- .../Diagnostics/DiagnosticServiceTests.cs | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs index 6c40aa56f0003..778492e7c330a 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticServiceTests.cs @@ -41,7 +41,11 @@ public async Task TestGetDiagnostics1() var diagnosticService = GetDiagnosticService(workspace); diagnosticService.Register(source); - diagnosticService.DiagnosticsUpdated += (s, o) => { mutex.Set(); }; + diagnosticService.DiagnosticsUpdated += (s, o) => + { + foreach (var _ in o) + mutex.Set(); + }; var id = Tuple.Create(workspace, document); var diagnostic = RaiseDiagnosticEvent(mutex, source, workspace, document.Project.Id, document.Id, id); @@ -71,7 +75,11 @@ public async Task TestGetDiagnostics2() var diagnosticService = GetDiagnosticService(workspace); diagnosticService.Register(source); - diagnosticService.DiagnosticsUpdated += (s, o) => { mutex.Set(); }; + diagnosticService.DiagnosticsUpdated += (s, o) => + { + foreach (var _ in o) + mutex.Set(); + }; var id = Tuple.Create(workspace, document); RaiseDiagnosticEvent(mutex, source, workspace, document.Project.Id, document.Id, id); @@ -148,16 +156,20 @@ public async Task TestCleared() void MarkCalled(object sender, ImmutableArray args) { - // event is serialized. no concurrent call - if (++count == 3) + foreach (var _ in args) { - mutex.Set(); + // event is serialized. no concurrent call + if (++count == 3) + { + mutex.Set(); + } } } void MarkSet(object sender, ImmutableArray args) { - mutex.Set(); + foreach (var _ in args) + mutex.Set(); } } From 5a84d4e3eb08a68c1983c1d46994e8a0c2236184 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 5 Oct 2023 08:43:28 -0500 Subject: [PATCH 5/5] Add comments regarding cached and non-cached analyzer events --- .../DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs index 50d33629a416f..660a611ec28b1 100644 --- a/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs +++ b/src/Features/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_IncrementalAnalyzer.cs @@ -85,6 +85,8 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi } } + // Send events for cached analyzers as a batch. The preceding loop is expected to quickly aggregate + // results from cached analyzers, since it will not wait for analyzers that are not already complete. AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); // Then, compute the diagnostics for non-cached state sets, and cache and raise diagnostic reported events for these diagnostics. @@ -99,6 +101,8 @@ private async Task AnalyzeDocumentForKindAsync(TextDocument document, AnalysisKi PersistAndAddDiagnosticsArgsIfNeeded(ref argsBuilder.AsRef(), computedData, analyzer, state); + // Send events for non-cached analyzers as soon as they complete, to avoid delaying error list + // updates when a subset of the analyzers takes a noticeably longer time to complete. AnalyzerService.RaiseDiagnosticsUpdated(argsBuilder.ToImmutableAndClear()); } }