Merge pull request #45786 from mavasani/Issue45779

Ensure that we de-dupe the configuration/suppression code actions

src/EditorFeatures/CSharpTest/Diagnostics/Suppression/SuppressionTests.cs

@@ -275,13 +275,12 @@ void Method()
                     var allFixes = (await fixService.GetFixesAsync(document, span, includeConfigurationFixes: true, cancellationToken: CancellationToken.None))
                         .SelectMany(fixCollection => fixCollection.Fixes);
 
-                    var cs0219Fixes = allFixes.Where(fix => fix.PrimaryDiagnostic.Id == "CS0219");
+                    var cs0219Fixes = allFixes.Where(fix => fix.PrimaryDiagnostic.Id == "CS0219").ToArray();
 
-                    // Ensure that both the fixes have identical equivalence key, and hence get de-duplicated in LB menu.
-                    Assert.Equal(2, cs0219Fixes.Count());
-                    var cs0219EquivalenceKey = cs0219Fixes.First().Action.EquivalenceKey;
+                    // Ensure that there are no duplicate suppression fixes.
+                    Assert.Equal(1, cs0219Fixes.Length);
+                    var cs0219EquivalenceKey = cs0219Fixes[0].Action.EquivalenceKey;
                     Assert.NotNull(cs0219EquivalenceKey);
-                    Assert.Equal(cs0219EquivalenceKey, cs0219Fixes.Last().Action.EquivalenceKey);
 
                     // Ensure that there *is* a fix for the other warning and that it has a *different*
                     // equivalence key so that it *doesn't* get de-duplicated

src/EditorFeatures/Core/Implementation/CodeFixes/CodeFixService.cs

@@ -215,11 +215,13 @@ int GetValue(CodeFixCollection c)
             // TODO (https://github.com/dotnet/roslyn/issues/4932): Don't restrict CodeFixes in Interactive
             if (document.Project.Solution.Workspace.Kind != WorkspaceKind.Interactive && includeConfigurationFixes)
             {
+                // Ensure that we do not register duplicate configuration fixes.
+                using var _ = PooledHashSet<string>.GetInstance(out var registeredConfigurationFixTitles);
                 foreach (var spanAndDiagnostic in aggregatedDiagnostics)
                 {
                     await AppendConfigurationsAsync(
                         document, spanAndDiagnostic.Key, spanAndDiagnostic.Value,
-                        result, cancellationToken).ConfigureAwait(false);
+                        result, registeredConfigurationFixTitles, cancellationToken).ConfigureAwait(false);
                 }
             }
 
@@ -446,8 +448,12 @@ public async Task<Document> ApplyCodeFixesForSpecificDiagnosticIdAsync(Document
         }
 
         private async Task AppendConfigurationsAsync(
-            Document document, TextSpan diagnosticsSpan, IEnumerable<DiagnosticData> diagnostics,
-            ArrayBuilder<CodeFixCollection> result, CancellationToken cancellationToken)
+            Document document,
+            TextSpan diagnosticsSpan,
+            IEnumerable<DiagnosticData> diagnostics,
+            ArrayBuilder<CodeFixCollection> result,
+            PooledHashSet<string> registeredConfigurationFixTitles,
+            CancellationToken cancellationToken)
         {
             if (!_configurationProvidersMap.TryGetValue(document.Project.Language, out var lazyConfigurationProviders) || lazyConfigurationProviders.Value == null)
             {
@@ -462,9 +468,12 @@ public async Task<Document> ApplyCodeFixesForSpecificDiagnosticIdAsync(Document
                     await AppendFixesOrConfigurationsAsync(
                         document, diagnosticsSpan, diagnostics, fixAllForInSpan: false, result, provider,
                         hasFix: d => provider.IsFixableDiagnostic(d),
-                        getFixes: dxs => provider.GetFixesAsync(
-                            document, diagnosticsSpan, dxs, cancellationToken),
-                        cancellationToken: cancellationToken).ConfigureAwait(false);
+                        getFixes: async dxs =>
+                        {
+                            var fixes = await provider.GetFixesAsync(document, diagnosticsSpan, dxs, cancellationToken).ConfigureAwait(false);
+                            return fixes.WhereAsArray(f => registeredConfigurationFixTitles.Add(f.Action.Title));
+                        },
+                        cancellationToken).ConfigureAwait(false);
                 }
             }
         }

src/EditorFeatures/Test/CodeFixes/CodeFixServiceTests.cs

@@ -92,6 +92,36 @@ public async Task TestGetFixesAsyncWithDuplicateDiagnostics()
             Assert.Equal(2, codeFix.ContextDiagnosticsCount);
         }
 
+        [Fact, WorkItem(45779, "https://github.com/dotnet/roslyn/issues/45779")]
+        public async Task TestGetFixesAsyncHasNoDuplicateConfigurationActions()
+        {
+            var codeFix = new MockFixer();
+
+            // Add analyzers with duplicate ID and/or category to get duplicate diagnostics.
+            var analyzerReference = new MockAnalyzerReference(
+                    codeFix,
+                    ImmutableArray.Create<DiagnosticAnalyzer>(
+                        new MockAnalyzerReference.MockDiagnosticAnalyzer("ID1", "Category1"),
+                        new MockAnalyzerReference.MockDiagnosticAnalyzer("ID1", "Category1"),
+                        new MockAnalyzerReference.MockDiagnosticAnalyzer("ID1", "Category2"),
+                        new MockAnalyzerReference.MockDiagnosticAnalyzer("ID2", "Category2")));
+
+            var tuple = ServiceSetup(codeFix, includeConfigurationFixProviders: true);
+            using var workspace = tuple.workspace;
+            GetDocumentAndExtensionManager(tuple.analyzerService, workspace, out var document, out var extensionManager, analyzerReference);
+
+            // Verify registered configuration code actions do not have duplicates.
+            var fixCollections = await tuple.codeFixService.GetFixesAsync(document, TextSpan.FromBounds(0, 0), includeConfigurationFixes: true, cancellationToken: CancellationToken.None);
+            var codeActions = fixCollections.SelectMany(c => c.Fixes.Select(f => f.Action)).ToImmutableArray();
+            Assert.Equal(7, codeActions.Length);
+            var uniqueTitles = new HashSet<string>();
+            foreach (var codeAction in codeActions)
+            {
+                Assert.True(codeAction is AbstractConfigurationActionWithNestedActions);
+                Assert.True(uniqueTitles.Add(codeAction.Title));
+            }
+        }
+
         [Fact]
         public async Task TestGetCodeFixWithExceptionInRegisterMethod()
         {
@@ -171,7 +201,9 @@ private async Task GetFirstDiagnosticWithFixAsync(CodeFixProvider codefix)
             Assert.False(extensionManager.IsIgnored(codefix));
         }
 
-        private static (TestWorkspace workspace, TestDiagnosticAnalyzerService analyzerService, CodeFixService codeFixService, IErrorLoggerService errorLogger) ServiceSetup(CodeFixProvider codefix)
+        private static (TestWorkspace workspace, TestDiagnosticAnalyzerService analyzerService, CodeFixService codeFixService, IErrorLoggerService errorLogger) ServiceSetup(
+            CodeFixProvider codefix,
+            bool includeConfigurationFixProviders = false)
         {
             var fixers = SpecializedCollections.SingletonEnumerable(
                 new Lazy<CodeFixProvider, CodeChangeProviderMetadata>(
@@ -187,9 +219,12 @@ private static (TestWorkspace workspace, TestDiagnosticAnalyzerService analyzerS
             var diagnosticService = new TestDiagnosticAnalyzerService();
             var logger = SpecializedCollections.SingletonEnumerable(new Lazy<IErrorLoggerService>(() => new TestErrorLogger()));
             var errorLogger = logger.First().Value;
+            var configurationFixProviders = includeConfigurationFixProviders
+                ? TestExportProvider.ExportProviderWithCSharpAndVisualBasic.GetExports<IConfigurationFixProvider, CodeChangeProviderMetadata>()
+                : SpecializedCollections.EmptyEnumerable<Lazy<IConfigurationFixProvider, CodeChangeProviderMetadata>>();
             var fixService = new CodeFixService(
                 workspace.ExportProvider.GetExportedValue<IThreadingContext>(),
-                diagnosticService, logger, fixers, SpecializedCollections.EmptyEnumerable<Lazy<IConfigurationFixProvider, CodeChangeProviderMetadata>>());
+                diagnosticService, logger, fixers, configurationFixProviders);
             return (workspace, diagnosticService, fixService, errorLogger);
         }
 
@@ -295,20 +330,30 @@ public ImmutableArray<CodeFixProvider> GetFixers()
 
             public class MockDiagnosticAnalyzer : DiagnosticAnalyzer
             {
+                public MockDiagnosticAnalyzer(ImmutableArray<(string id, string category)> reportedDiagnosticIdsWithCategories)
+                    => SupportedDiagnostics = CreateSupportedDiagnostics(reportedDiagnosticIdsWithCategories);
+
+                public MockDiagnosticAnalyzer(string diagnosticId, string category)
+                    : this(ImmutableArray.Create((diagnosticId, category)))
+                {
+                }
+
                 public MockDiagnosticAnalyzer(ImmutableArray<string> reportedDiagnosticIds)
-                    => SupportedDiagnostics = CreateSupportedDiagnostics(reportedDiagnosticIds);
+                    : this(reportedDiagnosticIds.SelectAsArray(id => (id, "InternalCategory")))
+                {
+                }
 
                 public MockDiagnosticAnalyzer()
                     : this(ImmutableArray.Create(MockFixer.Id))
                 {
                 }
 
-                private static ImmutableArray<DiagnosticDescriptor> CreateSupportedDiagnostics(ImmutableArray<string> reportedDiagnosticIds)
+                private static ImmutableArray<DiagnosticDescriptor> CreateSupportedDiagnostics(ImmutableArray<(string id, string category)> reportedDiagnosticIdsWithCategories)
                 {
                     var builder = ArrayBuilder<DiagnosticDescriptor>.GetInstance();
-                    foreach (var diagnosticId in reportedDiagnosticIds)
+                    foreach (var (diagnosticId, category) in reportedDiagnosticIdsWithCategories)
                     {
-                        var descriptor = new DiagnosticDescriptor(diagnosticId, "MockDiagnostic", "MockDiagnostic", "InternalCategory", DiagnosticSeverity.Warning, isEnabledByDefault: true);
+                        var descriptor = new DiagnosticDescriptor(diagnosticId, "MockDiagnostic", "MockDiagnostic", category, DiagnosticSeverity.Warning, isEnabledByDefault: true);
                         builder.Add(descriptor);
                     }