From 2fffa6de21ef9600eb3e0e5f9959817155388db9 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Mon, 23 Nov 2020 13:03:01 -0800 Subject: [PATCH] Fix the ability to expand the list of analyzers in a reference This was broken by 7fa2c964f8433b928c5ff966427ce3798724cdd2, where I asserted that GetDiagnosticItems was only called when we already knew we would have items. This was broken since right before we called the method the collection would get created, which changed the answer it would return. A small refactoring ensures we create the list and fill it in at once. This also makes it clear we aren't passing a non-trivial LINQ query across method boundaries anyways in a way that might result in multiple enumerations. Fixes https://github.com/dotnet/roslyn/issues/49524 --- .../TestAnalyzerReferenceByLanguage.cs | 11 ++-- .../BaseDiagnosticItemSource.cs | 13 +++-- .../CpsDiagnosticItemSourceTests.vb | 51 +++++++++++++++++++ 3 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 src/VisualStudio/Core/Test/SolutionExplorer/CpsDiagnosticItemSourceTests.vb diff --git a/src/EditorFeatures/TestUtilities/Diagnostics/TestAnalyzerReferenceByLanguage.cs b/src/EditorFeatures/TestUtilities/Diagnostics/TestAnalyzerReferenceByLanguage.cs index 070fe33410042..7775cd555c514 100644 --- a/src/EditorFeatures/TestUtilities/Diagnostics/TestAnalyzerReferenceByLanguage.cs +++ b/src/EditorFeatures/TestUtilities/Diagnostics/TestAnalyzerReferenceByLanguage.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.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -14,10 +12,13 @@ internal class TestAnalyzerReferenceByLanguage : AnalyzerReference { private readonly IReadOnlyDictionary> _analyzersMap; - public TestAnalyzerReferenceByLanguage(IReadOnlyDictionary> analyzersMap) - => _analyzersMap = analyzersMap; + public TestAnalyzerReferenceByLanguage(IReadOnlyDictionary> analyzersMap, string? fullPath = null) + { + _analyzersMap = analyzersMap; + FullPath = fullPath; + } - public override string FullPath => null; + public override string? FullPath { get; } public override string Display => nameof(TestAnalyzerReferenceByLanguage); public override object Id => Display; diff --git a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticItemSource.cs b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticItemSource.cs index 1d0453ca43081..4ab3c0f934f3a 100644 --- a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticItemSource.cs +++ b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/BaseDiagnosticItemSource.cs @@ -83,8 +83,7 @@ public IEnumerable Items _specificDiagnosticOptions = project.CompilationOptions!.SpecificDiagnosticOptions; _analyzerConfigOptions = project.GetAnalyzerConfigOptions(); - _diagnosticItems = new BulkObservableCollection(); - _diagnosticItems.AddRange(GetDiagnosticItems(project.Language, project.CompilationOptions, _analyzerConfigOptions)); + _diagnosticItems = CreateDiagnosticItems(project.Language, project.CompilationOptions, _analyzerConfigOptions); Workspace.WorkspaceChanged += OnWorkspaceChangedLookForOptionsChanges; } @@ -97,7 +96,7 @@ public IEnumerable Items } } - private IEnumerable GetDiagnosticItems(string language, CompilationOptions options, AnalyzerConfigOptionsResult? analyzerConfigOptions) + private BulkObservableCollection CreateDiagnosticItems(string language, CompilationOptions options, AnalyzerConfigOptionsResult? analyzerConfigOptions) { // Within an analyzer assembly, an individual analyzer may report multiple different diagnostics // with the same ID. Or, multiple analyzers may report diagnostics with the same ID. Or a @@ -110,7 +109,9 @@ private IEnumerable GetDiagnosticItems(string language, Comp Contract.ThrowIfFalse(HasItems); - return AnalyzerReference.GetAnalyzers(language) + var collection = new BulkObservableCollection(); + collection.AddRange( + AnalyzerReference.GetAnalyzers(language) .SelectMany(a => _diagnosticAnalyzerService.AnalyzerInfoCache.GetDiagnosticDescriptors(a)) .GroupBy(d => d.Id) .OrderBy(g => g.Key, StringComparer.CurrentCulture) @@ -119,7 +120,9 @@ private IEnumerable GetDiagnosticItems(string language, Comp var selectedDiagnostic = g.OrderBy(d => d, s_comparer).First(); var effectiveSeverity = selectedDiagnostic.GetEffectiveSeverity(options, analyzerConfigOptions); return CreateItem(selectedDiagnostic, effectiveSeverity, language); - }); + })); + + return collection; } private void OnWorkspaceChangedLookForOptionsChanges(object sender, WorkspaceChangeEventArgs e) diff --git a/src/VisualStudio/Core/Test/SolutionExplorer/CpsDiagnosticItemSourceTests.vb b/src/VisualStudio/Core/Test/SolutionExplorer/CpsDiagnosticItemSourceTests.vb new file mode 100644 index 0000000000000..82812dbb1c7d7 --- /dev/null +++ b/src/VisualStudio/Core/Test/SolutionExplorer/CpsDiagnosticItemSourceTests.vb @@ -0,0 +1,51 @@ +' Licensed to the .NET Foundation under one or more agreements. +' 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 Microsoft.CodeAnalysis +Imports Microsoft.CodeAnalysis.Diagnostics +Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces +Imports Microsoft.CodeAnalysis.Test.Utilities +Imports Microsoft.Internal.VisualStudio.PlatformUI +Imports Microsoft.VisualStudio.LanguageServices.Implementation.SolutionExplorer +Imports Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework +Imports Microsoft.VisualStudio.Shell + +Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.SolutionExplorer + + Public Class CpsDiagnosticItemSourceTests + + Public Sub AnalyzerHasDiagnostics() + Dim workspaceXml = + + + + + + Using workspace = TestWorkspace.Create(workspaceXml) + Dim project = workspace.Projects.Single() + + Dim analyzers = New Dictionary(Of String, ImmutableArray(Of DiagnosticAnalyzer)) + + ' The choice here of this analyzer to test with is arbitray -- there's nothing special about this + ' analyzer versus any other one. + analyzers.Add(LanguageNames.VisualBasic, ImmutableArray.Create(Of DiagnosticAnalyzer)(New Microsoft.CodeAnalysis.VisualBasic.UseAutoProperty.VisualBasicUseAutoPropertyAnalyzer())) + + Const analyzerPath = "C:\Analyzer.dll" + workspace.OnAnalyzerReferenceAdded(project.Id, New TestAnalyzerReferenceByLanguage(analyzers, analyzerPath)) + + Dim source As IAttachedCollectionSource = New CpsDiagnosticItemSource( + workspace, + project.FilePath, + project.Id, + New MockHierarchyItem() With {.CanonicalName = "\net472\analyzerdependency\" + analyzerPath}, + New FakeAnalyzersCommandHandler, workspace.GetService(Of IDiagnosticAnalyzerService)) + + Assert.True(source.HasItems) + Dim diagnostic = Assert.IsAssignableFrom(Of ITreeDisplayItem)(Assert.Single(source.Items)) + Assert.Contains(IDEDiagnosticIds.UseAutoPropertyDiagnosticId, diagnostic.Text) + End Using + End Sub + End Class +End Namespace